Merge #13269: refactoring: Drop UpdateTransaction in favor of UpdateInput

6aa33feadb Drop UpdateTransaction in favor of UpdateInput (Ben Woosley)

Pull request description:

  Updating the input explicitly requires the caller to present a mutable
  input, which more clearly communicates the effects and intent of the call
  (and, often, the enclosing loop).

  In most cases, this input is already immediately available and need not be
  looked up.

Tree-SHA512: 8c7914a8b7ae975d8ad0e9d760e3c5da65776a5f79d060b8ffb6b3ff7a32235f71ad705f2185b368d9263742d7796bb562395d22b806d90e8502d8c496011e57
This commit is contained in:
Wladimir J. van der Laan 2018-06-05 18:46:12 +02:00
commit f0fd39f376
No known key found for this signature in database
GPG key ID: 1E4AED62986CD25D
7 changed files with 14 additions and 21 deletions

View file

@ -637,7 +637,7 @@ static void MutateTxSign(CMutableTransaction& tx, const std::string& flagStr)
// Sign what we can: // Sign what we can:
for (unsigned int i = 0; i < mergedTx.vin.size(); i++) { for (unsigned int i = 0; i < mergedTx.vin.size(); i++) {
const CTxIn& txin = mergedTx.vin[i]; CTxIn& txin = mergedTx.vin[i];
const Coin& coin = view.AccessCoin(txin.prevout); const Coin& coin = view.AccessCoin(txin.prevout);
if (coin.IsSpent()) { if (coin.IsSpent()) {
continue; continue;
@ -652,7 +652,7 @@ static void MutateTxSign(CMutableTransaction& tx, const std::string& flagStr)
// ... and merge in other signatures: // ... and merge in other signatures:
sigdata = CombineSignatures(prevPubKey, MutableTransactionSignatureChecker(&mergedTx, i, amount), sigdata, DataFromTransaction(txv, i)); sigdata = CombineSignatures(prevPubKey, MutableTransactionSignatureChecker(&mergedTx, i, amount), sigdata, DataFromTransaction(txv, i));
UpdateTransaction(mergedTx, i, sigdata); UpdateInput(txin, sigdata);
} }
tx = mergedTx; tx = mergedTx;

View file

@ -748,7 +748,7 @@ static UniValue combinerawtransaction(const JSONRPCRequest& request)
} }
} }
UpdateTransaction(mergedTx, i, sigdata); UpdateInput(txin, sigdata);
} }
return EncodeHexTx(mergedTx); return EncodeHexTx(mergedTx);
@ -882,7 +882,7 @@ UniValue SignTransaction(CMutableTransaction& mtx, const UniValue& prevTxsUnival
} }
sigdata = CombineSignatures(prevPubKey, TransactionSignatureChecker(&txConst, i, amount), sigdata, DataFromTransaction(mtx, i)); sigdata = CombineSignatures(prevPubKey, TransactionSignatureChecker(&txConst, i, amount), sigdata, DataFromTransaction(mtx, i));
UpdateTransaction(mtx, i, sigdata); UpdateInput(txin, sigdata);
ScriptError serror = SCRIPT_ERR_OK; ScriptError serror = SCRIPT_ERR_OK;
if (!VerifyScript(txin.scriptSig, prevPubKey, &txin.scriptWitness, STANDARD_SCRIPT_VERIFY_FLAGS, TransactionSignatureChecker(&txConst, i, amount), &serror)) { if (!VerifyScript(txin.scriptSig, prevPubKey, &txin.scriptWitness, STANDARD_SCRIPT_VERIFY_FLAGS, TransactionSignatureChecker(&txConst, i, amount), &serror)) {

View file

@ -199,12 +199,6 @@ void UpdateInput(CTxIn& input, const SignatureData& data)
input.scriptWitness = data.scriptWitness; input.scriptWitness = data.scriptWitness;
} }
void UpdateTransaction(CMutableTransaction& tx, unsigned int nIn, const SignatureData& data)
{
assert(tx.vin.size() > nIn);
UpdateInput(tx.vin[nIn], data);
}
bool SignSignature(const SigningProvider &provider, const CScript& fromPubKey, CMutableTransaction& txTo, unsigned int nIn, const CAmount& amount, int nHashType) bool SignSignature(const SigningProvider &provider, const CScript& fromPubKey, CMutableTransaction& txTo, unsigned int nIn, const CAmount& amount, int nHashType)
{ {
assert(nIn < txTo.vin.size()); assert(nIn < txTo.vin.size());
@ -213,7 +207,7 @@ bool SignSignature(const SigningProvider &provider, const CScript& fromPubKey, C
SignatureData sigdata; SignatureData sigdata;
bool ret = ProduceSignature(provider, creator, fromPubKey, sigdata); bool ret = ProduceSignature(provider, creator, fromPubKey, sigdata);
UpdateTransaction(txTo, nIn, sigdata); UpdateInput(txTo.vin.at(nIn), sigdata);
return ret; return ret;
} }

View file

@ -73,7 +73,6 @@ SignatureData CombineSignatures(const CScript& scriptPubKey, const BaseSignature
/** Extract signature data from a transaction, and insert it. */ /** Extract signature data from a transaction, and insert it. */
SignatureData DataFromTransaction(const CMutableTransaction& tx, unsigned int nIn); SignatureData DataFromTransaction(const CMutableTransaction& tx, unsigned int nIn);
void UpdateTransaction(CMutableTransaction& tx, unsigned int nIn, const SignatureData& data);
void UpdateInput(CTxIn& input, const SignatureData& data); void UpdateInput(CTxIn& input, const SignatureData& data);
/* Check whether we know how to sign for an output like this, assuming we /* Check whether we know how to sign for an output like this, assuming we

View file

@ -629,7 +629,7 @@ BOOST_AUTO_TEST_CASE(test_witness)
CreateCreditAndSpend(keystore2, scriptMulti, output2, input2, false); CreateCreditAndSpend(keystore2, scriptMulti, output2, input2, false);
CheckWithFlag(output2, input2, 0, false); CheckWithFlag(output2, input2, 0, false);
BOOST_CHECK(*output1 == *output2); BOOST_CHECK(*output1 == *output2);
UpdateTransaction(input1, 0, CombineSignatures(output1->vout[0].scriptPubKey, MutableTransactionSignatureChecker(&input1, 0, output1->vout[0].nValue), DataFromTransaction(input1, 0), DataFromTransaction(input2, 0))); UpdateInput(input1.vin[0], CombineSignatures(output1->vout[0].scriptPubKey, MutableTransactionSignatureChecker(&input1, 0, output1->vout[0].nValue), DataFromTransaction(input1, 0), DataFromTransaction(input2, 0)));
CheckWithFlag(output1, input1, STANDARD_SCRIPT_VERIFY_FLAGS, true); CheckWithFlag(output1, input1, STANDARD_SCRIPT_VERIFY_FLAGS, true);
// P2SH 2-of-2 multisig // P2SH 2-of-2 multisig
@ -640,7 +640,7 @@ BOOST_AUTO_TEST_CASE(test_witness)
CheckWithFlag(output2, input2, 0, true); CheckWithFlag(output2, input2, 0, true);
CheckWithFlag(output2, input2, SCRIPT_VERIFY_P2SH, false); CheckWithFlag(output2, input2, SCRIPT_VERIFY_P2SH, false);
BOOST_CHECK(*output1 == *output2); BOOST_CHECK(*output1 == *output2);
UpdateTransaction(input1, 0, CombineSignatures(output1->vout[0].scriptPubKey, MutableTransactionSignatureChecker(&input1, 0, output1->vout[0].nValue), DataFromTransaction(input1, 0), DataFromTransaction(input2, 0))); UpdateInput(input1.vin[0], CombineSignatures(output1->vout[0].scriptPubKey, MutableTransactionSignatureChecker(&input1, 0, output1->vout[0].nValue), DataFromTransaction(input1, 0), DataFromTransaction(input2, 0)));
CheckWithFlag(output1, input1, SCRIPT_VERIFY_P2SH, true); CheckWithFlag(output1, input1, SCRIPT_VERIFY_P2SH, true);
CheckWithFlag(output1, input1, STANDARD_SCRIPT_VERIFY_FLAGS, true); CheckWithFlag(output1, input1, STANDARD_SCRIPT_VERIFY_FLAGS, true);
@ -652,7 +652,7 @@ BOOST_AUTO_TEST_CASE(test_witness)
CheckWithFlag(output2, input2, 0, true); CheckWithFlag(output2, input2, 0, true);
CheckWithFlag(output2, input2, SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_WITNESS, false); CheckWithFlag(output2, input2, SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_WITNESS, false);
BOOST_CHECK(*output1 == *output2); BOOST_CHECK(*output1 == *output2);
UpdateTransaction(input1, 0, CombineSignatures(output1->vout[0].scriptPubKey, MutableTransactionSignatureChecker(&input1, 0, output1->vout[0].nValue), DataFromTransaction(input1, 0), DataFromTransaction(input2, 0))); UpdateInput(input1.vin[0], CombineSignatures(output1->vout[0].scriptPubKey, MutableTransactionSignatureChecker(&input1, 0, output1->vout[0].nValue), DataFromTransaction(input1, 0), DataFromTransaction(input2, 0)));
CheckWithFlag(output1, input1, SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_WITNESS, true); CheckWithFlag(output1, input1, SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_WITNESS, true);
CheckWithFlag(output1, input1, STANDARD_SCRIPT_VERIFY_FLAGS, true); CheckWithFlag(output1, input1, STANDARD_SCRIPT_VERIFY_FLAGS, true);
@ -664,7 +664,7 @@ BOOST_AUTO_TEST_CASE(test_witness)
CheckWithFlag(output2, input2, SCRIPT_VERIFY_P2SH, true); CheckWithFlag(output2, input2, SCRIPT_VERIFY_P2SH, true);
CheckWithFlag(output2, input2, SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_WITNESS, false); CheckWithFlag(output2, input2, SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_WITNESS, false);
BOOST_CHECK(*output1 == *output2); BOOST_CHECK(*output1 == *output2);
UpdateTransaction(input1, 0, CombineSignatures(output1->vout[0].scriptPubKey, MutableTransactionSignatureChecker(&input1, 0, output1->vout[0].nValue), DataFromTransaction(input1, 0), DataFromTransaction(input2, 0))); UpdateInput(input1.vin[0], CombineSignatures(output1->vout[0].scriptPubKey, MutableTransactionSignatureChecker(&input1, 0, output1->vout[0].nValue), DataFromTransaction(input1, 0), DataFromTransaction(input2, 0)));
CheckWithFlag(output1, input1, SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_WITNESS, true); CheckWithFlag(output1, input1, SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_WITNESS, true);
CheckWithFlag(output1, input1, STANDARD_SCRIPT_VERIFY_FLAGS, true); CheckWithFlag(output1, input1, STANDARD_SCRIPT_VERIFY_FLAGS, true);
} }

View file

@ -315,7 +315,7 @@ BOOST_FIXTURE_TEST_CASE(checkinputs_test, TestChain100Setup)
// Sign // Sign
SignatureData sigdata; SignatureData sigdata;
ProduceSignature(keystore, MutableTransactionSignatureCreator(&valid_with_witness_tx, 0, 11*CENT, SIGHASH_ALL), spend_tx.vout[1].scriptPubKey, sigdata); ProduceSignature(keystore, MutableTransactionSignatureCreator(&valid_with_witness_tx, 0, 11*CENT, SIGHASH_ALL), spend_tx.vout[1].scriptPubKey, sigdata);
UpdateTransaction(valid_with_witness_tx, 0, sigdata); UpdateInput(valid_with_witness_tx.vin[0], sigdata);
// This should be valid under all script flags. // This should be valid under all script flags.
ValidateCheckInputsForAllFlags(valid_with_witness_tx, 0, true); ValidateCheckInputsForAllFlags(valid_with_witness_tx, 0, true);
@ -343,7 +343,7 @@ BOOST_FIXTURE_TEST_CASE(checkinputs_test, TestChain100Setup)
for (int i=0; i<2; ++i) { for (int i=0; i<2; ++i) {
SignatureData sigdata; SignatureData sigdata;
ProduceSignature(keystore, MutableTransactionSignatureCreator(&tx, i, 11*CENT, SIGHASH_ALL), spend_tx.vout[i].scriptPubKey, sigdata); ProduceSignature(keystore, MutableTransactionSignatureCreator(&tx, i, 11*CENT, SIGHASH_ALL), spend_tx.vout[i].scriptPubKey, sigdata);
UpdateTransaction(tx, i, sigdata); UpdateInput(tx.vin[i], sigdata);
} }
// This should be valid under all script flags // This should be valid under all script flags

View file

@ -2609,7 +2609,7 @@ bool CWallet::SignTransaction(CMutableTransaction &tx)
// sign the new tx // sign the new tx
int nIn = 0; int nIn = 0;
for (const auto& input : tx.vin) { for (auto& input : tx.vin) {
std::map<uint256, CWalletTx>::const_iterator mi = mapWallet.find(input.prevout.hash); std::map<uint256, CWalletTx>::const_iterator mi = mapWallet.find(input.prevout.hash);
if(mi == mapWallet.end() || input.prevout.n >= mi->second.tx->vout.size()) { if(mi == mapWallet.end() || input.prevout.n >= mi->second.tx->vout.size()) {
return false; return false;
@ -2620,7 +2620,7 @@ bool CWallet::SignTransaction(CMutableTransaction &tx)
if (!ProduceSignature(*this, MutableTransactionSignatureCreator(&tx, nIn, amount, SIGHASH_ALL), scriptPubKey, sigdata)) { if (!ProduceSignature(*this, MutableTransactionSignatureCreator(&tx, nIn, amount, SIGHASH_ALL), scriptPubKey, sigdata)) {
return false; return false;
} }
UpdateTransaction(tx, nIn, sigdata); UpdateInput(input, sigdata);
nIn++; nIn++;
} }
return true; return true;
@ -3050,7 +3050,7 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CTransac
strFailReason = _("Signing transaction failed"); strFailReason = _("Signing transaction failed");
return false; return false;
} else { } else {
UpdateTransaction(txNew, nIn, sigdata); UpdateInput(txNew.vin.at(nIn), sigdata);
} }
nIn++; nIn++;