From 5a803ae7653a6d52832b6f28d0733e15e7eaf6dd Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Mon, 2 Aug 2021 15:51:42 +0200 Subject: [PATCH] Merge bitcoin/bitcoin#22378: test: remove confusing `MAX_BLOCK_BASE_SIZE` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 607076d01bf23c69ac21950c17b01fb4e1130774 test: remove confusing `MAX_BLOCK_BASE_SIZE` (Sebastian Falbesoner) 4af97c74edcda56cd15523bf3a335adea2bad14a test: introduce `get_weight()` helper for CBlock (Sebastian Falbesoner) a084ebe1330bcec15715e08b0f65319142927ad1 test: introduce `get_weight()` helper for CTransaction (Sebastian Falbesoner) Pull request description: This is a very late follow-up PR to #10618, which removed the constant `MAX_BLOCK_BASE_SIZE` from the core implementation about four years ago (see also #10608 in why it was considered confusing and superfluous). Since there is also no point in still keeping it in the functional test framework, the PR switches to weight-based accounting on the relevant test code parts and use `MAX_BLOCK_WEIGHT` instead for the block limit checks. To prepare that, the first two commits introduce `get_weight()` helpers for the classes CTransaction and CBlock, respectively. ACKs for top commit: MarcoFalke: review ACK 607076d01bf23c69ac21950c17b01fb4e1130774 🚴 Tree-SHA512: d59aa0b6b3dfd0a849b8063e66de275d252f705f99e25cd3bf6daec028b47d946777ee5b42a060f5283cb18e917ac073119c2c0e11bbc21211f69ef0a6ed335a --- test/functional/feature_block.py | 26 +++++++++---------- .../mining_prioritisetransaction.py | 4 +-- test/functional/test_framework/messages.py | 5 ++++ 3 files changed, 20 insertions(+), 15 deletions(-) diff --git a/test/functional/feature_block.py b/test/functional/feature_block.py index 9776a549d0..973f7d0fa8 100755 --- a/test/functional/feature_block.py +++ b/test/functional/feature_block.py @@ -329,24 +329,24 @@ class FullBlockTest(BitcoinTestFramework): self.move_tip(15) b23 = self.next_block(23, spend=out[6]) tx = CTransaction() - script_length = MAX_BLOCK_SIZE - len(b23.serialize()) - 69 + script_length = MAX_BLOCK_SIZE - b23.get_weight() - 69 script_output = CScript([b'\x00' * script_length]) tx.vout.append(CTxOut(0, script_output)) tx.vin.append(CTxIn(COutPoint(b23.vtx[1].sha256, 0))) b23 = self.update_block(23, [tx]) # Make sure the math above worked out to produce a max-sized block - assert_equal(len(b23.serialize()), MAX_BLOCK_SIZE) + assert_equal(b23.get_weight(), MAX_BLOCK_SIZE) self.send_blocks([b23], True) self.save_spendable_output() self.log.info("Reject a block of size MAX_BLOCK_SIZE + 1") self.move_tip(15) b24 = self.next_block(24, spend=out[6]) - script_length = MAX_BLOCK_SIZE - len(b24.serialize()) - 69 + script_length = MAX_BLOCK_SIZE - b24.get_weight() - 69 script_output = CScript([b'\x00' * (script_length + 1)]) tx.vout = [CTxOut(0, script_output)] b24 = self.update_block(24, [tx]) - assert_equal(len(b24.serialize()), MAX_BLOCK_SIZE + 1) + assert_equal(b24.get_weight(), MAX_BLOCK_SIZE + 1) self.send_blocks([b24], success=False, reject_reason='bad-blk-length', reconnect=True) b25 = self.next_block(25, spend=out[7]) @@ -500,13 +500,13 @@ class FullBlockTest(BitcoinTestFramework): # Until block is full, add tx's with 1 satoshi to p2sh_script, the rest to OP_TRUE tx_new = None tx_last = tx - total_size = len(b39.serialize()) - while(total_size < MAX_BLOCK_SIZE): + total_weight = b39.get_weight() + while total_weight < MAX_BLOCK_SIZE: tx_new = self.create_tx(tx_last, 1, 1, p2sh_script) tx_new.vout.append(CTxOut(tx_last.vout[1].nValue - 1, CScript([OP_TRUE]))) tx_new.rehash() - total_size += len(tx_new.serialize()) - if total_size >= MAX_BLOCK_SIZE: + total_weight += tx_new.get_weight() + if total_weight >= MAX_BLOCK_SIZE: break b39.vtx.append(tx_new) # add tx to block tx_last = tx_new @@ -517,7 +517,7 @@ class FullBlockTest(BitcoinTestFramework): # Make sure we didn't accidentally make too big a block. Note that the # size of the block has non-determinism due to the ECDSA signature in # the first transaction. - while (len(b39.serialize()) >= MAX_BLOCK_SIZE): + while b39.get_weight() >= MAX_BLOCK_SIZE: del b39.vtx[-1] b39 = self.update_block(39, []) @@ -938,7 +938,7 @@ class FullBlockTest(BitcoinTestFramework): tx.vout.append(CTxOut(0, script_output)) tx.vin.append(CTxIn(COutPoint(b64a.vtx[1].sha256, 0))) b64a = self.update_block("64a", [tx]) - assert_equal(len(b64a.serialize()), MAX_BLOCK_SIZE + 8) + assert_equal(b64a.get_weight(), MAX_BLOCK_SIZE + 8) self.send_blocks([b64a], success=False, reject_reason='non-canonical ReadCompactSize()') # dashd doesn't disconnect us for sending a bloated block, but if we subsequently @@ -952,7 +952,7 @@ class FullBlockTest(BitcoinTestFramework): b64 = CBlock(b64a) b64.vtx = copy.deepcopy(b64a.vtx) assert_equal(b64.hash, b64a.hash) - assert_equal(len(b64.serialize()), MAX_BLOCK_SIZE) + assert_equal(b64.get_weight(), MAX_BLOCK_SIZE) self.blocks[64] = b64 b64 = self.update_block(64, []) self.send_blocks([b64], True) @@ -1290,12 +1290,12 @@ class FullBlockTest(BitcoinTestFramework): for i in range(89, LARGE_REORG_SIZE + 89): b = self.next_block(i, spend) tx = CTransaction() - script_length = MAX_BLOCK_SIZE - len(b.serialize()) - 69 + script_length = MAX_BLOCK_SIZE - b.get_weight() - 69 script_output = CScript([b'\x00' * script_length]) tx.vout.append(CTxOut(0, script_output)) tx.vin.append(CTxIn(COutPoint(b.vtx[1].sha256, 0))) b = self.update_block(i, [tx]) - assert_equal(len(b.serialize()), MAX_BLOCK_SIZE) + assert_equal(b.get_weight(), MAX_BLOCK_SIZE) blocks.append(b) self.save_spendable_output() spend = self.get_spendable_output() diff --git a/test/functional/mining_prioritisetransaction.py b/test/functional/mining_prioritisetransaction.py index 31b0c15adb..6fee0e876e 100755 --- a/test/functional/mining_prioritisetransaction.py +++ b/test/functional/mining_prioritisetransaction.py @@ -55,8 +55,8 @@ class PrioritiseTransactionTest(BitcoinTestFramework): txids[i] = create_lots_of_big_transactions(self.nodes[0], self.txouts, utxos[start_range:end_range], end_range - start_range, (i+1)*base_fee) # Make sure that the size of each group of transactions exceeds - # MAX_BLOCK_SIZE -- otherwise the test needs to be revised to create - # more transactions. + # MAX_BLOCK_SIZE -- otherwise the test needs to be revised to + # create more transactions. mempool = self.nodes[0].getrawmempool(True) sizes = [0, 0, 0] for i in range(3): diff --git a/test/functional/test_framework/messages.py b/test/functional/test_framework/messages.py index 100b91c9a2..7611ec118c 100755 --- a/test/functional/test_framework/messages.py +++ b/test/functional/test_framework/messages.py @@ -546,6 +546,7 @@ class CTransaction: def get_vsize(self): return len(self.serialize()) + # it's just a helper that return vsize to reduce conflicts during backporting def get_weight(self): return self.get_vsize() @@ -681,6 +682,10 @@ class CBlock(CBlockHeader): self.nNonce += 1 self.rehash() + # it's just a helper that return vsize to reduce conflicts during backporting + def get_weight(self): + return len(self.serialize()) + def __repr__(self): return "CBlock(nVersion=%i hashPrevBlock=%064x hashMerkleRoot=%064x nTime=%s nBits=%08x nNonce=%08x vtx=%s)" \ % (self.nVersion, self.hashPrevBlock, self.hashMerkleRoot,