Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check oder of iteration #599

Closed
wants to merge 6 commits into from
Closed

Check oder of iteration #599

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
01b20ef
bringing changes from #583 wip
mpoke Dec 14, 2022
e219e41
bringing changes from #583 provider done
mpoke Dec 14, 2022
2f1d376
bringing changes from #583 consumer done
mpoke Dec 14, 2022
22ee82f
IsPendingConsumerRemovalProp -> PendingConsumerRemovalPropExists
mpoke Dec 15, 2022
e440824
use uint64 in keys instead of time.Time
mpoke Dec 15, 2022
ee70409
Use SDK utility function instead of encoding/binary (#601)
mpoke Dec 16, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion tests/e2e/stop_consumer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ func (s *CCVTestSuite) checkConsumerChainIsRemoved(chainID string, checkChannel
}

// check UnbondingOps were deleted and undelegation entries aren't onHold
providerKeeper.IterateOverUnbondingOpIndex(
providerKeeper.IterateUnbondingOpIndex(
s.providerCtx(),
chainID,
func(vscID uint64, ubdIndex []uint64) (stop bool) {
Expand Down
21 changes: 1 addition & 20 deletions x/ccv/consumer/keeper/genesis.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,25 +128,6 @@ func (k Keeper) ExportGenesis(ctx sdk.Context) (genesis *consumertypes.GenesisSt
return false // do not stop the iteration
})

heightToVCIDs := []consumertypes.HeightToValsetUpdateID{}
k.IterateHeightToValsetUpdateID(ctx, func(height, vscID uint64) (stop bool) {
hv := consumertypes.HeightToValsetUpdateID{
Height: height,
ValsetUpdateId: vscID,
}
heightToVCIDs = append(heightToVCIDs, hv)
return false // do not stop the iteration
})

outstandingDowntimes := []consumertypes.OutstandingDowntime{}
k.IterateOutstandingDowntime(ctx, func(addr string) (stop bool) {
od := consumertypes.OutstandingDowntime{
ValidatorConsensusAddress: addr,
}
outstandingDowntimes = append(outstandingDowntimes, od)
return false // do not stop the iteration
})

// TODO: update GetLastTransmissionBlockHeight to not return an error
ltbh, err := k.GetLastTransmissionBlockHeight(ctx)
if err != nil {
Expand All @@ -160,7 +141,7 @@ func (k Keeper) ExportGenesis(ctx sdk.Context) (genesis *consumertypes.GenesisSt
valset,
k.GetHeightToValsetUpdateIDs(ctx),
k.GetPendingPackets(ctx),
outstandingDowntimes,
k.GetOutstandingDowntimes(ctx),
*ltbh,
params,
)
Expand Down
30 changes: 21 additions & 9 deletions x/ccv/consumer/keeper/keeper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -275,15 +275,15 @@ func (k Keeper) VerifyProviderChain(ctx sdk.Context, connectionHops []string) er
return nil
}

// SetHeightValsetUpdateID sets the valset update id for a given block height
func (k Keeper) SetHeightValsetUpdateID(ctx sdk.Context, height, valsetUpdateId uint64) {
// SetHeightValsetUpdateID sets the vscID for a given block height
func (k Keeper) SetHeightValsetUpdateID(ctx sdk.Context, height, vscID uint64) {
store := ctx.KVStore(k.storeKey)
valBytes := make([]byte, 8)
binary.BigEndian.PutUint64(valBytes, valsetUpdateId)
binary.BigEndian.PutUint64(valBytes, vscID)
store.Set(types.HeightValsetUpdateIDKey(height), valBytes)
}

// GetHeightValsetUpdateID gets the valset update id recorded for a given block height
// GetHeightValsetUpdateID gets the vscID recorded for a given block height
func (k Keeper) GetHeightValsetUpdateID(ctx sdk.Context, height uint64) uint64 {
store := ctx.KVStore(k.storeKey)
bz := store.Get(types.HeightValsetUpdateIDKey(height))
Expand All @@ -293,13 +293,17 @@ func (k Keeper) GetHeightValsetUpdateID(ctx sdk.Context, height uint64) uint64 {
return binary.BigEndian.Uint64(bz)
}

// DeleteHeightValsetUpdateID deletes the valset update id for a given block height
// DeleteHeightValsetUpdateID deletes the vscID for a given block height
func (k Keeper) DeleteHeightValsetUpdateID(ctx sdk.Context, height uint64) {
store := ctx.KVStore(k.storeKey)
store.Delete(types.HeightValsetUpdateIDKey(height))
}

// IterateHeightToValsetUpdateID iterates over the block height to valset update ID mapping in store
// IterateHeightToValsetUpdateID iterates over the block height to vscID mapping in store.
//
// Note that the block height to vscID mapping is stored under keys with the following format:
// HeightValsetUpdateIDBytePrefix | height
// Thus, the iteration is in ascending order of heights.
func (k Keeper) IterateHeightToValsetUpdateID(ctx sdk.Context, cb func(height, vscID uint64) (stop bool)) {
store := ctx.KVStore(k.storeKey)
iterator := sdk.KVStorePrefixIterator(store, []byte{types.HeightValsetUpdateIDBytePrefix})
Expand Down Expand Up @@ -341,7 +345,11 @@ func (k Keeper) DeleteOutstandingDowntime(ctx sdk.Context, consAddress string) {
store.Delete(types.OutstandingDowntimeKey(consAddr))
}

// IterateOutstandingDowntime iterates over the validator addresses of outstanding downtime flags
// IterateOutstandingDowntime iterates over the outstanding downtime flags.
//
// Note that the outstanding downtime flags are stored under keys with the following format:
// OutstandingDowntimeBytePrefix | consAddress
// Thus, the iteration is in ascending order of consAddresses.
func (k Keeper) IterateOutstandingDowntime(ctx sdk.Context, cb func(address string) (stop bool)) {
store := ctx.KVStore(k.storeKey)
iterator := sdk.KVStorePrefixIterator(store, []byte{types.OutstandingDowntimeBytePrefix})
Expand Down Expand Up @@ -384,7 +392,11 @@ func (k Keeper) DeleteCCValidator(ctx sdk.Context, addr []byte) {
store.Delete(types.CrossChainValidatorKey(addr))
}

// GetAllCCValidator returns all cross-chain validators
// GetAllCCValidator returns all cross-chain validators.
//
// Note that the cross-chain validators are stored under keys with the following format:
// CrossChainValidatorBytePrefix | address
// Thus, the iteration is in ascending order of addresses.
func (k Keeper) GetAllCCValidator(ctx sdk.Context) (validators []types.CrossChainValidator) {
store := ctx.KVStore(k.storeKey)
iterator := sdk.KVStorePrefixIterator(store, []byte{types.CrossChainValidatorBytePrefix})
Expand Down Expand Up @@ -463,7 +475,7 @@ func (k Keeper) GetOutstandingDowntimes(ctx sdk.Context) []consumertypes.Outstan
ValidatorConsensusAddress: addr,
}
outstandingDowntimes = append(outstandingDowntimes, od)
return false
return false // do not stop iteration
})
return outstandingDowntimes
}
23 changes: 10 additions & 13 deletions x/ccv/provider/keeper/genesis.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package keeper

import (
"fmt"
"time"

sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/interchain-security/x/ccv/provider/types"
Expand Down Expand Up @@ -33,12 +32,12 @@ func (k Keeper) InitGenesis(ctx sdk.Context, genState *types.GenesisState) {
for _, prop := range genState.ConsumerAdditionProposals {
// prevent implicit memory aliasing
prop := prop
if err := k.SetPendingConsumerAdditionProp(ctx, &prop); err != nil {
panic(fmt.Errorf("pending create consumer chain proposal could not be persisted: %w", err))
}
k.SetPendingConsumerAdditionProp(ctx, &prop)
}
for _, prop := range genState.ConsumerRemovalProposals {
k.SetPendingConsumerRemovalProp(ctx, prop.ChainId, prop.StopTime)
// prevent implicit memory aliasing
prop := prop
k.SetPendingConsumerRemovalProp(ctx, &prop)
}
for _, ubdOp := range genState.UnbondingOps {
k.SetUnbondingOp(ctx, ubdOp)
Expand Down Expand Up @@ -116,7 +115,7 @@ func (k Keeper) ExportGenesis(ctx sdk.Context) *types.GenesisState {
panic(fmt.Errorf("cannot find genesis for consumer chain %s with client %s", chainID, clientID))
}
cs.SlashDowntimeAck = k.GetSlashAcks(ctx, chainID)
k.IterateOverUnbondingOpIndex(ctx, chainID, func(vscID uint64, ubdIndex []uint64) (stop bool) {
k.IterateUnbondingOpIndex(ctx, chainID, func(vscID uint64, ubdIndex []uint64) (stop bool) {
cs.UnbondingOpsIndex = append(cs.UnbondingOpsIndex,
types.UnbondingOpIndex{ValsetUpdateId: vscID, UnbondingOpIndex: ubdIndex},
)
Expand All @@ -138,7 +137,7 @@ func (k Keeper) ExportGenesis(ctx sdk.Context) *types.GenesisState {
})

ubdOps := []ccv.UnbondingOp{}
k.IterateOverUnbondingOps(ctx, func(id uint64, ubdOp ccv.UnbondingOp) (stop bool) {
k.IterateUnbondingOps(ctx, func(id uint64, ubdOp ccv.UnbondingOp) (stop bool) {
ubdOps = append(ubdOps, ubdOp)
return false // do not stop the iteration
})
Expand All @@ -149,26 +148,25 @@ func (k Keeper) ExportGenesis(ctx sdk.Context) *types.GenesisState {
}

addProps := []types.ConsumerAdditionProposal{}
k.IteratePendingConsumerAdditionProps(ctx, func(_ time.Time, prop types.ConsumerAdditionProposal) (stop bool) {
k.IteratePendingConsumerAdditionProps(ctx, func(prop types.ConsumerAdditionProposal) (stop bool) {
addProps = append(addProps, prop)
return false // do not stop the iteration
})

remProps := []types.ConsumerRemovalProposal{}
k.IteratePendingConsumerRemovalProps(ctx, func(_ time.Time, prop types.ConsumerRemovalProposal) (stop bool) {
k.IteratePendingConsumerRemovalProps(ctx, func(prop types.ConsumerRemovalProposal) (stop bool) {
remProps = append(remProps, prop)
return false // do not stop the iteration
})

// Export key assignment states
validatorConsumerPubKeys := []types.ValidatorConsumerPubKey{}
k.IterateAllValidatorConsumerPubKeys(ctx, func(chainID string, providerAddr sdk.ConsAddress, consumerKey tmcrypto.PublicKey) (stop bool) {
k.IterateAllValidatorConsumerPubKeys(ctx, func(chainID string, providerAddr sdk.ConsAddress, consumerKey tmcrypto.PublicKey) {
validatorConsumerPubKeys = append(validatorConsumerPubKeys, types.ValidatorConsumerPubKey{
ChainId: chainID,
ProviderAddr: providerAddr,
ConsumerKey: &consumerKey,
})
return false // do not stop the iteration
shaspitz marked this conversation as resolved.
Show resolved Hide resolved
})

validatorsByConsumerAddr := []types.ValidatorByConsumerAddr{}
Expand All @@ -184,13 +182,12 @@ func (k Keeper) ExportGenesis(ctx sdk.Context) *types.GenesisState {
consumerAddrsToPrune := []types.ConsumerAddrsToPrune{}
// ConsumerAddrsToPrune are added only for registered consumer chains
k.IterateConsumerChains(ctx, func(ctx sdk.Context, chainID string, _ string) (stopOuter bool) {
k.IterateConsumerAddrsToPrune(ctx, chainID, func(vscID uint64, consumerAddrs types.AddressList) (stopInner bool) {
k.IterateConsumerAddrsToPrune(ctx, chainID, func(vscID uint64, consumerAddrs types.AddressList) {
consumerAddrsToPrune = append(consumerAddrsToPrune, types.ConsumerAddrsToPrune{
ChainId: chainID,
VscId: vscID,
ConsumerAddrs: &consumerAddrs,
})
return false // do not stop the iteration
})
return false // do not stop the iteration
})
Expand Down
2 changes: 1 addition & 1 deletion x/ccv/provider/keeper/genesis_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ func TestInitAndExportGenesis(t *testing.T) {
addProp, found := pk.GetPendingConsumerAdditionProp(ctx, oneHourFromNow, cChainIDs[0])
require.True(t, found)
require.Equal(t, provGenesis.ConsumerAdditionProposals[0], addProp)
require.True(t, pk.GetPendingConsumerRemovalProp(ctx, cChainIDs[0], oneHourFromNow))
require.True(t, pk.IsPendingConsumerRemovalProp(ctx, cChainIDs[0], oneHourFromNow))
require.Equal(t, provGenesis.Params, pk.GetParams(ctx))

gotConsTmPubKey, found := pk.GetValidatorConsumerPubKey(ctx, cChainIDs[0], provAddr)
Expand Down
14 changes: 12 additions & 2 deletions x/ccv/provider/keeper/grpc_query.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,13 @@ func (k Keeper) QueryConsumerChainStarts(goCtx context.Context, req *types.Query
return nil, status.Error(codes.InvalidArgument, "empty request")
}

// get all pending consumer addition proposals
ctx := sdk.UnwrapSDKContext(goCtx)
props := k.GetAllConsumerAdditionProps(ctx)
props := types.ConsumerAdditionProposals{}
k.IteratePendingConsumerAdditionProps(ctx, func(prop types.ConsumerAdditionProposal) (stop bool) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the reasoning behind using the iterate method instead of GetAllConsumerAdditionProps?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GetAllConsumerAdditionProps was only used here. Just wanted to cleanup the code. We could revert back if you think it's useful to have it. @MSalopek was also saying he prefers to have it.

props.Pending = append(props.Pending, &prop)
return false // do not stop the iteration
})

return &types.QueryConsumerChainStartProposalsResponse{Proposals: &props}, nil
}
Expand All @@ -67,8 +72,13 @@ func (k Keeper) QueryConsumerChainStops(goCtx context.Context, req *types.QueryC
return nil, status.Error(codes.InvalidArgument, "empty request")
}

// get all pending consumer removal proposals
ctx := sdk.UnwrapSDKContext(goCtx)
props := k.GetAllConsumerRemovalProps(ctx)
props := types.ConsumerRemovalProposals{}
k.IteratePendingConsumerRemovalProps(ctx, func(prop types.ConsumerRemovalProposal) (stop bool) {
props.Pending = append(props.Pending, &prop)
return false // do not stop the iteration
})

return &types.QueryConsumerChainStopProposalsResponse{Proposals: &props}, nil
}
Expand Down
3 changes: 1 addition & 2 deletions x/ccv/provider/keeper/hooks.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ func (h Hooks) AfterValidatorRemoved(ctx sdk.Context, valConsAddr sdk.ConsAddres
chainID string,
providerAddr sdk.ConsAddress,
consumerKey tmprotocrypto.PublicKey,
) (stop bool) {
) {
if providerAddr.Equals(valConsAddr) {
toDelete = append(toDelete,
StoreKey{
Expand All @@ -114,7 +114,6 @@ func (h Hooks) AfterValidatorRemoved(ctx sdk.Context, valConsAddr sdk.ConsAddres
ConsumerKey: consumerKey,
})
}
return false // do not stop
})
for _, key := range toDelete {
consumerAddr := utils.TMCryptoPublicKeyToConsAddr(key.ConsumerKey)
Expand Down
Loading