8000 ABCI++: application to control how many txs it receives in PrepareProposal · Issue #980 · cometbft/cometbft · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

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
sergio-mena opened this issue Jun 15, 2023 · 0 comments · Fixed by #1003
Closed
Tracked by #9

ABCI++: application to control how many txs it receives in PrepareProposal #980

sergio-mena opened this issue Jun 15, 2023 · 0 comments · Fixed by #1003
Labels
abci Application blockchain interface P:new-use-cases Priority: Enable new use cases for application developers

Comments

@sergio-mena
Copy link
Contributor
sergio-mena commented Jun 15, 2023

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 in PrepProp.
Probably with a special value (-1?) to indicate "send everything".

[See discussion in tendermint/tendermint#7750 for further details]

@sergio-mena sergio-mena added the abci Application blockchain interface label Jun 15, 2023
@mergify mergify bot closed this as completed in #1003 Jun 20, 2023
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
@github-project-automation github-project-automation bot moved this from Todo to Done in CometBFT 2023 Jun 20, 2023
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>
@thanethomson thanethomson added the P:new-use-cases Priority: Enable new use cases for application developers label Jun 20, 2023
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
Projects
No open projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants
0