From 2d48d7dcfb93eeec36dd6f5cbad289b89ec69324 Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Sun, 16 Aug 2020 21:35:23 -0400 Subject: [PATCH 1/2] Simplify and fix CWallet::SignTransaction Backport CWallet::SignTransaction from master which is simpler and not broken. --- src/wallet/wallet.cpp | 50 +++++++------------------------------------ 1 file changed, 8 insertions(+), 42 deletions(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index af3f9583497..bd2b0145b83 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2474,50 +2474,16 @@ bool CWallet::SignTransaction(CMutableTransaction& tx) const bool CWallet::SignTransaction(CMutableTransaction& tx, const std::map& coins, int sighash, std::map& input_errors) const { - // Sign the tx with ScriptPubKeyMans - // Because each ScriptPubKeyMan can sign more than one input, we need to keep track of each ScriptPubKeyMan that has signed this transaction. - // Each iteration, we may sign more txins than the txin that is specified in that iteration. - // We assume that each input is signed by only one ScriptPubKeyMan. - std::set visited_spk_mans; - for (unsigned int i = 0; i < tx.vin.size(); i++) { - // Get the prevout - CTxIn& txin = tx.vin[i]; - auto coin = coins.find(txin.prevout); - if (coin == coins.end() || coin->second.IsSpent()) { - input_errors[i] = "Input not found or already spent"; - continue; - } - - // Check if this input is complete - SignatureData sigdata = DataFromTransaction(tx, i, coin->second.out); - if (sigdata.complete) { - continue; - } - - // Input needs to be signed, find the right ScriptPubKeyMan - std::set spk_mans = GetScriptPubKeyMans(coin->second.out.scriptPubKey, sigdata); - if (spk_mans.size() == 0) { - input_errors[i] = "Unable to sign input, missing keys"; - continue; - } - - for (auto& spk_man : spk_mans) { - // If we've already been signed by this spk_man, skip it - if (visited_spk_mans.count(spk_man->GetID()) > 0) { - continue; - } - - // Sign the tx. - // spk_man->SignTransaction will return true if the transaction is complete, - // so we can exit early and return true if that happens. - if (spk_man->SignTransaction(tx, coins, sighash, input_errors)) { - return true; - } - - // Add this spk_man to visited_spk_mans so we can skip it later - visited_spk_mans.insert(spk_man->GetID()); + // Try to sign with all ScriptPubKeyMans + for (ScriptPubKeyMan* spk_man : GetAllScriptPubKeyMans()) { + // spk_man->SignTransaction will return true if the transaction is complete, + // so we can exit early and return true if that happens + if (spk_man->SignTransaction(tx, coins, sighash, input_errors)) { + return true; } } + + // At this point, one input was not fully signed otherwise we would have exited already return false; } From 6a326cf66f04948ffe729e5540a69e159a7840ad Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Thu, 3 Dec 2020 14:43:03 -0500 Subject: [PATCH 2/2] tests: Test that a fully signed tx given to signrawtx is unchanged Tests that a fully signed transaction given to signrawtransactionwithwallet is both unchanged and marked as complete. This tests for a regression in 0.20 where the transaction would not be marked as complete. Github-Pull: #20562 Rebased-From: 773c42b265fb2212b5cb8785b7226a206d063543 --- test/functional/rpc_signrawtransaction.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/test/functional/rpc_signrawtransaction.py b/test/functional/rpc_signrawtransaction.py index 17686f3a785..d76a29fd995 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__':