tests: Fix bitcoin-tx signing test case #6390

Merged
merged 1 commit into from Jul 8, 2015

Conversation

Projects
None yet
3 participants
@laanwj
Member

laanwj commented Jul 7, 2015

Fixes wrong scriptPubkey problem in #6389, causing the transaction to not actually be signed.

Also change bitcoin-tx so that in MutateTxSign if SignSignature fails, signing is marked as incomplete. This makes sure that errors in e.g. scriptPubKeys are detected.

@laanwj laanwj added the Tests label Jul 7, 2015

tests: Fix bitcoin-tx signing testcase
Fixes wrong scriptPubkey problem, which caused the transaction to
not actually be signed.
@jgarzik

This comment has been minimized.

Show comment
Hide comment
@jgarzik

jgarzik Jul 7, 2015

Contributor

ACK, with comments:

  • bitcoin-tx notably does nothing with fComplete.
  • a "exit(1) if not completely signed" command line switch would enable tests to notice this sort of failure
Contributor

jgarzik commented Jul 7, 2015

ACK, with comments:

  • bitcoin-tx notably does nothing with fComplete.
  • a "exit(1) if not completely signed" command line switch would enable tests to notice this sort of failure
@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj Jul 7, 2015

Member

Yes, thinking of it, the change to bitcoin-tx is redundant, even if fComplete was somehow reported.

The VerifyScript later in the iteration will already set fComplete to false if the signing was not successful.

Will remove it.

Member

laanwj commented Jul 7, 2015

Yes, thinking of it, the change to bitcoin-tx is redundant, even if fComplete was somehow reported.

The VerifyScript later in the iteration will already set fComplete to false if the signing was not successful.

Will remove it.

@laanwj laanwj merged commit 133601f into bitcoin:master Jul 8, 2015

1 check failed

continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details

laanwj added a commit that referenced this pull request Jul 8, 2015

Merge pull request #6390
133601f tests: Fix bitcoin-tx signing testcase (Wladimir J. van der Laan)
@paveljanik

This comment has been minimized.

Show comment
Hide comment
@paveljanik

paveljanik Jul 8, 2015

Post merge ACK, thanks!

Looks like this is the only example and usage of sign in bitcoin-tx. I'll try to come up with more test cases, more inputs etc. It is not as straightforward as createrawtransaction/signrawtransaction and the syntax doesn't help at all. The same applies to doc ("see signrawtransaction").

Post merge ACK, thanks!

Looks like this is the only example and usage of sign in bitcoin-tx. I'll try to come up with more test cases, more inputs etc. It is not as straightforward as createrawtransaction/signrawtransaction and the syntax doesn't help at all. The same applies to doc ("see signrawtransaction").

@str4d str4d referenced this pull request in zcash/zcash Feb 15, 2017

Merged

Bitcoin 0.12 test PRs 1 #2101

zkbot added a commit to zcash/zcash that referenced this pull request Feb 15, 2017

zkbot added a commit to zcash/zcash that referenced this pull request Feb 20, 2017

zkbot added a commit to zcash/zcash that referenced this pull request Mar 3, 2017

Auto merge of #2101 - str4d:2074-tests, r=arcalinea
Bitcoin 0.12 test PRs 1

Cherry-picked from the following upstream PRs:

- bitcoin/bitcoin#6337
- bitcoin/bitcoin#6390
- bitcoin/bitcoin#5515
- bitcoin/bitcoin#6287 (partial, remainder included in bitcoin/bitcoin#6703)
- bitcoin/bitcoin#6465

Part of #2074.

@dagurval dagurval referenced this pull request in bitcoinxt/bitcoinxt Apr 27, 2018

Merged

tests: Fix bitcoin-tx signing testcase #402

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment