Skip to content

Commit

Permalink
[backport#10953] [Refactor] Combine scriptPubKey and amount as CTxOut…
Browse files Browse the repository at this point in the history
… in CScriptCheck

Summary:
3a131b7 Rename out to m_tx_out in CScriptCheck (Johnson Lau)
e912118 [Refactor] Combine scriptPubKey and amount as CTxOut in CScriptCheck (Johnson Lau)

Pull request description:

  This simplifies CScriptCheck by combining scriptPubKey and amount

---

Backport of Core [[bitcoin/bitcoin#10953 | PR10953]]

Test Plan:
  ninja check && ninja check-functional

Reviewers: #bitcoin_abc, Fabien

Reviewed By: #bitcoin_abc, Fabien

Differential Revision: https://reviews.bitcoinabc.org/D6932
  • Loading branch information
sipa authored and majcosta committed Jul 15, 2020
1 parent 5153554 commit fddb00f
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 27 deletions.
12 changes: 6 additions & 6 deletions src/test/script_p2sh_tests.cpp
Expand Up @@ -115,12 +115,12 @@ BOOST_AUTO_TEST_CASE(sign) {
for (int j = 0; j < 8; j++) {
CScript sigSave = txTo[i].vin[0].scriptSig;
txTo[i].vin[0].scriptSig = txTo[j].vin[0].scriptSig;
const CTxOut &output = txFrom.vout[txTo[i].vin[0].prevout.GetN()];
bool sigOK = CScriptCheck(
output.scriptPubKey, output.nValue, CTransaction(txTo[i]), 0,
SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_STRICTENC |
SCRIPT_ENABLE_SIGHASH_FORKID,
false, txdata)();
bool sigOK =
CScriptCheck(txFrom.vout[txTo[i].vin[0].prevout.GetN()],
CTransaction(txTo[i]), 0,
SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_STRICTENC |
SCRIPT_ENABLE_SIGHASH_FORKID,
false, txdata)();
if (i == j) {
BOOST_CHECK_MESSAGE(sigOK,
strprintf("VerifySignature %d %d", i, j));
Expand Down
3 changes: 1 addition & 2 deletions src/test/transaction_tests.cpp
Expand Up @@ -488,8 +488,7 @@ BOOST_AUTO_TEST_CASE(test_big_transaction) {

for (size_t i = 0; i < mtx.vin.size(); i++) {
std::vector<CScriptCheck> vChecks;
CTxOut &out = coins[tx.vin[i].prevout.GetN()].GetTxOut();
CScriptCheck check(out.scriptPubKey, out.nValue, tx, i,
CScriptCheck check(coins[tx.vin[i].prevout.GetN()].GetTxOut(), tx, i,
STANDARD_SCRIPT_VERIFY_FLAGS, false, txdata);
vChecks.push_back(CScriptCheck());
check.swap(vChecks.back());
Expand Down
14 changes: 6 additions & 8 deletions src/validation.cpp
Expand Up @@ -996,9 +996,9 @@ void UpdateCoins(CCoinsViewCache &view, const CTransaction &tx, int nHeight) {

bool CScriptCheck::operator()() {
const CScript &scriptSig = ptxTo->vin[nIn].scriptSig;
if (!VerifyScript(scriptSig, scriptPubKey, nFlags,
CachingTransactionSignatureChecker(ptxTo, nIn, amount,
cacheStore, txdata),
if (!VerifyScript(scriptSig, m_tx_out.scriptPubKey, nFlags,
CachingTransactionSignatureChecker(
ptxTo, nIn, m_tx_out.nValue, cacheStore, txdata),
metrics, &error)) {
return false;
}
Expand Down Expand Up @@ -1072,12 +1072,10 @@ bool CheckInputs(const CTransaction &tx, TxValidationState &state,
// check that our caching is not introducing consensus failures through
// additional data in, eg, the coins being spent being checked as a part
// of CScriptCheck.
const CScript &scriptPubKey = coin.GetTxOut().scriptPubKey;
const Amount amount = coin.GetTxOut().nValue;

// Verify signature
CScriptCheck check(scriptPubKey, amount, tx, i, flags, sigCacheStore,
txdata, &txLimitSigChecks, pBlockLimitSigChecks);
CScriptCheck check(coin.GetTxOut(), tx, i, flags, sigCacheStore, txdata,
&txLimitSigChecks, pBlockLimitSigChecks);
if (pvChecks) {
pvChecks->push_back(std::move(check));
} else if (!check()) {
Expand All @@ -1094,7 +1092,7 @@ bool CheckInputs(const CTransaction &tx, TxValidationState &state,
// NOT_STANDARD instead of CONSENSUS to avoid downstream users
// splitting the network between upgraded and non-upgraded nodes
// by banning CONSENSUS-failing data providers.
CScriptCheck check2(scriptPubKey, amount, tx, i, mandatoryFlags,
CScriptCheck check2(coin.GetTxOut(), tx, i, mandatoryFlags,
sigCacheStore, txdata);
if (check2()) {
return state.Invalid(
Expand Down
18 changes: 7 additions & 11 deletions src/validation.h
Expand Up @@ -581,8 +581,7 @@ bool CheckSequenceLocks(const CTxMemPool &pool, const CTransaction &tx,
*/
class CScriptCheck {
private:
CScript scriptPubKey;
Amount amount;
CTxOut m_tx_out;
const CTransaction *ptxTo;
unsigned int nIn;
uint32_t nFlags;
Expand All @@ -595,28 +594,25 @@ class CScriptCheck {

public:
CScriptCheck()
: amount(), ptxTo(nullptr), nIn(0), nFlags(0), cacheStore(false),
: ptxTo(nullptr), nIn(0), nFlags(0), cacheStore(false),
error(ScriptError::UNKNOWN), txdata(), pTxLimitSigChecks(nullptr),
pBlockLimitSigChecks(nullptr) {}

CScriptCheck(const CScript &scriptPubKeyIn, const Amount amountIn,
const CTransaction &txToIn, unsigned int nInIn,
uint32_t nFlagsIn, bool cacheIn,
CScriptCheck(const CTxOut &outIn, const CTransaction &txToIn,
unsigned int nInIn, uint32_t nFlagsIn, bool cacheIn,
const PrecomputedTransactionData &txdataIn,
TxSigCheckLimiter *pTxLimitSigChecksIn = nullptr,
CheckInputsLimiter *pBlockLimitSigChecksIn = nullptr)
: scriptPubKey(scriptPubKeyIn), amount(amountIn), ptxTo(&txToIn),
nIn(nInIn), nFlags(nFlagsIn), cacheStore(cacheIn),
error(ScriptError::UNKNOWN), txdata(txdataIn),
: m_tx_out(outIn), ptxTo(&txToIn), nIn(nInIn), nFlags(nFlagsIn),
cacheStore(cacheIn), error(ScriptError::UNKNOWN), txdata(txdataIn),
pTxLimitSigChecks(pTxLimitSigChecksIn),
pBlockLimitSigChecks(pBlockLimitSigChecksIn) {}

bool operator()();

void swap(CScriptCheck &check) {
scriptPubKey.swap(check.scriptPubKey);
std::swap(ptxTo, check.ptxTo);
std::swap(amount, check.amount);
std::swap(m_tx_out, check.m_tx_out);
std::swap(nIn, check.nIn);
std::swap(nFlags, check.nFlags);
std::swap(cacheStore, check.cacheStore);
Expand Down

0 comments on commit fddb00f

Please sign in to comment.