Merge #19413: refactor: Remove confusing BlockIndex global

fa0dfdf447 refactor: Remove confusing BlockIndex global (MarcoFalke)

Pull request description:

  The global `::BlockIndex()` is problematic for several reasons:

  * It returns a mutable reference to the block tree, without the appropriate lock annotation (`m_block_index` is guarded by `cs_main`). The current code is fine, but in the future this might lead to accidental races and data corruption.
  * The rpc server shouldn't rely on node globals, but rather a context that is passed in to the RPC method.
  * Tests might want to spin up their own block tree, and thus should also not rely on a single global.

  Fix all issues by removing the global

ACKs for top commit:
  promag:
    Code review ACK fa0dfdf447.
  jonatack:
    re-ACK fa0dfdf

Tree-SHA512: 8f158fc5e1c67e73588a21c25677b3fa0fe442313b13ec24b87054806c59607d6ba0c062a865ce3e0ee568706bd0d1faa84febda21aff5bcd65dab172f74c52f
This commit is contained in:
MarcoFalke
2020-07-03 07:37:15 -04:00
5 changed files with 22 additions and 32 deletions

View File

@ -333,7 +333,7 @@ BOOST_FIXTURE_TEST_CASE(coin_mark_dirty_immature_credit, TestChain100Setup)
BOOST_CHECK_EQUAL(wtx.GetImmatureCredit(), 50*COIN);
}
static int64_t AddTx(CWallet& wallet, uint32_t lockTime, int64_t mockTime, int64_t blockTime)
static int64_t AddTx(ChainstateManager& chainman, CWallet& wallet, uint32_t lockTime, int64_t mockTime, int64_t blockTime)
{
CMutableTransaction tx;
CWalletTx::Confirmation confirm;
@ -341,7 +341,8 @@ static int64_t AddTx(CWallet& wallet, uint32_t lockTime, int64_t mockTime, int64
SetMockTime(mockTime);
CBlockIndex* block = nullptr;
if (blockTime > 0) {
auto inserted = ::BlockIndex().emplace(GetRandHash(), new CBlockIndex);
LOCK(cs_main);
auto inserted = chainman.BlockIndex().emplace(GetRandHash(), new CBlockIndex);
assert(inserted.second);
const uint256& hash = inserted.first->first;
block = inserted.first->second;
@ -363,24 +364,24 @@ static int64_t AddTx(CWallet& wallet, uint32_t lockTime, int64_t mockTime, int64
BOOST_AUTO_TEST_CASE(ComputeTimeSmart)
{
// New transaction should use clock time if lower than block time.
BOOST_CHECK_EQUAL(AddTx(m_wallet, 1, 100, 120), 100);
BOOST_CHECK_EQUAL(AddTx(*m_node.chainman, m_wallet, 1, 100, 120), 100);
// Test that updating existing transaction does not change smart time.
BOOST_CHECK_EQUAL(AddTx(m_wallet, 1, 200, 220), 100);
BOOST_CHECK_EQUAL(AddTx(*m_node.chainman, m_wallet, 1, 200, 220), 100);
// New transaction should use clock time if there's no block time.
BOOST_CHECK_EQUAL(AddTx(m_wallet, 2, 300, 0), 300);
BOOST_CHECK_EQUAL(AddTx(*m_node.chainman, m_wallet, 2, 300, 0), 300);
// New transaction should use block time if lower than clock time.
BOOST_CHECK_EQUAL(AddTx(m_wallet, 3, 420, 400), 400);
BOOST_CHECK_EQUAL(AddTx(*m_node.chainman, m_wallet, 3, 420, 400), 400);
// New transaction should use latest entry time if higher than
// min(block time, clock time).
BOOST_CHECK_EQUAL(AddTx(m_wallet, 4, 500, 390), 400);
BOOST_CHECK_EQUAL(AddTx(*m_node.chainman, m_wallet, 4, 500, 390), 400);
// If there are future entries, new transaction should use time of the
// newest entry that is no more than 300 seconds ahead of the clock time.
BOOST_CHECK_EQUAL(AddTx(m_wallet, 5, 50, 600), 300);
BOOST_CHECK_EQUAL(AddTx(*m_node.chainman, m_wallet, 5, 50, 600), 300);
// Reset mock time for other tests.
SetMockTime(0);