Propogate on chain consensus params to tendermint params#3591
Conversation
PR SummaryMedium Risk Overview Both optimistic and non-optimistic finalize paths call new The FlatKV EVM Migrate integration-test matrix job is removed from Reviewed by Cursor Bugbot for commit 7e9d07b. Bugbot is set up for automated code reviews on this repo. Configure here. |
|
The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).
|
| if params.Synchrony != nil { | ||
| cp.Synchrony = &tmproto.SynchronyParams{ | ||
| Precision: cloneDuration(params.Synchrony.Precision), | ||
| MessageDelay: cloneDuration(params.Synchrony.MessageDelay), | ||
| } | ||
| } | ||
| if params.Timeout != nil { | ||
| cp.Timeout = &tmproto.TimeoutParams{ | ||
| Propose: cloneDuration(params.Timeout.Propose), | ||
| ProposeDelta: cloneDuration(params.Timeout.ProposeDelta), | ||
| Vote: cloneDuration(params.Timeout.Vote), | ||
| VoteDelta: cloneDuration(params.Timeout.VoteDelta), | ||
| Commit: cloneDuration(params.Timeout.Commit), | ||
| BypassCommitTimeout: params.Timeout.BypassCommitTimeout, | ||
| } |
There was a problem hiding this comment.
these need to be committed to tm state from app state
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3591 +/- ##
==========================================
- Coverage 58.96% 58.08% -0.88%
==========================================
Files 2208 2134 -74
Lines 181736 173238 -8498
==========================================
- Hits 107162 100631 -6531
+ Misses 64957 63688 -1269
+ Partials 9617 8919 -698
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
ce48c06 to
8f011e4
Compare
Describe your changes and provide context
Fix propagation of on-chain consensus parameter updates into Tendermint consensus state.
Governance proposals can update
baseappconsensus params such asTimeoutParams, but the app was buildingResponseFinalizeBlock.ConsensusParamUpdatesfrom the legacy EndBlock response, which only carriesBlock,Evidence,Validator, andVersion. As a result, params likeTimeout,Synchrony, andABCIcould be updated in app state but never applied by Tendermint.This change reads the full consensus params from the state selected for commit and sends the complete set in
ResponseFinalizeBlock.ConsensusParamUpdates.FinalizeBlocker.BaseApp.GetConsensusParamsForStateToCommit().BlockEvidenceValidatorVersionSynchronyTimeoutABCITesting performed to validate your change