Add a comment explaining the use of MAX_BLOCK_BASE_SIZE. #10608
Conversation
|
so |
| * The maximum allowed size for a block excluding witness data, in bytes (network rule). | ||
| * This parameter is largely superfluous because it is directly implied by the above block | ||
| * weight limit, even when BIP 141 is not active. It continues to exist for use in | ||
| * various early tests that run before the witness data has been checked. |
There was a problem hiding this comment.
Minor nit: Accidental double spaces after * on the two lines above.
There was a problem hiding this comment.
intentional indentation
There was a problem hiding this comment.
Just out of curiosity, why the indentation? :-)
There was a problem hiding this comment.
After re-reading the diff I think I get it: the indentation is to clarify that the current line is part of the sentence of the line above.
|
ACK 1887337 modulo clarification of "network rule" vs "network consensus rule" as suggested by @instagibbs |
|
@instagibbs if you decrease it below a million you will break consensus. If you remove it entirely you will not, or set it to eleven million you will not. It's a consensus rule but a redundant one which is subsumed by weight. Do you see a way to make that more clear? |
| * The maximum allowed size for a block excluding witness data, in bytes (network rule). | ||
| * This parameter is largely superfluous because it is directly implied by the above block | ||
| * weight limit, even when BIP 141 is not active. It continues to exist for use in | ||
| * various early tests that run before the witness data has been checked. |
There was a problem hiding this comment.
"tests" is overloaded and could be confused for unit/integration tests. "early checks" might be better.
Why are those checks not removed if they don't change the rules? Would be good to mention the reason for it also here.
There was a problem hiding this comment.
There are many early checks in validation which limit the resources used handling invalid things or avoid incorrect invalidation in the presence of malleability. Most of them explain themselves.
E.g. in this case you cannot check the weight limit until after you've verified the witness commitment, which takes a bunch of hashing-- otherwise you might invalidate a block you received with an tampered witness (take a valid block add another 2mb of junk to one of the witnesses) instead of just dropping it and the party that gave it to you. But you can check the base size before doing that.
|
#10618 is an alternative that just removes the constant. |
1887337 Add a comment explaining the use of MAX_BLOCK_BASE_SIZE. (Gregory Maxwell) Tree-SHA512: 4c643a3696241fbf4eac8d58bb26586f319338b28ee86d20394a8ea362911b467853eb40c43487ede753209a3c7bee2e576d2ca80627e9fc924fabefbcaea34b
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.
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.
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.
607076d test: remove confusing `MAX_BLOCK_BASE_SIZE` (Sebastian Falbesoner) 4af97c7 test: introduce `get_weight()` helper for CBlock (Sebastian Falbesoner) a084ebe 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 607076d 🚴 Tree-SHA512: d59aa0b6b3dfd0a849b8063e66de275d252f705f99e25cd3bf6daec028b47d946777ee5b42a060f5283cb18e917ac073119c2c0e11bbc21211f69ef0a6ed335a
No description provided.