docs: add CIP for block limits on pfbs and tx size #220
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| | cip | TBD(27) | | ||
| | - | - | | ||
| | title | Block limits for number of PFBs/non-PFBs and transaction size | | ||
| | description | Set limits for number of PFBs and non-PFBs per block and transaction size | | ||
| | author | Josh Stein ([@jcstein](https://github.com/jcstein)), Nina Barbakadze ([@ninabarbakadze](https://github.com/ninabarbakadze)) | | ||
| | discussions-to | <https://forum.celestia.org/t/cip-limit-number-of-pfbs-and-non-pfbs-per-block-increase-transaction-size-limit/1843> | | ||
| | status | Draft | | ||
| | type | Standards Track | | ||
| | category | Core | | ||
| | created | 2024-10-16 | | ||
|
|
||
| ## Abstract | ||
|
|
||
| This CIP proposes to set limits for the number of PayForBlobs (PFBs) messages and non-PFBs messages per block and set the limit for transaction size. The proposal is to set the limits to 600 PFBs messages and 200 non-PFBs messages per block and set the transaction size limit to 2MiB. Setting PFB and non-PFBs limits is not consensus-breaking. Setting the transaction size limit is consensus-breaking. | ||
|
|
||
| ## Specification | ||
|
|
||
| 1. The number of PFBs per block is limited to 600 by setting `MaxPFBMessages` to 600. | ||
|
|
||
| 1. The number of non-PFBs messages per block is limited to 200 by setting `MaxNonPFBMessages` to 200. | ||
|
|
||
jcstein marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| 1. It's important to note that these limits are not strictly enforced. While they are defined by the `celestia-app` implementation, a validator could potentially modify the `PrepareProposal` logic, run a custom binary, and produce blocks that exceed the specified limits for PFB or non-PFBs transactions. | ||
|
|
||
| 1. Transaction size is limited to 2MiB by setting `MaxTxSize` to 2097152, which is 2MiB in bytes. From version v3 and above, in `CheckTx`, `PrepareProposal`, and `ProcessProposal`, each transaction's size is checked against the `appconsts.MaxTxSize` threshold. This ensures that transactions over the limit are rejected or excluded at all stages, from initial submission to execution. | ||
jcstein marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| ## Rationale | ||
|
|
||
| The rationale for this proposal is twofold: | ||
|
|
||
| 1. To prevent long block times on the network by limiting the number of PFBs and non-PFBs messages per block. This was initially not considered consensus-breaking, but it has a meaningful effect on users and should be formalized in a CIP. | ||
|
There was a problem hiding this comment. We can do this at a later point but it might be helpful to remember later on how these numbers were chosen There was a problem hiding this comment. we could point to this PR celestiaorg/celestia-app#390 as I am adding the results to it and will open it for review afterwards There was a problem hiding this comment. The PR 309 referenced is from deciding on the prefix for addresses and unrelated to this? celestiaorg/celestia-app#390 @rach-id Can you please provide a condensed explanation of what you learned in the PR that was intended to be linked? to explain why these values were chosen? There was a problem hiding this comment. my bad, this PR: celestiaorg/celestia-app#3904 That PR contains benchmarks that we run to measure the processing time different ABCI methods take with different transaction types. Then, to limit that processing time to ~0.25sec, we added a soft limiter in prepare proposal so that the default block created by the block construction mechanism respects that processing time limit. This soft limiter is a simple cap on the number of messages a block can contain, based on running the PRs benchmarks on 4 CPU 16GB RAM machines, which is the suggested validator configuration. This way, the default blocks don't take much time to process. However, if a block not respecting those limits reaches consensus, it still can be included and won't be wasted.
jcstein marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| 1. To set the transaction size limit to 2MiB, even with 8MiB blocks, to prevent issues with gossiping large transactions. Gossiping an 8MiB transaction without chunking could be detrimental to the network. This is a consensus-breaking change. | ||
|
|
||
| ## Backwards Compatibility | ||
|
|
||
| This proposal is meant to be included with v3 and the [Ginger Network Upgrade](./cip-25.md). It is backwards compatible with v2. | ||
|
|
||
| ## Security Considerations | ||
|
|
||
| This proposal does not introduce any new security risks. However, it does impact network behavior and user experience, which should be carefully considered during implementation. | ||
|
|
||
| ## Copyright | ||
|
|
||
| Copyright and related rights waived via [CC0](https://github.com/celestiaorg/CIPs/blob/main/LICENSE). | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We discussed this in all core devs but IMO there wasn't a clear decision made in the call. I think we could split this into two CIPs:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think @cmwaters explanation is that these both impact how a block is constructed, so was okay to leave it as 1 CIP.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm also happy to scrap this and have it split to 2 CIPs by authors of the features. but I worry this will likely just add more delay to these being drafted.