test: remove confusing MAX_BLOCK_BASE_SIZE
#22378
Conversation
a00f02e to
e26ad3b
Compare
| 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_BASE_SIZE + 1) | ||
| assert_equal(b24.get_weight(), MAX_BLOCK_WEIGHT + 1 * 4) |
There was a problem hiding this comment.
Does this 1 * 4 make sense? If you wanna add 1 to MAX_BLOCK_WEIGHT I think you have to use (MAX_BLOCK_WEIGHT + 1) * 4.
There was a problem hiding this comment.
bytes in the scriptSig count for 4 weight units.
Not sure, but it might be possible to pad the scriptWitness to get exactly +1 weight unit
There was a problem hiding this comment.
@brunoerg: MAX_BLOCK_WEIGHT + 4 is the smallest over-weight block you can reach if you only change non-witness data (as in this test scenario, the scriptPubKey), as every byte added increases the block weight by 4 WU
(What you probably meant was (MAX_BLOCK_BASE_SIZE + 1) * 4 but that constant doesn't exist anymore in this PR :))
@MarcoFalke: yes I guess it's possible to reach exactly MAX_BLOCK_WEIGHT+1 by tweaking the witness. Didn't want to change too much of the surrounding logic though, can be done in a follow-up I guess. E.g. adding test coverage for the rejection state bad-blk-weight is on my TODO list, it could be appropriate to include it in such a PR.
The constant `MAX_BLOCK_BASE_SIZE` has been removed from the core implementation years ago due to being confusing and superfluous, as it is implied by the block weight limit (see PRs bitcoin#10618 and bitcoin#10608). Since there is also no point in still keeping it in the functional test framework, we switch to weight-based accounting on the relevant test code parts and use `MAX_BLOCK_WEIGHT` instead for the block limit checks.
e26ad3b to
607076d
Compare
Replace MAX_BLOCK_BASE_SIZE with MAX_BLOCK_WEIGHT to resolve conflict with bitcoin#22378
Replace MAX_BLOCK_BASE_SIZE with MAX_BLOCK_WEIGHT to resolve conflict with bitcoin#22378
Replace MAX_BLOCK_BASE_SIZE with MAX_BLOCK_WEIGHT to resolve conflict with bitcoin#22378
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
There was a problem hiding this comment.
Concept Ack!
Since the core codebase does not use MAX_BLOCK_BASE_SIZE anymore, it makes sense to update our test implementations too accordingly.
Replacing them with MAX_BLOCK_WEIGHT just like we did in the codebase is the appropriate approach to make our tests more relevant to the core
Reviewed the codebase too! Merging this PR will remove the existence of MAX_BLOCK_BASE_SIZE entirely from the codebase.
There was a problem hiding this comment.
review ACK 607076d 🚴
Show signature and timestamp
Signature:
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512
review ACK 607076d01bf23c69ac21950c17b01fb4e1130774 🚴
-----BEGIN PGP SIGNATURE-----
iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
pUhangv/fJc3Jt7y5CNtkgkk5wpM7WvrVo7kr5id3uDLFwc4XvV/+tu46Lj8wKnM
VaKCogYIZUfDY0ybaQTPWUQsc7mRsCQHCZrzdqULXKlgilkbptrtw2Hnib5Yji/V
z8i55R4h27Vz9/+TnTQnoIVnN2yJb74ZDOhIplt+9EK8jsQlIpVSwvTd6jzybqfB
feKe5yfWdVXBa2Wt+7vI8jLB657ImRRpNjo0vmzsYV8dBKO69M6bLOFCyZyJI4vR
d/yV6Oz5mqBH3IE+k1yc+A9C1HDs7RTOk8Br9rAFF6l3GIRW5FZpojAHo6C52XGT
/uVcpiHj+KOkgQCsKDOsyULt+d64KVbXpyTm/ZBrtMsPiyWz3pf5sAaIFPKlJbbY
rCPJOz4BisFWea7r4SJnGRNzIn6AW5WAPTMkGhmBzbrp11sbGC43pavGyWlVXAkv
d2Lzhx6uzWSZMPdcyrF2tSMTt27iMpGt6yiTSVe5akPurgtlPtEP5K7rU3AadT+Y
9XhNZRKq
=6Hi6
-----END PGP SIGNATURE-----
Timestamp of file with hash ed68a39acc95435083178c51bd7a5f00064c323a9eb72384644b3c4b2d16a9a6 -
This is a very late follow-up PR to #10618, which removed the constant
MAX_BLOCK_BASE_SIZEfrom 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_WEIGHTinstead for the block limit checks. To prepare that, the first two commits introduceget_weight()helpers for the classes CTransaction and CBlock, respectively.