Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

docs: add CIP for block limits on pfbs and tx size #220

Closed
wants to merge 14 commits into from
44 changes: 44 additions & 0 deletions cips/cip-tx-limits.md
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 sizes |
jcstein marked this conversation as resolved.
Show resolved Hide resolved
| 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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can do this at a later point but it might be helpful to remember later on how these numbers were chosen

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

@jcstein jcstein Oct 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR 309 referenced is from deciding on the prefix for addresses and unrelated to this? celestiaorg/celestia-app#390
Screenshot 2024-10-17 at 12 26 35

@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?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).