zcash-hackworks
/
bitcoin
mirror of https://github.com/zcash-hackworks/bitcoin.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

Merge pull request #5875 

aa8c827 P2P regression test for new AcceptBlock behavior (Suhas Daftuar)
9be0e68 Be stricter in processing unrequested blocks (Suhas Daftuar)
This commit is contained in:
Wladimir J. van der Laan 2015-06-03 16:20:30 +02:00
parent 9d67b10091 aa8c827968
commit 9d60602444
No known key found for this signature in database
GPG Key ID: 74810B012346C9A6
7 changed files with 260 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
qa/pull-tester/rpc-tests.sh
View File

@ -51,6 +51,7 @@ testScriptsExt=(
'invalidblockrequest.py'
'rawtransactions.py'
# 'forknotify.py'
'p2p-acceptblock.py'
);
extArg="-extended"

226
qa/rpc-tests/p2p-acceptblock.py Executable file
View File

@ -0,0 +1,226 @@
#!/usr/bin/env python2
#
# Distributed under the MIT/X11 software license, see the accompanying
# file COPYING or http://www.opensource.org/licenses/mit-license.php.
#
from test_framework.mininode import *
from test_framework.test_framework import BitcoinTestFramework
from test_framework.util import *
import time
from test_framework.blocktools import create_block, create_coinbase
'''
AcceptBlockTest -- test processing of unrequested blocks.
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
whitelist localhost, but node1 does. They will each be on their own chain for
this test.
We have one NodeConn connection to each, test_node and white_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.
3. Mine a block that forks the previous block, and deliver to each node from
corresponding peer.
Node0 should not process this block (just accept the header), because it is
unrequested and doesn't have more work than the tip.
Node1 should process because this is coming from a whitelisted peer.
4. Send another block that builds on the forking block.
Node0 should process this block but be stuck on the shorter chain, because
it's missing an intermediate block.
Node1 should reorg to this longer chain.
5. Send a duplicate of the block in #3 to Node0.
Node0 should not process the block because it is unrequested, and stay on
the shorter chain.
6. Send Node0 an inv for the height 3 block produced in #4 above.
Node0 should figure out that Node0 has the missing height 2 block and send a
getdata.
7. Send Node0 the missing block again.
Node0 should process and the tip should advance.
'''
# TestNode: bare-bones "peer". Used mostly as a conduit for a test to sending
# p2p messages to a node, generating the messages in the main testing logic.
class TestNode(NodeConnCB):
def __init__(self):
NodeConnCB.__init__(self)
self.create_callback_map()
self.connection = None
def add_connection(self, conn):
self.connection = conn
# Track the last getdata message we receive (used in the test)
def on_getdata(self, conn, message):
self.last_getdata = message
# Spin until verack message is received from the node.
# We use this to signal that our test can begin. This
# is called from the testing thread, so it needs to acquire
# the global lock.
def wait_for_verack(self):
while True:
with mininode_lock:
if self.verack_received:
return
time.sleep(0.05)
# Wrapper for the NodeConn's send_message function
def send_message(self, message):
self.connection.send_message(message)
class AcceptBlockTest(BitcoinTestFramework):
def add_options(self, parser):
parser.add_option("--testbinary", dest="testbinary",
default=os.getenv("BITCOIND", "bitcoind"),
help="bitcoind binary to test")
def setup_chain(self):
initialize_chain_clean(self.options.tmpdir, 2)
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.
self.nodes = []
self.nodes.append(start_node(0, self.options.tmpdir, ["-debug"],
binary=self.options.testbinary))
self.nodes.append(start_node(1, self.options.tmpdir,
["-debug", "-whitelist=127.0.0.1"],
binary=self.options.testbinary))
def run_test(self):
# Setup the p2p connections and start up the network thread.
test_node = TestNode() # connects to node0 (not whitelisted)
white_node = TestNode() # connects to node1 (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))
test_node.add_connection(connections[0])
white_node.add_connection(connections[1])
NetworkThread().start() # Start up network handling in another thread
# Test logic begins here
test_node.wait_for_verack()
white_node.wait_for_verack()
# 1. Have both nodes mine a block (leave IBD)
[ n.generate(1) for n in self.nodes ]
tips = [ int ("0x" + n.getbestblockhash() + "L", 0) for n in self.nodes ]
# 2. Send one block that builds on each tip.
# This should be accepted.
blocks_h2 = [] # the height 2 blocks on each node's chain
for i in xrange(2):
blocks_h2.append(create_block(tips[i], create_coinbase(), time.time()+1))
blocks_h2[i].solve()
test_node.send_message(msg_block(blocks_h2[0]))
white_node.send_message(msg_block(blocks_h2[1]))
time.sleep(1)
assert_equal(self.nodes[0].getblockcount(), 2)
assert_equal(self.nodes[1].getblockcount(), 2)
print "First height 2 block accepted by both nodes"
# 3. Send another block that builds on the original tip.
blocks_h2f = [] # Blocks at height 2 that fork off the main chain
for i in xrange(2):
blocks_h2f.append(create_block(tips[i], create_coinbase(), blocks_h2[i].nTime+1))
blocks_h2f[i].solve()
test_node.send_message(msg_block(blocks_h2f[0]))
white_node.send_message(msg_block(blocks_h2f[1]))
time.sleep(1) # Give time to process the block
for x in self.nodes[0].getchaintips():
if x['hash'] == blocks_h2f[0].hash:
assert_equal(x['status'], "headers-only")
for x in self.nodes[1].getchaintips():
if x['hash'] == blocks_h2f[1].hash:
assert_equal(x['status'], "valid-headers")
print "Second height 2 block accepted only from whitelisted peer"
# 4. Now send another block that builds on the forking chain.
blocks_h3 = []
for i in xrange(2):
blocks_h3.append(create_block(blocks_h2f[i].sha256, create_coinbase(), blocks_h2f[i].nTime+1))
blocks_h3[i].solve()
test_node.send_message(msg_block(blocks_h3[0]))
white_node.send_message(msg_block(blocks_h3[1]))
time.sleep(1)
# Since the earlier block was not processed by node0, the new block
# can't be fully validated.
for x in self.nodes[0].getchaintips():
if x['hash'] == blocks_h3[0].hash:
assert_equal(x['status'], "headers-only")
# But this block should be accepted by node0 since it has more work.
try:
self.nodes[0].getblock(blocks_h3[0].hash)
print "Unrequested more-work block accepted from non-whitelisted peer"
except:
raise AssertionError("Unrequested more work block was not processed")
# Node1 should have accepted and reorged.
assert_equal(self.nodes[1].getblockcount(), 3)
print "Successfully reorged to length 3 chain from whitelisted peer"
# 5. Test handling of unrequested block on the node that didn't process
# Should still not be processed (even though it has a child that has more
# work).
test_node.send_message(msg_block(blocks_h2f[0]))
# Here, if the sleep is too short, the test could falsely succeed (if the
# node hasn't processed the block by the time the sleep returns, and then
# the node processes it and incorrectly advances the tip).
# But this would be caught later on, when we verify that an inv triggers
# a getdata request for this block.
time.sleep(1)
assert_equal(self.nodes[0].getblockcount(), 2)
print "Unrequested block that would complete more-work chain was ignored"
# 6. Try to get node to request the missing block.
# Poke the node with an inv for block at height 3 and see if that
# triggers a getdata on block 2 (it should if block 2 is missing).
with mininode_lock:
# Clear state so we can check the getdata request
test_node.last_getdata = None
test_node.send_message(msg_inv([CInv(2, blocks_h3[0].sha256)]))
time.sleep(1)
with mininode_lock:
getdata = test_node.last_getdata
# Check that the getdata is for the right block
assert_equal(len(getdata.inv), 1)
assert_equal(getdata.inv[0].hash, blocks_h2f[0].sha256)
print "Inv at tip triggered getdata for unprocessed block"
# 7. Send the missing block for the third time (now it is requested)
test_node.send_message(msg_block(blocks_h2f[0]))
time.sleep(1)
assert_equal(self.nodes[0].getblockcount(), 3)
print "Successfully reorged to length 3 chain from non-whitelisted peer"
[ c.disconnect_node() for c in connections ]
if __name__ == '__main__':
AcceptBlockTest().main()

40
src/main.cpp
View File

@ -296,7 +296,8 @@ void FinalizeNode(NodeId nodeid) {
}
// Requires cs_main.
void MarkBlockAsReceived(const uint256& hash) {
// Returns a bool indicating whether we requested this block.
bool MarkBlockAsReceived(const uint256& hash) {
map<uint256, pair<NodeId, list<QueuedBlock>::iterator> >::iterator itInFlight = mapBlocksInFlight.find(hash);
if (itInFlight != mapBlocksInFlight.end()) {
CNodeState *state = State(itInFlight->second.first);
@ -306,7 +307,9 @@ void MarkBlockAsReceived(const uint256& hash) {
state->nBlocksInFlight--;
state->nStallingSince = 0;
mapBlocksInFlight.erase(itInFlight);
return true;
}
return false;
}
// Requires cs_main.
@ -2826,7 +2829,7 @@ bool AcceptBlockHeader(const CBlockHeader& block, CValidationState& state, CBloc
return true;
}
bool AcceptBlock(CBlock& block, CValidationState& state, CBlockIndex** ppindex, CDiskBlockPos* dbp)
bool AcceptBlock(CBlock& block, CValidationState& state, CBlockIndex** ppindex, bool fRequested, CDiskBlockPos* dbp)
{
const CChainParams& chainparams = Params();
AssertLockHeld(cs_main);
@ -2836,13 +2839,18 @@ bool AcceptBlock(CBlock& block, CValidationState& state, CBlockIndex** ppindex,
if (!AcceptBlockHeader(block, state, &pindex))
return false;
// If we're pruning, ensure that we don't allow a peer to dump a copy
// of old blocks. But we might need blocks that are not on the main chain
// to handle a reorg, even if we've processed once.
if (pindex->nStatus & BLOCK_HAVE_DATA || chainActive.Contains(pindex)) {
// TODO: deal better with duplicate blocks.
// return state.DoS(20, error("AcceptBlock(): already have block %d %s", pindex->nHeight, pindex->GetBlockHash().ToString()), REJECT_DUPLICATE, "duplicate");
return true;
// Try to process all requested blocks that we don't have, but only
// process an unrequested block if it's new and has enough work to
// advance our tip.
bool fAlreadyHave = pindex->nStatus & BLOCK_HAVE_DATA;
bool fHasMoreWork = (chainActive.Tip() ? pindex->nChainWork > chainActive.Tip()->nChainWork : true);
// TODO: deal better with return value and error conditions for duplicate
// and unrequested blocks.
if (fAlreadyHave) return true;
if (!fRequested) { // If we didn't ask for it:
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 ((!CheckBlock(block, state)) || !ContextualCheckBlock(block, state, pindex->pprev)) {
@ -2891,21 +2899,22 @@ static bool IsSuperMajority(int minVersion, const CBlockIndex* pstart, unsigned
}
bool ProcessNewBlock(CValidationState &state, CNode* pfrom, CBlock* pblock, CDiskBlockPos *dbp)
bool ProcessNewBlock(CValidationState &state, CNode* pfrom, CBlock* pblock, bool fForceProcessing, CDiskBlockPos *dbp)
{
// Preliminary checks
bool checked = CheckBlock(*pblock, state);
{
LOCK(cs_main);
MarkBlockAsReceived(pblock->GetHash());
bool fRequested = MarkBlockAsReceived(pblock->GetHash());
fRequested |= fForceProcessing;
if (!checked) {
return error("%s: CheckBlock FAILED", __func__);
}
// Store to disk
CBlockIndex *pindex = NULL;
bool ret = AcceptBlock(*pblock, state, &pindex, dbp);
bool ret = AcceptBlock(*pblock, state, &pindex, fRequested, dbp);
if (pindex && pfrom) {
mapBlockSource[pindex->GetBlockHash()] = pfrom->GetId();
}
@ -3453,7 +3462,7 @@ bool LoadExternalBlockFile(FILE* fileIn, CDiskBlockPos *dbp)
// process in case the block isn't known yet
if (mapBlockIndex.count(hash) == 0 || (mapBlockIndex[hash]->nStatus & BLOCK_HAVE_DATA) == 0) {
CValidationState state;
if (ProcessNewBlock(state, NULL, &block, dbp))
if (ProcessNewBlock(state, NULL, &block, true, dbp))
nLoaded++;
if (state.IsError())
break;
@ -3475,7 +3484,7 @@ bool LoadExternalBlockFile(FILE* fileIn, CDiskBlockPos *dbp)
LogPrintf("%s: Processing out of order child %s of %s\n", __func__, block.GetHash().ToString(),
head.ToString());
CValidationState dummy;
if (ProcessNewBlock(dummy, NULL, &block, &it->second))
if (ProcessNewBlock(dummy, NULL, &block, true, &it->second))
{
nLoaded++;
queue.push_back(block.GetHash());
@ -4462,7 +4471,8 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv,
pfrom->AddInventoryKnown(inv);
CValidationState state;
ProcessNewBlock(state, pfrom, &block);
// Process all blocks from whitelisted peers, even if not requested.
ProcessNewBlock(state, pfrom, &block, pfrom->fWhitelisted, NULL);
int nDoS;
if (state.IsInvalid(nDoS)) {
pfrom->PushMessage("reject", strCommand, state.GetRejectCode(),

7
src/main.h
View File

@ -153,10 +153,11 @@ void UnregisterNodeSignals(CNodeSignals& nodeSignals);
* @param[out] state This may be set to an Error state if any error occurred processing it, including during validation/connection/etc of otherwise unrelated blocks during reorganisation; or it may be set to an Invalid state if pblock is itself invalid (but this is not guaranteed even when the block is checked). If you want to *possibly* get feedback on whether pblock is valid, you must also install a CValidationInterface (see validationinterface.h) - this will have its BlockChecked method called whenever *any* block completes validation.
* @param[in] pfrom The node which we are receiving the block from; it is added to mapBlockSource and may be penalised if the block is invalid.
* @param[in] pblock The block we want to process.
* @param[in] fForceProcessing Process this block even if unrequested; used for non-network block sources and whitelisted peers.
* @param[out] dbp If pblock is stored to disk (or already there), this will be set to its location.
* @return True if state.IsValid()
*/
bool ProcessNewBlock(CValidationState &state, CNode* pfrom, CBlock* pblock, CDiskBlockPos *dbp = NULL);
bool ProcessNewBlock(CValidationState &state, CNode* pfrom, CBlock* pblock, bool fForceProcessing, CDiskBlockPos *dbp);
/** Check whether enough disk space is available for an incoming block */
bool CheckDiskSpace(uint64_t nAdditionalBytes = 0);
/** Open a block file (blk?????.dat) */
@ -400,8 +401,8 @@ bool ContextualCheckBlock(const CBlock& block, CValidationState& state, CBlockIn
/** Check a block is completely valid from start to finish (only works on top of our current best block, with cs_main held) */
bool TestBlockValidity(CValidationState &state, const CBlock& block, CBlockIndex *pindexPrev, bool fCheckPOW = true, bool fCheckMerkleRoot = true);
/** Store block on disk. If dbp is provided, the file is known to already reside on disk */
bool AcceptBlock(CBlock& block, CValidationState& state, CBlockIndex **pindex, CDiskBlockPos* dbp = NULL);
/** Store block on disk. If dbp is non-NULL, the file is known to already reside on disk */
bool AcceptBlock(CBlock& block, CValidationState& state, CBlockIndex **pindex, bool fRequested, CDiskBlockPos* dbp);
bool AcceptBlockHeader(const CBlockHeader& block, CValidationState& state, CBlockIndex **ppindex= NULL);

2
src/miner.cpp
View File

@ -434,7 +434,7 @@ static bool ProcessBlockFound(CBlock* pblock, CWallet& wallet, CReserveKey& rese
// Process this block the same as if we had received it from another node
CValidationState state;
if (!ProcessNewBlock(state, NULL, pblock))
if (!ProcessNewBlock(state, NULL, pblock, true, NULL))
return error("BitcoinMiner: ProcessNewBlock, block not accepted");
return true;

4
src/rpcmining.cpp
View File

@ -164,7 +164,7 @@ Value generate(const Array& params, bool fHelp)
++pblock->nNonce;
}
CValidationState state;
if (!ProcessNewBlock(state, NULL, pblock))
if (!ProcessNewBlock(state, NULL, pblock, true, NULL))
throw JSONRPCError(RPC_INTERNAL_ERROR, "ProcessNewBlock, block not accepted");
++nHeight;
blockHashes.push_back(pblock->GetHash().GetHex());
@ -650,7 +650,7 @@ Value submitblock(const Array& params, bool fHelp)
CValidationState state;
submitblock_StateCatcher sc(block.GetHash());
RegisterValidationInterface(&sc);
bool fAccepted = ProcessNewBlock(state, NULL, &block);
bool fAccepted = ProcessNewBlock(state, NULL, &block, true, NULL);
UnregisterValidationInterface(&sc);
if (fBlockPresent)
{

2
src/test/miner_tests.cpp
View File

@ -84,7 +84,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity)
pblock->hashMerkleRoot = pblock->BuildMerkleTree();
pblock->nNonce = blockinfo[i].nonce;
CValidationState state;
BOOST_CHECK(ProcessNewBlock(state, NULL, pblock));
BOOST_CHECK(ProcessNewBlock(state, NULL, pblock, true, NULL));
BOOST_CHECK(state.IsValid());
pblock->hashPrevBlock = pblock->GetHash();
}