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

Inconsistent block size limit handling in proposal preparation #3189

Closed
grarco opened this issue May 7, 2024 · 3 comments · Fixed by #3220
Closed

Inconsistent block size limit handling in proposal preparation #3189

grarco opened this issue May 7, 2024 · 3 comments · Fixed by #3220

Comments

@grarco
Copy link
Contributor

grarco commented May 7, 2024

Reported by Informal Systems.

Description

The process of preparing proposals involves the BlockAllocator, which sets the maximum size of all transactions.
This maximum size, determined by the parameter max_proposal_bytes in the BlockAllocator configuration (/ block_alloc.rs#L141), should align with the max_tx_bytes parameter in the RequestPrepareProposal .
However, there lacks a validation step to ensure this alignment, potentially leading to discrepancies between these parameters over time.

Problem Scenarios

The absence of consistency checking between max_proposal_bytes and max_tx_bytes poses several risks. Firstly, it violates Requirement 2 [PrepareProposal, tx-size] of the ABCI++ 0.37 specification, which mandates that the total size of transactions in a proposal does not exceed max_tx_bytes when calling ResponsePrepareProposal . Secondly, such discrepancies compromise liveness, as proposals from honest validators exceeding the maximum allowed would be rejected by the consensus engine.

Recommendation

To address these issues, it would be desirable to implement a validation step to ensure that max_proposal_bytes does not exceed max_tx_bytes , or that the total size of transaction bytes in the proposal does not exceed the max_tx_bytes specified in the RequestPrepareProposal .

@grarco grarco added this to the Later / optional milestone May 7, 2024
@grarco grarco changed the title Inconsistent Block Size Limit Handling in Proposal Preparation Inconsistent block size limit handling in proposal preparation May 7, 2024
@grarco grarco added the audit label May 7, 2024
@batconjurer
Copy link
Member

Given that this parameter might be changed in namada storage by a fork, I think the best way to handle this is to take the minimum of the tx byte limit provided in the PrepareProposal request and the value in storage. The main alternative I see is to use the cometbft config parameter, but then updating this value would require a hardfork since we can't change values passed to cometbft without restarting a node, although a single source of truth would be nice.

@sug0
Copy link
Contributor

sug0 commented May 10, 2024

hmmm I just realized why we started using max_proposal_bytes in the first place. ProcessProposal doesn't include max_tx_bytes, so it's impossible to validate the value the validator was using

@batconjurer
Copy link
Member

batconjurer commented May 10, 2024

hmmm I just realized why we started using max_proposal_bytes in the first place. ProcessProposal doesn't include max_tx_bytes, so it's impossible to validate the value the validator was using

Yep, I was actually just wondering if your pr handles process proposal correctly now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants