Skip to content

Commit 168fa67

Browse files
committed
Use undo data in ConnectBlock
1 parent 817edfb commit 168fa67

File tree

14 files changed

+365
-178
lines changed

14 files changed

+365
-178
lines changed

src/bench/connectblock.cpp

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
#include <script/interpreter.h>
1010
#include <sync.h>
1111
#include <test/util/setup_common.h>
12+
#include <undo.h>
1213
#include <validation.h>
1314

1415
#include <cassert>
@@ -100,7 +101,11 @@ void BenchmarkConnectBlock(benchmark::Bench& bench, std::vector<CKey>& keys, std
100101
auto* pindex{chainman->m_blockman.AddToBlockIndex(test_block, chainman->m_best_header)}; // Doing this here doesn't impact the benchmark
101102
CCoinsViewCache viewNew{&chainstate.CoinsTip()};
102103

103-
assert(chainstate.ConnectBlock(test_block, test_block_state, pindex, viewNew));
104+
CBlockUndo blockundo;
105+
uint256 block_hash{test_block.GetHash()};
106+
assert(SpendBlock(block_hash, pindex, chainstate.m_chainman.GetParams(), test_block, viewNew, test_block_state, blockundo));
107+
108+
assert(chainstate.ConnectBlock(test_block, test_block_state, pindex, block_hash, blockundo));
104109
});
105110
}
106111

src/coins.cpp

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -302,6 +302,22 @@ bool CCoinsViewCache::HaveInputs(const CTransaction& tx) const
302302
return true;
303303
}
304304

305+
std::optional<std::vector<Coin>> CCoinsViewCache::GetInputs(const CTransaction& tx) const
306+
{
307+
if (tx.IsCoinBase()) {
308+
return {};
309+
}
310+
std::vector<Coin> coins;
311+
coins.reserve(tx.vin.size());
312+
for (const auto& txin : tx.vin) {
313+
const std::optional<Coin> coin = GetCoin(txin.prevout);
314+
if (!coin) return std::nullopt;
315+
Assume(!coin->IsSpent());
316+
coins.emplace_back(*coin);
317+
}
318+
return coins;
319+
}
320+
305321
void CCoinsViewCache::ReallocateCache()
306322
{
307323
// Cache should be empty when we're calling this.

src/coins.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -469,6 +469,8 @@ class CCoinsViewCache : public CCoinsViewBacked
469469
//! Check whether all prevouts of the transaction are present in the UTXO set represented by this view
470470
bool HaveInputs(const CTransaction& tx) const;
471471

472+
std::optional<std::vector<Coin>> GetInputs(const CTransaction& tx) const;
473+
472474
//! Force a reallocation of the cache map. This is required when downsizing
473475
//! the cache because the map's allocator may be hanging onto a lot of
474476
//! memory despite having called .clear().

src/consensus/tx_verify.cpp

Lines changed: 6 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -123,15 +123,15 @@ unsigned int GetLegacySigOpCount(const CTransaction& tx)
123123
return nSigOps;
124124
}
125125

126-
unsigned int GetP2SHSigOpCount(const CTransaction& tx, const CCoinsViewCache& inputs)
126+
unsigned int GetP2SHSigOpCount(const CTransaction& tx, std::span<const Coin> inputs)
127127
{
128128
if (tx.IsCoinBase())
129129
return 0;
130130

131131
unsigned int nSigOps = 0;
132132
for (unsigned int i = 0; i < tx.vin.size(); i++)
133133
{
134-
const Coin& coin = inputs.AccessCoin(tx.vin[i].prevout);
134+
const Coin& coin = inputs[i];
135135
assert(!coin.IsSpent());
136136
const CTxOut &prevout = coin.out;
137137
if (prevout.scriptPubKey.IsPayToScriptHash())
@@ -140,7 +140,7 @@ unsigned int GetP2SHSigOpCount(const CTransaction& tx, const CCoinsViewCache& in
140140
return nSigOps;
141141
}
142142

143-
int64_t GetTransactionSigOpCost(const CTransaction& tx, const CCoinsViewCache& inputs, uint32_t flags)
143+
int64_t GetTransactionSigOpCost(const CTransaction& tx, std::span<const Coin> inputs, uint32_t flags)
144144
{
145145
int64_t nSigOps = GetLegacySigOpCount(tx) * WITNESS_SCALE_FACTOR;
146146

@@ -153,26 +153,19 @@ int64_t GetTransactionSigOpCost(const CTransaction& tx, const CCoinsViewCache& i
153153

154154
for (unsigned int i = 0; i < tx.vin.size(); i++)
155155
{
156-
const Coin& coin = inputs.AccessCoin(tx.vin[i].prevout);
156+
const Coin& coin = inputs[i];
157157
assert(!coin.IsSpent());
158158
const CTxOut &prevout = coin.out;
159159
nSigOps += CountWitnessSigOps(tx.vin[i].scriptSig, prevout.scriptPubKey, &tx.vin[i].scriptWitness, flags);
160160
}
161161
return nSigOps;
162162
}
163163

164-
bool Consensus::CheckTxInputs(const CTransaction& tx, TxValidationState& state, const CCoinsViewCache& inputs, int nSpendHeight, CAmount& txfee)
164+
bool Consensus::CheckTxInputs(const CTransaction& tx, TxValidationState& state, std::span<const Coin> inputs, int nSpendHeight, CAmount& txfee)
165165
{
166-
// are the actual inputs available?
167-
if (!inputs.HaveInputs(tx)) {
168-
return state.Invalid(TxValidationResult::TX_MISSING_INPUTS, "bad-txns-inputs-missingorspent",
169-
strprintf("%s: inputs missing/spent", __func__));
170-
}
171-
172166
CAmount nValueIn = 0;
173167
for (unsigned int i = 0; i < tx.vin.size(); ++i) {
174-
const COutPoint &prevout = tx.vin[i].prevout;
175-
const Coin& coin = inputs.AccessCoin(prevout);
168+
const Coin& coin = inputs[i];
176169
assert(!coin.IsSpent());
177170

178171
// If prev is coinbase, check that it's matured

src/consensus/tx_verify.h

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
#include <consensus/amount.h>
99

1010
#include <stdint.h>
11+
#include <undo.h>
1112
#include <vector>
1213

1314
class CBlockIndex;
@@ -24,7 +25,7 @@ namespace Consensus {
2425
* @param[out] txfee Set to the transaction fee if successful.
2526
* Preconditions: tx.IsCoinBase() is false.
2627
*/
27-
[[nodiscard]] bool CheckTxInputs(const CTransaction& tx, TxValidationState& state, const CCoinsViewCache& inputs, int nSpendHeight, CAmount& txfee);
28+
[[nodiscard]] bool CheckTxInputs(const CTransaction& tx, TxValidationState& state, std::span<const Coin> inputs, int nSpendHeight, CAmount& txfee);
2829
} // namespace Consensus
2930

3031
/** Auxiliary functions for transaction validation (ideally should not be exposed) */
@@ -39,11 +40,11 @@ unsigned int GetLegacySigOpCount(const CTransaction& tx);
3940
/**
4041
* Count ECDSA signature operations in pay-to-script-hash inputs.
4142
*
42-
* @param[in] mapInputs Map of previous transactions that have outputs we're spending
43+
* @param[in] inputs Previous transactions that have outputs we're spending
4344
* @return maximum number of sigops required to validate this transaction's inputs
4445
* @see CTransaction::FetchInputs
4546
*/
46-
unsigned int GetP2SHSigOpCount(const CTransaction& tx, const CCoinsViewCache& mapInputs);
47+
unsigned int GetP2SHSigOpCount(const CTransaction& tx, std::span<const Coin> inputs);
4748

4849
/**
4950
* Compute total signature operation cost of a transaction.
@@ -52,7 +53,7 @@ unsigned int GetP2SHSigOpCount(const CTransaction& tx, const CCoinsViewCache& ma
5253
* @param[in] flags Script verification flags
5354
* @return Total signature operation cost of tx
5455
*/
55-
int64_t GetTransactionSigOpCost(const CTransaction& tx, const CCoinsViewCache& inputs, uint32_t flags);
56+
int64_t GetTransactionSigOpCost(const CTransaction& tx, std::span<const Coin> inputs, uint32_t flags);
5657

5758
/**
5859
* Check if transaction is final and can be included in a block with the

src/node/psbt.cpp

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,13 @@ PSBTAnalysis AnalyzePSBT(PartiallySignedTransaction psbtx)
137137

138138
if (success) {
139139
CTransaction ctx = CTransaction(mtx);
140-
size_t size(GetVirtualTransactionSize(ctx, GetTransactionSigOpCost(ctx, view, STANDARD_SCRIPT_VERIFY_FLAGS), ::nBytesPerSigOp));
140+
std::vector<Coin> spent_outputs;
141+
spent_outputs.reserve(ctx.vin.size());
142+
for (const auto& input : ctx.vin) {
143+
spent_outputs.emplace_back(view.AccessCoin(input.prevout));
144+
}
145+
146+
size_t size(GetVirtualTransactionSize(ctx, GetTransactionSigOpCost(ctx, spent_outputs, STANDARD_SCRIPT_VERIFY_FLAGS), ::nBytesPerSigOp));
141147
result.estimated_vsize = size;
142148
// Estimate fee rate
143149
CFeeRate feerate(fee, size);

src/test/coinstatsindex_tests.cpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
#include <test/util/index.h>
1010
#include <test/util/setup_common.h>
1111
#include <test/util/validation.h>
12+
#include <undo.h>
1213
#include <validation.h>
1314

1415
#include <boost/test/unit_test.hpp>
@@ -100,7 +101,9 @@ BOOST_FIXTURE_TEST_CASE(coinstatsindex_unclean_shutdown, TestChain100Setup)
100101
BOOST_CHECK(CheckBlock(block, state, params.GetConsensus()));
101102
BOOST_CHECK(m_node.chainman->AcceptBlock(new_block, state, &new_block_index, true, nullptr, nullptr, true));
102103
CCoinsViewCache view(&chainstate.CoinsTip());
103-
BOOST_CHECK(chainstate.ConnectBlock(block, state, new_block_index, view));
104+
CBlockUndo blockundo;
105+
assert(SpendBlock(block.GetHash(), new_block_index, chainstate.m_chainman.GetParams(), block, view, state, blockundo));
106+
BOOST_CHECK(chainstate.ConnectBlock(block, state, new_block_index, block.GetHash(), blockundo));
104107
}
105108
// Send block connected notification, then stop the index without
106109
// sending a chainstate flushed notification. Prior to #24138, this

src/test/fuzz/coins_view.cpp

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -255,7 +255,9 @@ FUZZ_TARGET(coins_view, .init = initialize_coins_view)
255255
// It is not allowed to call CheckTxInputs if CheckTransaction failed
256256
return;
257257
}
258-
if (Consensus::CheckTxInputs(transaction, state, coins_view_cache, fuzzed_data_provider.ConsumeIntegralInRange<int>(0, std::numeric_limits<int>::max()), tx_fee_out)) {
258+
std::optional<std::vector<Coin>> coins{coins_view_cache.GetInputs(transaction)};
259+
if (!coins) return;
260+
if (Consensus::CheckTxInputs(transaction, state, *coins, fuzzed_data_provider.ConsumeIntegralInRange<int>(0, std::numeric_limits<int>::max()), tx_fee_out)) {
259261
assert(MoneyRange(tx_fee_out));
260262
}
261263
},
@@ -266,7 +268,10 @@ FUZZ_TARGET(coins_view, .init = initialize_coins_view)
266268
// consensus/tx_verify.cpp:130: unsigned int GetP2SHSigOpCount(const CTransaction &, const CCoinsViewCache &): Assertion `!coin.IsSpent()' failed.
267269
return;
268270
}
269-
(void)GetP2SHSigOpCount(transaction, coins_view_cache);
271+
272+
std::optional<std::vector<Coin>> coins{coins_view_cache.GetInputs(transaction)};
273+
if (!coins) return;
274+
(void)GetP2SHSigOpCount(transaction, *coins);
270275
},
271276
[&] {
272277
const CTransaction transaction{random_mutable_transaction};
@@ -281,7 +286,9 @@ FUZZ_TARGET(coins_view, .init = initialize_coins_view)
281286
// script/interpreter.cpp:1705: size_t CountWitnessSigOps(const CScript &, const CScript &, const CScriptWitness *, unsigned int): Assertion `(flags & SCRIPT_VERIFY_P2SH) != 0' failed.
282287
return;
283288
}
284-
(void)GetTransactionSigOpCost(transaction, coins_view_cache, flags);
289+
std::optional<std::vector<Coin>> coins{coins_view_cache.GetInputs(transaction)};
290+
if (!coins) return;
291+
(void)GetTransactionSigOpCost(transaction, *coins, flags);
285292
},
286293
[&] {
287294
(void)IsWitnessStandard(CTransaction{random_mutable_transaction}, coins_view_cache);

src/test/script_p2sh_tests.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -362,7 +362,7 @@ BOOST_AUTO_TEST_CASE(AreInputsStandard)
362362

363363
BOOST_CHECK(::AreInputsStandard(CTransaction(txTo), coins));
364364
// 22 P2SH sigops for all inputs (1 for vin[0], 6 for vin[3], 15 for vin[4]
365-
BOOST_CHECK_EQUAL(GetP2SHSigOpCount(CTransaction(txTo), coins), 22U);
365+
BOOST_CHECK_EQUAL(GetP2SHSigOpCount(CTransaction(txTo), *(Assert(coins.GetInputs(CTransaction(txTo))))), 22U);
366366

367367
CMutableTransaction txToNonStd1;
368368
txToNonStd1.vout.resize(1);
@@ -374,7 +374,7 @@ BOOST_AUTO_TEST_CASE(AreInputsStandard)
374374
txToNonStd1.vin[0].scriptSig << std::vector<unsigned char>(sixteenSigops.begin(), sixteenSigops.end());
375375

376376
BOOST_CHECK(!::AreInputsStandard(CTransaction(txToNonStd1), coins));
377-
BOOST_CHECK_EQUAL(GetP2SHSigOpCount(CTransaction(txToNonStd1), coins), 16U);
377+
BOOST_CHECK_EQUAL(GetP2SHSigOpCount(CTransaction(txToNonStd1), *(Assert(coins.GetInputs(CTransaction(txToNonStd1))))), 16U);
378378

379379
CMutableTransaction txToNonStd2;
380380
txToNonStd2.vout.resize(1);
@@ -386,7 +386,7 @@ BOOST_AUTO_TEST_CASE(AreInputsStandard)
386386
txToNonStd2.vin[0].scriptSig << std::vector<unsigned char>(twentySigops.begin(), twentySigops.end());
387387

388388
BOOST_CHECK(!::AreInputsStandard(CTransaction(txToNonStd2), coins));
389-
BOOST_CHECK_EQUAL(GetP2SHSigOpCount(CTransaction(txToNonStd2), coins), 20U);
389+
BOOST_CHECK_EQUAL(GetP2SHSigOpCount(CTransaction(txToNonStd2), *(Assert(coins.GetInputs(CTransaction(txToNonStd2))))), 20U);
390390
}
391391

392392
BOOST_AUTO_TEST_SUITE_END()

src/test/sigopcount_tests.cpp

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -134,10 +134,10 @@ BOOST_AUTO_TEST_CASE(GetTxSigOpCost)
134134
// Legacy counting only includes signature operations in scriptSigs and scriptPubKeys
135135
// of a transaction and does not take the actual executed sig operations into account.
136136
// spendingTx in itself does not contain a signature operation.
137-
assert(GetTransactionSigOpCost(CTransaction(spendingTx), coins, flags) == 0);
137+
assert(GetTransactionSigOpCost(CTransaction(spendingTx), *Assert(coins.GetInputs(CTransaction(spendingTx))), flags) == 0);
138138
// creationTx contains two signature operations in its scriptPubKey, but legacy counting
139139
// is not accurate.
140-
assert(GetTransactionSigOpCost(CTransaction(creationTx), coins, flags) == MAX_PUBKEYS_PER_MULTISIG * WITNESS_SCALE_FACTOR);
140+
assert(GetTransactionSigOpCost(CTransaction(creationTx), {}, flags) == MAX_PUBKEYS_PER_MULTISIG * WITNESS_SCALE_FACTOR);
141141
// Sanity check: script verification fails because of an invalid signature.
142142
assert(VerifyWithFlag(CTransaction(creationTx), spendingTx, flags) == SCRIPT_ERR_CHECKMULTISIGVERIFY);
143143
}
@@ -149,7 +149,7 @@ BOOST_AUTO_TEST_CASE(GetTxSigOpCost)
149149
CScript scriptSig = CScript() << OP_0 << OP_0 << ToByteVector(redeemScript);
150150

151151
BuildTxs(spendingTx, coins, creationTx, scriptPubKey, scriptSig, CScriptWitness());
152-
assert(GetTransactionSigOpCost(CTransaction(spendingTx), coins, flags) == 2 * WITNESS_SCALE_FACTOR);
152+
assert(GetTransactionSigOpCost(CTransaction(spendingTx), *Assert(coins.GetInputs(CTransaction(spendingTx))), flags) == 2 * WITNESS_SCALE_FACTOR);
153153
assert(VerifyWithFlag(CTransaction(creationTx), spendingTx, flags) == SCRIPT_ERR_CHECKMULTISIGVERIFY);
154154
}
155155

@@ -163,22 +163,22 @@ BOOST_AUTO_TEST_CASE(GetTxSigOpCost)
163163

164164

165165
BuildTxs(spendingTx, coins, creationTx, scriptPubKey, scriptSig, scriptWitness);
166-
assert(GetTransactionSigOpCost(CTransaction(spendingTx), coins, flags) == 1);
166+
assert(GetTransactionSigOpCost(CTransaction(spendingTx), *Assert(coins.GetInputs(CTransaction(spendingTx))), flags) == 1);
167167
// No signature operations if we don't verify the witness.
168-
assert(GetTransactionSigOpCost(CTransaction(spendingTx), coins, flags & ~SCRIPT_VERIFY_WITNESS) == 0);
168+
assert(GetTransactionSigOpCost(CTransaction(spendingTx), *Assert(coins.GetInputs(CTransaction(spendingTx))), flags & ~SCRIPT_VERIFY_WITNESS) == 0);
169169
assert(VerifyWithFlag(CTransaction(creationTx), spendingTx, flags) == SCRIPT_ERR_EQUALVERIFY);
170170

171171
// The sig op cost for witness version != 0 is zero.
172172
assert(scriptPubKey[0] == 0x00);
173173
scriptPubKey[0] = 0x51;
174174
BuildTxs(spendingTx, coins, creationTx, scriptPubKey, scriptSig, scriptWitness);
175-
assert(GetTransactionSigOpCost(CTransaction(spendingTx), coins, flags) == 0);
175+
assert(GetTransactionSigOpCost(CTransaction(spendingTx), *Assert(coins.GetInputs(CTransaction(spendingTx))), flags) == 0);
176176
scriptPubKey[0] = 0x00;
177177
BuildTxs(spendingTx, coins, creationTx, scriptPubKey, scriptSig, scriptWitness);
178178

179179
// The witness of a coinbase transaction is not taken into account.
180180
spendingTx.vin[0].prevout.SetNull();
181-
assert(GetTransactionSigOpCost(CTransaction(spendingTx), coins, flags) == 0);
181+
assert(GetTransactionSigOpCost(CTransaction(spendingTx), {}, flags) == 0);
182182
}
183183

184184
// P2WPKH nested in P2SH
@@ -191,7 +191,7 @@ BOOST_AUTO_TEST_CASE(GetTxSigOpCost)
191191
scriptWitness.stack.emplace_back(0);
192192

193193
BuildTxs(spendingTx, coins, creationTx, scriptPubKey, scriptSig, scriptWitness);
194-
assert(GetTransactionSigOpCost(CTransaction(spendingTx), coins, flags) == 1);
194+
assert(GetTransactionSigOpCost(CTransaction(spendingTx), *Assert(coins.GetInputs(CTransaction(spendingTx))), flags) == 1);
195195
assert(VerifyWithFlag(CTransaction(creationTx), spendingTx, flags) == SCRIPT_ERR_EQUALVERIFY);
196196
}
197197

@@ -206,8 +206,8 @@ BOOST_AUTO_TEST_CASE(GetTxSigOpCost)
206206
scriptWitness.stack.emplace_back(witnessScript.begin(), witnessScript.end());
207207

208208
BuildTxs(spendingTx, coins, creationTx, scriptPubKey, scriptSig, scriptWitness);
209-
assert(GetTransactionSigOpCost(CTransaction(spendingTx), coins, flags) == 2);
210-
assert(GetTransactionSigOpCost(CTransaction(spendingTx), coins, flags & ~SCRIPT_VERIFY_WITNESS) == 0);
209+
assert(GetTransactionSigOpCost(CTransaction(spendingTx), *Assert(coins.GetInputs(CTransaction(spendingTx))), flags) == 2);
210+
assert(GetTransactionSigOpCost(CTransaction(spendingTx), *Assert(coins.GetInputs(CTransaction(spendingTx))), flags & ~SCRIPT_VERIFY_WITNESS) == 0);
211211
assert(VerifyWithFlag(CTransaction(creationTx), spendingTx, flags) == SCRIPT_ERR_CHECKMULTISIGVERIFY);
212212
}
213213

@@ -223,7 +223,7 @@ BOOST_AUTO_TEST_CASE(GetTxSigOpCost)
223223
scriptWitness.stack.emplace_back(witnessScript.begin(), witnessScript.end());
224224

225225
BuildTxs(spendingTx, coins, creationTx, scriptPubKey, scriptSig, scriptWitness);
226-
assert(GetTransactionSigOpCost(CTransaction(spendingTx), coins, flags) == 2);
226+
assert(GetTransactionSigOpCost(CTransaction(spendingTx), *Assert(coins.GetInputs(CTransaction(spendingTx))), flags) == 2);
227227
assert(VerifyWithFlag(CTransaction(creationTx), spendingTx, flags) == SCRIPT_ERR_CHECKMULTISIGVERIFY);
228228
}
229229
}

0 commit comments

Comments
 (0)