Merge bitcoin/bitcoin#22711: test: check for specific block reject reasons in p2p_segwit.py

45827fd718 test: check for block reject reasons in p2p_segwit.py [2/2] (Sebastian Falbesoner)
4eb532ff8b test: check for block reject reasons in p2p_segwit.py [1/2] (Sebastian Falbesoner)
b1488c4dce test: fix reference to block processing test in p2p_segwit.py (Sebastian Falbesoner)

Pull request description:

  In the test `p2p_segwit.py`, there are many instances where we send a segwit block to a node with the expectation that it is rejected. For this purpose, the helper function `test_witness_block` exists which allows also to check for a specific reject `reason` that is asserted in the debug log:
  502d22ceed/test/functional/p2p_segwit.py (L119-L120)

  This PR aims to increase the precision of the tests by adding the expected reject reasons to all `test_witness_block` call instances (found via `grep`ing after `test_witness_block(.*accepted=False`). For some blocks that are rejected due to failed script verification, the exact reason is only shown in the debug log if parallel script verification is disabled. For this reason, the addition of the reasons is split up in two commits:
  * first, all instances are tackled that are not subject to script verification, i.e. it doesn't matter whether parallel script verification is enabled or not (e.g. `bad-blk-weight`, `bad-witness-merkle-match`...)
  * then, we explicitely set `-par=1` to only use one script thread, and add the remaining reasons (`non-mandatory-script-verify-flag` with the more specific reason in parantheses)

ACKs for top commit:
  stratospher:
    tested ACK 45827fd.

Tree-SHA512: 00f31867f11d48b38a42b1e79a1303bda1c797ccf3d8c73e6107d70b062604d51ee2a3f2251e7f068dfafdaf09469d27dfee438d9bc9ebaef7febc4b6ef90a95
This commit is contained in:
MarcoFalke 2021-10-25 14:53:34 +02:00
commit 49e40f5704
No known key found for this signature in database
GPG Key ID: CE2B75697E69A548

View File

@ -8,7 +8,12 @@ import random
import struct
import time
from test_framework.blocktools import create_block, create_coinbase, add_witness_commitment, WITNESS_COMMITMENT_HEADER
from test_framework.blocktools import (
WITNESS_COMMITMENT_HEADER,
add_witness_commitment,
create_block,
create_coinbase,
)
from test_framework.key import ECKey
from test_framework.messages import (
BIP125_SEQUENCE_NUMBER,
@ -194,7 +199,7 @@ class SegWitTest(BitcoinTestFramework):
self.num_nodes = 2
# This test tests SegWit both pre and post-activation, so use the normal BIP9 activation.
self.extra_args = [
["-acceptnonstdtxn=1", f"-testactivationheight=segwit@{SEGWIT_HEIGHT}", "-whitelist=noban@127.0.0.1"],
["-acceptnonstdtxn=1", f"-testactivationheight=segwit@{SEGWIT_HEIGHT}", "-whitelist=noban@127.0.0.1", "-par=1"],
["-acceptnonstdtxn=0", f"-testactivationheight=segwit@{SEGWIT_HEIGHT}"],
]
self.supports_cli = False
@ -505,8 +510,8 @@ class SegWitTest(BitcoinTestFramework):
# 'block-validation-failed' (if script check threads > 1) or
# 'non-mandatory-script-verify-flag (Witness program was passed an
# empty witness)' (otherwise).
# TODO: support multiple acceptable reject reasons.
test_witness_block(self.nodes[0], self.test_node, block, accepted=False, with_witness=False)
test_witness_block(self.nodes[0], self.test_node, block, accepted=False, with_witness=False,
reason='non-mandatory-script-verify-flag (Witness program was passed an empty witness)')
self.utxo.pop(0)
self.utxo.append(UTXO(txid, 2, value))
@ -786,7 +791,7 @@ class SegWitTest(BitcoinTestFramework):
block_3.rehash()
block_3.solve()
test_witness_block(self.nodes[0], self.test_node, block_3, accepted=False)
test_witness_block(self.nodes[0], self.test_node, block_3, accepted=False, reason='bad-witness-merkle-match')
# Add a different commitment with different nonce, but in the
# right location, and with some funds burned(!).
@ -852,7 +857,7 @@ class SegWitTest(BitcoinTestFramework):
# Change the nonce -- should not cause the block to be permanently
# failed
block.vtx[0].wit.vtxinwit[0].scriptWitness.stack = [ser_uint256(1)]
test_witness_block(self.nodes[0], self.test_node, block, accepted=False)
test_witness_block(self.nodes[0], self.test_node, block, accepted=False, reason='bad-witness-merkle-match')
# Changing the witness reserved value doesn't change the block hash
block.vtx[0].wit.vtxinwit[0].scriptWitness.stack = [ser_uint256(0)]
@ -861,7 +866,7 @@ class SegWitTest(BitcoinTestFramework):
@subtest # type: ignore
def test_witness_block_size(self):
# TODO: Test that non-witness carrying blocks can't exceed 1MB
# Skipping this test for now; this is covered in p2p-fullblocktest.py
# Skipping this test for now; this is covered in feature_block.py
# Test that witness-bearing blocks are limited at ceil(base + wit/4) <= 1MB.
block = self.build_next_block()
@ -917,7 +922,7 @@ class SegWitTest(BitcoinTestFramework):
# limit
assert len(block.serialize()) > 2 * 1024 * 1024
test_witness_block(self.nodes[0], self.test_node, block, accepted=False)
test_witness_block(self.nodes[0], self.test_node, block, accepted=False, reason='bad-blk-weight')
# Now resize the second transaction to make the block fit.
cur_length = len(block.vtx[-1].wit.vtxinwit[0].scriptWitness.stack[0])
@ -989,7 +994,8 @@ class SegWitTest(BitcoinTestFramework):
self.update_witness_block_with_transactions(block, [tx])
# Extra witness data should not be allowed.
test_witness_block(self.nodes[0], self.test_node, block, accepted=False)
test_witness_block(self.nodes[0], self.test_node, block, accepted=False,
reason='non-mandatory-script-verify-flag (Witness provided for non-witness script)')
# Try extra signature data. Ok if we're not spending a witness output.
block.vtx[1].wit.vtxinwit = []
@ -1014,7 +1020,8 @@ class SegWitTest(BitcoinTestFramework):
self.update_witness_block_with_transactions(block, [tx2])
# This has extra witness data, so it should fail.
test_witness_block(self.nodes[0], self.test_node, block, accepted=False)
test_witness_block(self.nodes[0], self.test_node, block, accepted=False,
reason='non-mandatory-script-verify-flag (Stack size must be exactly one after execution)')
# Now get rid of the extra witness, but add extra scriptSig data
tx2.vin[0].scriptSig = CScript([OP_TRUE])
@ -1026,7 +1033,8 @@ class SegWitTest(BitcoinTestFramework):
block.solve()
# This has extra signature data for a witness input, so it should fail.
test_witness_block(self.nodes[0], self.test_node, block, accepted=False)
test_witness_block(self.nodes[0], self.test_node, block, accepted=False,
reason='non-mandatory-script-verify-flag (Witness requires empty scriptSig)')
# Now get rid of the extra scriptsig on the witness input, and verify
# success (even with extra scriptsig data in the non-witness input)
@ -1064,7 +1072,8 @@ class SegWitTest(BitcoinTestFramework):
tx2.rehash()
self.update_witness_block_with_transactions(block, [tx, tx2])
test_witness_block(self.nodes[0], self.test_node, block, accepted=False)
test_witness_block(self.nodes[0], self.test_node, block, accepted=False,
reason='non-mandatory-script-verify-flag (Push value size limit exceeded)')
# Now reduce the length of the stack element
tx2.wit.vtxinwit[0].scriptWitness.stack[0] = b'a' * (MAX_SCRIPT_ELEMENT_SIZE)
@ -1104,7 +1113,8 @@ class SegWitTest(BitcoinTestFramework):
self.update_witness_block_with_transactions(block, [tx, tx2])
test_witness_block(self.nodes[0], self.test_node, block, accepted=False)
test_witness_block(self.nodes[0], self.test_node, block, accepted=False,
reason='non-mandatory-script-verify-flag (Script is too big)')
# Try again with one less byte in the witness script
witness_script = CScript([b'a' * MAX_SCRIPT_ELEMENT_SIZE] * 19 + [OP_DROP] * 62 + [OP_TRUE])
@ -1176,7 +1186,7 @@ class SegWitTest(BitcoinTestFramework):
block = self.build_next_block()
self.update_witness_block_with_transactions(block, [tx2])
test_witness_block(self.nodes[0], self.test_node, block, accepted=False)
test_witness_block(self.nodes[0], self.test_node, block, accepted=False, reason='bad-txnmrklroot')
# Now try using a too short vtxinwit
tx2.wit.vtxinwit.pop()
@ -1184,6 +1194,8 @@ class SegWitTest(BitcoinTestFramework):
block.vtx = [block.vtx[0]]
self.update_witness_block_with_transactions(block, [tx2])
# This block doesn't result in a specific reject reason, but an iostream exception:
# "Exception 'CDataStream::read(): end of data: unspecified iostream_category error' (...) caught"
test_witness_block(self.nodes[0], self.test_node, block, accepted=False)
# Now make one of the intermediate witnesses be incorrect
@ -1193,7 +1205,8 @@ class SegWitTest(BitcoinTestFramework):
block.vtx = [block.vtx[0]]
self.update_witness_block_with_transactions(block, [tx2])
test_witness_block(self.nodes[0], self.test_node, block, accepted=False)
test_witness_block(self.nodes[0], self.test_node, block, accepted=False,
reason='non-mandatory-script-verify-flag (Operation not valid with the current stack size)')
# Fix the broken witness and the block should be accepted.
tx2.wit.vtxinwit[5].scriptWitness.stack = [b'a', witness_script]
@ -1415,7 +1428,7 @@ class SegWitTest(BitcoinTestFramework):
self.sync_blocks()
block2 = self.build_next_block()
self.update_witness_block_with_transactions(block2, [spend_tx])
test_witness_block(self.nodes[0], self.test_node, block2, accepted=False)
test_witness_block(self.nodes[0], self.test_node, block2, accepted=False, reason='bad-txns-premature-spend-of-coinbase')
# Advancing one more block should allow the spend.
self.generate(self.nodes[0], 1)
@ -1564,13 +1577,17 @@ class SegWitTest(BitcoinTestFramework):
# Too-large input value
sign_p2pk_witness_input(witness_script, tx, 0, hashtype, prev_utxo.nValue + 1, key)
self.update_witness_block_with_transactions(block, [tx])
test_witness_block(self.nodes[0], self.test_node, block, accepted=False)
test_witness_block(self.nodes[0], self.test_node, block, accepted=False,
reason='non-mandatory-script-verify-flag (Script evaluated without error '
'but finished with a false/empty top stack element')
# Too-small input value
sign_p2pk_witness_input(witness_script, tx, 0, hashtype, prev_utxo.nValue - 1, key)
block.vtx.pop() # remove last tx
self.update_witness_block_with_transactions(block, [tx])
test_witness_block(self.nodes[0], self.test_node, block, accepted=False)
test_witness_block(self.nodes[0], self.test_node, block, accepted=False,
reason='non-mandatory-script-verify-flag (Script evaluated without error '
'but finished with a false/empty top stack element')
# Now try correct value
sign_p2pk_witness_input(witness_script, tx, 0, hashtype, prev_utxo.nValue, key)
@ -1672,7 +1689,8 @@ class SegWitTest(BitcoinTestFramework):
tx2.vin[0].scriptSig = CScript([signature, pubkey])
block = self.build_next_block()
self.update_witness_block_with_transactions(block, [tx, tx2])
test_witness_block(self.nodes[0], self.test_node, block, accepted=False)
test_witness_block(self.nodes[0], self.test_node, block, accepted=False,
reason='non-mandatory-script-verify-flag (Witness requires empty scriptSig)')
# Move the signature to the witness.
block.vtx.pop()
@ -1918,7 +1936,7 @@ class SegWitTest(BitcoinTestFramework):
block_2 = self.build_next_block()
self.update_witness_block_with_transactions(block_2, [tx2])
test_witness_block(self.nodes[0], self.test_node, block_2, accepted=False)
test_witness_block(self.nodes[0], self.test_node, block_2, accepted=False, reason='bad-blk-sigops')
# Try dropping the last input in tx2, and add an output that has
# too many sigops (contributing to legacy sigop count).
@ -1931,7 +1949,7 @@ class SegWitTest(BitcoinTestFramework):
tx2.rehash()
block_3 = self.build_next_block()
self.update_witness_block_with_transactions(block_3, [tx2])
test_witness_block(self.nodes[0], self.test_node, block_3, accepted=False)
test_witness_block(self.nodes[0], self.test_node, block_3, accepted=False, reason='bad-blk-sigops')
# If we drop the last checksig in this output, the tx should succeed.
block_4 = self.build_next_block()