Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tx signature verification unittest #69

Closed
wants to merge 1 commit into from

Conversation

TrustHenry
Copy link
Member

@TrustHenry TrustHenry commented Jul 5, 2019

This creates a new transaction and signs it as a publickey
of the previous transaction to create and validate the input.
Required PR : Implement transaction verification #60

…vious transaction to create and validate the input.
@Geod24 Geod24 added the status-blocked label Jul 5, 2019
@AndrejMitrovic
Copy link
Contributor

@AndrejMitrovic AndrejMitrovic commented Jul 8, 2019

There's a few issues with this PR:

  • The test code should not live here. It should live in agora.common.Transaction because it is self-contained and doesn't require networking code.
  • It does not need the "TransactionManager"
  • It doesn't seem to use the public key in the outputs array for testing the signature, instead it directly uses the key in the key_pairs array (PublicKey pubkey = key_pairs[0].address; assert(pubkey.verify(input.signature, inputHash[]));).

So I suggest moving unittest code to agora.common.Transaction, and writing something similar to:

/// This creates a new transaction and signs it as a publickey
/// of the previous transaction to create and validate the input.
unittest
{
    immutable(KeyPair)[] key_pairs;
    key_pairs ~= KeyPair.random();
    key_pairs ~= KeyPair.random();

    // first transaction (coinbase to first public key)
    Transaction tx0 =
    {
        inputs  : [Input(Hash.init, 0)],
        outputs : [Output(40_000, key_pairs[0].address)]
    };

    Input input =
    {
        previous : hashFull(tx0),  // hash of previous utxo
        index : 0  // first item in the outputs array
    };

    // Signs the previous hash value.
    Signature signature = key_pairs[0].secret.sign(hashFull(input)[]);
    input.signature = signature;

    Output output =
    {
        value : 20_000,
        address : key_pairs[1].address,
    };

    // second transaction (spends the utxo from the first transaction)
    Transaction tx1 =
    {
        inputs : [input],
        outputs : [output]
    };

    // Verify the transaction's only output
    assert(tx0.outputs[0].address.verify(
        tx1.inputs[0].signature,
        hashFull(tx1.inputs[0])[]));
}

@Geod24 please verify my logic in this unittest. But I think it should be reasonable..

@TrustHenry
Copy link
Member Author

@TrustHenry TrustHenry commented Jul 8, 2019

@AndrejMitrovic agora.common.Transaction You're talking about me adding it to this file?

@AndrejMitrovic
Copy link
Contributor

@AndrejMitrovic AndrejMitrovic commented Jul 8, 2019

Yes to move the unittest there. The source/agora/test folder is only for code that needs networking support.

@TrustHenry
Copy link
Member Author

@TrustHenry TrustHenry commented Jul 8, 2019

I uploaded it again. I'll close this PR.

@TrustHenry TrustHenry closed this Jul 8, 2019
@TrustHenry TrustHenry deleted the henry/txvGP branch Jul 23, 2019
TrustHenry added a commit to TrustHenry/agora that referenced this issue May 26, 2020
AndrejMitrovic pushed a commit that referenced this issue May 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status-blocked
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants