diff --git a/x/opchild/keeper/abci_listener.go b/x/opchild/keeper/abci_listener.go deleted file mode 100644 index 170a12d0..00000000 --- a/x/opchild/keeper/abci_listener.go +++ /dev/null @@ -1,69 +0,0 @@ -package keeper - -import ( - "context" - "sync" - - abci "github.com/cometbft/cometbft/abci/types" - sdk "github.com/cosmos/cosmos-sdk/types" - - "github.com/cosmos/cosmos-sdk/baseapp" - store "github.com/cosmos/cosmos-sdk/store/types" -) - -var _ baseapp.StreamingService = &ABCIListener{} - -// ABCIListener is the abci listener to check whether current block is empty or not. -type ABCIListener struct { - txCount uint64 - *Keeper -} - -func newABCIListener(k *Keeper) ABCIListener { - return ABCIListener{txCount: 0, Keeper: k} -} - -// ListenDeliverTx updates the steaming service with the latest DeliverTx messages -func (listener *ABCIListener) ListenDeliverTx(ctx context.Context, _ abci.RequestDeliverTx, _ abci.ResponseDeliverTx) error { - listener.txCount++ - - return nil -} - -// Stream is the streaming service loop, awaits kv pairs and writes them to some destination stream or file -func (listener *ABCIListener) Stream(wg *sync.WaitGroup) error { return nil } - -// Listeners returns the streaming service's listeners for the BaseApp to register -func (listener *ABCIListener) Listeners() map[store.StoreKey][]store.WriteListener { return nil } - -// ListenBeginBlock updates the streaming service with the latest BeginBlock messages -func (listener *ABCIListener) ListenBeginBlock(ctx context.Context, req abci.RequestBeginBlock, res abci.ResponseBeginBlock) error { - listener.txCount = 0 - - return nil -} - -// ListenEndBlock updates the steaming service with the latest EndBlock messages -func (listener *ABCIListener) ListenEndBlock(ctx context.Context, req abci.RequestEndBlock, res abci.ResponseEndBlock) error { - - // if a block is empty, then remove historical info. - sdkCtx := sdk.UnwrapSDKContext(ctx) - - // ignore first tx in a block. - // - https://github.com/skip-mev/block-sdk/issues/215 - if listener.txCount == 1 && sdkCtx.BlockHeight() != 1 { - listener.DeleteHistoricalInfo(sdkCtx, sdkCtx.BlockHeight()) - } - - return nil -} - -// ListenCommit updates the steaming service with the latest Commit event -func (listener *ABCIListener) ListenCommit(ctx context.Context, res abci.ResponseCommit) error { - return nil -} - -// Closer is the interface that wraps the basic Close method. -func (listener *ABCIListener) Close() error { - return nil -} 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/keeper.go b/x/opchild/keeper/keeper.go index 9a8fb01e..fc600036 100644 --- a/x/opchild/keeper/keeper.go +++ b/x/opchild/keeper/keeper.go @@ -20,8 +20,7 @@ type Keeper struct { bridgeHook types.BridgeHook // Msg server router - router *baseapp.MsgServiceRouter - abciListener *ABCIListener + router *baseapp.MsgServiceRouter // Legacy Proposal router legacyRouter govv1beta1.Router @@ -45,22 +44,15 @@ func NewKeeper( panic("authority is not a valid acc address") } - abciListener := &ABCIListener{} - k := Keeper{ - cdc: cdc, - storeKey: key, - authKeeper: ak, - bankKeeper: bk, - bridgeHook: bh, - router: router, - authority: authority, - abciListener: abciListener, + return Keeper{ + cdc: cdc, + storeKey: key, + authKeeper: ak, + bankKeeper: bk, + bridgeHook: bh, + router: router, + authority: authority, } - - _abciListener := newABCIListener(&k) - *abciListener = _abciListener - - return k } // GetAuthority returns the x/move module's authority. @@ -73,11 +65,6 @@ func (k Keeper) Logger(ctx sdk.Context) log.Logger { return ctx.Logger().With("module", "x/"+types.ModuleName) } -// GetABCIListener return ABCIListener pointer -func (k Keeper) GetABCIListener() *ABCIListener { - return k.abciListener -} - // Router returns the gov keeper's router func (keeper Keeper) Router() *baseapp.MsgServiceRouter { return keeper.router 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 }