From bd38e1bcd9181cf49c1d30f03398af6e215eb803 Mon Sep 17 00:00:00 2001 From: beer-1 <147697694+beer-1@users.noreply.github.com> Date: Wed, 22 Nov 2023 17:15:34 +0900 Subject: [PATCH 1/2] change to store historical info for all blocks --- x/opchild/keeper/historical_info.go | 60 +++++---------------- x/opchild/keeper/historical_info_test.go | 67 +++++++----------------- x/opchild/keeper/val_state_change.go | 6 --- 3 files changed, 32 insertions(+), 101 deletions(-) diff --git a/x/opchild/keeper/historical_info.go b/x/opchild/keeper/historical_info.go index f95152fd..aadffe94 100644 --- a/x/opchild/keeper/historical_info.go +++ b/x/opchild/keeper/historical_info.go @@ -1,43 +1,28 @@ package keeper import ( - "encoding/binary" - "cosmossdk.io/math" - "github.com/cosmos/cosmos-sdk/store/prefix" sdk "github.com/cosmos/cosmos-sdk/types" cosmostypes "github.com/cosmos/cosmos-sdk/x/staking/types" - - "github.com/initia-labs/OPinit/x/opchild/types" ) -// GetHistoricalInfo fetch height historical info that is equal or lower than the given height. +// GetHistoricalInfo gets the historical info at a given height func (k Keeper) GetHistoricalInfo(ctx sdk.Context, height int64) (cosmostypes.HistoricalInfo, bool) { store := ctx.KVStore(k.storeKey) + key := cosmostypes.GetHistoricalInfoKey(height) - // increase height by 1 because iterator is exclusive. - height += 1 - - prefixStore := prefix.NewStore(store, types.HistoricalInfoKey) - - end := make([]byte, 8) - binary.BigEndian.PutUint64(end, uint64(height)) - - iterator := prefixStore.ReverseIterator(nil, end) - defer iterator.Close() - - if !iterator.Valid() { + value := store.Get(key) + if value == nil { return cosmostypes.HistoricalInfo{}, false } - value := iterator.Value() return cosmostypes.MustUnmarshalHistoricalInfo(k.cdc, value), true } // SetHistoricalInfo sets the historical info at a given height func (k Keeper) SetHistoricalInfo(ctx sdk.Context, height int64, hi *cosmostypes.HistoricalInfo) { store := ctx.KVStore(k.storeKey) - key := types.GetHistoricalInfoKey(uint64(height)) + key := cosmostypes.GetHistoricalInfoKey(height) value := k.cdc.MustMarshal(hi) store.Set(key, value) } @@ -45,7 +30,7 @@ func (k Keeper) SetHistoricalInfo(ctx sdk.Context, height int64, hi *cosmostypes // DeleteHistoricalInfo deletes the historical info at a given height func (k Keeper) DeleteHistoricalInfo(ctx sdk.Context, height int64) { store := ctx.KVStore(k.storeKey) - key := types.GetHistoricalInfoKey(uint64(height)) + key := cosmostypes.GetHistoricalInfoKey(height) store.Delete(key) } @@ -53,7 +38,7 @@ func (k Keeper) DeleteHistoricalInfo(ctx sdk.Context, height int64) { // TrackHistoricalInfo saves the latest historical-info and deletes the oldest // heights that are below pruning height func (k Keeper) TrackHistoricalInfo(ctx sdk.Context) { - entryNum := uint64(k.HistoricalEntries(ctx)) + entryNum := k.HistoricalEntries(ctx) // Prune store to ensure we only have parameter-defined historical entries. // In most cases, this will involve removing a single historical entry. @@ -62,31 +47,12 @@ func (k Keeper) TrackHistoricalInfo(ctx sdk.Context) { // Since the entries to be deleted are always in a continuous range, we can iterate // over the historical entries starting from the most recent version to be pruned // and then return at the first empty entry. - height := uint64(ctx.BlockHeight()) - if height > entryNum { - store := ctx.KVStore(k.storeKey) - prefixStore := prefix.NewStore(store, types.HistoricalInfoKey) - - end := make([]byte, 8) - binary.BigEndian.PutUint64(end, height-entryNum) - - iterator := prefixStore.ReverseIterator(nil, end) - defer iterator.Close() - - // our historical info does not exist for every block to allow - // empty block, so it is possible when ibc request deleted block - // historical info. Then opchild module returns height historical - // historical info that is lower than the given height. - // - // Whenever we delete historical info, we have to leave first info - // for safety. - if iterator.Valid() { - iterator.Next() - } - - for ; iterator.Valid(); iterator.Next() { - key := iterator.Key() - prefixStore.Delete(key) + for i := ctx.BlockHeight() - int64(entryNum); i >= 0; i-- { + _, found := k.GetHistoricalInfo(ctx, i) + if found { + k.DeleteHistoricalInfo(ctx, i) + } else { + break } } diff --git a/x/opchild/keeper/historical_info_test.go b/x/opchild/keeper/historical_info_test.go index 7f75d934..62659b5a 100644 --- a/x/opchild/keeper/historical_info_test.go +++ b/x/opchild/keeper/historical_info_test.go @@ -3,65 +3,36 @@ package keeper_test import ( "testing" - tmtypes "github.com/cometbft/cometbft/proto/tendermint/types" - cosmostypes "github.com/cosmos/cosmos-sdk/x/staking/types" "github.com/stretchr/testify/require" + + cosmostypes "github.com/cosmos/cosmos-sdk/x/staking/types" ) func Test_HistoricalInfo(t *testing.T) { ctx, input := createDefaultTestInput(t) - historicalInfo := cosmostypes.HistoricalInfo{ - Header: tmtypes.Header{ - Height: 100, - ChainID: "testnet", - }, - Valset: []cosmostypes.Validator{{ - OperatorAddress: "hihi", - }}, - } - - input.OPChildKeeper.SetHistoricalInfo(ctx, 100, &historicalInfo) - - _historicalInfo, found := input.OPChildKeeper.GetHistoricalInfo(ctx, 101) - require.True(t, found) - require.Equal(t, historicalInfo.Header.Height, _historicalInfo.Header.Height) - require.Equal(t, historicalInfo.Header.ChainID, _historicalInfo.Header.ChainID) - require.Equal(t, historicalInfo.Valset[0].OperatorAddress, _historicalInfo.Valset[0].OperatorAddress) - - _, found = input.OPChildKeeper.GetHistoricalInfo(ctx, 99) - require.False(t, found) -} - -func Test_TrackHistoricalInfo(t *testing.T) { - ctx, input := createDefaultTestInput(t) - emptyHistoricalInfo := cosmostypes.HistoricalInfo{} - input.OPChildKeeper.SetHistoricalInfo(ctx, 100, &emptyHistoricalInfo) - input.OPChildKeeper.SetHistoricalInfo(ctx, 101, &emptyHistoricalInfo) - input.OPChildKeeper.SetHistoricalInfo(ctx, 102, &emptyHistoricalInfo) - input.OPChildKeeper.SetHistoricalInfo(ctx, 103, &emptyHistoricalInfo) - - ctx = ctx.WithBlockHeight(104) params := input.OPChildKeeper.GetParams(ctx) - params.HistoricalEntries = 1 + params.HistoricalEntries = 2 input.OPChildKeeper.SetParams(ctx, params) - input.OPChildKeeper.TrackHistoricalInfo(ctx) + input.OPChildKeeper.TrackHistoricalInfo(ctx.WithBlockHeight(1)) + input.OPChildKeeper.TrackHistoricalInfo(ctx.WithBlockHeight(2)) + input.OPChildKeeper.TrackHistoricalInfo(ctx.WithBlockHeight(3)) - _, found := input.OPChildKeeper.GetHistoricalInfo(ctx, 102) - require.True(t, found) - _, found = input.OPChildKeeper.GetHistoricalInfo(ctx, 101) + _, found := input.OPChildKeeper.GetHistoricalInfo(ctx, 1) require.False(t, found) - _, found = input.OPChildKeeper.GetHistoricalInfo(ctx, 100) - require.False(t, found) -} -func Test_DeleteHistoricalInfo(t *testing.T) { - ctx, input := createDefaultTestInput(t) - emptyHistoricalInfo := cosmostypes.HistoricalInfo{} - input.OPChildKeeper.SetHistoricalInfo(ctx, 100, &emptyHistoricalInfo) + historicalInfo, found := input.OPChildKeeper.GetHistoricalInfo(ctx, 2) + require.True(t, found) + require.Equal(t, cosmostypes.HistoricalInfo{ + Header: ctx.WithBlockHeight(2).BlockHeader(), + Valset: nil, + }, historicalInfo) - input.OPChildKeeper.DeleteHistoricalInfo(ctx, 100) - _, found := input.OPChildKeeper.GetHistoricalInfo(ctx, 100) - require.False(t, found) + historicalInfo, found = input.OPChildKeeper.GetHistoricalInfo(ctx, 3) + require.True(t, found) + require.Equal(t, cosmostypes.HistoricalInfo{ + Header: ctx.WithBlockHeight(3).BlockHeader(), + Valset: nil, + }, historicalInfo) } diff --git a/x/opchild/keeper/val_state_change.go b/x/opchild/keeper/val_state_change.go index 62900af5..1f733150 100644 --- a/x/opchild/keeper/val_state_change.go +++ b/x/opchild/keeper/val_state_change.go @@ -19,12 +19,6 @@ func (k Keeper) BlockValidatorUpdates(ctx sdk.Context) []abci.ValidatorUpdate { panic(err) } - // if there is no validator updates, - // delete tracking info to prevent empty block creation. - if len(updates) == 0 { - k.DeleteHistoricalInfo(ctx, ctx.BlockHeight()) - } - return updates } From 2e5632076588d4b31bcb2ce59a93e74d29cccf1f Mon Sep 17 00:00:00 2001 From: beer-1 <147697694+beer-1@users.noreply.github.com> Date: Mon, 27 Nov 2023 13:36:37 +0900 Subject: [PATCH 2/2] throw invalid param update error when max validator is zero --- x/opchild/types/errors.go | 1 + x/opchild/types/params.go | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/x/opchild/types/errors.go b/x/opchild/types/errors.go index 3135eb10..a1efba47 100644 --- a/x/opchild/types/errors.go +++ b/x/opchild/types/errors.go @@ -16,4 +16,5 @@ var ( ErrDepositAlreadyFinalized = errorsmod.Register(ModuleName, 9, "deposit already finalized") ErrInvalidAmount = errorsmod.Register(ModuleName, 10, "invalid amount") ErrInvalidSequence = errorsmod.Register(ModuleName, 11, "invalid sequence") + ErrZeroMaxValidators = errorsmod.Register(ModuleName, 12, "max validators must be non-zero") ) diff --git a/x/opchild/types/params.go b/x/opchild/types/params.go index 1c405946..cc14edd0 100644 --- a/x/opchild/types/params.go +++ b/x/opchild/types/params.go @@ -48,5 +48,9 @@ func (p Params) Validate() error { return err } + if p.MaxValidators == 0 { + return ErrZeroMaxValidators + } + return nil }