Skip to content

Commit

Permalink
[BUG][P2P] Update mapBlockSource before processing new received block
Browse files Browse the repository at this point in the history 
This bug is visible in the numerous commented "expected_disconnect"
lines in feature_block.py.
When the validation state is sent via the BlockChecked signal, after
ConnectBlock failures in ConnectTip, the mapBlockSource is not updated
yet, so there is no node to disconnect.
  • Loading branch information
@random-zebra
random-zebra committed May 17, 2021
1 parent db5e692 commit a6d74e7
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 31 deletions.
7 changes: 5 additions & 2 deletions src/net_processing.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1726,13 +1726,16 @@ bool static ProcessMessage(CNode* pfrom, std::string strCommand, CDataStream& vR
pfrom->AddInventoryKnown(inv);
CValidationState state;
if (!mapBlockIndex.count(hashBlock)) {
WITH_LOCK(cs_main, MarkBlockAsReceived(hashBlock); );
{
LOCK(cs_main);
MarkBlockAsReceived(hashBlock);
mapBlockSource.emplace(hashBlock, pfrom->GetId());
}
bool fAccepted = true;
ProcessNewBlock(state, pfrom, pblock, nullptr, &fAccepted);
if (!fAccepted) {
CheckBlockSpam(state, pfrom, hashBlock);
}
WITH_LOCK(cs_main, mapBlockSource.emplace(hashBlock, pfrom->GetId()); );
int nDoS;
if(state.IsInvalid(nDoS)) {
assert (state.GetRejectCode() < REJECT_INTERNAL); // Blocks are never rejected with internal reject codes
Expand Down
44 changes: 15 additions & 29 deletions test/functional/feature_block.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -181,8 +181,7 @@ def run_test(self):
self.log.info("Reject a block where the miner creates too much coinbase reward")
self.move_tip(6)
b9 = self.next_block(9, spend=out[4], additional_coinbase_value=1)
# !TODO: fix expect_disconnect
self.send_blocks([b9], False, "bad-blk-amount", reconnect=False)
self.send_blocks([b9], False, "bad-blk-amount", reconnect=True)

# Create a fork that ends in a block with too much fee (the one that causes the reorg)
# genesis -> b1 (0) -> b2 (1) -> b5 (2) -> b6 (3)
Expand All @@ -194,8 +193,7 @@ def run_test(self):
self.send_blocks([b10], False)

b11 = self.next_block(11, spend=out[4], additional_coinbase_value=1)
# !TODO: fix expect_disconnect
self.send_blocks([b11], False, "bad-blk-amount", reconnect=False)
self.send_blocks([b11], False, "bad-blk-amount", reconnect=True)

# Try again, but with a valid fork first
# genesis -> b1 (0) -> b2 (1) -> b5 (2) -> b6 (3)
Expand All @@ -208,8 +206,7 @@ def run_test(self):
b13 = self.next_block(13, spend=out[4])
self.save_spendable_output()
b14 = self.next_block(14, spend=out[5], additional_coinbase_value=1)
# !TODO: fix expect_disconnect
self.send_blocks([b12, b13, b14], False, "bad-blk-amount", reconnect=False)
self.send_blocks([b12, b13, b14], False, "bad-blk-amount", reconnect=True)

# New tip should be b13.
assert_equal(node.getbestblockhash(), b13.hash)
Expand Down Expand Up @@ -237,8 +234,7 @@ def run_test(self):
self.log.info("Reject a block with a spend from a re-org'ed out tx")
self.move_tip(15)
b17 = self.next_block(17, spend=txout_b3)
# !TODO: fix expect_disconnect
self.send_blocks([b17], False, "bad-txns-inputs-missingorspent", reconnect=False)
self.send_blocks([b17], False, "bad-txns-inputs-missingorspent", reconnect=True)

# Attempt to spend a transaction created on a different fork (on a fork this time)
# genesis -> b1 (0) -> b2 (1) -> b5 (2) -> b6 (3)
Expand Down Expand Up @@ -402,14 +398,12 @@ def run_test(self):
txout_b37 = PreviousSpendableOutput(b37.vtx[1], 0)
tx = self.create_and_sign_transaction(out[11].tx, out[11].n, 0)
b37 = self.update_block(37, [tx])
# !TODO: fix expect_disconnect
self.send_blocks([b37], False, "bad-txns-inputs-missingorspent", reconnect=False)
self.send_blocks([b37], False, "bad-txns-inputs-missingorspent", reconnect=True)

# attempt to spend b37's first non-coinbase tx, at which point b37 was still considered valid
self.move_tip(35)
b38 = self.next_block(38, spend=txout_b37)
# !TODO: fix expect_disconnect
self.send_blocks([b38], False, "bad-txns-inputs-missingorspent", reconnect=False)
self.send_blocks([b38], False, "bad-txns-inputs-missingorspent", reconnect=True)

# Check P2SH SigOp counting
#
Expand Down Expand Up @@ -501,8 +495,7 @@ def run_test(self):
tx.rehash()
new_txs.append(tx)
self.update_block(40, new_txs)
# !TODO: fix expect_disconnect
self.send_blocks([b40], False, "bad-blk-sigops", reconnect=False)
self.send_blocks([b40], False, "bad-blk-sigops", reconnect=True)

# same as b40, but one less sigop
self.log.info("Accept a block with the max number of P2SH sigops")
Expand Down Expand Up @@ -727,17 +720,15 @@ def run_test(self):
tx.vout.append(CTxOut(0, CScript([OP_TRUE])))
tx.calc_sha256()
b58 = self.update_block(58, [tx])
# !TODO: fix expect_disconnect
self.send_blocks([b58], False, "bad-txns-inputs-missingorspent", reconnect=False)
self.send_blocks([b58], False, "bad-txns-inputs-missingorspent", reconnect=True)

# tx with output value > input value
self.log.info("Reject a block with a transaction with outputs > inputs")
self.move_tip(57)
b59 = self.next_block(59)
tx = self.create_and_sign_transaction(out[17].tx, out[17].n, 251 * COIN)
b59 = self.update_block(59, [tx])
# !TODO: fix expect_disconnect
self.send_blocks([b59], False, "bad-txns-in-belowout", reconnect=False)
self.send_blocks([b59], False, "bad-txns-in-belowout", reconnect=True)

# reset to good chain
self.move_tip(57)
Expand Down Expand Up @@ -827,8 +818,7 @@ def run_test(self):
tx1 = self.create_and_sign_transaction(out[20].tx, out[20].n, out[20].tx.vout[0].nValue)
tx2 = self.create_and_sign_transaction(tx1, 0, 1)
b66 = self.update_block(66, [tx2, tx1])
# !TODO: fix expect_disconnect
self.send_blocks([b66], False, "bad-txns-inputs-missingorspent", reconnect=False)
self.send_blocks([b66], False, "bad-txns-inputs-missingorspent", reconnect=True)

# Attempt to double-spend a transaction created in a block
#
Expand All @@ -843,8 +833,7 @@ def run_test(self):
tx2 = self.create_and_sign_transaction(tx1, 0, 1)
tx3 = self.create_and_sign_transaction(tx1, 0, 2)
b67 = self.update_block(67, [tx1, tx2, tx3])
# !TODO: fix expect_disconnect
self.send_blocks([b67], False, "bad-txns-inputs-missingorspent", reconnect=False)
self.send_blocks([b67], False, "bad-txns-inputs-missingorspent", reconnect=True)

# More tests of block subsidy
#
Expand All @@ -863,8 +852,7 @@ def run_test(self):
b68 = self.next_block(68, additional_coinbase_value=10)
tx = self.create_and_sign_transaction(out[20].tx, out[20].n, out[20].tx.vout[0].nValue - 9)
b68 = self.update_block(68, [tx])
# !TODO: fix expect_disconnect
self.send_blocks([b68], False, "bad-blk-amount", reconnect=False)
self.send_blocks([b68], False, "bad-blk-amount", reconnect=True)

self.log.info("Accept a block claiming the correct subsidy in the coinbase transaction")
self.move_tip(65)
Expand All @@ -888,8 +876,7 @@ def run_test(self):
tx.vin.append(CTxIn(COutPoint(bogus_tx.sha256, 0), b"", 0xffffffff))
tx.vout.append(CTxOut(1, b""))
b70 = self.update_block(70, [tx])
# !TODO: fix expect_disconnect
self.send_blocks([b70], False, "bad-txns-inputs-missingorspent", reconnect=False)
self.send_blocks([b70], False, "bad-txns-inputs-missingorspent", reconnect=True)

# Test accepting an invalid block which has the same hash as a valid one (via merkle tree tricks)
#
Expand All @@ -898,7 +885,7 @@ def run_test(self):
#
# b72 is a good block.
# b71 is a copy of 72, but re-adds one of its transactions. However, it has the same hash as b72.
self.log.info("Reject a block containing a duplicate transaction but with the same Merkle root (Merkle tree malleability")
self.log.info("Reject a block containing a duplicate transaction but with the same Merkle root (Merkle tree malleability)")
self.move_tip(69)
b72 = self.next_block(72)
tx1 = self.create_and_sign_transaction(out[21].tx, out[21].n, 2)
Expand Down Expand Up @@ -1125,8 +1112,7 @@ def run_test(self):
b89a = self.next_block("89a", spend=out[32])
tx = self.create_tx(tx1, 0, 0, CScript([OP_TRUE]))
b89a = self.update_block("89a", [tx])
# !TODO: fix expect_disconnect
self.send_blocks([b89a], False, 'bad-txns-inputs-missingorspent', reconnect=False)
self.send_blocks([b89a], False, 'bad-txns-inputs-missingorspent', reconnect=True)

# !TODO: add long-reorg test

Expand Down

0 comments on commit a6d74e7

Please sign in to comment.