feat(abci): Add round to Prepare/ProcessProposal, FinalizeBlock#498
feat(abci): Add round to Prepare/ProcessProposal, FinalizeBlock#498
Conversation
internal/consensus/replay_test.go
Outdated
There was a problem hiding this comment.
Is round important for replay blocks?
There was a problem hiding this comment.
When we replay blocks on ABCI app, we should provide the same round as previously to make it deterministic. If we just send 0, it will be non-deterministic (block can be accepted at round 123456, then TD is restarted and the block will be replayed with round 0).
For current functionality it might not be important, but is important for API consistency.
There was a problem hiding this comment.
Do you mean deterministic and consistent on the ABCI app level?
There was a problem hiding this comment.
Yes, ABCI app shall get the same round every time it gets a message.
shotonoff
left a comment
shotonoff
left a comment
There was a problem hiding this comment.
🚀 LGTM
pease have a look at the comments
| return nil | ||
| } | ||
|
|
||
| func roundKey(appHash tmbytes.HexBytes, height int64, round int32) string { |
There was a problem hiding this comment.
do I understand correctly, that app-hash might be the same between rounds at the same height?
There was a problem hiding this comment.
yes, but apphash it is redundant here (as an additional check), in theory height+round should be enough
internal/state/execution_test.go
Outdated
| Status: abci.ResponseProcessProposal_ACCEPT, | ||
| }, nil) | ||
| uncommittedState, err := blockExec.ProcessProposal(ctx, block1, state, true) | ||
| uncommittedState, err := blockExec.ProcessProposal(ctx, block1, 12, state, true) |
There was a problem hiding this comment.
No, just a random height, changed to constant to make it more obvious
| // Merkle root of the next validator set. | ||
| bytes next_validators_hash = 7; | ||
| // Round number for the block. | ||
| int32 round = 8; |
There was a problem hiding this comment.
WDYT usage 1XX position for round, the same as for other dash related fields?
3 participants
Issue being fixed or feature implemented
For better transaction processing in same-block execution mode, ABCI clients need round number in PrepareProposal, ProcessProposal, and FinalizeBlock.
What was done?
How Has This Been Tested?
Breaking Changes
Checklist:
For repository code-owners and collaborators only