Skip to content

Commit f0fd39f

Browse files
committed
Merge #13269: refactoring: Drop UpdateTransaction in favor of UpdateInput
6aa33fe 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
2 parents 2140f6c + 6aa33fe commit f0fd39f

File tree

7 files changed

+14
-21
lines changed

7 files changed

+14
-21
lines changed

src/bitcoin-tx.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -637,7 +637,7 @@ static void MutateTxSign(CMutableTransaction& tx, const std::string& flagStr)
637637

638638
// Sign what we can:
639639
for (unsigned int i = 0; i < mergedTx.vin.size(); i++) {
640-
const CTxIn& txin = mergedTx.vin[i];
640+
CTxIn& txin = mergedTx.vin[i];
641641
const Coin& coin = view.AccessCoin(txin.prevout);
642642
if (coin.IsSpent()) {
643643
continue;
@@ -652,7 +652,7 @@ static void MutateTxSign(CMutableTransaction& tx, const std::string& flagStr)
652652

653653
// ... and merge in other signatures:
654654
sigdata = CombineSignatures(prevPubKey, MutableTransactionSignatureChecker(&mergedTx, i, amount), sigdata, DataFromTransaction(txv, i));
655-
UpdateTransaction(mergedTx, i, sigdata);
655+
UpdateInput(txin, sigdata);
656656
}
657657

658658
tx = mergedTx;

src/rpc/rawtransaction.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -748,7 +748,7 @@ static UniValue combinerawtransaction(const JSONRPCRequest& request)
748748
}
749749
}
750750

751-
UpdateTransaction(mergedTx, i, sigdata);
751+
UpdateInput(txin, sigdata);
752752
}
753753

754754
return EncodeHexTx(mergedTx);
@@ -882,7 +882,7 @@ UniValue SignTransaction(CMutableTransaction& mtx, const UniValue& prevTxsUnival
882882
}
883883
sigdata = CombineSignatures(prevPubKey, TransactionSignatureChecker(&txConst, i, amount), sigdata, DataFromTransaction(mtx, i));
884884

885-
UpdateTransaction(mtx, i, sigdata);
885+
UpdateInput(txin, sigdata);
886886

887887
ScriptError serror = SCRIPT_ERR_OK;
888888
if (!VerifyScript(txin.scriptSig, prevPubKey, &txin.scriptWitness, STANDARD_SCRIPT_VERIFY_FLAGS, TransactionSignatureChecker(&txConst, i, amount), &serror)) {

src/script/sign.cpp

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -199,12 +199,6 @@ void UpdateInput(CTxIn& input, const SignatureData& data)
199199
input.scriptWitness = data.scriptWitness;
200200
}
201201

202-
void UpdateTransaction(CMutableTransaction& tx, unsigned int nIn, const SignatureData& data)
203-
{
204-
assert(tx.vin.size() > nIn);
205-
UpdateInput(tx.vin[nIn], data);
206-
}
207-
208202
bool SignSignature(const SigningProvider &provider, const CScript& fromPubKey, CMutableTransaction& txTo, unsigned int nIn, const CAmount& amount, int nHashType)
209203
{
210204
assert(nIn < txTo.vin.size());
@@ -213,7 +207,7 @@ bool SignSignature(const SigningProvider &provider, const CScript& fromPubKey, C
213207

214208
SignatureData sigdata;
215209
bool ret = ProduceSignature(provider, creator, fromPubKey, sigdata);
216-
UpdateTransaction(txTo, nIn, sigdata);
210+
UpdateInput(txTo.vin.at(nIn), sigdata);
217211
return ret;
218212
}
219213

src/script/sign.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,6 @@ SignatureData CombineSignatures(const CScript& scriptPubKey, const BaseSignature
7373

7474
/** Extract signature data from a transaction, and insert it. */
7575
SignatureData DataFromTransaction(const CMutableTransaction& tx, unsigned int nIn);
76-
void UpdateTransaction(CMutableTransaction& tx, unsigned int nIn, const SignatureData& data);
7776
void UpdateInput(CTxIn& input, const SignatureData& data);
7877

7978
/* Check whether we know how to sign for an output like this, assuming we

src/test/transaction_tests.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -629,7 +629,7 @@ BOOST_AUTO_TEST_CASE(test_witness)
629629
CreateCreditAndSpend(keystore2, scriptMulti, output2, input2, false);
630630
CheckWithFlag(output2, input2, 0, false);
631631
BOOST_CHECK(*output1 == *output2);
632-
UpdateTransaction(input1, 0, CombineSignatures(output1->vout[0].scriptPubKey, MutableTransactionSignatureChecker(&input1, 0, output1->vout[0].nValue), DataFromTransaction(input1, 0), DataFromTransaction(input2, 0)));
632+
UpdateInput(input1.vin[0], CombineSignatures(output1->vout[0].scriptPubKey, MutableTransactionSignatureChecker(&input1, 0, output1->vout[0].nValue), DataFromTransaction(input1, 0), DataFromTransaction(input2, 0)));
633633
CheckWithFlag(output1, input1, STANDARD_SCRIPT_VERIFY_FLAGS, true);
634634

635635
// P2SH 2-of-2 multisig
@@ -640,7 +640,7 @@ BOOST_AUTO_TEST_CASE(test_witness)
640640
CheckWithFlag(output2, input2, 0, true);
641641
CheckWithFlag(output2, input2, SCRIPT_VERIFY_P2SH, false);
642642
BOOST_CHECK(*output1 == *output2);
643-
UpdateTransaction(input1, 0, CombineSignatures(output1->vout[0].scriptPubKey, MutableTransactionSignatureChecker(&input1, 0, output1->vout[0].nValue), DataFromTransaction(input1, 0), DataFromTransaction(input2, 0)));
643+
UpdateInput(input1.vin[0], CombineSignatures(output1->vout[0].scriptPubKey, MutableTransactionSignatureChecker(&input1, 0, output1->vout[0].nValue), DataFromTransaction(input1, 0), DataFromTransaction(input2, 0)));
644644
CheckWithFlag(output1, input1, SCRIPT_VERIFY_P2SH, true);
645645
CheckWithFlag(output1, input1, STANDARD_SCRIPT_VERIFY_FLAGS, true);
646646

@@ -652,7 +652,7 @@ BOOST_AUTO_TEST_CASE(test_witness)
652652
CheckWithFlag(output2, input2, 0, true);
653653
CheckWithFlag(output2, input2, SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_WITNESS, false);
654654
BOOST_CHECK(*output1 == *output2);
655-
UpdateTransaction(input1, 0, CombineSignatures(output1->vout[0].scriptPubKey, MutableTransactionSignatureChecker(&input1, 0, output1->vout[0].nValue), DataFromTransaction(input1, 0), DataFromTransaction(input2, 0)));
655+
UpdateInput(input1.vin[0], CombineSignatures(output1->vout[0].scriptPubKey, MutableTransactionSignatureChecker(&input1, 0, output1->vout[0].nValue), DataFromTransaction(input1, 0), DataFromTransaction(input2, 0)));
656656
CheckWithFlag(output1, input1, SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_WITNESS, true);
657657
CheckWithFlag(output1, input1, STANDARD_SCRIPT_VERIFY_FLAGS, true);
658658

@@ -664,7 +664,7 @@ BOOST_AUTO_TEST_CASE(test_witness)
664664
CheckWithFlag(output2, input2, SCRIPT_VERIFY_P2SH, true);
665665
CheckWithFlag(output2, input2, SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_WITNESS, false);
666666
BOOST_CHECK(*output1 == *output2);
667-
UpdateTransaction(input1, 0, CombineSignatures(output1->vout[0].scriptPubKey, MutableTransactionSignatureChecker(&input1, 0, output1->vout[0].nValue), DataFromTransaction(input1, 0), DataFromTransaction(input2, 0)));
667+
UpdateInput(input1.vin[0], CombineSignatures(output1->vout[0].scriptPubKey, MutableTransactionSignatureChecker(&input1, 0, output1->vout[0].nValue), DataFromTransaction(input1, 0), DataFromTransaction(input2, 0)));
668668
CheckWithFlag(output1, input1, SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_WITNESS, true);
669669
CheckWithFlag(output1, input1, STANDARD_SCRIPT_VERIFY_FLAGS, true);
670670
}

src/test/txvalidationcache_tests.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -315,7 +315,7 @@ BOOST_FIXTURE_TEST_CASE(checkinputs_test, TestChain100Setup)
315315
// Sign
316316
SignatureData sigdata;
317317
ProduceSignature(keystore, MutableTransactionSignatureCreator(&valid_with_witness_tx, 0, 11*CENT, SIGHASH_ALL), spend_tx.vout[1].scriptPubKey, sigdata);
318-
UpdateTransaction(valid_with_witness_tx, 0, sigdata);
318+
UpdateInput(valid_with_witness_tx.vin[0], sigdata);
319319

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

349349
// This should be valid under all script flags

src/wallet/wallet.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2609,7 +2609,7 @@ bool CWallet::SignTransaction(CMutableTransaction &tx)
26092609

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

30563056
nIn++;

0 commit comments

Comments
 (0)