Skip to content

Commit 765c0b3

Browse files
author
Antoine Riard
committed
refactor: combine Chain::findFirstBlockWithTime/findFirstBlockWithTimeAndHeight
As suggested in #14711, pass height to CChain::FindEarliestAtLeast to simplify Chain interface by combining findFirstBlockWithTime and findFirstBlockWithTimeAndHeight into one Extend findearliestatleast_edge_test in consequence
1 parent 8a8b03e commit 765c0b3

File tree

7 files changed

+44
-54
lines changed

7 files changed

+44
-54
lines changed

‎src/chain.cpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -59,10 +59,11 @@ const CBlockIndex *CChain::FindFork(const CBlockIndex *pindex) const {
5959
return pindex;
6060
}
6161

62-
CBlockIndex* CChain::FindEarliestAtLeast(int64_t nTime) const
62+
CBlockIndex* CChain::FindEarliestAtLeast(int64_t nTime, int height) const
6363
{
64-
std::vector<CBlockIndex*>::const_iterator lower = std::lower_bound(vChain.begin(), vChain.end(), nTime,
65-
[](CBlockIndex* pBlock, const int64_t& time) -> bool { return pBlock->GetBlockTimeMax() < time; });
64+
std::pair<int64_t, int> blockparams = std::make_pair(nTime, height);
65+
std::vector<CBlockIndex*>::const_iterator lower = std::lower_bound(vChain.begin(), vChain.end(), blockparams,
66+
[](CBlockIndex* pBlock, const std::pair<int64_t, int>& blockparams) -> bool { return pBlock->GetBlockTimeMax() < blockparams.first || pBlock->nHeight < blockparams.second; });
6667
return (lower == vChain.end() ? nullptr : *lower);
6768
}
6869

‎src/chain.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -465,8 +465,8 @@ class CChain {
465465
/** Find the last common block between this chain and a block index entry. */
466466
const CBlockIndex *FindFork(const CBlockIndex *pindex) const;
467467

468-
/** Find the earliest block with timestamp equal or greater than the given. */
469-
CBlockIndex* FindEarliestAtLeast(int64_t nTime) const;
468+
/** Find the earliest block with timestamp equal or greater than the given time and height equal or greater than the given height. */
469+
CBlockIndex* FindEarliestAtLeast(int64_t nTime, int height) const;
470470
};
471471

472472
#endif // BITCOIN_CHAIN_H

‎src/interfaces/chain.cpp

Lines changed: 2 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -83,29 +83,15 @@ class LockImpl : public Chain::Lock
8383
CBlockIndex* block = ::chainActive[height];
8484
return block && ((block->nStatus & BLOCK_HAVE_DATA) != 0) && block->nTx > 0;
8585
}
86-
Optional<int> findFirstBlockWithTime(int64_t time, uint256* hash) override
86+
Optional<int> findFirstBlockWithTimeAndHeight(int64_t time, int height, uint256* hash) override
8787
{
88-
CBlockIndex* block = ::chainActive.FindEarliestAtLeast(time);
88+
CBlockIndex* block = ::chainActive.FindEarliestAtLeast(time, height);
8989
if (block) {
9090
if (hash) *hash = block->GetBlockHash();
9191
return block->nHeight;
9292
}
9393
return nullopt;
9494
}
95-
Optional<int> findFirstBlockWithTimeAndHeight(int64_t time, int height) override
96-
{
97-
// TODO: Could update CChain::FindEarliestAtLeast() to take a height
98-
// parameter and use it with std::lower_bound() to make this
99-
// implementation more efficient and allow combining
100-
// findFirstBlockWithTime and findFirstBlockWithTimeAndHeight into one
101-
// method.
102-
for (CBlockIndex* block = ::chainActive[height]; block; block = ::chainActive.Next(block)) {
103-
if (block->GetBlockTime() >= time) {
104-
return block->nHeight;
105-
}
106-
}
107-
return nullopt;
108-
}
10995
Optional<int> findPruned(int start_height, Optional<int> stop_height) override
11096
{
11197
if (::fPruneMode) {

‎src/interfaces/chain.h

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -100,21 +100,12 @@ class Chain
100100
//! pruned), and contains transactions.
101101
virtual bool haveBlockOnDisk(int height) = 0;
102102

103-
//! Return height of the first block in the chain with timestamp equal
104-
//! or greater than the given time, or nullopt if there is no block with
105-
//! a high enough timestamp. Also return the block hash as an optional
106-
//! output parameter (to avoid the cost of a second lookup in case this
107-
//! information is needed.)
108-
virtual Optional<int> findFirstBlockWithTime(int64_t time, uint256* hash) = 0;
109-
110103
//! Return height of the first block in the chain with timestamp equal
111104
//! or greater than the given time and height equal or greater than the
112-
//! given height, or nullopt if there is no such block.
113-
//!
114-
//! Calling this with height 0 is equivalent to calling
115-
//! findFirstBlockWithTime, but less efficient because it requires a
116-
//! linear instead of a binary search.
117-
virtual Optional<int> findFirstBlockWithTimeAndHeight(int64_t time, int height) = 0;
105+
//! given height, or nullopt if there is no block with a high enough
106+
//! timestamp and height. Also return the block hash as an optional output parameter
107+
//! (to avoid the cost of a second lookup in case this information is needed.)
108+
virtual Optional<int> findFirstBlockWithTimeAndHeight(int64_t time, int height, uint256* hash) = 0;
118109

119110
//! Return height of last block in the specified range which is pruned, or
120111
//! nullopt if no block in the range is pruned. Range is inclusive.

‎src/rpc/blockchain.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1007,7 +1007,7 @@ static UniValue pruneblockchain(const JSONRPCRequest& request)
10071007
// too low to be a block time (corresponds to timestamp from Sep 2001).
10081008
if (heightParam > 1000000000) {
10091009
// Add a 2 hour buffer to include blocks which might have had old timestamps
1010-
CBlockIndex* pindex = chainActive.FindEarliestAtLeast(heightParam - TIMESTAMP_WINDOW);
1010+
CBlockIndex* pindex = chainActive.FindEarliestAtLeast(heightParam - TIMESTAMP_WINDOW, 0);
10111011
if (!pindex) {
10121012
throw JSONRPCError(RPC_INVALID_PARAMETER, "Could not find block with at least the specified timestamp.");
10131013
}

‎src/test/skiplist_tests.cpp

Lines changed: 29 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,7 @@ BOOST_AUTO_TEST_CASE(findearliestatleast_test)
136136
// Pick a random element in vBlocksMain.
137137
int r = InsecureRandRange(vBlocksMain.size());
138138
int64_t test_time = vBlocksMain[r].nTime;
139-
CBlockIndex *ret = chain.FindEarliestAtLeast(test_time);
139+
CBlockIndex* ret = chain.FindEarliestAtLeast(test_time, 0);
140140
BOOST_CHECK(ret->nTimeMax >= test_time);
141141
BOOST_CHECK((ret->pprev==nullptr) || ret->pprev->nTimeMax < test_time);
142142
BOOST_CHECK(vBlocksMain[r].GetAncestor(ret->nHeight) == ret);
@@ -158,22 +158,34 @@ BOOST_AUTO_TEST_CASE(findearliestatleast_edge_test)
158158
CChain chain;
159159
chain.SetTip(&blocks.back());
160160

161-
BOOST_CHECK_EQUAL(chain.FindEarliestAtLeast(50)->nHeight, 0);
162-
BOOST_CHECK_EQUAL(chain.FindEarliestAtLeast(100)->nHeight, 0);
163-
BOOST_CHECK_EQUAL(chain.FindEarliestAtLeast(150)->nHeight, 3);
164-
BOOST_CHECK_EQUAL(chain.FindEarliestAtLeast(200)->nHeight, 3);
165-
BOOST_CHECK_EQUAL(chain.FindEarliestAtLeast(250)->nHeight, 6);
166-
BOOST_CHECK_EQUAL(chain.FindEarliestAtLeast(300)->nHeight, 6);
167-
BOOST_CHECK(!chain.FindEarliestAtLeast(350));
168-
169-
BOOST_CHECK_EQUAL(chain.FindEarliestAtLeast(0)->nHeight, 0);
170-
BOOST_CHECK_EQUAL(chain.FindEarliestAtLeast(-1)->nHeight, 0);
171-
172-
BOOST_CHECK_EQUAL(chain.FindEarliestAtLeast(std::numeric_limits<int64_t>::min())->nHeight, 0);
173-
BOOST_CHECK_EQUAL(chain.FindEarliestAtLeast(-int64_t(std::numeric_limits<unsigned int>::max()) - 1)->nHeight, 0);
174-
BOOST_CHECK(!chain.FindEarliestAtLeast(std::numeric_limits<int64_t>::max()));
175-
BOOST_CHECK(!chain.FindEarliestAtLeast(std::numeric_limits<unsigned int>::max()));
176-
BOOST_CHECK(!chain.FindEarliestAtLeast(int64_t(std::numeric_limits<unsigned int>::max()) + 1));
161+
BOOST_CHECK_EQUAL(chain.FindEarliestAtLeast(50, 0)->nHeight, 0);
162+
BOOST_CHECK_EQUAL(chain.FindEarliestAtLeast(100, 0)->nHeight, 0);
163+
BOOST_CHECK_EQUAL(chain.FindEarliestAtLeast(150, 0)->nHeight, 3);
164+
BOOST_CHECK_EQUAL(chain.FindEarliestAtLeast(200, 0)->nHeight, 3);
165+
BOOST_CHECK_EQUAL(chain.FindEarliestAtLeast(250, 0)->nHeight, 6);
166+
BOOST_CHECK_EQUAL(chain.FindEarliestAtLeast(300, 0)->nHeight, 6);
167+
BOOST_CHECK(!chain.FindEarliestAtLeast(350, 0));
168+
169+
BOOST_CHECK_EQUAL(chain.FindEarliestAtLeast(0, 0)->nHeight, 0);
170+
BOOST_CHECK_EQUAL(chain.FindEarliestAtLeast(-1, 0)->nHeight, 0);
171+
172+
BOOST_CHECK_EQUAL(chain.FindEarliestAtLeast(std::numeric_limits<int64_t>::min(), 0)->nHeight, 0);
173+
BOOST_CHECK_EQUAL(chain.FindEarliestAtLeast(-int64_t(std::numeric_limits<unsigned int>::max()) - 1, 0)->nHeight, 0);
174+
BOOST_CHECK(!chain.FindEarliestAtLeast(std::numeric_limits<int64_t>::max(), 0));
175+
BOOST_CHECK(!chain.FindEarliestAtLeast(std::numeric_limits<unsigned int>::max(), 0));
176+
BOOST_CHECK(!chain.FindEarliestAtLeast(int64_t(std::numeric_limits<unsigned int>::max()) + 1, 0));
177+
178+
BOOST_CHECK_EQUAL(chain.FindEarliestAtLeast(0, -1)->nHeight, 0);
179+
BOOST_CHECK_EQUAL(chain.FindEarliestAtLeast(0, 0)->nHeight, 0);
180+
BOOST_CHECK_EQUAL(chain.FindEarliestAtLeast(0, 3)->nHeight, 3);
181+
BOOST_CHECK_EQUAL(chain.FindEarliestAtLeast(0, 8)->nHeight, 8);
182+
BOOST_CHECK(!chain.FindEarliestAtLeast(0, 9));
183+
184+
CBlockIndex* ret1 = chain.FindEarliestAtLeast(100, 2);
185+
BOOST_CHECK(ret1->nTimeMax >= 100 && ret1->nHeight == 2);
186+
BOOST_CHECK(!chain.FindEarliestAtLeast(300, 9));
187+
CBlockIndex* ret2 = chain.FindEarliestAtLeast(200, 4);
188+
BOOST_CHECK(ret2->nTimeMax >= 200 && ret2->nHeight == 4);
177189
}
178190

179191
BOOST_AUTO_TEST_SUITE_END()

‎src/wallet/wallet.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1722,7 +1722,7 @@ int64_t CWallet::RescanFromTime(int64_t startTime, const WalletRescanReserver& r
17221722
uint256 start_block;
17231723
{
17241724
auto locked_chain = chain().lock();
1725-
const Optional<int> start_height = locked_chain->findFirstBlockWithTime(startTime - TIMESTAMP_WINDOW, &start_block);
1725+
const Optional<int> start_height = locked_chain->findFirstBlockWithTimeAndHeight(startTime - TIMESTAMP_WINDOW, 0, &start_block);
17261726
const Optional<int> tip_height = locked_chain->getHeight();
17271727
WalletLogPrintf("%s: Rescanning last %i blocks\n", __func__, tip_height && start_height ? *tip_height - *start_height + 1 : 0);
17281728
}
@@ -4338,7 +4338,7 @@ std::shared_ptr<CWallet> CWallet::CreateWalletFromFile(interfaces::Chain& chain,
43384338
// No need to read and scan block if block was created before
43394339
// our wallet birthday (as adjusted for block time variability)
43404340
if (walletInstance->nTimeFirstKey) {
4341-
if (Optional<int> first_block = locked_chain->findFirstBlockWithTimeAndHeight(walletInstance->nTimeFirstKey - TIMESTAMP_WINDOW, rescan_height)) {
4341+
if (Optional<int> first_block = locked_chain->findFirstBlockWithTimeAndHeight(walletInstance->nTimeFirstKey - TIMESTAMP_WINDOW, rescan_height, nullptr)) {
43424342
rescan_height = *first_block;
43434343
}
43444344
}

0 commit comments

Comments
 (0)