-
Notifications
You must be signed in to change notification settings - Fork 636
ABCI++: application to control how many txs it receives in PrepareProposal #980
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
Closed
Tracked by
#9
Labels
abci
Application blockchain interface
P:new-use-cases
Priority: Enable new use cases for application developers
Comments
27 tasks
3 tasks
mergify bot
pushed a commit
that referenced
this issue
Jun 20, 2023
…e app (#1003) Closes #980 This PR is the result of internal discussions (last year, but also last week), and interactions with the Vega team in order to address their use case. The gist of the agreed solution is to interpret value -1 for `ConsensusParams.Block.MaxBytes` as the Application wanting to enforce a size limit at its level, via `PrepareProposal` and `ProcessProposal`. Additionally, it instructs CometBFT to send all transactions in the mempool upon calling `PrepareProposal` so that the application can apply its own transaction ordering policies to *all* transactions known to the node. This solution is also the most _surgical_ among all candidate solutions in that the least amount of logic/spec needed to be added/removed/modified. So, at CometBFT level, a value of -1 for `ConsensusParams.Block.MaxBytes` implies: * The limit enforced at CometBFT level is the generic `types.MaxBlockSizeBytes` (hard-coded to 100MB) * CometBFT will reap all transactions from the mempool when creating a proposed block (just before calling `PrepareProposal`) --- #### PR checklist - [x] Tests written/updated - [x] Changelog entry added in `.changelog` (we use [unclog](https://github.com/informalsystems/unclog) to manage our changelog) - [x] Updated relevant documentation (`docs/` or `spec/`) and code comments
mergify bot
pushed a commit
that referenced
this issue
Jun 20, 2023
…e app (#1003) Closes #980 This PR is the result of internal discussions (last year, but also last week), and interactions with the Vega team in order to address their use case. The gist of the agreed solution is to interpret value -1 for `ConsensusParams.Block.MaxBytes` as the Application wanting to enforce a size limit at its level, via `PrepareProposal` and `ProcessProposal`. Additionally, it instructs CometBFT to send all transactions in the mempool upon calling `PrepareProposal` so that the application can apply its own transaction ordering policies to *all* transactions known to the node. This solution is also the most _surgical_ among all candidate solutions in that the least amount of logic/spec needed to be added/removed/modified. So, at CometBFT level, a value of -1 for `ConsensusParams.Block.MaxBytes` implies: * The limit enforced at CometBFT level is the generic `types.MaxBlockSizeBytes` (hard-coded to 100MB) * CometBFT will reap all transactions from the mempool when creating a proposed block (just before calling `PrepareProposal`) --- #### PR checklist - [x] Tests written/updated - [x] Changelog entry added in `.changelog` (we use [unclog](https://github.com/informalsystems/unclog) to manage our changelog) - [x] Updated relevant documentation (`docs/` or `spec/`) and code comments (cherry picked from commit c138e78)
sergio-mena
added a commit
that referenced
this issue
Jun 20, 2023
…e app (#1003) (#1006) Closes #980 This PR is the result of internal discussions (last year, but also last week), and interactions with the Vega team in order to address their use case. The gist of the agreed solution is to interpret value -1 for `ConsensusParams.Block.MaxBytes` as the Application wanting to enforce a size limit at its level, via `PrepareProposal` and `ProcessProposal`. Additionally, it instructs CometBFT to send all transactions in the mempool upon calling `PrepareProposal` so that the application can apply its own transaction ordering policies to *all* transactions known to the node. This solution is also the most _surgical_ among all candidate solutions in that the least amount of logic/spec needed to be added/removed/modified. So, at CometBFT level, a value of -1 for `ConsensusParams.Block.MaxBytes` implies: * The limit enforced at CometBFT level is the generic `types.MaxBlockSizeBytes` (hard-coded to 100MB) * CometBFT will reap all transactions from the mempool when creating a proposed block (just before calling `PrepareProposal`) --- #### PR checklist - [x] Tests written/updated - [x] Changelog entry added in `.changelog` (we use [unclog](https://github.com/informalsystems/unclog) to manage our changelog) - [x] Updated relevant documentation (`docs/` or `spec/`) and code comments (cherry picked from commit c138e78) Co-authored-by: Sergio Mena <sergio@informal.systems>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
abci
Application blockchain interface
P:new-use-cases
Priority: Enable new use cases for application developers
Uh oh!
There was an error while loading. Please reload this page.
Was: tendermint/tendermint#7750
Context
As is currently implemneted in ABCI 2.0, CometBFT sends the application up to
ConsensusParams.Block.MaxBytes - CommitBytes - HeaderBytes - EvidenceBytes
of txs. It also sends this number so that applications know the maximum amount of transactions they can return (if they want to add txs or modify them). If applications tend to merge or squash a lot of txs, or they need to filter through and either delay or remove txs, having this limit means they will tend to have undersized blocks.Current Proposal
The current proposal is the "one shot" approach. There will be a new
config.toml
or consensus param that controls the max number/size of transactions that CometBFT includes in raw proposal blocks it send to the app inPrepProp
.Probably with a special value (-1?) to indicate "send everything".
[See discussion in tendermint/tendermint#7750 for further details]
The text was updated successfully, but these errors were encountered: