Implement and test AccumulateChanges #468
Conversation
jtremback
requested review from
AdityaSripal,
colin-axner and
fedekunze
as code owners
jtremback
changed the title
|
@AdityaSripal A couple questions about the |
Correct, however the type ABCI expects is We're keeping this in store as a list both because its easier to just stick to the type expected by ABCI, and because go-maps are not marshalled deterministically which will cause app hash divergence |
| abci "github.com/tendermint/tendermint/abci/types" | ||
| ) | ||
|
|
||
| func AccumulateChanges(currentChanges, newChanges []abci.ValidatorUpdate) []abci.ValidatorUpdate { |
There was a problem hiding this comment.
Maybe this could go in the types package? I think even just a file on the top level could be fine. I'd recommend avoiding utils/utils.go and go with file naming that is descriptive
Function needs godoc/comments/tests as well
|
|
||
| var out []abci.ValidatorUpdate | ||
|
|
||
| for _, update := range m { |
There was a problem hiding this comment.
Is this used in the state machine? Iteration over maps is nondeterministic, seems likely this could result in app hash mismatch
| name string | ||
| packet channeltypes.Packet | ||
| newChanges types.ValidatorSetChangePacketData | ||
| expectedPendingChanges types.ValidatorSetChangePacketData |
There was a problem hiding this comment.
| expectedPendingChanges types.ValidatorSetChangePacketData | |
| expPendingChanges types.ValidatorSetChangePacketData |
nit: naming consistency
| suite.Require().True(ok) | ||
| suite.Require().Equal(&pd, actualPd, "pending changes not equal to packet data after successful packet receive. case: %s", tc.name) | ||
| // Sort to avoid dumb inequalities |
There was a problem hiding this comment.
Maybe it would be better to require pending changes to always be sorted?
|
Closed in favor of cosmos/interchain-security#6 |
…github.com/stretchr/testify-1.8.0 build(deps): bump github.com/stretchr/testify from 1.7.5 to 1.8.0
Description
This creates a utility which accumulates validator set changes by updating those validators whose powers have changed. It then uses this utility in the child's OnRecvPacket to update the stored changes instead of overwriting them
closes: https://github.com/cosmos/cosmos-sdk/issues/10123
Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.
docs/) or specification (x/<module>/spec/)godoccomments.Unreleasedsection inCHANGELOG.mdFiles changedin the Github PR explorerCodecov Reportin the comment section below once CI passes