mirror of
https://github.com/dashpay/dash.git
synced 2024-12-25 12:02:48 +01:00
Merge #11458: Don't process unrequested, low-work blocks
01b52ce
Add comment explaining forced processing of compact blocks (Suhas Daftuar)08fd822
qa: add test for minchainwork use in acceptblock (Suhas Daftuar)ce8cd7a
Don't process unrequested, low-work blocks (Suhas Daftuar) Pull request description: A peer could try to waste our resources by sending us unrequested blocks with low work (eg to fill up our disk). Sincee265200
we no longer request blocks until we know we're on a chain with more than nMinimumChainWork (our anti-DoS threshold), but we would still process unrequested blocks that had more work than our tip (which generally has low-work during IBD), even though we may not yet have found a headers chain with sufficient work. Fix this and add a test. Tree-SHA512: 1a4fb0bbd78054b84683f995c8c3194dd44fa914dc351ae4379c7c1a6f83224f609f8b9c2d9dde28741426c6af008ffffea836d21aa31a5ebaa00f8e0f81229e
This commit is contained in:
parent
9938dd83d4
commit
35d60de2b5
@ -2449,7 +2449,16 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
|
||||
mapBlockSource.emplace(pblock->GetHash(), std::make_pair(pfrom->GetId(), false));
|
||||
}
|
||||
bool fNewBlock = false;
|
||||
ProcessNewBlock(chainparams, pblock, true, &fNewBlock);
|
||||
// Setting fForceProcessing to true means that we bypass some of
|
||||
// our anti-DoS protections in AcceptBlock, which filters
|
||||
// unrequested blocks that might be trying to waste our resources
|
||||
// (eg disk space). Because we only try to reconstruct blocks when
|
||||
// we're close to caught up (via the CanDirectFetch() requirement
|
||||
// above, combined with the behavior of not requesting blocks until
|
||||
// we have a chain with at least nMinimumChainWork), and we ignore
|
||||
// compact blocks with less work than our tip, it is safe to treat
|
||||
// reconstructed compact blocks as having been requested.
|
||||
ProcessNewBlock(chainparams, pblock, /*fForceProcessing=*/true, &fNewBlock);
|
||||
if (fNewBlock) {
|
||||
pfrom->nLastBlockTime = GetTime();
|
||||
} else {
|
||||
@ -2529,7 +2538,11 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
|
||||
bool fNewBlock = false;
|
||||
// Since we requested this block (it was in mapBlocksInFlight), force it to be processed,
|
||||
// even if it would not be a candidate for new tip (missing previous block, chain not long enough, etc)
|
||||
ProcessNewBlock(chainparams, pblock, true, &fNewBlock);
|
||||
// This bypasses some anti-DoS logic in AcceptBlock (eg to prevent
|
||||
// disk-space attacks), but this should be safe due to the
|
||||
// protections in the compact block handler -- see related comment
|
||||
// in compact block optimistic reconstruction handling.
|
||||
ProcessNewBlock(chainparams, pblock, /*fForceProcessing=*/true, &fNewBlock);
|
||||
if (fNewBlock) {
|
||||
pfrom->nLastBlockTime = GetTime();
|
||||
} else {
|
||||
|
@ -3464,6 +3464,12 @@ static bool AcceptBlock(const std::shared_ptr<const CBlock>& pblock, CValidation
|
||||
if (pindex->nTx != 0) return true; // This is a previously-processed block that was pruned
|
||||
if (!fHasMoreWork) return true; // Don't process less-work chains
|
||||
if (fTooFarAhead) return true; // Block height is too high
|
||||
|
||||
// Protect against DoS attacks from low-work chains.
|
||||
// If our tip is behind, a peer could try to send us
|
||||
// low-work blocks on a fake chain that we would never
|
||||
// request; don't process these.
|
||||
if (pindex->nChainWork < nMinimumChainWork) return true;
|
||||
}
|
||||
if (fNewBlock) *fNewBlock = true;
|
||||
|
||||
|
@ -8,17 +8,22 @@ Since behavior differs when receiving unrequested blocks from whitelisted peers
|
||||
versus non-whitelisted peers, this tests the behavior of both (effectively two
|
||||
separate tests running in parallel).
|
||||
|
||||
Setup: two nodes, node0 and node1, not connected to each other. Node0 does not
|
||||
Setup: three nodes, node0+node1+node2, not connected to each other. Node0 does not
|
||||
whitelist localhost, but node1 does. They will each be on their own chain for
|
||||
this test.
|
||||
this test. Node2 will have nMinimumChainWork set to 0x10, so it won't process
|
||||
low-work unrequested blocks.
|
||||
|
||||
We have one NodeConn connection to each, test_node and white_node respectively.
|
||||
We have one NodeConn connection to each, test_node, white_node, and min_work_node,
|
||||
respectively.
|
||||
|
||||
The test:
|
||||
1. Generate one block on each node, to leave IBD.
|
||||
|
||||
2. Mine a new block on each tip, and deliver to each node from node's peer.
|
||||
The tip should advance.
|
||||
The tip should advance for node0 and node1, but node2 should skip processing
|
||||
due to nMinimumChainWork.
|
||||
|
||||
Node2 is unused in tests 3-7:
|
||||
|
||||
3. Mine a block that forks the previous block, and deliver to each node from
|
||||
corresponding peer.
|
||||
@ -46,6 +51,10 @@ The test:
|
||||
|
||||
7. Send Node0 the missing block again.
|
||||
Node0 should process and the tip should advance.
|
||||
|
||||
8. Test Node2 is able to sync when connected to node0 (which should have sufficient
|
||||
work on its chain).
|
||||
|
||||
"""
|
||||
|
||||
from test_framework.mininode import *
|
||||
@ -62,52 +71,60 @@ class AcceptBlockTest(BitcoinTestFramework):
|
||||
|
||||
def set_test_params(self):
|
||||
self.setup_clean_chain = True
|
||||
self.num_nodes = 2
|
||||
self.extra_args = [[], ["-whitelist=127.0.0.1"]]
|
||||
self.num_nodes = 3
|
||||
self.extra_args = [[], ["-whitelist=127.0.0.1"], ["-minimumchainwork=0x10"]]
|
||||
|
||||
def setup_network(self):
|
||||
# Node0 will be used to test behavior of processing unrequested blocks
|
||||
# from peers which are not whitelisted, while Node1 will be used for
|
||||
# the whitelisted case.
|
||||
# Node2 will be used for non-whitelisted peers to test the interaction
|
||||
# with nMinimumChainWork.
|
||||
self.setup_nodes()
|
||||
|
||||
def run_test(self):
|
||||
# Setup the p2p connections and start up the network thread.
|
||||
test_node = NodeConnCB() # connects to node0 (not whitelisted)
|
||||
white_node = NodeConnCB() # connects to node1 (whitelisted)
|
||||
min_work_node = NodeConnCB() # connects to node2 (not whitelisted)
|
||||
|
||||
connections = []
|
||||
connections.append(NodeConn('127.0.0.1', p2p_port(0), self.nodes[0], test_node))
|
||||
connections.append(NodeConn('127.0.0.1', p2p_port(1), self.nodes[1], white_node))
|
||||
connections.append(NodeConn('127.0.0.1', p2p_port(2), self.nodes[2], min_work_node))
|
||||
test_node.add_connection(connections[0])
|
||||
white_node.add_connection(connections[1])
|
||||
min_work_node.add_connection(connections[2])
|
||||
|
||||
NetworkThread().start() # Start up network handling in another thread
|
||||
|
||||
# Test logic begins here
|
||||
test_node.wait_for_verack()
|
||||
white_node.wait_for_verack()
|
||||
min_work_node.wait_for_verack()
|
||||
|
||||
# 1. Have both nodes mine a block (leave IBD)
|
||||
# 1. Have nodes mine a block (nodes1/2 leave IBD)
|
||||
[ n.generate(1) for n in self.nodes ]
|
||||
tips = [ int("0x" + n.getbestblockhash(), 0) for n in self.nodes ]
|
||||
|
||||
# 2. Send one block that builds on each tip.
|
||||
# This should be accepted.
|
||||
# This should be accepted by nodes 1/2
|
||||
blocks_h2 = [] # the height 2 blocks on each node's chain
|
||||
block_time = self.mocktime + 1
|
||||
for i in range(2):
|
||||
for i in range(3):
|
||||
blocks_h2.append(create_block(tips[i], create_coinbase(2), block_time + 1))
|
||||
blocks_h2[i].solve()
|
||||
block_time += 1
|
||||
test_node.send_message(msg_block(blocks_h2[0]))
|
||||
white_node.send_message(msg_block(blocks_h2[1]))
|
||||
min_work_node.send_message(msg_block(blocks_h2[2]))
|
||||
|
||||
for x in [test_node, white_node]:
|
||||
for x in [test_node, white_node, min_work_node]:
|
||||
x.sync_with_ping()
|
||||
assert_equal(self.nodes[0].getblockcount(), 2)
|
||||
assert_equal(self.nodes[1].getblockcount(), 2)
|
||||
self.log.info("First height 2 block accepted by both nodes")
|
||||
assert_equal(self.nodes[2].getblockcount(), 1)
|
||||
self.log.info("First height 2 block accepted by node0/node1; correctly rejected by node2")
|
||||
|
||||
# 3. Send another block that builds on the original tip.
|
||||
blocks_h2f = [] # Blocks at height 2 that fork off the main chain
|
||||
@ -221,6 +238,11 @@ class AcceptBlockTest(BitcoinTestFramework):
|
||||
assert_equal(self.nodes[0].getblockcount(), 290)
|
||||
self.log.info("Successfully reorged to longer chain from non-whitelisted peer")
|
||||
|
||||
# 8. Connect node2 to node0 and ensure it is able to sync
|
||||
connect_nodes(self.nodes[0], 2)
|
||||
sync_blocks([self.nodes[0], self.nodes[2]])
|
||||
self.log.info("Successfully synced nodes 2 and 0")
|
||||
|
||||
[ c.disconnect_node() for c in connections ]
|
||||
|
||||
if __name__ == '__main__':
|
||||
|
Loading…
Reference in New Issue
Block a user