boxtec
/
bitcoin
mirror of https://github.com/bitcoin/bitcoin
4
1
Fork 0
Code Issues Projects Releases Wiki Activity

Merge #9026: Fix handling of invalid compact blocks

Browse Source 
d4833ff Bump the protocol version to distinguish new banning behavior. (Suhas Daftuar)
88c3549 Fix compact block handling to not ban if block is invalid (Suhas Daftuar)
c93beac [qa] Test that invalid compactblocks don't result in ban (Suhas Daftuar)
pull/9087/head
Pieter Wuille 8 years ago
parent 9f554e03eb d4833ff747
commit dc6b9406bd
No known key found for this signature in database
GPG Key ID: DBA1A67379A1A931
9 changed files with 81 additions and 21 deletions
Show Stats Download Patch File Download Diff File

37
qa/rpc-tests/p2p-compactblocks.py
Unescape View File

@ -708,6 +708,33 @@ class CompactBlocksTest(BitcoinTestFramework):
l.last_cmpctblock.header_and_shortids.header.calc_sha256()
assert_equal(l.last_cmpctblock.header_and_shortids.header.sha256, block.sha256)
# Test that we don't get disconnected if we relay a compact block with valid header,
# but invalid transactions.
def test_invalid_tx_in_compactblock(self, node, test_node, use_segwit):
assert(len(self.utxos))
utxo = self.utxos[0]
block = self.build_block_with_transactions(node, utxo, 5)
del block.vtx[3]
block.hashMerkleRoot = block.calc_merkle_root()
if use_segwit:
# If we're testing with segwit, also drop the coinbase witness,
# but include the witness commitment.
add_witness_commitment(block)
block.vtx[0].wit.vtxinwit = []
block.solve()
# Now send the compact block with all transactions prefilled, and
# verify that we don't get disconnected.
comp_block = HeaderAndShortIDs()
comp_block.initialize_from_block(block, prefill_list=[0, 1, 2, 3, 4], use_witness=use_segwit)
msg = msg_cmpctblock(comp_block.to_p2p())
test_node.send_and_ping(msg)
# Check that the tip didn't advance
assert(int(node.getbestblockhash(), 16) is not block.sha256)
test_node.sync_with_ping()
# Helper for enabling cb announcements
# Send the sendcmpct request and sync headers
def request_cb_announcements(self, peer, node, version):
@ -798,6 +825,11 @@ class CompactBlocksTest(BitcoinTestFramework):
self.test_end_to_end_block_relay(self.nodes[0], [self.segwit_node, self.test_node, self.old_node])
self.test_end_to_end_block_relay(self.nodes[1], [self.segwit_node, self.test_node, self.old_node])
print("\tTesting handling of invalid compact blocks...")
self.test_invalid_tx_in_compactblock(self.nodes[0], self.test_node, False)
self.test_invalid_tx_in_compactblock(self.nodes[1], self.segwit_node, False)
self.test_invalid_tx_in_compactblock(self.nodes[1], self.old_node, False)
# Advance to segwit activation
print ("\nAdvancing to segwit activation\n")
self.activate_segwit(self.nodes[1])
@ -844,6 +876,11 @@ class CompactBlocksTest(BitcoinTestFramework):
self.request_cb_announcements(self.segwit_node, self.nodes[1], 2)
self.test_end_to_end_block_relay(self.nodes[1], [self.segwit_node, self.test_node, self.old_node])
print("\tTesting handling of invalid compact blocks...")
self.test_invalid_tx_in_compactblock(self.nodes[0], self.test_node, False)
self.test_invalid_tx_in_compactblock(self.nodes[1], self.segwit_node, True)
self.test_invalid_tx_in_compactblock(self.nodes[1], self.old_node, True)
print("\tTesting invalid index in cmpctblock message...")
self.test_invalid_cmpctblock_message()

2
src/blockencodings.cpp
Unescape View File

@ -167,7 +167,7 @@ ReadStatus PartiallyDownloadedBlock::FillBlock(CBlock& block, const std::vector<
// check its own merkle root and cache that check.
if (state.CorruptionPossible())
return READ_STATUS_FAILED; // Possible Short ID collision
return READ_STATUS_INVALID;
return READ_STATUS_CHECKBLOCK_FAILED;
}
LogPrint("cmpctblock", "Successfully reconstructed block %s with %lu txn prefilled, %lu txn from mempool and %lu txn requested\n", header.GetHash().ToString(), prefilled_count, mempool_count, vtx_missing.size());

2
src/blockencodings.h
Unescape View File

@ -124,6 +124,8 @@ typedef enum ReadStatus_t
READ_STATUS_OK,
READ_STATUS_INVALID, // Invalid object, peer is sending bogus crap
READ_STATUS_FAILED, // Failed to process object
READ_STATUS_CHECKBLOCK_FAILED, // Used only by FillBlock to indicate a
// failure in CheckBlock.
} ReadStatus;
class CBlockHeaderAndShortTxIDs {

46
src/main.cpp
Unescape View File

@ -180,8 +180,10 @@ namespace {
* Sources of received blocks, saved to be able to send them reject
* messages or ban them when processing happens afterwards. Protected by
* cs_main.
* Set mapBlockSource[hash].second to false if the node should not be
* punished if the block is invalid.
*/
map<uint256, NodeId> mapBlockSource;
map<uint256, std::pair<NodeId, bool>> mapBlockSource;
/**
* Filter for transactions that were recently rejected by
@ -3785,7 +3787,7 @@ static bool AcceptBlock(const CBlock& block, CValidationState& state, const CCha
return true;
}
bool ProcessNewBlock(CValidationState& state, const CChainParams& chainparams, CNode* pfrom, const CBlock* pblock, bool fForceProcessing, const CDiskBlockPos* dbp)
bool ProcessNewBlock(CValidationState& state, const CChainParams& chainparams, CNode* pfrom, const CBlock* pblock, bool fForceProcessing, const CDiskBlockPos* dbp, bool fMayBanPeerIfInvalid)
{
{
LOCK(cs_main);
@ -3795,7 +3797,7 @@ bool ProcessNewBlock(CValidationState& state, const CChainParams& chainparams, C
bool fNewBlock = false;
bool ret = AcceptBlock(*pblock, state, chainparams, &pindex, fForceProcessing, dbp, &fNewBlock);
if (pindex && pfrom) {
mapBlockSource[pindex->GetBlockHash()] = pfrom->GetId();
mapBlockSource[pindex->GetBlockHash()] = std::make_pair(pfrom->GetId(), fMayBanPeerIfInvalid);
if (fNewBlock) pfrom->nLastBlockTime = GetTime();
}
CheckBlockIndex(chainparams.GetConsensus());
@ -4775,16 +4777,16 @@ void PeerLogicValidation::BlockChecked(const CBlock& block, const CValidationSta
LOCK(cs_main);
const uint256 hash(block.GetHash());
std::map<uint256, NodeId>::iterator it = mapBlockSource.find(hash);
std::map<uint256, std::pair<NodeId, bool>>::iterator it = mapBlockSource.find(hash);
int nDoS = 0;
if (state.IsInvalid(nDoS)) {
if (it != mapBlockSource.end() && State(it->second)) {
if (it != mapBlockSource.end() && State(it->second.first)) {
assert (state.GetRejectCode() < REJECT_INTERNAL); // Blocks are never rejected with internal reject codes
CBlockReject reject = {(unsigned char)state.GetRejectCode(), state.GetRejectReason().substr(0, MAX_REJECT_MESSAGE_LENGTH), hash};
State(it->second)->rejects.push_back(reject);
if (nDoS > 0)
Misbehaving(it->second, nDoS);
State(it->second.first)->rejects.push_back(reject);
if (nDoS > 0 && it->second.second)
Misbehaving(it->second.first, nDoS);
}
}
if (it != mapBlockSource.end())
@ -5893,6 +5895,23 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv,
invs.push_back(CInv(MSG_BLOCK | GetFetchFlags(pfrom, chainActive.Tip(), chainparams.GetConsensus()), resp.blockhash));
connman.PushMessage(pfrom, NetMsgType::GETDATA, invs);
} else {
// Block is either okay, or possibly we received
// READ_STATUS_CHECKBLOCK_FAILED.
// Note that CheckBlock can only fail for one of a few reasons:
// 1. bad-proof-of-work (impossible here, because we've already
// accepted the header)
// 2. merkleroot doesn't match the transactions given (already
// caught in FillBlock with READ_STATUS_FAILED, so
// impossible here)
// 3. the block is otherwise invalid (eg invalid coinbase,
// block is too big, too many legacy sigops, etc).
// So if CheckBlock failed, #3 is the only possibility.
// Under BIP 152, we don't DoS-ban unless proof of work is
// invalid (we don't require all the stateless checks to have
// been run). This is handled below, so just treat this as
// though the block was successfully read, and rely on the
// handling in ProcessNewBlock to ensure the block index is
// updated, reject messages go out, etc.
MarkBlockAsReceived(resp.blockhash); // it is now an empty pointer
fBlockRead = true;
}
@ -5901,16 +5920,15 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv,
CValidationState state;
// 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(state, chainparams, pfrom, &block, true, NULL);
// BIP 152 permits peers to relay compact blocks after validating
// the header only; we should not punish peers if the block turns
// out to be invalid.
ProcessNewBlock(state, chainparams, pfrom, &block, true, NULL, false);
int nDoS;
if (state.IsInvalid(nDoS)) {
assert (state.GetRejectCode() < REJECT_INTERNAL); // Blocks are never rejected with internal reject codes
connman.PushMessage(pfrom, NetMsgType::REJECT, strCommand, (unsigned char)state.GetRejectCode(),
state.GetRejectReason().substr(0, MAX_REJECT_MESSAGE_LENGTH), block.GetHash());
if (nDoS > 0) {
LOCK(cs_main);
Misbehaving(pfrom->GetId(), nDoS);
}
}
}
}
@ -6081,7 +6099,7 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv,
// need it even though it is not a candidate for a new best tip.
forceProcessing |= MarkBlockAsReceived(block.GetHash());
}
ProcessNewBlock(state, chainparams, pfrom, &block, forceProcessing, NULL);
ProcessNewBlock(state, chainparams, pfrom, &block, forceProcessing, NULL, true);
int nDoS;
if (state.IsInvalid(nDoS)) {
assert (state.GetRejectCode() < REJECT_INTERNAL); // Blocks are never rejected with internal reject codes

2
src/main.h
Unescape View File

@ -223,7 +223,7 @@ static const uint64_t MIN_DISK_SPACE_FOR_BLOCK_FILES = 550 * 1024 * 1024;
* @param[out] dbp The already known disk position of pblock, or NULL if not yet stored.
* @return True if state.IsValid()
*/
bool ProcessNewBlock(CValidationState& state, const CChainParams& chainparams, CNode* pfrom, const CBlock* pblock, bool fForceProcessing, const CDiskBlockPos* dbp);
bool ProcessNewBlock(CValidationState& state, const CChainParams& chainparams, CNode* pfrom, const CBlock* pblock, bool fForceProcessing, const CDiskBlockPos* dbp, bool fMayBanPeerIfInvalid);
/** Check whether enough disk space is available for an incoming block */
bool CheckDiskSpace(uint64_t nAdditionalBytes = 0);
/** Open a block file (blk?????.dat) */

4
src/rpc/mining.cpp
Unescape View File

@ -132,7 +132,7 @@ UniValue generateBlocks(boost::shared_ptr<CReserveScript> coinbaseScript, int nG
continue;
}
CValidationState state;
if (!ProcessNewBlock(state, Params(), NULL, pblock, true, NULL))
if (!ProcessNewBlock(state, Params(), NULL, pblock, true, NULL, false))
throw JSONRPCError(RPC_INTERNAL_ERROR, "ProcessNewBlock, block not accepted");
++nHeight;
blockHashes.push_back(pblock->GetHash().GetHex());
@ -757,7 +757,7 @@ UniValue submitblock(const JSONRPCRequest& request)
CValidationState state;
submitblock_StateCatcher sc(block.GetHash());
RegisterValidationInterface(&sc);
bool fAccepted = ProcessNewBlock(state, Params(), NULL, &block, true, NULL);
bool fAccepted = ProcessNewBlock(state, Params(), NULL, &block, true, NULL, false);
UnregisterValidationInterface(&sc);
if (fBlockPresent)
{

2
src/test/miner_tests.cpp
Unescape View File

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

2
src/test/test_bitcoin.cpp
Unescape View File

@ -127,7 +127,7 @@ TestChain100Setup::CreateAndProcessBlock(const std::vector<CMutableTransaction>&
while (!CheckProofOfWork(block.GetHash(), block.nBits, chainparams.GetConsensus())) ++block.nNonce;
CValidationState state;
ProcessNewBlock(state, chainparams, NULL, &block, true, NULL);
ProcessNewBlock(state, chainparams, NULL, &block, true, NULL, false);
CBlock result = block;
return result;

5
src/version.h
Unescape View File

@ -9,7 +9,7 @@
* network protocol versioning
*/
static const int PROTOCOL_VERSION = 70014;
static const int PROTOCOL_VERSION = 70015;
//! initial proto version, to be increased after version/verack negotiation
static const int INIT_PROTO_VERSION = 209;
@ -42,4 +42,7 @@ static const int FEEFILTER_VERSION = 70013;
//! short-id-based block download starts with this version
static const int SHORT_IDS_BLOCKS_VERSION = 70014;
//! not banning for invalid compact blocks starts with this version
static const int INVALID_CB_NO_BAN_VERSION = 70015;
#endif // BITCOIN_VERSION_H

Write Preview
Loading…
Cancel
Save