fix: panic recovery in PrepareProposal and ProcessProposal Handlers #14381
Conversation
alexanderbez
added
the
backport/v0.47.x
| "hash", fmt.Sprintf("%X", req.Hash), | ||
| "panic", err, | ||
| ) | ||
| resp = abci.ResponseProcessProposal{Status: abci.ResponseProcessProposal_REJECT} |
There was a problem hiding this comment.
Wouldn't the blockchain stop producing blocks if there is a deterministic bug that makes all proposal to be rejected? Does not it has effectively the same effect than a halt? 🤔
There was a problem hiding this comment.
I mean it is still better than stopping the node, but it is still stuck then?
There was a problem hiding this comment.
Since ProcessProposal is defined by the application and could very well define behavior that depends on the proposal's contents, i.e. txs, then the behavior cannot be guaranteed to be deterministic.
That being said, if a panic is triggered, the proposal is rejected. As such, Tendermint will proceed to the next validator to propose a new proposal in PrepareProposal and the round starts again. At some point a valid proposal will need to be produced and accepted. If no such proposal occurs, then yes, the chain will face a liveness halt, but this will be very clear and investigation will need to occur.
There was a problem hiding this comment.
Just a quick followup in case folks come back to this comment in the future:
PrepareProposalMAY be non-deterministicProcessProposalMAY NOT be non-deterministic, i.e. it must be deterministic
This means if ProcessProposal panics and we reject, all honest validator processes p will prevote nil and the round will proceed again until a valid proposal is proposed.
There was a problem hiding this comment.
we should document this here: https://docs.cosmos.network/main/building-apps/app-mempool
|
[Cosmos SDK] Kudos, SonarCloud Quality Gate passed!
|
Description
Closes: #14375
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!to the type prefix if API or client breaking changeCHANGELOG.mdReviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...
!in the type prefix if API or client breaking change