-
Notifications
You must be signed in to change notification settings - Fork 780
Description
During our E2E fuzz testing using the CometBFT 0.37.x branch, where one of the tests is randomly killing and restarting nodes, it seems that PrepareProposal/ProcessProposal invocation during replay is inconsistent with the spec.
The spec currently says:
For every round, if the local process is the proposer of the current round, CometBFT calls
PrepareProposal, followed byProcessProposal. These two always come together because they reflect the same proposal that the process also delivers to itself.
However, it seems that there is an edge case with the following scenario:
- A validator node becomes the proposer, calls
PrepareProposal, generates and broadcasts the proposal (proposal A). Then the node crashes/is killed. - When the node restarts, it enters the consensus replay mode.
- CometBFT replays the timeout event, triggering a new round where the validator node is again the proposer (as this is a replay, the height/round is the same). This causes the recovering validator node to run
PrepareProposalagain, generating a new proposal B. Since signing this proposal would be an equivocation, the proposal is dropped. - CometBFT then replays proposal A, which causes it to be processed by the app via
ProcessProposal.
This seems to contradict the specification, because based on it the following sequence would be expected in this scenario:
- B := PrepareProposal()
- ProcessProposal(B)
- ProcessProposal(A)
However, the actual sequence is:
- B := PrepareProposal()
- ProcessProposal(A)
So either the specification needs to be updated to clarify that these other sequences are possible or the implementation should be fixed.