diff options
author | Carl Dong <contact@carldong.me> | 2021-04-23 15:24:47 -0400 |
---|---|---|
committer | Carl Dong <contact@carldong.me> | 2021-05-27 13:50:11 -0400 |
commit | e6b4aa6eb53dc555ecab2922af35e7a2572faf4f (patch) | |
tree | 42927d85d5a098a2a333d32cbcc0333eaba78283 /src | |
parent | 9ecade14252ad1972f668d2d2e4ef44fdfcb944a (diff) | |
download | bitcoin-e6b4aa6eb53dc555ecab2922af35e7a2572faf4f.tar.xz |
miner: Pass in chainman to RegenerateCommitments
Pass in chainman instead of prev_block so that we can enforce the
block.hashPrevBlock refers to prev_block invariant in the function
itself.
We should probably rethink BlockAssembler's API and somehow include
commitment regeneration functionality in there. Something like a variant
of CreateNewBlock that takes in a std::vector<TxRef> and return a CBlock
instead of CBlockTemplate. That could avoid reaching for
LookupBlockIndex at all.
Diffstat (limited to 'src')
-rw-r--r-- | src/miner.cpp | 12 | ||||
-rw-r--r-- | src/miner.h | 2 | ||||
-rw-r--r-- | src/rpc/mining.cpp | 3 | ||||
-rw-r--r-- | src/test/util/setup_common.cpp | 3 |
4 files changed, 13 insertions, 7 deletions
diff --git a/src/miner.cpp b/src/miner.cpp index 3bc7fdd458..eccddbb04f 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -39,13 +39,21 @@ int64_t UpdateTime(CBlockHeader* pblock, const Consensus::Params& consensusParam return nNewTime - nOldTime; } -void RegenerateCommitments(CBlock& block, CBlockIndex* prev_block) +void RegenerateCommitments(CBlock& block, ChainstateManager& chainman) { CMutableTransaction tx{*block.vtx.at(0)}; tx.vout.erase(tx.vout.begin() + GetWitnessCommitmentIndex(block)); block.vtx.at(0) = MakeTransactionRef(tx); - WITH_LOCK(::cs_main, assert(g_chainman.m_blockman.LookupBlockIndex(block.hashPrevBlock) == prev_block)); + CBlockIndex* prev_block; + { + // TODO: Temporary scope to check correctness of refactored code. + // Should be removed manually after merge of + // https://github.com/bitcoin/bitcoin/pull/20158 + LOCK(::cs_main); + assert(std::addressof(g_chainman.m_blockman) == std::addressof(chainman.m_blockman)); + prev_block = chainman.m_blockman.LookupBlockIndex(block.hashPrevBlock); + } GenerateCoinbaseCommitment(block, prev_block, Params().GetConsensus()); block.hashMerkleRoot = BlockMerkleRoot(block); diff --git a/src/miner.h b/src/miner.h index becf362b79..10a80f4392 100644 --- a/src/miner.h +++ b/src/miner.h @@ -203,6 +203,6 @@ void IncrementExtraNonce(CBlock* pblock, const CBlockIndex* pindexPrev, unsigned int64_t UpdateTime(CBlockHeader* pblock, const Consensus::Params& consensusParams, const CBlockIndex* pindexPrev); /** Update an old GenerateCoinbaseCommitment from CreateNewBlock after the block txs have changed */ -void RegenerateCommitments(CBlock& block, CBlockIndex* prev_block); +void RegenerateCommitments(CBlock& block, ChainstateManager& chainman); #endif // BITCOIN_MINER_H diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index 8190a2f006..6826e6fd07 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -378,8 +378,7 @@ static RPCHelpMan generateblock() // Add transactions block.vtx.insert(block.vtx.end(), txs.begin(), txs.end()); - CBlockIndex* prev_block = WITH_LOCK(::cs_main, return chainman.m_blockman.LookupBlockIndex(block.hashPrevBlock)); - RegenerateCommitments(block, prev_block); + RegenerateCommitments(block, chainman); { LOCK(cs_main); diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index f92e4c4b99..f53c505e7c 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -246,8 +246,7 @@ CBlock TestChain100Setup::CreateAndProcessBlock(const std::vector<CMutableTransa for (const CMutableTransaction& tx : txns) { block.vtx.push_back(MakeTransactionRef(tx)); } - CBlockIndex* prev_block = WITH_LOCK(::cs_main, return g_chainman.m_blockman.LookupBlockIndex(block.hashPrevBlock)); - RegenerateCommitments(block, prev_block); + RegenerateCommitments(block, *Assert(m_node.chainman)); while (!CheckProofOfWork(block.GetHash(), block.nBits, chainparams.GetConsensus())) ++block.nNonce; |