Merge #12431: Only call NotifyBlockTip when chainActive changes

f98b54352 Only call NotifyBlockTip when the active chain changes (James O'Beirne)
152b7fb25 [tests] Add a (failing) test for waitforblockheight (James O'Beirne)

Pull request description:

  This is a subset of the more controversial https://github.com/bitcoin/bitcoin/pull/12407, but this also adds a test demonstrating the bug.

  In InvalidateBlock, we're calling NotifyBlockTip with the now-invalid block's prev regardless of what chain the ancestor block is on. This could create numerous issues, but it at least screws up `waitforblockheight` (or anything else relying on `rpc/blockchain.cpp:latestblock`) when InvalidateBlock is called on a block not in chainActive, which can happen via RPC.

  Only call NotifyBlockTip when the block being marked invalid is on the active chain.

Tree-SHA512: 9a54fe5e8c7eb489daf5df4483c0986129e871e2ca931a456ba869ecb5d5a8d4f7bd27ccc9e711e9292c9ed79ddef896c85d0e81fc76883503e327995b0e914f
This commit is contained in:
Wladimir J. van der Laan 2018-03-15 16:55:50 +01:00
commit 947c25ead2
No known key found for this signature in database
GPG key ID: 1E4AED62986CD25D
2 changed files with 62 additions and 1 deletions

View file

@ -2780,7 +2780,11 @@ bool CChainState::InvalidateBlock(CValidationState& state, const CChainParams& c
}
InvalidChainFound(pindex);
uiInterface.NotifyBlockTip(IsInitialBlockDownload(), pindex->pprev);
// Only notify about a new block tip if the active chain was modified.
if (pindex_was_in_chain) {
uiInterface.NotifyBlockTip(IsInitialBlockDownload(), pindex->pprev);
}
return true;
}
bool InvalidateBlock(CValidationState& state, const CChainParams& chainparams, CBlockIndex *pindex) {

View file

@ -32,6 +32,18 @@ from test_framework.util import (
assert_is_hex_string,
assert_is_hash_string,
)
from test_framework.blocktools import (
create_block,
create_coinbase,
)
from test_framework.messages import (
msg_block,
)
from test_framework.mininode import (
P2PInterface,
network_thread_start,
)
class BlockchainTest(BitcoinTestFramework):
def set_test_params(self):
@ -46,6 +58,7 @@ class BlockchainTest(BitcoinTestFramework):
self._test_getdifficulty()
self._test_getnetworkhashps()
self._test_stopatheight()
self._test_waitforblockheight()
assert self.nodes[0].verifychain(4, 0)
def _test_getblockchaininfo(self):
@ -241,6 +254,50 @@ class BlockchainTest(BitcoinTestFramework):
self.start_node(0)
assert_equal(self.nodes[0].getblockcount(), 207)
def _test_waitforblockheight(self):
self.log.info("Test waitforblockheight")
node = self.nodes[0]
# Start a P2P connection since we'll need to create some blocks.
node.add_p2p_connection(P2PInterface())
network_thread_start()
node.p2p.wait_for_verack()
current_height = node.getblock(node.getbestblockhash())['height']
# Create a fork somewhere below our current height, invalidate the tip
# of that fork, and then ensure that waitforblockheight still
# works as expected.
#
# (Previously this was broken based on setting
# `rpc/blockchain.cpp:latestblock` incorrectly.)
#
b20hash = node.getblockhash(20)
b20 = node.getblock(b20hash)
def solve_and_send_block(prevhash, height, time):
b = create_block(prevhash, create_coinbase(height), time)
b.solve()
node.p2p.send_message(msg_block(b))
node.p2p.sync_with_ping()
return b
b21f = solve_and_send_block(int(b20hash, 16), 21, b20['time'] + 1)
b22f = solve_and_send_block(b21f.sha256, 22, b21f.nTime + 1)
node.invalidateblock(b22f.hash)
def assert_waitforheight(height, timeout=2):
assert_equal(
node.waitforblockheight(height, timeout)['height'],
current_height)
assert_waitforheight(0)
assert_waitforheight(current_height - 1)
assert_waitforheight(current_height)
assert_waitforheight(current_height + 1)
if __name__ == '__main__':
BlockchainTest().main()