diff options
| -rwxr-xr-x | qa/rpc-tests/p2p-compactblocks.py | 7 | ||||
| -rwxr-xr-x | qa/rpc-tests/p2p-segwit.py | 14 | ||||
| -rwxr-xr-x | qa/rpc-tests/sendheaders.py | 11 | ||||
| -rwxr-xr-x | qa/rpc-tests/test_framework/comptool.py | 19 | ||||
| -rw-r--r-- | src/main.cpp | 26 |
5 files changed, 46 insertions, 31 deletions
diff --git a/qa/rpc-tests/p2p-compactblocks.py b/qa/rpc-tests/p2p-compactblocks.py index e0b72e6840..ab4b809ded 100755 --- a/qa/rpc-tests/p2p-compactblocks.py +++ b/qa/rpc-tests/p2p-compactblocks.py @@ -27,6 +27,7 @@ class TestNode(SingleNodeConnCB): self.last_cmpctblock = None self.block_announced = False self.last_getdata = None + self.last_getheaders = None self.last_getblocktxn = None self.last_block = None self.last_blocktxn = None @@ -64,6 +65,9 @@ class TestNode(SingleNodeConnCB): def on_getdata(self, conn, message): self.last_getdata = message + def on_getheaders(self, conn, message): + self.last_getheaders = message + def on_getblocktxn(self, conn, message): self.last_getblocktxn = message @@ -396,6 +400,9 @@ class CompactBlocksTest(BitcoinTestFramework): if announce == "inv": test_node.send_message(msg_inv([CInv(2, block.sha256)])) + success = wait_until(lambda: test_node.last_getheaders is not None, timeout=30) + assert(success) + test_node.send_header_for_blocks([block]) else: test_node.send_header_for_blocks([block]) success = wait_until(lambda: test_node.last_getdata is not None, timeout=30) diff --git a/qa/rpc-tests/p2p-segwit.py b/qa/rpc-tests/p2p-segwit.py index 09ab1b80fc..6ecd84c3f0 100755 --- a/qa/rpc-tests/p2p-segwit.py +++ b/qa/rpc-tests/p2p-segwit.py @@ -64,6 +64,9 @@ class TestNode(NodeConnCB): self.getdataset.add(inv.hash) self.last_getdata = message + def on_getheaders(self, conn, message): + self.last_getheaders = message + def on_pong(self, conn, message): self.last_pong = message @@ -97,6 +100,10 @@ class TestNode(NodeConnCB): test_function = lambda: self.last_getdata != None self.sync(test_function, timeout) + def wait_for_getheaders(self, timeout=60): + test_function = lambda: self.last_getheaders != None + self.sync(test_function, timeout) + def wait_for_inv(self, expected_inv, timeout=60): test_function = lambda: self.last_inv != expected_inv self.sync(test_function, timeout) @@ -111,12 +118,15 @@ class TestNode(NodeConnCB): def announce_block_and_wait_for_getdata(self, block, use_header, timeout=60): with mininode_lock: self.last_getdata = None + self.last_getheaders = None + msg = msg_headers() + msg.headers = [ CBlockHeader(block) ] if use_header: - msg = msg_headers() - msg.headers = [ CBlockHeader(block) ] self.send_message(msg) else: self.send_message(msg_inv(inv=[CInv(2, block.sha256)])) + self.wait_for_getheaders() + self.send_message(msg) self.wait_for_getdata() return diff --git a/qa/rpc-tests/sendheaders.py b/qa/rpc-tests/sendheaders.py index 81b2442e6a..37b98c576e 100755 --- a/qa/rpc-tests/sendheaders.py +++ b/qa/rpc-tests/sendheaders.py @@ -348,14 +348,13 @@ class SendHeadersTest(BitcoinTestFramework): if j == 0: # Announce via inv test_node.send_block_inv(tip) - test_node.wait_for_getdata([tip], timeout=5) + test_node.wait_for_getheaders(timeout=5) + # Should have received a getheaders now + test_node.send_header_for_blocks(blocks) # Test that duplicate inv's won't result in duplicate # getdata requests, or duplicate headers announcements - inv_node.send_block_inv(tip) - # Should have received a getheaders as well! - test_node.send_header_for_blocks(blocks) - test_node.wait_for_getdata([x.sha256 for x in blocks[0:-1]], timeout=5) - [ inv_node.send_block_inv(x.sha256) for x in blocks[0:-1] ] + [ inv_node.send_block_inv(x.sha256) for x in blocks ] + test_node.wait_for_getdata([x.sha256 for x in blocks], timeout=5) inv_node.sync_with_ping() else: # Announce via headers diff --git a/qa/rpc-tests/test_framework/comptool.py b/qa/rpc-tests/test_framework/comptool.py index 7c92d3f828..17679fc7e1 100755 --- a/qa/rpc-tests/test_framework/comptool.py +++ b/qa/rpc-tests/test_framework/comptool.py @@ -111,6 +111,11 @@ class TestNode(NodeConnCB): m.locator = self.block_store.get_locator(self.bestblockhash) self.conn.send_message(m) + def send_header(self, header): + m = msg_headers() + m.headers.append(header) + self.conn.send_message(m) + # This assumes BIP31 def send_ping(self, nonce): self.pingMap[nonce] = True @@ -345,8 +350,16 @@ class TestManager(object): # Either send inv's to each node and sync, or add # to invqueue for later inv'ing. if (test_instance.sync_every_block): - [ c.cb.send_inv(block) for c in self.connections ] - self.sync_blocks(block.sha256, 1) + # if we expect success, send inv and sync every block + # if we expect failure, just push the block and see what happens. + if outcome == True: + [ c.cb.send_inv(block) for c in self.connections ] + self.sync_blocks(block.sha256, 1) + else: + [ c.send_message(msg_block(block)) for c in self.connections ] + [ c.cb.send_ping(self.ping_counter) for c in self.connections ] + self.wait_for_pings(self.ping_counter) + self.ping_counter += 1 if (not self.check_results(tip, outcome)): raise AssertionError("Test failed at test %d" % test_number) else: @@ -354,6 +367,8 @@ class TestManager(object): elif isinstance(b_or_t, CBlockHeader): block_header = b_or_t self.block_store.add_header(block_header) + [ c.cb.send_header(block_header) for c in self.connections ] + else: # Tx test runner assert(isinstance(b_or_t, CTransaction)) tx = b_or_t diff --git a/src/main.cpp b/src/main.cpp index 52f53ce87a..4293a6bebf 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -5370,28 +5370,12 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv, if (inv.type == MSG_BLOCK) { UpdateBlockAvailability(pfrom->GetId(), inv.hash); if (!fAlreadyHave && !fImporting && !fReindex && !mapBlocksInFlight.count(inv.hash)) { - // First request the headers preceding the announced block. In the normal fully-synced - // case where a new block is announced that succeeds the current tip (no reorganization), - // there are no such headers. - // Secondly, and only when we are close to being synced, we request the announced block directly, - // to avoid an extra round-trip. Note that we must *first* ask for the headers, so by the - // time the block arrives, the header chain leading up to it is already validated. Not - // doing this will result in the received block being rejected as an orphan in case it is - // not a direct successor. + // We used to request the full block here, but since headers-announcements are now the + // primary method of announcement on the network, and since, in the case that a node + // fell back to inv we probably have a reorg which we should get the headers for first, + // we now only provide a getheaders response here. When we receive the headers, we will + // then ask for the blocks we need. connman.PushMessage(pfrom, NetMsgType::GETHEADERS, chainActive.GetLocator(pindexBestHeader), inv.hash); - CNodeState *nodestate = State(pfrom->GetId()); - if (CanDirectFetch(chainparams.GetConsensus()) && - nodestate->nBlocksInFlight < MAX_BLOCKS_IN_TRANSIT_PER_PEER && - (!IsWitnessEnabled(chainActive.Tip(), chainparams.GetConsensus()) || State(pfrom->GetId())->fHaveWitness)) { - inv.type |= nFetchFlags; - if (nodestate->fSupportsDesiredCmpctVersion) - vToFetch.push_back(CInv(MSG_CMPCT_BLOCK, inv.hash)); - else - vToFetch.push_back(inv); - // Mark block as in flight already, even though the actual "getdata" message only goes out - // later (within the same cs_main lock, though). - MarkBlockAsInFlight(pfrom->GetId(), inv.hash, chainparams.GetConsensus()); - } LogPrint("net", "getheaders (%d) %s to peer=%d\n", pindexBestHeader->nHeight, inv.hash.ToString(), pfrom->id); } } |