diff options
author | MarcoFalke <falke.marco@gmail.com> | 2020-12-10 20:06:28 +0100 |
---|---|---|
committer | MarcoFalke <falke.marco@gmail.com> | 2020-12-10 20:06:35 +0100 |
commit | 953dddbd20584cb41b869d74ea7dfc911b405dac (patch) | |
tree | 0c2bb3a6127b5e3cb2360585e63e0f45157bedec /test/functional | |
parent | 75bf23d8613a28928e18a0b05135d08e354452e0 (diff) | |
parent | 6a326cf66f04948ffe729e5540a69e159a7840ad (diff) | |
download | bitcoin-953dddbd20584cb41b869d74ea7dfc911b405dac.tar.xz |
Merge #19740: [0.20] wallet: Simplify and fix CWallet::SignTransaction
6a326cf66f04948ffe729e5540a69e159a7840ad tests: Test that a fully signed tx given to signrawtx is unchanged (Andrew Chow)
2d48d7dcfb93eeec36dd6f5cbad289b89ec69324 Simplify and fix CWallet::SignTransaction (Andrew Chow)
Pull request description:
Backport `CWallet::SignTransaction` from master which is simpler and not broken.
Previously `CWallet::SignTransaction` would return false for a fully signed transaction. This is a regression and obviously incorrect - a fully signed transaction is always complete. This occurs because `CWallet::SignTransaction` would iterate through each input and skip any further checks if the input was already signed. It would then end up falling through to the `return false` catch-all thus erroneously saying a fully signed transaction is incomplete. The change to attempting to use all `ScriptPubKeyMan`s fixes this problem since the `LegacyScriptPubKeyMan` (the only spkm implemented in 0.20) will verify inputs during its signing attempt and correctly return that it is complete when the inputs verify. Thus a fully signed transaction will be correctly identified as complete, `LegacyScriptPubKeyMan::SignTranaction` will return true, and so `CWallet::Transaction` will return true too.
Note that this is not a backport of any specific commit. Rather it is the end result of the changes we have made to this function in master.
Fixes #19737
ACKs for top commit:
fjahr:
Code review ACK 6a326cf66f04948ffe729e5540a69e159a7840ad
MarcoFalke:
re-ACK 6a326cf66f 👓
Tree-SHA512: 7b8e04cfc264de01a2de08a99477c1a110fa0965dcbd4305bf4632a165f28b090746789195f5cb584eb6d85e27d4801a09d2dad28e508d7898c7c088e771812b
Diffstat (limited to 'test/functional')
-rwxr-xr-x | test/functional/rpc_signrawtransaction.py | 14 |
1 files changed, 14 insertions, 0 deletions
diff --git a/test/functional/rpc_signrawtransaction.py b/test/functional/rpc_signrawtransaction.py index 17686f3a78..d76a29fd99 100755 --- a/test/functional/rpc_signrawtransaction.py +++ b/test/functional/rpc_signrawtransaction.py @@ -146,6 +146,19 @@ class SignRawTransactionsTest(BitcoinTestFramework): assert_equal(rawTxSigned['errors'][1]['witness'], ["304402203609e17b84f6a7d30c80bfa610b5b4542f32a8a0d5447a12fb1366d7f01cc44a0220573a954c4518331561406f90300e8f3358f51928d43c212a8caed02de67eebee01", "025476c2e83188368da1ff3e292e7acafcdb3566bb0ad253f62fc70f07aeee6357"]) assert not rawTxSigned['errors'][0]['witness'] + def test_fully_signed_tx(self): + self.log.info("Test signing a fully signed transaction does nothing") + self.nodes[0].walletpassphrase("password", 9999) + self.nodes[0].generate(101) + rawtx = self.nodes[0].createrawtransaction([], [{self.nodes[0].getnewaddress(): 10}]) + fundedtx = self.nodes[0].fundrawtransaction(rawtx) + signedtx = self.nodes[0].signrawtransactionwithwallet(fundedtx["hex"]) + assert_equal(signedtx["complete"], True) + signedtx2 = self.nodes[0].signrawtransactionwithwallet(signedtx["hex"]) + assert_equal(signedtx2["complete"], True) + assert_equal(signedtx["hex"], signedtx2["hex"]) + self.nodes[0].walletlock() + def witness_script_test(self): # Now test signing transaction to P2SH-P2WSH addresses without wallet # Create a new P2SH-P2WSH 1-of-1 multisig address: @@ -212,6 +225,7 @@ class SignRawTransactionsTest(BitcoinTestFramework): self.script_verification_error_test() self.witness_script_test() self.test_with_lock_outputs() + self.test_fully_signed_tx() if __name__ == '__main__': |