Skip to content

Commit 51de5bf

Browse files
kwvgkwvg
committed
merge bitcoin#24804: Sanity assert GetAncestor() != nullptr where appropriate
1 parent bf0b580 commit 51de5bf

File tree

5 files changed

+39
-29
lines changed

5 files changed

+39
-29
lines changed

‎src/consensus/tx_verify.cpp‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
#include <primitives/transaction.h>
1313
#include <script/interpreter.h>
1414
#include <tinyformat.h>
15+
#include <util/check.h>
1516
#include <util/moneystr.h>
1617

1718

@@ -76,7 +77,7 @@ std::pair<int, int64_t> CalculateSequenceLocks(const CTransaction &tx, int flags
7677
int nCoinHeight = prevHeights[txinIndex];
7778

7879
if (txin.nSequence & CTxIn::SEQUENCE_LOCKTIME_TYPE_FLAG) {
79-
int64_t nCoinTime = block.GetAncestor(std::max(nCoinHeight-1, 0))->GetMedianTimePast();
80+
const int64_t nCoinTime{Assert(block.GetAncestor(std::max(nCoinHeight - 1, 0)))->GetMedianTimePast()};
8081
// NOTE: Subtract 1 to maintain nLockTime semantics
8182
// BIP 68 relative lock times have the semantics of calculating
8283
// the first block or time at which the transaction would be

‎src/rpc/blockchain.cpp‎

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1875,9 +1875,9 @@ static RPCHelpMan getchaintxstats()
18751875
}
18761876
}
18771877

1878-
const CBlockIndex* pindexPast = pindex->GetAncestor(pindex->nHeight - blockcount);
1879-
int nTimeDiff = pindex->GetMedianTimePast() - pindexPast->GetMedianTimePast();
1880-
int nTxDiff = pindex->nChainTx - pindexPast->nChainTx;
1878+
const CBlockIndex& past_block{*CHECK_NONFATAL(pindex->GetAncestor(pindex->nHeight - blockcount))};
1879+
const int64_t nTimeDiff{pindex->GetMedianTimePast() - past_block.GetMedianTimePast()};
1880+
const int nTxDiff = pindex->nChainTx - past_block.nChainTx;
18811881

18821882
UniValue ret(UniValue::VOBJ);
18831883
ret.pushKV("time", (int64_t)pindex->nTime);
@@ -2018,8 +2018,7 @@ static RPCHelpMan getblockstats()
20182018

20192019
ChainstateManager& chainman = EnsureAnyChainman(request.context);
20202020
LOCK(cs_main);
2021-
const CBlockIndex* pindex{ParseHashOrHeight(request.params[0], chainman)};
2022-
CHECK_NONFATAL(pindex != nullptr);
2021+
const CBlockIndex& pindex{*CHECK_NONFATAL(ParseHashOrHeight(request.params[0], chainman))};
20232022

20242023
std::set<std::string> stats;
20252024
if (!request.params[1].isNull()) {
@@ -2030,8 +2029,8 @@ static RPCHelpMan getblockstats()
20302029
}
20312030
}
20322031

2033-
const CBlock block = GetBlockChecked(chainman.m_blockman, pindex);
2034-
const CBlockUndo blockUndo = GetUndoChecked(chainman.m_blockman, pindex);
2032+
const CBlock& block = GetBlockChecked(chainman.m_blockman, &pindex);
2033+
const CBlockUndo& blockUndo = GetUndoChecked(chainman.m_blockman, &pindex);
20352034

20362035
const bool do_all = stats.size() == 0; // Calculate everything if nothing selected (default)
20372036
const bool do_mediantxsize = do_all || stats.count("mediantxsize") != 0;
@@ -2137,22 +2136,22 @@ static RPCHelpMan getblockstats()
21372136
ret_all.pushKV("avgfee", (block.vtx.size() > 1) ? totalfee / (block.vtx.size() - 1) : 0);
21382137
ret_all.pushKV("avgfeerate", total_size ? totalfee / total_size : 0); // Unit: sat/byte
21392138
ret_all.pushKV("avgtxsize", (block.vtx.size() > 1) ? total_size / (block.vtx.size() - 1) : 0);
2140-
ret_all.pushKV("blockhash", pindex->GetBlockHash().GetHex());
2139+
ret_all.pushKV("blockhash", pindex.GetBlockHash().GetHex());
21412140
ret_all.pushKV("feerate_percentiles", feerates_res);
2142-
ret_all.pushKV("height", (int64_t)pindex->nHeight);
2141+
ret_all.pushKV("height", (int64_t)pindex.nHeight);
21432142
ret_all.pushKV("ins", inputs);
21442143
ret_all.pushKV("maxfee", maxfee);
21452144
ret_all.pushKV("maxfeerate", maxfeerate);
21462145
ret_all.pushKV("maxtxsize", maxtxsize);
21472146
ret_all.pushKV("medianfee", CalculateTruncatedMedian(fee_array));
2148-
ret_all.pushKV("mediantime", pindex->GetMedianTimePast());
2147+
ret_all.pushKV("mediantime", pindex.GetMedianTimePast());
21492148
ret_all.pushKV("mediantxsize", CalculateTruncatedMedian(txsize_array));
21502149
ret_all.pushKV("minfee", (minfee == MAX_MONEY) ? 0 : minfee);
21512150
ret_all.pushKV("minfeerate", (minfeerate == MAX_MONEY) ? 0 : minfeerate);
21522151
ret_all.pushKV("mintxsize", mintxsize == MaxBlockSize() ? 0 : mintxsize);
21532152
ret_all.pushKV("outs", outputs);
2154-
ret_all.pushKV("subsidy", GetBlockSubsidy(pindex, Params().GetConsensus()));
2155-
ret_all.pushKV("time", pindex->GetBlockTime());
2153+
ret_all.pushKV("subsidy", GetBlockSubsidy(&pindex, Params().GetConsensus()));
2154+
ret_all.pushKV("time", pindex.GetBlockTime());
21562155
ret_all.pushKV("total_out", total_out);
21572156
ret_all.pushKV("total_size", total_size);
21582157
ret_all.pushKV("totalfee", totalfee);

‎src/test/miner_tests.cpp‎

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -465,16 +465,18 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity)
465465
BOOST_CHECK(CheckFinalTxAtTip(*Assert(m_node.chainman->ActiveChain().Tip()), CTransaction{tx})); // Locktime passes
466466
BOOST_CHECK(!TestSequenceLocks(CTransaction{tx})); // Sequence locks fail
467467

468-
for (int i = 0; i < CBlockIndex::nMedianTimeSpan; i++)
469-
m_node.chainman->ActiveChain().Tip()->GetAncestor(m_node.chainman->ActiveChain().Tip()->nHeight - i)->nTime += 512; //Trick the MedianTimePast
470-
468+
const int SEQUENCE_LOCK_TIME = 512; // Sequence locks pass 512 seconds later
469+
for (int i = 0; i < CBlockIndex::nMedianTimeSpan; ++i)
470+
m_node.chainman->ActiveChain().Tip()->GetAncestor(m_node.chainman->ActiveChain().Tip()->nHeight - i)->nTime += SEQUENCE_LOCK_TIME; // Trick the MedianTimePast
471471
{
472472
CBlockIndex* active_chain_tip = m_node.chainman->ActiveChain().Tip();
473-
BOOST_CHECK(SequenceLocks(CTransaction(tx), flags, prevheights, CreateBlockIndex(active_chain_tip->nHeight + 1, active_chain_tip))); // Sequence locks pass 512 seconds later
473+
BOOST_CHECK(SequenceLocks(CTransaction(tx), flags, prevheights, CreateBlockIndex(active_chain_tip->nHeight + 1, active_chain_tip)));
474474
}
475475

476-
for (int i = 0; i < CBlockIndex::nMedianTimeSpan; i++)
477-
m_node.chainman->ActiveChain().Tip()->GetAncestor(m_node.chainman->ActiveChain().Tip()->nHeight - i)->nTime -= 512; //undo tricked MTP
476+
for (int i = 0; i < CBlockIndex::nMedianTimeSpan; ++i) {
477+
CBlockIndex* ancestor{Assert(m_node.chainman->ActiveChain().Tip()->GetAncestor(m_node.chainman->ActiveChain().Tip()->nHeight - i))};
478+
ancestor->nTime -= SEQUENCE_LOCK_TIME; // undo tricked MTP
479+
}
478480

479481
// absolute height locked
480482
tx.vin[0].prevout.hash = txFirst[2]->GetHash();
@@ -519,11 +521,12 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity)
519521
// but relative locked txs will if inconsistently added to mempool.
520522
// For now these will still generate a valid template until BIP68 soft fork
521523
BOOST_CHECK_EQUAL(pblocktemplate->block.vtx.size(), 3U);
522-
// However if we advance height by 1 and time by 512, all of them should be mined
523-
for (int i = 0; i < CBlockIndex::nMedianTimeSpan; i++)
524-
m_node.chainman->ActiveChain().Tip()->GetAncestor(m_node.chainman->ActiveChain().Tip()->nHeight - i)->nTime += 512; //Trick the MedianTimePast
525-
524+
// However if we advance height by 1 and time by SEQUENCE_LOCK_TIME, all of them should be mined
525+
for (int i = 0; i < CBlockIndex::nMedianTimeSpan; ++i) {
526+
CBlockIndex* ancestor{Assert(m_node.chainman->ActiveChain().Tip()->GetAncestor(m_node.chainman->ActiveChain().Tip()->nHeight - i))};
527+
ancestor->nTime += SEQUENCE_LOCK_TIME; // Trick the MedianTimePast
526528
} // unlock cs_main while calling createAndProcessEmptyBlock
529+
}
527530

528531
// Mine an empty block
529532
createAndProcessEmptyBlock();

‎src/validation.cpp‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -223,7 +223,12 @@ bool CheckSequenceLocksAtTip(CBlockIndex* tip,
223223
maxInputHeight = std::max(maxInputHeight, height);
224224
}
225225
}
226-
lp->maxInputBlock = tip->GetAncestor(maxInputHeight);
226+
// tip->GetAncestor(maxInputHeight) should never return a nullptr
227+
// because maxInputHeight is always less than the tip height.
228+
// It would, however, be a bad bug to continue execution, since a
229+
// LockPoints object with the maxInputBlock member set to nullptr
230+
// signifies no relative lock time.
231+
lp->maxInputBlock = Assert(tip->GetAncestor(maxInputHeight));
227232
}
228233
}
229234
return EvaluateSequenceLocks(index, lockPair);
@@ -4732,10 +4737,11 @@ bool CChainState::ReplayBlocks()
47324737
// Roll forward from the forking point to the new tip.
47334738
int nForkHeight = pindexFork ? pindexFork->nHeight : 0;
47344739
for (int nHeight = nForkHeight + 1; nHeight <= pindexNew->nHeight; ++nHeight) {
4735-
const CBlockIndex* pindex = pindexNew->GetAncestor(nHeight);
4736-
LogPrintf("Rolling forward %s (%i)\n", pindex->GetBlockHash().ToString(), nHeight);
4740+
const CBlockIndex& pindex{*Assert(pindexNew->GetAncestor(nHeight))};
4741+
4742+
LogPrintf("Rolling forward %s (%i)\n", pindex.GetBlockHash().ToString(), nHeight);
47374743
uiInterface.ShowProgress(_("Replaying blocks…").translated, (int) ((nHeight - nForkHeight) * 100.0 / (pindexNew->nHeight - nForkHeight)) , false);
4738-
if (!RollforwardBlock(pindex, cache)) return false;
4744+
if (!RollforwardBlock(&pindex, cache)) return false;
47394745
}
47404746

47414747
cache.SetBestBlock(pindexNew->GetBlockHash());

‎src/versionbits.cpp‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,9 @@
22
// Distributed under the MIT software license, see the accompanying
33
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
44

5-
#include <versionbits.h>
65
#include <consensus/params.h>
6+
#include <util/check.h>
7+
#include <versionbits.h>
78

89
#include <limits>
910

@@ -190,7 +191,7 @@ int AbstractThresholdConditionChecker::GetStateSinceHeightFor(const CBlockIndex*
190191
// if we are computing for the last block of a period, then pindexPrev points to the second to last block of the period, and
191192
// if we are computing for the first block of a period, then pindexPrev points to the last block of the previous period.
192193
// The parent of the genesis block is represented by nullptr.
193-
pindexPrev = pindexPrev->GetAncestor(pindexPrev->nHeight - ((pindexPrev->nHeight + 1) % nPeriod));
194+
pindexPrev = Assert(pindexPrev->GetAncestor(pindexPrev->nHeight - ((pindexPrev->nHeight + 1) % nPeriod)));
194195

195196
const CBlockIndex* previousPeriodParent = pindexPrev->GetAncestor(pindexPrev->nHeight - nPeriod);
196197

0 commit comments

Comments
 (0)