diff --git a/src/chain.h b/src/chain.h index c6d1640768e..f5bfdb2fb4b 100644 --- a/src/chain.h +++ b/src/chain.h @@ -292,7 +292,7 @@ public: std::string ToString() const; //! Check whether this block index entry is valid up to the passed validity level. - bool IsValid(enum BlockStatus nUpTo = BLOCK_VALID_TRANSACTIONS) const + bool IsValid(enum BlockStatus nUpTo) const EXCLUSIVE_LOCKS_REQUIRED(::cs_main) { AssertLockHeld(::cs_main); diff --git a/src/test/fuzz/chain.cpp b/src/test/fuzz/chain.cpp index 0363f317b63..09053e4815c 100644 --- a/src/test/fuzz/chain.cpp +++ b/src/test/fuzz/chain.cpp @@ -30,7 +30,7 @@ FUZZ_TARGET(chain) (void)disk_block_index->GetMedianTimePast(); (void)disk_block_index->GetUndoPos(); (void)disk_block_index->HaveNumChainTxs(); - (void)disk_block_index->IsValid(); + (void)disk_block_index->IsValid(BLOCK_VALID_TRANSACTIONS); } const CBlockHeader block_header = disk_block_index->GetBlockHeader(); diff --git a/src/validation.cpp b/src/validation.cpp index 88180caca90..3cfcd2728cd 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -3851,9 +3851,9 @@ void Chainstate::ResetBlockFailureFlags(CBlockIndex *pindex) { int nHeight = pindex->nHeight; - // Remove the invalidity flag from this block and all its descendants. + // Remove the invalidity flag from this block and all its descendants and ancestors. for (auto& [_, block_index] : m_blockman.m_block_index) { - if (!block_index.IsValid() && block_index.GetAncestor(nHeight) == pindex) { + if ((block_index.nStatus & BLOCK_FAILED_MASK) && (block_index.GetAncestor(nHeight) == pindex || pindex->GetAncestor(block_index.nHeight) == &block_index)) { block_index.nStatus &= ~BLOCK_FAILED_MASK; m_blockman.m_dirty_blockindex.insert(&block_index); if (block_index.IsValid(BLOCK_VALID_TRANSACTIONS) && block_index.HaveNumChainTxs() && setBlockIndexCandidates.value_comp()(m_chain.Tip(), &block_index)) { @@ -3865,15 +3865,6 @@ void Chainstate::ResetBlockFailureFlags(CBlockIndex *pindex) { } } } - - // Remove the invalidity flag from all ancestors too. - while (pindex != nullptr) { - if (pindex->nStatus & BLOCK_FAILED_MASK) { - pindex->nStatus &= ~BLOCK_FAILED_MASK; - m_blockman.m_dirty_blockindex.insert(pindex); - } - pindex = pindex->pprev; - } } void Chainstate::TryAddBlockIndexCandidate(CBlockIndex* pindex) diff --git a/src/validation.h b/src/validation.h index 727b0255826..624a2ee8d6b 100644 --- a/src/validation.h +++ b/src/validation.h @@ -745,7 +745,7 @@ public: /** Set invalidity status to all descendants of a block */ void SetBlockFailureFlags(CBlockIndex* pindex) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); - /** Remove invalidity status from a block and its descendants. */ + /** Remove invalidity status from a block, its descendants and ancestors and reconsider them for activation */ void ResetBlockFailureFlags(CBlockIndex* pindex) EXCLUSIVE_LOCKS_REQUIRED(cs_main); /** Replay blocks that aren't fully applied to the database. */ diff --git a/test/functional/rpc_invalidateblock.py b/test/functional/rpc_invalidateblock.py index e2eb033f09c..031f1dd86ce 100755 --- a/test/functional/rpc_invalidateblock.py +++ b/test/functional/rpc_invalidateblock.py @@ -85,6 +85,23 @@ class InvalidateTest(BitcoinTestFramework): self.wait_until(lambda: self.nodes[0].getblockcount() == 4, timeout=5) self.wait_until(lambda: self.nodes[1].getblockcount() == 4, timeout=5) + self.log.info("Verify that ancestors can become chain tip candidates when we reconsider blocks") + # Invalidate node0's current chain (1' -> 2' -> 3' -> 4') so that we don't reorg back to it in this test + badhash = self.nodes[0].getblockhash(1) + self.nodes[0].invalidateblock(badhash) + # Reconsider the tip so that node0's chain becomes this chain again : 1 -> 2 -> 3 -> 4 -> 5 -> 6 -> header 7 + self.nodes[0].reconsiderblock(tip) + blockhash_3 = self.nodes[0].getblockhash(3) + blockhash_4 = self.nodes[0].getblockhash(4) + blockhash_6 = self.nodes[0].getblockhash(6) + assert_equal(self.nodes[0].getbestblockhash(), blockhash_6) + + # Invalidate block 4 so that chain becomes : 1 -> 2 -> 3 + self.nodes[0].invalidateblock(blockhash_4) + assert_equal(self.nodes[0].getbestblockhash(), blockhash_3) + assert_equal(self.nodes[0].getblockchaininfo()['blocks'], 3) + assert_equal(self.nodes[0].getblockchaininfo()['headers'], 3) + self.log.info("Verify that we reconsider all ancestors as well") blocks = self.generatetodescriptor(self.nodes[1], 10, ADDRESS_BCRT1_UNSPENDABLE_DESCRIPTOR, sync_fun=self.no_op) assert_equal(self.nodes[1].getbestblockhash(), blocks[-1]) @@ -111,6 +128,14 @@ class InvalidateTest(BitcoinTestFramework): # Should report consistent blockchain info assert_equal(self.nodes[1].getblockchaininfo()["headers"], self.nodes[1].getblockchaininfo()["blocks"]) + # Reconsider the header + self.nodes[0].reconsiderblock(block.hash) + # Since header doesn't have block data, it can't be chain tip + # Check if it's possible for an ancestor (with block data) to be the chain tip + assert_equal(self.nodes[0].getbestblockhash(), blockhash_6) + assert_equal(self.nodes[0].getblockchaininfo()['blocks'], 6) + assert_equal(self.nodes[0].getblockchaininfo()['headers'], 7) + self.log.info("Verify that invalidating an unknown block throws an error") assert_raises_rpc_error(-5, "Block not found", self.nodes[1].invalidateblock, "00" * 32) assert_equal(self.nodes[1].getbestblockhash(), blocks[-1])