Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rpc, util: Remove caching from BlockFinder #2490

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions src/gridcoin/contract/contract.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -418,16 +418,14 @@ Contract GRC::MakeLegacyContract(

void GRC::ReplayContracts(CBlockIndex* pindex_end, CBlockIndex* pindex_start)
{
static BlockFinder blockFinder;

CBlockIndex*& pindex = pindex_start;

// If there is no pindex_start (i.e. default value of nullptr), then set standard lookback. A Non-standard lookback
// where there is a specific pindex_start argument supplied, is only used in the GRC InitializeContracts call for
// when the beacon database in LevelDB has not already been populated.
if (!pindex)
{
pindex = blockFinder.FindByMinTime(pindexBest->nTime - Beacon::MAX_AGE);
pindex = GRC::BlockFinder::FindByMinTime(pindexBest->nTime - Beacon::MAX_AGE);
}

if (pindex->nHeight < (fTestNet ? 1 : 164618)) {
Expand Down
6 changes: 2 additions & 4 deletions src/gridcoin/gridcoin.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ bool InitializeResearchRewardAccounting(CBlockIndex* pindexBest)

const int64_t start_height = Params().GetConsensus().ResearchAgeHeight;

if (!Tally::Initialize(BlockFinder().FindByHeight(start_height))) {
if (!Tally::Initialize(BlockFinder::FindByHeight(start_height))) {
return error("Failed to initialize tally.");
}

Expand Down Expand Up @@ -169,9 +169,7 @@ void InitializeContracts(CBlockIndex* pindexBest)
LogPrintf("Gridcoin: replaying contracts...");
uiInterface.InitMessage(_("Replaying contracts..."));

static BlockFinder blockFinder;

CBlockIndex* pindex_start = blockFinder.FindByMinTime(pindexBest->nTime - Beacon::MAX_AGE);
CBlockIndex* pindex_start = GRC::BlockFinder::FindByMinTime(pindexBest->nTime - Beacon::MAX_AGE);

const int& V11_height = Params().GetConsensus().BlockV11Height;
const int& lookback_window_low_height = pindex_start->nHeight;
Expand Down
4 changes: 1 addition & 3 deletions src/gridcoin/scraper/scraper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -571,11 +571,9 @@ const CBlockIndex* GetBeaconConsensusHeight() EXCLUSIVE_LOCKS_REQUIRED(cs_main)
{
AssertLockHeld(cs_main);

static BlockFinder block_finder;

// Use 4 times the BLOCK_GRANULARITY which moves the consensus block every hour.
// TODO: Make the mod a function of SCRAPER_CMANIFEST_RETENTION_TIME in scraper.h.
return block_finder.FindByHeight(
return BlockFinder::FindByHeight(
(nBestHeight - CONSENSUS_LOOKBACK)
- (nBestHeight - CONSENSUS_LOOKBACK) % (BLOCK_GRANULARITY * 4));
}
Expand Down
23 changes: 0 additions & 23 deletions src/gridcoin/support/block_finder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,8 @@
#include "main.h"
#include "gridcoin/support/block_finder.h"

#include <cstdlib>

using namespace GRC;

BlockFinder::BlockFinder()
: cache(nullptr)
{}

CBlockIndex* BlockFinder::FindByHeight(int height)
{
// If the height is at the bottom half of the chain, start searching from
Expand All @@ -23,11 +17,6 @@ CBlockIndex* BlockFinder::FindByHeight(int height)

if(index != nullptr)
{
// Use the cache if it's closer to the target than the current
// start block.
if (cache && abs(height - index->nHeight) > std::abs(height - cache->nHeight))
index = cache;

// Traverse towards the tail.
while (index && index->pprev && index->nHeight > height)
index = index->pprev;
Expand All @@ -37,7 +26,6 @@ CBlockIndex* BlockFinder::FindByHeight(int height)
index = index->pnext;
}

cache = index;
return index;
}

Expand All @@ -52,11 +40,6 @@ CBlockIndex* BlockFinder::FindByMinTime(int64_t time)

if(index != nullptr)
{
// If we have a cache that's closer to target than our current index,
// use it.
if(cache && abs(time - index->nTime) > abs(time - int64_t(cache->nTime)))
index = cache;

// Move back until the previous block is no longer younger than "time".
while(index && index->pprev && index->pprev->nTime > time)
index = index->pprev;
Expand All @@ -66,11 +49,5 @@ CBlockIndex* BlockFinder::FindByMinTime(int64_t time)
index = index->pnext;
}

cache = index;
return index;
}

void BlockFinder::Reset()
{
cache = nullptr;
}
22 changes: 3 additions & 19 deletions src/gridcoin/support/block_finder.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,22 +14,17 @@ namespace GRC {
class BlockFinder
{
public:
//!
//! \brief Constructor.
//!
BlockFinder();

//!
//! \brief Find a block with a specific height.
//!
//! Traverses the chain from head or tail, depending on what's closest to
//! find the block that matches \p height. This is a caching operation
//! find the block that matches \p height.
//!
//! \param nHeight Block height to find.
//! \return The block with the height closest to \p nHeight if found, otherwise
//! \a nullptr is returned.
//!
CBlockIndex* FindByHeight(int height);
static CBlockIndex* FindByHeight(int height);

//!
//! \brief Find block by time.
Expand All @@ -42,18 +37,7 @@ class BlockFinder
//! \return The youngest block which is not older than \p time, or the
//! head of the chain if it is older than \p time.
//!
CBlockIndex* FindByMinTime(int64_t time);

//!
//! \brief Reset finder cache.
//!
//! Clears the block finder cache. This should be used when blocks are removed
//! from the chain to avoid accessing deleted memory.
//!
void Reset();

private:
CBlockIndex* cache;
static CBlockIndex* FindByMinTime(int64_t time);
};
} // namespace GRC

Expand Down
4 changes: 1 addition & 3 deletions src/gridcoin/voting/registry.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -337,10 +337,8 @@ CBlockIndex* PollReference::GetEndingBlockIndexPtr() const
{
// Has poll ended?
if (Expired(GetAdjustedTime())) {
GRC::BlockFinder blockfinder;

// Find and return the last block that contains valid votes for the poll.
return blockfinder.FindByMinTime(Expiration());
return GRC::BlockFinder::FindByMinTime(Expiration());
}

return nullptr;
Expand Down
12 changes: 4 additions & 8 deletions src/rpc/blockchain.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,6 @@ double CoinToDouble(double surrogate);
extern void TxToJSON(const CTransaction& tx, const uint256 hashBlock, UniValue& entry);
UniValue ContractToJson(const GRC::Contract& contract);

GRC::BlockFinder RPCBlockFinder;

UniValue MRCToJson(const GRC::MRC& mrc) {
UniValue json(UniValue::VOBJ);

Expand Down Expand Up @@ -471,7 +469,7 @@ UniValue showblock(const UniValue& params, bool fHelp)

LOCK(cs_main);

CBlockIndex* pblockindex = RPCBlockFinder.FindByHeight(nHeight);
CBlockIndex* pblockindex = GRC::BlockFinder::FindByHeight(nHeight);

if (pblockindex == nullptr)
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Block not found");
Expand Down Expand Up @@ -587,7 +585,7 @@ UniValue getblockhash(const UniValue& params, bool fHelp)

LOCK(cs_main);

CBlockIndex* RPCpblockindex = RPCBlockFinder.FindByHeight(nHeight);
CBlockIndex* RPCpblockindex = GRC::BlockFinder::FindByHeight(nHeight);

return RPCpblockindex->phashBlock->GetHex();
}
Expand Down Expand Up @@ -634,9 +632,8 @@ UniValue getblockbynumber(const UniValue& params, bool fHelp)
LOCK(cs_main);

CBlock block;
static GRC::BlockFinder block_finder;

CBlockIndex* pblockindex = block_finder.FindByHeight(nHeight);
CBlockIndex* pblockindex = GRC::BlockFinder::FindByHeight(nHeight);
ReadBlockFromDisk(block, pblockindex, Params().GetConsensus());

return blockToJSON(block, pblockindex, params.size() > 1 ? params[1].get_bool() : false);
Expand All @@ -660,9 +657,8 @@ UniValue getblockbymintime(const UniValue& params, bool fHelp)
LOCK(cs_main);

CBlock block;
static GRC::BlockFinder block_finder;

CBlockIndex* pblockindex = block_finder.FindByMinTime(nTimestamp);
CBlockIndex* pblockindex = GRC::BlockFinder::FindByMinTime(nTimestamp);
ReadBlockFromDisk(block, pblockindex, Params().GetConsensus());

return blockToJSON(block, pblockindex, params.size() > 1 ? params[1].get_bool() : false);
Expand Down
2 changes: 0 additions & 2 deletions src/rpc/dataacq.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,6 @@
using namespace GRC;
using namespace std;

extern GRC::BlockFinder RPCBlockFinder;

// Brod
static bool compare_second(const pair<std::string, int64_t> &p1, const pair<std::string, int64_t> &p2)
{
Expand Down
8 changes: 2 additions & 6 deletions src/rpc/rawtransaction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1075,9 +1075,7 @@ UniValue consolidatemsunspent(const UniValue& params, bool fHelp)
if (nMaxInputs == 0 || nMaxInputs > nEqMaxInputs)
nMaxInputs = nEqMaxInputs;

GRC::BlockFinder blockfinder;

CBlockIndex* pblkindex = blockfinder.FindByHeight((nBlockStart - 1));
CBlockIndex* pblkindex = GRC::BlockFinder::FindByHeight((nBlockStart - 1));

if (!pblkindex)
throw JSONRPCError(RPC_PARSE_ERROR, "Block not found");
Expand Down Expand Up @@ -1279,9 +1277,7 @@ UniValue scanforunspent(const UniValue& params, bool fHelp)
{
LOCK(cs_main);

GRC::BlockFinder blockfinder;

CBlockIndex* pblkindex = blockfinder.FindByHeight((nBlockStart - 1));
CBlockIndex* pblkindex = GRC::BlockFinder::FindByHeight((nBlockStart - 1));

if (!pblkindex)
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Block not found");
Expand Down
23 changes: 9 additions & 14 deletions src/test/gridcoin/block_finder_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -46,46 +46,41 @@ BOOST_AUTO_TEST_SUITE(block_finder_tests);
BOOST_AUTO_TEST_CASE(FindBlockInNormalChainShouldWork)
{
BlockChain<100> chain;
GRC::BlockFinder finder;
for(auto& block : chain.blocks)
BOOST_CHECK_EQUAL(&block, finder.FindByHeight(block.nHeight));
BOOST_CHECK_EQUAL(&block, GRC::BlockFinder::FindByHeight(block.nHeight));
}

BOOST_AUTO_TEST_CASE(FindBlockAboveHighestHeightShouldReturnHighestBlock)
{
BlockChain<100> chain;
GRC::BlockFinder finder;
CBlockIndex& last = chain.blocks.back();
BOOST_CHECK_EQUAL(&last, finder.FindByHeight(101));
BOOST_CHECK_EQUAL(&last, GRC::BlockFinder::FindByHeight(101));
}

BOOST_AUTO_TEST_CASE(FindBlockByHeightShouldWorkOnChainsWithJustOneBlock)
{
BlockChain<1> chain;
GRC::BlockFinder finder;
BOOST_CHECK_EQUAL(&chain.blocks.front(), finder.FindByHeight(0));
BOOST_CHECK_EQUAL(&chain.blocks.front(), finder.FindByHeight(1));
BOOST_CHECK_EQUAL(&chain.blocks.front(), finder.FindByHeight(-1));
BOOST_CHECK_EQUAL(&chain.blocks.front(), GRC::BlockFinder::FindByHeight(0));
BOOST_CHECK_EQUAL(&chain.blocks.front(), GRC::BlockFinder::FindByHeight(1));
BOOST_CHECK_EQUAL(&chain.blocks.front(), GRC::BlockFinder::FindByHeight(-1));
}

BOOST_AUTO_TEST_CASE(FindBlockByTimeShouldReturnNextYoungestBlock)
{
// Chain with block times 0, 10, 20, 30, 40 etc.
BlockChain<10> chain;
GRC::BlockFinder finder;

// Finding the block older than time 10 should return block #2
// which has time 20.
BOOST_CHECK_EQUAL(&chain.blocks[2], finder.FindByMinTime(11));
BOOST_CHECK_EQUAL(&chain.blocks[1], finder.FindByMinTime(10));
BOOST_CHECK_EQUAL(&chain.blocks[1], finder.FindByMinTime(9));
BOOST_CHECK_EQUAL(&chain.blocks[2], GRC::BlockFinder::FindByMinTime(11));
BOOST_CHECK_EQUAL(&chain.blocks[1], GRC::BlockFinder::FindByMinTime(10));
BOOST_CHECK_EQUAL(&chain.blocks[1], GRC::BlockFinder::FindByMinTime(9));
}

BOOST_AUTO_TEST_CASE(FindBlockByTimeShouldReturnLastBlockIfOlderThanTime)
{
BlockChain<10> chain;
GRC::BlockFinder finder;
BOOST_CHECK_EQUAL(&chain.blocks.back(), finder.FindByMinTime(999999));
BOOST_CHECK_EQUAL(&chain.blocks.back(), GRC::BlockFinder::FindByMinTime(999999));
}

BOOST_AUTO_TEST_SUITE_END()
2 changes: 1 addition & 1 deletion src/wallet/wallet.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2860,7 +2860,7 @@ void CWallet::GetKeyBirthTimes(std::map<CKeyID, int64_t> &mapKeyBirth) const EXC
mapKeyBirth[it->first] = it->second.nCreateTime;

// map in which we'll infer heights of other keys
CBlockIndex *pindexMax = GRC::BlockFinder().FindByHeight(std::max(0, nBestHeight - 144)); // the tip can be reorganised; use a 144-block safety margin
CBlockIndex *pindexMax = GRC::BlockFinder::FindByHeight(nBestHeight);
std::map<CKeyID, CBlockIndex*> mapKeyFirstBlock;
std::set<CKeyID> setKeys;
GetKeys(setKeys);
Expand Down