Browse Source

Merge #8872: Remove block-request logic from INV message processing

037159c Remove block-request logic from INV message processing (Matt Corallo)
3451203 [qa] Respond to getheaders and do not assume a getdata on inv (Matt Corallo)
d768f15 [qa] Make comptool push blocks instead of relying on inv-fetch (mrbandrews)
0.14
Wladimir J. van der Laan 8 years ago
parent
commit
0c577f2638
No known key found for this signature in database
GPG Key ID: 74810B012346C9A6
  1. 7
      qa/rpc-tests/p2p-compactblocks.py
  2. 12
      qa/rpc-tests/p2p-segwit.py
  3. 11
      qa/rpc-tests/sendheaders.py
  4. 15
      qa/rpc-tests/test_framework/comptool.py
  5. 26
      src/main.cpp

7
qa/rpc-tests/p2p-compactblocks.py

@ -27,6 +27,7 @@ class TestNode(SingleNodeConnCB): @@ -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): @@ -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): @@ -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)

12
qa/rpc-tests/p2p-segwit.py

@ -64,6 +64,9 @@ class TestNode(NodeConnCB): @@ -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): @@ -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): @@ -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
if use_header:
self.last_getheaders = None
msg = msg_headers()
msg.headers = [ CBlockHeader(block) ]
if use_header:
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

11
qa/rpc-tests/sendheaders.py

@ -348,14 +348,13 @@ class SendHeadersTest(BitcoinTestFramework): @@ -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

15
qa/rpc-tests/test_framework/comptool.py

@ -111,6 +111,11 @@ class TestNode(NodeConnCB): @@ -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): @@ -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):
# 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): @@ -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

26
src/main.cpp

@ -5370,28 +5370,12 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv, @@ -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);
}
}

Loading…
Cancel
Save