phantom/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-04-29 14:59:39 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions 1

Merge #19740: [0.20] wallet: Simplify and fix CWallet::SignTransaction

Browse source 
6a326cf66f tests: Test that a fully signed tx given to signrawtx is unchanged (Andrew Chow)
2d48d7dcfb 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 6a326cf66f
  MarcoFalke:
    re-ACK 6a326cf66f 👓

Tree-SHA512: 7b8e04cfc264de01a2de08a99477c1a110fa0965dcbd4305bf4632a165f28b090746789195f5cb584eb6d85e27d4801a09d2dad28e508d7898c7c088e771812b
This commit is contained in:
MarcoFalke 2020-12-10 20:06:28 +01:00
commit 953dddbd20
No known key found for this signature in database
GPG key ID: D2EA4850E7528B25
2 changed files with 22 additions and 42 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

50
src/wallet/wallet.cpp
View file

@ -2474,50 +2474,16 @@ bool CWallet::SignTransaction(CMutableTransaction& tx) const
bool CWallet::SignTransaction(CMutableTransaction& tx, const std::map<COutPoint, Coin>& coins, int sighash, std::map<int, std::string>& 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<uint256> 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<ScriptPubKeyMan*> 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;
}

14
test/functional/rpc_signrawtransaction.py
View file

@ -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__':