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

Add bitcoin-tx JSON tests #8829

Merged
merged 1 commit into from Sep 29, 2016

Conversation

Projects
None yet
6 participants
@jnewbery
Copy link
Member

commented Sep 28, 2016

This PR adds new test cases to test bitcoin-tx JSON output, as suggested by @sipa in #8817 .

@laanwj

This comment has been minimized.

Copy link
Member

commented Sep 29, 2016

Awesome, thanks!
utACK 54e5d7c

@MarcoFalke

This comment has been minimized.

Copy link
Member

commented Sep 29, 2016

utACK 54e5d7c

@MarcoFalke MarcoFalke merged commit 54e5d7c into bitcoin:master Sep 29, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

MarcoFalke added a commit that referenced this pull request Sep 29, 2016

Merge #8829: Add bitcoin-tx JSON tests
54e5d7c Add bitcoin-tx JSON tests (jnewbery)

@MarcoFalke MarcoFalke added this to the 0.13.1 milestone Sep 29, 2016

@paveljanik

This comment has been minimized.

Copy link
Contributor

commented Sep 29, 2016

ups, merged in the middle of my testing :-)

postmerge ACK 54e5d7c

@droark

This comment has been minimized.

Copy link
Contributor

commented Sep 29, 2016

Question: Was txcreate2.json supposed to be empty? I'm assuming some test data was supposed to be in it. :)

@jnewbery

This comment has been minimized.

Copy link
Member Author

commented Sep 29, 2016

Interesting. The txcreate2 test is attempting to create a transaction with a single pay-to-script output for zero satoshi. When outputting as json, bitcoin-tx returns the empty string. When outputting as hex, it returns 01000000000100000000000000000000000000, ie:

version |number in|number out|out1 amount     |out1 script length|nlocktime
01000000 00        01         0000000000000000 00                 00000000

I think that's garbage and bitcoin-tx should reject any attempt to add an output with zero satoshi. I'll open a PR to fix that.

Interestingly, bitcoin-tx does output a json object if I try to create a zero-value transaction output to a non-empty script:

$ bitcoin-tx -json -create outscript=0:"OP_DROP"
{
    "txid": "f0851b68202f736b792649cfc960259c2374badcb644ab20cac726b5f72f61c9",
    "version": 1,
    "locktime": 0,
    "vin": [
    ],
    "vout": [
        {
            "value": 0.00,
            "n": 0,
            "scriptPubKey": {
                "asm": "OP_DROP",
                "hex": "75",
                "type": "nonstandard"
            }
        }
    ],
    "hex": "0100000000010000000000000000017500000000"
}

txcreate2 is also the only test case that tests creating a pay-to-script, so we should add a new test case that tests adding a valid pay-to-script output.

@jnewbery jnewbery deleted the jnewbery:test-bitcoin-tx-json branch Sep 29, 2016

@sipa

This comment has been minimized.

Copy link
Member

commented Sep 29, 2016

@jnewbery

This comment has been minimized.

Copy link
Member Author

commented Sep 29, 2016

ah, ok. Thanks. Then the fix is in bitcoin-tx to properly output zero-value, empty-script outputs.

@laanwj

This comment has been minimized.

Copy link
Member

commented Sep 29, 2016

Good to see that we're finding bugs in bitcoin-tx by adding tests :)

@jnewbery

This comment has been minimized.

Copy link
Member Author

commented Sep 29, 2016

I've mischaracterised the problem here. bitcoin-tx is outputting the transaction correctly in hex and json. However, when I try to pipe that same hex transaction back into bitcoin-tx, it fails with error: invalid transaction encoding.

@sipa

This comment has been minimized.

Copy link
Member

commented Sep 29, 2016

@jnewbery

This comment has been minimized.

Copy link
Member Author

commented Sep 29, 2016

Thanks @sipa . New PR here: #8837 to make bitcoin-tx set the flag correctly and parse partial transactions.

@droark - I've added #8836 to fix the fact that bitcoin-util-test.py would always succeed if the output comparison file was empty. That PR also fixes txcreate2.json so it's no longer empty.

MarcoFalke added a commit to MarcoFalke/bitcoin that referenced this pull request Oct 3, 2016

@laanwj

This comment has been minimized.

Copy link
Member

commented Oct 13, 2016

This was backported in #8832, removing tag

codablock added a commit to codablock/dash that referenced this pull request Jan 12, 2018

Merge bitcoin#8829: Add bitcoin-tx JSON tests
54e5d7c Add bitcoin-tx JSON tests (jnewbery)

lateminer added a commit to lateminer/bitcoin that referenced this pull request Nov 10, 2018

andvgal added a commit to energicryptocurrency/energi that referenced this pull request Jan 6, 2019

Merge bitcoin#8829: Add bitcoin-tx JSON tests
54e5d7c Add bitcoin-tx JSON tests (jnewbery)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.