Drop UpdateTransaction in favor of UpdateInput

Updating the input explicitly requires the caller to present a mutable
input, which more clearly communicates the effects and intent of the method.

In most cases, this input is already immediately available and need not be
looked up.
This commit is contained in:
Ben Woosley 2018-05-17 17:54:18 -07:00
parent 1b53e4f67c
commit 6aa33feadb
No known key found for this signature in database
GPG key ID: 4D8CA4BA18040906
7 changed files with 14 additions and 21 deletions

View file

@ -629,7 +629,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;
@ -644,7 +644,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());
@ -214,7 +208,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

@ -80,7 +80,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

@ -2608,7 +2608,7 @@ bool CWallet::SignTransaction(CMutableTransaction &tx)
// sign the new tx // sign the new tx
CTransaction txNewConst(tx); CTransaction txNewConst(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;
@ -2619,7 +2619,7 @@ bool CWallet::SignTransaction(CMutableTransaction &tx)
if (!ProduceSignature(*this, TransactionSignatureCreator(&txNewConst, nIn, amount, SIGHASH_ALL), scriptPubKey, sigdata)) { if (!ProduceSignature(*this, TransactionSignatureCreator(&txNewConst, 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++;