From d8de3c6c29b79f1c77d00aa3ba71ae8e5ff7b602 Mon Sep 17 00:00:00 2001 From: zale144 Date: Thu, 18 Jul 2024 10:39:11 +0200 Subject: [PATCH] fix(delayedack): handle finalization errors per rollapp (#966) Co-authored-by: Daniel T <30197399+danwt@users.noreply.github.com> --- CHANGELOG.md | 1 + app/app.go | 2 +- app/apptesting/test_suite.go | 8 +- app/export.go | 2 +- app/keepers/keepers.go | 40 +- app/keepers/modules.go | 6 +- ibctesting/utils_test.go | 2 +- x/delayedack/ibc_middleware.go | 4 +- x/delayedack/keeper/finalize.go | 22 +- x/delayedack/keeper/invariants_test.go | 4 +- x/eibc/module.go | 9 +- x/eibc/module_simulation.go | 8 +- .../client/cli/tx_submit_fraud_proposal.go | 6 +- .../block_height_to_finalization_queue.go | 126 +++-- ...block_height_to_finalization_queue_test.go | 441 +++++++++++++++++- x/rollapp/keeper/fraud_handler_test.go | 6 +- x/rollapp/keeper/invariants_test.go | 9 +- x/rollapp/keeper/keeper.go | 18 +- x/rollapp/keeper/rollapp_suite_test.go | 2 +- x/rollapp/module.go | 23 +- x/rollapp/module_simulation.go | 6 +- 21 files changed, 590 insertions(+), 155 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dd442d549..63f3d4b2c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -148,6 +148,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ - (rollapp) [#681](https://github.com/dymensionxyz/dymension/pull/681) Accept rollapp initial state with arbitrary height - (ibc) [#678](https://github.com/dymensionxyz/dymension/pull/678) Apply a pfm patch - (rollapp) [#671](https://github.com/dymensionxyz/dymension/pull/671) Fix rollapp genesis token not registered as IBC denom +- (delayedack) [#670](https://github.com/dymensionxyz/dymension/issues/670) Finalize error handling per rollapp - (dependencies) [#677](https://github.com/dymensionxyz/dymension/pull/677) Bump cosmos ecosystem dependencies - (hygiene) [#676](https://github.com/dymensionxyz/dymension/pull/676) Lint tests - (rollapp) [#657](https://github.com/dymensionxyz/dymension/pull/657) Verification of broken invariant logic diff --git a/app/app.go b/app/app.go index 050518945..b7fc0f4d6 100644 --- a/app/app.go +++ b/app/app.go @@ -215,7 +215,7 @@ func New( SignModeHandler: encodingConfig.TxConfig.SignModeHandler(), MaxTxGasWanted: maxGasWanted, ExtensionOptionChecker: nil, // uses default - RollappKeeper: app.RollappKeeper, + RollappKeeper: *app.RollappKeeper, }) if err != nil { panic(err) diff --git a/app/apptesting/test_suite.go b/app/apptesting/test_suite.go index e432eeaf9..022727c38 100644 --- a/app/apptesting/test_suite.go +++ b/app/apptesting/test_suite.go @@ -5,10 +5,12 @@ import ( codectypes "github.com/cosmos/cosmos-sdk/codec/types" "github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1" sdk "github.com/cosmos/cosmos-sdk/types" - "github.com/dymensionxyz/dymension/v3/app" "github.com/stretchr/testify/suite" + "github.com/dymensionxyz/dymension/v3/app" + bankutil "github.com/cosmos/cosmos-sdk/x/bank/testutil" + rollappkeeper "github.com/dymensionxyz/dymension/v3/x/rollapp/keeper" rollapptypes "github.com/dymensionxyz/dymension/v3/x/rollapp/types" sequencerkeeper "github.com/dymensionxyz/dymension/v3/x/sequencer/keeper" @@ -38,7 +40,7 @@ func (s *KeeperTestHelper) CreateRollappWithName(name string) string { MaxSequencers: 5, } - msgServer := rollappkeeper.NewMsgServerImpl(s.App.RollappKeeper) + msgServer := rollappkeeper.NewMsgServerImpl(*s.App.RollappKeeper) _, err := msgServer.CreateRollapp(s.Ctx, &msgCreateRollapp) s.Require().NoError(err) return name @@ -84,7 +86,7 @@ func (s *KeeperTestHelper) PostStateUpdate(ctx sdk.Context, rollappId, seqAddr s Version: 0, BDs: bds, } - msgServer := rollappkeeper.NewMsgServerImpl(s.App.RollappKeeper) + msgServer := rollappkeeper.NewMsgServerImpl(*s.App.RollappKeeper) _, err = msgServer.UpdateState(ctx, &updateState) return startHeight + numOfBlocks, err } diff --git a/app/export.go b/app/export.go index d14856995..0cbc32d2f 100644 --- a/app/export.go +++ b/app/export.go @@ -36,7 +36,7 @@ func (app *App) ExportAppStateAndValidators( return servertypes.ExportedApp{}, err } - validators, err := staking.WriteValidators(ctx, &app.StakingKeeper) + validators, err := staking.WriteValidators(ctx, app.StakingKeeper) if err != nil { return servertypes.ExportedApp{}, err } diff --git a/app/keepers/keepers.go b/app/keepers/keepers.go index 2204247d6..6f9907082 100644 --- a/app/keepers/keepers.go +++ b/app/keepers/keepers.go @@ -99,7 +99,7 @@ type AppKeepers struct { AuthzKeeper authzkeeper.Keeper BankKeeper bankkeeper.Keeper CapabilityKeeper *capabilitykeeper.Keeper - StakingKeeper stakingkeeper.Keeper + StakingKeeper *stakingkeeper.Keeper SlashingKeeper slashingkeeper.Keeper MintKeeper mintkeeper.Keeper DistrKeeper distrkeeper.Keeper @@ -133,7 +133,7 @@ type AppKeepers struct { ScopedIBCKeeper capabilitykeeper.ScopedKeeper ScopedTransferKeeper capabilitykeeper.ScopedKeeper - RollappKeeper rollappmodulekeeper.Keeper + RollappKeeper *rollappmodulekeeper.Keeper SequencerKeeper sequencermodulekeeper.Keeper StreamerKeeper streamermodulekeeper.Keeper EIBCKeeper eibckeeper.Keeper @@ -211,7 +211,7 @@ func (a *AppKeepers) InitKeepers( govModuleAddress, ) - stakingKeeper := stakingkeeper.NewKeeper( + a.StakingKeeper = stakingkeeper.NewKeeper( appCodec, a.keys[stakingtypes.StoreKey], a.AccountKeeper, @@ -222,7 +222,7 @@ func (a *AppKeepers) InitKeepers( a.MintKeeper = mintkeeper.NewKeeper( appCodec, a.keys[minttypes.StoreKey], - stakingKeeper, + a.StakingKeeper, a.AccountKeeper, a.BankKeeper, authtypes.FeeCollectorName, @@ -234,7 +234,7 @@ func (a *AppKeepers) InitKeepers( a.keys[distrtypes.StoreKey], a.AccountKeeper, a.BankKeeper, - stakingKeeper, + a.StakingKeeper, authtypes.FeeCollectorName, govModuleAddress, ) @@ -243,16 +243,10 @@ func (a *AppKeepers) InitKeepers( appCodec, cdc, a.keys[slashingtypes.StoreKey], - stakingKeeper, + a.StakingKeeper, govModuleAddress, ) - // TODO: move back to SetupHooks after https://github.com/dymensionxyz/dymension/pull/970 is merged - stakingKeeper.SetHooks( - stakingtypes.NewMultiStakingHooks(a.DistrKeeper.Hooks(), a.SlashingKeeper.Hooks()), - ) - a.StakingKeeper = *stakingKeeper - a.FeeGrantKeeper = feegrantkeeper.NewKeeper( appCodec, a.keys[feegrant.StoreKey], @@ -277,7 +271,7 @@ func (a *AppKeepers) InitKeepers( authtypes.NewModuleAddress(govtypes.ModuleName), a.AccountKeeper, a.BankKeeper, - stakingKeeper, + a.StakingKeeper, a.FeeMarketKeeper, nil, geth.NewEVM, @@ -332,7 +326,7 @@ func (a *AppKeepers) InitKeepers( appCodec, a.keys[ibcexported.StoreKey], a.GetSubspace(ibcexported.ModuleName), - stakingKeeper, + a.StakingKeeper, a.UpgradeKeeper, a.ScopedIBCKeeper, ) @@ -370,7 +364,7 @@ func (a *AppKeepers) InitKeepers( a.BankKeeper, ) - a.RollappKeeper = *rollappmodulekeeper.NewKeeper( + a.RollappKeeper = rollappmodulekeeper.NewKeeper( appCodec, a.keys[rollappmoduletypes.StoreKey], a.GetSubspace(rollappmoduletypes.ModuleName), @@ -422,20 +416,20 @@ func (a *AppKeepers) InitKeepers( AddRoute(upgradetypes.RouterKey, upgrade.NewSoftwareUpgradeProposalHandler(a.UpgradeKeeper)). AddRoute(ibcclienttypes.RouterKey, ibcclient.NewClientProposalHandler(a.IBCKeeper.ClientKeeper)). AddRoute(streamermoduletypes.RouterKey, streamermodule.NewStreamerProposalHandler(a.StreamerKeeper)). - AddRoute(rollappmoduletypes.RouterKey, rollappmodule.NewRollappProposalHandler(&a.RollappKeeper)). + AddRoute(rollappmoduletypes.RouterKey, rollappmodule.NewRollappProposalHandler(a.RollappKeeper)). AddRoute(denommetadatamoduletypes.RouterKey, denommetadatamodule.NewDenomMetadataProposalHandler(a.DenomMetadataKeeper)). AddRoute(evmtypes.RouterKey, evm.NewEvmProposalHandler(a.EvmKeeper)) // Create evidence Keeper for to register the IBC light client misbehaviour evidence route // If evidence needs to be handled for the app, set routes in router here and seal a.EvidenceKeeper = *evidencekeeper.NewKeeper( - appCodec, a.keys[evidencetypes.StoreKey], stakingKeeper, a.SlashingKeeper, + appCodec, a.keys[evidencetypes.StoreKey], a.StakingKeeper, a.SlashingKeeper, ) govConfig := govtypes.DefaultConfig() a.GovKeeper = govkeeper.NewKeeper( appCodec, a.keys[govtypes.StoreKey], a.AccountKeeper, a.BankKeeper, - stakingKeeper, bApp.MsgServiceRouter(), govConfig, govModuleAddress, + a.StakingKeeper, bApp.MsgServiceRouter(), govConfig, govModuleAddress, ) a.GovKeeper.SetLegacyRouter(govRouter) @@ -457,7 +451,7 @@ func (a *AppKeepers) InitTransferStack() { a.DelayedAckKeeper, a.TransferKeeper, a.AccountKeeper.GetModuleAddress(txfeestypes.ModuleName), - a.RollappKeeper, + *a.RollappKeeper, ) a.TransferStack = packetforwardmiddleware.NewIBCMiddleware( a.TransferStack, @@ -475,8 +469,8 @@ func (a *AppKeepers) InitTransferStack() { delayedackmodule.WithRollappKeeper(a.RollappKeeper), ) a.TransferStack = a.delayedAckMiddleware - a.TransferStack = transfergenesis.NewIBCModule(a.TransferStack, a.DelayedAckKeeper, a.RollappKeeper, a.TransferKeeper, a.DenomMetadataKeeper) - a.TransferStack = transfergenesis.NewIBCModuleCanonicalChannelHack(a.TransferStack, a.RollappKeeper, a.IBCKeeper.ChannelKeeper) + a.TransferStack = transfergenesis.NewIBCModule(a.TransferStack, a.DelayedAckKeeper, *a.RollappKeeper, a.TransferKeeper, a.DenomMetadataKeeper) + a.TransferStack = transfergenesis.NewIBCModuleCanonicalChannelHack(a.TransferStack, *a.RollappKeeper, a.IBCKeeper.ChannelKeeper) // Create static IBC router, add transfer route, then set and seal it ibcRouter := ibcporttypes.NewRouter() @@ -485,6 +479,10 @@ func (a *AppKeepers) InitTransferStack() { } func (a *AppKeepers) SetupHooks() { + a.StakingKeeper.SetHooks( + stakingtypes.NewMultiStakingHooks(a.DistrKeeper.Hooks(), a.SlashingKeeper.Hooks()), + ) + // register the staking hooks a.LockupKeeper.SetHooks( lockuptypes.NewMultiLockupHooks( diff --git a/app/keepers/modules.go b/app/keepers/modules.go index a914cb4a8..e5d6a3e39 100644 --- a/app/keepers/modules.go +++ b/app/keepers/modules.go @@ -101,7 +101,7 @@ import ( var ModuleBasics = module.NewBasicManager( auth.AppModuleBasic{}, authzmodule.AppModuleBasic{}, - genutil.AppModuleBasic{}, + genutil.NewAppModuleBasic(genutiltypes.DefaultMessageValidator), bank.AppModuleBasic{}, capability.AppModuleBasic{}, consensus.AppModuleBasic{}, @@ -177,14 +177,14 @@ func (a *AppKeepers) SetupModules( mint.NewAppModule(appCodec, a.MintKeeper, a.AccountKeeper, nil, a.GetSubspace(minttypes.ModuleName)), slashing.NewAppModule(appCodec, a.SlashingKeeper, a.AccountKeeper, a.BankKeeper, a.StakingKeeper, a.GetSubspace(slashingtypes.ModuleName)), distr.NewAppModule(appCodec, a.DistrKeeper, a.AccountKeeper, a.BankKeeper, a.StakingKeeper, a.GetSubspace(distrtypes.ModuleName)), - staking.NewAppModule(appCodec, &a.StakingKeeper, a.AccountKeeper, a.BankKeeper, a.GetSubspace(stakingtypes.ModuleName)), + staking.NewAppModule(appCodec, a.StakingKeeper, a.AccountKeeper, a.BankKeeper, a.GetSubspace(stakingtypes.ModuleName)), upgrade.NewAppModule(a.UpgradeKeeper), evidence.NewAppModule(a.EvidenceKeeper), ibc.NewAppModule(a.IBCKeeper), params.NewAppModule(a.ParamsKeeper), packetforwardmiddleware.NewAppModule(a.PacketForwardMiddlewareKeeper, a.GetSubspace(packetforwardtypes.ModuleName)), ibctransfer.NewAppModule(a.TransferKeeper), - rollappmodule.NewAppModule(appCodec, &a.RollappKeeper, a.AccountKeeper, a.BankKeeper), + rollappmodule.NewAppModule(appCodec, a.RollappKeeper, a.AccountKeeper, a.BankKeeper), sequencermodule.NewAppModule(appCodec, a.SequencerKeeper, a.AccountKeeper, a.BankKeeper), streamermodule.NewAppModule(a.StreamerKeeper, a.AccountKeeper, a.BankKeeper, a.EpochsKeeper), delayedackmodule.NewAppModule(appCodec, a.DelayedAckKeeper), diff --git a/ibctesting/utils_test.go b/ibctesting/utils_test.go index 32687e0f1..aa1f4e02d 100644 --- a/ibctesting/utils_test.go +++ b/ibctesting/utils_test.go @@ -101,7 +101,7 @@ func (s *utilSuite) rollappCtx() sdk.Context { } func (s *utilSuite) rollappMsgServer() rollapptypes.MsgServer { - return rollappkeeper.NewMsgServerImpl(s.hubApp().RollappKeeper) + return rollappkeeper.NewMsgServerImpl(*s.hubApp().RollappKeeper) } // SetupTest creates a coordinator with 2 test chains. diff --git a/x/delayedack/ibc_middleware.go b/x/delayedack/ibc_middleware.go index e9d5c8a1e..7794f243b 100644 --- a/x/delayedack/ibc_middleware.go +++ b/x/delayedack/ibc_middleware.go @@ -39,9 +39,9 @@ func WithKeeper(k keeper.Keeper) option { } } -func WithRollappKeeper(k rollappkeeper.Keeper) option { +func WithRollappKeeper(k *rollappkeeper.Keeper) option { return func(m *IBCMiddleware) { - m.raKeeper = k + m.raKeeper = *k } } diff --git a/x/delayedack/keeper/finalize.go b/x/delayedack/keeper/finalize.go index 844c5bd57..a980c16f4 100644 --- a/x/delayedack/keeper/finalize.go +++ b/x/delayedack/keeper/finalize.go @@ -5,15 +5,14 @@ import ( "github.com/cosmos/ibc-go/v7/modules/core/exported" + "github.com/cometbft/cometbft/libs/log" sdk "github.com/cosmos/cosmos-sdk/types" capabilitytypes "github.com/cosmos/cosmos-sdk/x/capability/types" porttypes "github.com/cosmos/ibc-go/v7/modules/core/05-port/types" + "github.com/osmosis-labs/osmosis/v15/osmoutils" commontypes "github.com/dymensionxyz/dymension/v3/x/common/types" "github.com/dymensionxyz/dymension/v3/x/delayedack/types" - - "github.com/cometbft/cometbft/libs/log" - "github.com/osmosis-labs/osmosis/v15/osmoutils" ) // FinalizeRollappPackets finalizes the packets for the given rollapp until the given height which is @@ -45,7 +44,7 @@ func (k Keeper) finalizeRollappPacket( rollappID string, logger log.Logger, rollappPacket commontypes.RollappPacket, -) (err error) { +) error { logContext := []interface{}{ "rollappID", rollappID, "sequence", rollappPacket.Packet.Sequence, @@ -54,6 +53,7 @@ func (k Keeper) finalizeRollappPacket( "type", rollappPacket.Type, } + var packetErr error switch rollappPacket.Type { case commontypes.RollappPacket_ON_RECV: // TODO: makes more sense to modify the packet when calling the handler, instead storing in db "wrong" packet @@ -70,21 +70,21 @@ func (k Keeper) finalizeRollappPacket( eibc: effective transfer from fulfiller to original target */ if ack != nil { - err = osmoutils.ApplyFuncIfNoError(ctx, k.writeRecvAck(rollappPacket, ack)) + packetErr = osmoutils.ApplyFuncIfNoError(ctx, k.writeRecvAck(rollappPacket, ack)) } case commontypes.RollappPacket_ON_ACK: - err = osmoutils.ApplyFuncIfNoError(ctx, k.onAckPacket(rollappPacket, ibc)) + packetErr = osmoutils.ApplyFuncIfNoError(ctx, k.onAckPacket(rollappPacket, ibc)) case commontypes.RollappPacket_ON_TIMEOUT: - err = osmoutils.ApplyFuncIfNoError(ctx, k.onTimeoutPacket(rollappPacket, ibc)) + packetErr = osmoutils.ApplyFuncIfNoError(ctx, k.onTimeoutPacket(rollappPacket, ibc)) default: logger.Error("Unknown rollapp packet type", logContext...) } // Update the packet with the error - if err != nil { - rollappPacket.Error = err.Error() + if packetErr != nil { + rollappPacket.Error = packetErr.Error() } // Update status to finalized - _, err = k.UpdateRollappPacketWithStatus(ctx, rollappPacket, commontypes.Status_FINALIZED) + _, err := k.UpdateRollappPacketWithStatus(ctx, rollappPacket, commontypes.Status_FINALIZED) if err != nil { // If we failed finalizing the packet we return an error to abort the end blocker otherwise it's // invariant breaking @@ -92,7 +92,7 @@ func (k Keeper) finalizeRollappPacket( } logger.Debug("finalized IBC rollapp packet", logContext...) - return + return nil } func (k Keeper) writeRecvAck(rollappPacket commontypes.RollappPacket, ack exported.Acknowledgement) wrappedFunc { diff --git a/x/delayedack/keeper/invariants_test.go b/x/delayedack/keeper/invariants_test.go index 5453ac566..f4730a068 100644 --- a/x/delayedack/keeper/invariants_test.go +++ b/x/delayedack/keeper/invariants_test.go @@ -45,7 +45,6 @@ func (suite *DelayedAckTestSuite) TestInvariants() { sequence := uint64(0) for j := 0; j < numOfStates; j++ { - numOfBlocks := uint64(rand.Intn(10) + 1) for rollapp, sequencer := range seqPerRollapp { @@ -71,8 +70,7 @@ func (suite *DelayedAckTestSuite) TestInvariants() { } // progress finalization queue - err := suite.App.RollappKeeper.FinalizeRollappStates(suite.Ctx) - suite.Require().NoError(err) + suite.App.RollappKeeper.FinalizeRollappStates(suite.Ctx) // test fraud for rollapp := range seqPerRollapp { diff --git a/x/eibc/module.go b/x/eibc/module.go index dae1df483..71f55101c 100644 --- a/x/eibc/module.go +++ b/x/eibc/module.go @@ -5,18 +5,15 @@ import ( "encoding/json" "fmt" - // this line is used by starport scaffolding # 1 - - "github.com/grpc-ecosystem/grpc-gateway/runtime" - "github.com/spf13/cobra" - abci "github.com/cometbft/cometbft/abci/types" - "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/codec" cdctypes "github.com/cosmos/cosmos-sdk/codec/types" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/types/module" + "github.com/grpc-ecosystem/grpc-gateway/runtime" + "github.com/spf13/cobra" + "github.com/dymensionxyz/dymension/v3/x/eibc/client/cli" "github.com/dymensionxyz/dymension/v3/x/eibc/keeper" "github.com/dymensionxyz/dymension/v3/x/eibc/types" diff --git a/x/eibc/module_simulation.go b/x/eibc/module_simulation.go index 66015123d..0d2c8bee9 100644 --- a/x/eibc/module_simulation.go +++ b/x/eibc/module_simulation.go @@ -6,6 +6,7 @@ import ( "github.com/cosmos/cosmos-sdk/types/module" simtypes "github.com/cosmos/cosmos-sdk/types/simulation" "github.com/cosmos/cosmos-sdk/x/simulation" + "github.com/dymensionxyz/dymension/v3/app/params" "github.com/dymensionxyz/dymension/v3/testutil/sample" eibcsimulation "github.com/dymensionxyz/dymension/v3/x/eibc/simulation" @@ -21,10 +22,6 @@ var ( _ = baseapp.Paramspace ) -const ( -// this line is used by starport scaffolding # simapp/module/const -) - // GenerateGenesisState creates a randomized GenState of the module func (AppModule) GenerateGenesisState(simState *module.SimulationState) { accs := make([]string, len(simState.Accounts)) @@ -49,8 +46,5 @@ func (am AppModule) RegisterStoreDecoder(_ sdk.StoreDecoderRegistry) {} // WeightedOperations returns the all the gov module operations with their respective weights. func (am AppModule) WeightedOperations(simState module.SimulationState) []simtypes.WeightedOperation { operations := make([]simtypes.WeightedOperation, 0) - - // this line is used by starport scaffolding # simapp/module/operation - return operations } diff --git a/x/rollapp/client/cli/tx_submit_fraud_proposal.go b/x/rollapp/client/cli/tx_submit_fraud_proposal.go index 26a578003..33d496350 100644 --- a/x/rollapp/client/cli/tx_submit_fraud_proposal.go +++ b/x/rollapp/client/cli/tx_submit_fraud_proposal.go @@ -5,14 +5,12 @@ import ( "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/client/tx" - "github.com/spf13/cobra" - - "github.com/dymensionxyz/dymension/v3/x/rollapp/types" - govcli "github.com/cosmos/cosmos-sdk/x/gov/client/cli" govtypes "github.com/cosmos/cosmos-sdk/x/gov/types/v1beta1" + "github.com/spf13/cobra" "github.com/dymensionxyz/dymension/v3/utils" + "github.com/dymensionxyz/dymension/v3/x/rollapp/types" ) // NewCmdSubmitFraudProposal submits a fraud proposal diff --git a/x/rollapp/keeper/block_height_to_finalization_queue.go b/x/rollapp/keeper/block_height_to_finalization_queue.go index ab87acca9..b7ba678de 100644 --- a/x/rollapp/keeper/block_height_to_finalization_queue.go +++ b/x/rollapp/keeper/block_height_to_finalization_queue.go @@ -2,65 +2,117 @@ package keeper import ( "fmt" + "slices" "github.com/cosmos/cosmos-sdk/store/prefix" sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/osmosis-labs/osmosis/v15/osmoutils" + common "github.com/dymensionxyz/dymension/v3/x/common/types" "github.com/dymensionxyz/dymension/v3/x/rollapp/types" - "github.com/osmosis-labs/osmosis/v15/osmoutils" ) // FinalizeRollappStates is called every block to finalize states when their dispute period over. -func (k Keeper) FinalizeRollappStates(ctx sdk.Context) error { +func (k Keeper) FinalizeRollappStates(ctx sdk.Context) { if uint64(ctx.BlockHeight()) < k.DisputePeriodInBlocks(ctx) { - return nil + return } // check to see if there are pending states to be finalized finalizationHeight := uint64(ctx.BlockHeight() - int64(k.DisputePeriodInBlocks(ctx))) pendingFinalizationQueue := k.GetAllFinalizationQueueUntilHeightInclusive(ctx, finalizationHeight) - return osmoutils.ApplyFuncIfNoError(ctx, - // we trap at this granularity because we want to avoid iterating inside the - // wrapped function (above), and we want the function to be atomic, so - // we cannot trap inside finalizePending - func(ctx sdk.Context) error { - return k.finalizePending(ctx, pendingFinalizationQueue) - }) + k.FinalizeAllPending(ctx, pendingFinalizationQueue) } -func (k Keeper) finalizePending(ctx sdk.Context, pendingFinalizationQueue []types.BlockHeightToFinalizationQueue) error { - // Iterate over all the pending finalization queue +func (k Keeper) FinalizeAllPending(ctx sdk.Context, pendingFinalizationQueue []types.BlockHeightToFinalizationQueue) { + // Cache the rollapps that failed to finalize at current EndBlocker execution. + // The mapping is from rollappID to the first index of the state that failed to finalize. + failedRollapps := make(map[string]uint64) + // iterate over all the pending finalization height queues for _, blockHeightToFinalizationQueue := range pendingFinalizationQueue { - // finalize pending states - for _, stateInfoIndex := range blockHeightToFinalizationQueue.FinalizationQueue { - stateInfo := k.MustGetStateInfo(ctx, stateInfoIndex.RollappId, stateInfoIndex.Index) - if stateInfo.Status != common.Status_PENDING { - panic(fmt.Sprintf("invariant broken: stateInfo is not in pending state: rollapp: %s: status: %s", stateInfoIndex.RollappId, stateInfo.Status)) - } - stateInfo.Finalize() - // update the status of the stateInfo - k.SetStateInfo(ctx, stateInfo) - // update the LatestStateInfoIndex of the rollapp - k.SetLatestFinalizedStateIndex(ctx, stateInfoIndex) - // call the after-update-state hook - keeperHooks := k.GetHooks() - err := keeperHooks.AfterStateFinalized(ctx, stateInfoIndex.RollappId, &stateInfo) - if err != nil { - return fmt.Errorf("after state finalized: %w", err) - } - - // emit event - ctx.EventManager().EmitEvent( - sdk.NewEvent(types.EventTypeStatusChange, - stateInfo.GetEvents()..., - ), - ) + k.finalizeQueueForHeight(ctx, blockHeightToFinalizationQueue, failedRollapps) + } +} + +func (k Keeper) finalizeQueueForHeight(ctx sdk.Context, queue types.BlockHeightToFinalizationQueue, failedRollapps map[string]uint64) { + // finalize pending states + for _, stateInfoIndex := range queue.FinalizationQueue { + if _, failed := failedRollapps[stateInfoIndex.RollappId]; failed { + // if the rollapp has already failed to finalize for at least one state index, + // skip all subsequent state changes for this rollapp + continue } - k.RemoveBlockHeightToFinalizationQueue(ctx, blockHeightToFinalizationQueue.CreationHeight) + if err := k.finalizeStateForIndex(ctx, stateInfoIndex); err != nil { + // record the rollapp that had a failed state change at current height + failedRollapps[stateInfoIndex.RollappId] = stateInfoIndex.Index + } + } + k.updateQueueForHeight(ctx, queue, failedRollapps) +} + +func (k Keeper) finalizeStateForIndex(ctx sdk.Context, stateInfoIndex types.StateInfoIndex) error { + // if this fails, no state change will happen + err := osmoutils.ApplyFuncIfNoError(ctx, func(ctx sdk.Context) error { + return k.finalizePending(ctx, stateInfoIndex) + }) + if err != nil { + // TODO: think about (non)recoverable errors and how to handle them accordingly + k.Logger(ctx).Error( + "failed to finalize state", + "rollapp", stateInfoIndex.RollappId, + "index", stateInfoIndex.Index, + "error", err.Error(), + ) + return fmt.Errorf("finalize state: %w", err) } return nil } +func (k *Keeper) finalizePendingState(ctx sdk.Context, stateInfoIndex types.StateInfoIndex) error { + stateInfo := k.MustGetStateInfo(ctx, stateInfoIndex.RollappId, stateInfoIndex.Index) + if stateInfo.Status != common.Status_PENDING { + panic(fmt.Sprintf("invariant broken: stateInfo is not in pending state: rollapp: %s: status: %s", stateInfoIndex.RollappId, stateInfo.Status)) + } + stateInfo.Finalize() + // update the status of the stateInfo + k.SetStateInfo(ctx, stateInfo) + // update the LatestStateInfoIndex of the rollapp + k.SetLatestFinalizedStateIndex(ctx, stateInfoIndex) + // call the after-update-state hook + keeperHooks := k.GetHooks() + err := keeperHooks.AfterStateFinalized(ctx, stateInfoIndex.RollappId, &stateInfo) + if err != nil { + return fmt.Errorf("after state finalized: %w", err) + } + + // emit event + ctx.EventManager().EmitEvent( + sdk.NewEvent(types.EventTypeStatusChange, + stateInfo.GetEvents()..., + ), + ) + return nil +} + +func (k Keeper) updateQueueForHeight(ctx sdk.Context, blockHeightToFinalizationQueue types.BlockHeightToFinalizationQueue, failedRollapps map[string]uint64) { + // remove the blockHeightToFinalizationQueue if all the rollapps' states are finalized + if len(failedRollapps) == 0 { + k.RemoveBlockHeightToFinalizationQueue(ctx, blockHeightToFinalizationQueue.CreationHeight) + return + } + // remove from the queue only the rollapps that were successfully finalized at all indices. + // while iterating the queue for deleting the successfully finalized states, we remove them if + // - rollapp was not found in the failedRollapps map + // - if it was found, the indexes to be deleted should only be the ones up to the point of the index of the first failed state change + blockHeightToFinalizationQueue.FinalizationQueue = slices.DeleteFunc(blockHeightToFinalizationQueue.FinalizationQueue, + func(si types.StateInfoIndex) bool { + idx, failed := failedRollapps[si.RollappId] + return !failed || si.Index < idx + }) + // save the current queue with "leftover" rollapp's state changes + k.SetBlockHeightToFinalizationQueue(ctx, blockHeightToFinalizationQueue) +} + // SetBlockHeightToFinalizationQueue set a specific blockHeightToFinalizationQueue in the store from its index func (k Keeper) SetBlockHeightToFinalizationQueue(ctx sdk.Context, blockHeightToFinalizationQueue types.BlockHeightToFinalizationQueue) { store := prefix.NewStore(ctx.KVStore(k.storeKey), types.KeyPrefix(types.BlockHeightToFinalizationQueueKeyPrefix)) diff --git a/x/rollapp/keeper/block_height_to_finalization_queue_test.go b/x/rollapp/keeper/block_height_to_finalization_queue_test.go index b25855ed1..c8528ae12 100644 --- a/x/rollapp/keeper/block_height_to_finalization_queue_test.go +++ b/x/rollapp/keeper/block_height_to_finalization_queue_test.go @@ -1,17 +1,19 @@ package keeper_test import ( + "errors" "slices" "strconv" "testing" abci "github.com/cometbft/cometbft/abci/types" sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/stretchr/testify/require" + keepertest "github.com/dymensionxyz/dymension/v3/testutil/keeper" "github.com/dymensionxyz/dymension/v3/testutil/nullify" "github.com/dymensionxyz/dymension/v3/x/rollapp/keeper" "github.com/dymensionxyz/dymension/v3/x/rollapp/types" - "github.com/stretchr/testify/require" ) // Prevent strconv unused error @@ -22,7 +24,7 @@ func (suite *RollappTestSuite) TestGetAllFinalizationQueueUntilHeight() { initialHeight := uint64(10) suite.Ctx = suite.Ctx.WithBlockHeight(int64(initialHeight)) ctx := &suite.Ctx - keeper := suite.App.RollappKeeper + k := suite.App.RollappKeeper rollapp := suite.CreateDefaultRollapp() proposer := suite.CreateDefaultSequencer(*ctx, rollapp) @@ -34,18 +36,18 @@ func (suite *RollappTestSuite) TestGetAllFinalizationQueueUntilHeight() { suite.Require().Nil(err) // Get the pending finalization queue - suite.Len(keeper.GetAllFinalizationQueueUntilHeightInclusive(*ctx, initialHeight-1), 0) - suite.Len(keeper.GetAllFinalizationQueueUntilHeightInclusive(*ctx, initialHeight), 1) - suite.Len(keeper.GetAllFinalizationQueueUntilHeightInclusive(*ctx, initialHeight+1), 2) - suite.Len(keeper.GetAllFinalizationQueueUntilHeightInclusive(*ctx, initialHeight+100), 2) + suite.Len(k.GetAllFinalizationQueueUntilHeightInclusive(*ctx, initialHeight-1), 0) + suite.Len(k.GetAllFinalizationQueueUntilHeightInclusive(*ctx, initialHeight), 1) + suite.Len(k.GetAllFinalizationQueueUntilHeightInclusive(*ctx, initialHeight+1), 2) + suite.Len(k.GetAllFinalizationQueueUntilHeightInclusive(*ctx, initialHeight+100), 2) } func TestBlockHeightToFinalizationQueueGet(t *testing.T) { - keeper, ctx := keepertest.RollappKeeper(t) - items := createNBlockHeightToFinalizationQueue(keeper, ctx, 10) + k, ctx := keepertest.RollappKeeper(t) + items := createNBlockHeightToFinalizationQueue(k, ctx, 10) for _, item := range items { item := item - rst, found := keeper.GetBlockHeightToFinalizationQueue(ctx, + rst, found := k.GetBlockHeightToFinalizationQueue(ctx, item.CreationHeight, ) require.True(t, found) @@ -57,13 +59,13 @@ func TestBlockHeightToFinalizationQueueGet(t *testing.T) { } func TestBlockHeightToFinalizationQueueRemove(t *testing.T) { - keeper, ctx := keepertest.RollappKeeper(t) - items := createNBlockHeightToFinalizationQueue(keeper, ctx, 10) + k, ctx := keepertest.RollappKeeper(t) + items := createNBlockHeightToFinalizationQueue(k, ctx, 10) for _, item := range items { - keeper.RemoveBlockHeightToFinalizationQueue(ctx, + k.RemoveBlockHeightToFinalizationQueue(ctx, item.CreationHeight, ) - _, found := keeper.GetBlockHeightToFinalizationQueue(ctx, + _, found := k.GetBlockHeightToFinalizationQueue(ctx, item.CreationHeight, ) require.False(t, found) @@ -71,14 +73,188 @@ func TestBlockHeightToFinalizationQueueRemove(t *testing.T) { } func TestBlockHeightToFinalizationQueueGetAll(t *testing.T) { - keeper, ctx := keepertest.RollappKeeper(t) - items := createNBlockHeightToFinalizationQueue(keeper, ctx, 10) + k, ctx := keepertest.RollappKeeper(t) + items := createNBlockHeightToFinalizationQueue(k, ctx, 10) require.ElementsMatch(t, nullify.Fill(items), - nullify.Fill(keeper.GetAllBlockHeightToFinalizationQueue(ctx)), + nullify.Fill(k.GetAllBlockHeightToFinalizationQueue(ctx)), ) } +// TODO: Test FinalizeQueue function with failed states +func (suite *RollappTestSuite) TestFinalizeRollapps() { + type stateUpdate struct { + blockHeight int64 + startHeight uint64 + numBlocks uint64 + } + type queue struct { + rollappsLeft int + } + type blockEnd struct { + blockHeight func() int64 + wantNumFinalized int + wantQueue []queue + } + type rollappStateUpdate struct { + stateUpdate stateUpdate + malleatePostStateUpdate func(rollappId string) + } + type fields struct { + rollappStateUpdates []rollappStateUpdate + blockEnd blockEnd + } + + const initialHeight int64 = 10 + + tests := []struct { + name string + fields fields + }{ + { + name: "finalize two rollapps successfully", + fields: fields{ + rollappStateUpdates: []rollappStateUpdate{ + { + stateUpdate: stateUpdate{ + blockHeight: initialHeight, + startHeight: 1, + numBlocks: 10, + }, + }, { + stateUpdate: stateUpdate{ + blockHeight: initialHeight, + startHeight: 1, + numBlocks: 10, + }, + }, + }, + blockEnd: blockEnd{ + blockHeight: func() int64 { return initialHeight + int64(suite.App.RollappKeeper.DisputePeriodInBlocks(suite.Ctx)) }, + wantQueue: nil, + wantNumFinalized: 2, + }, + }, + }, { + name: "finalize two rollapps, one with failed state", + fields: fields{ + rollappStateUpdates: []rollappStateUpdate{ + { + stateUpdate: stateUpdate{ + blockHeight: initialHeight, + startHeight: 1, + numBlocks: 10, + }, + }, { + stateUpdate: stateUpdate{ + blockHeight: initialHeight, + startHeight: 1, + numBlocks: 10, + }, + malleatePostStateUpdate: func(rollappId string) { + suite.App.RollappKeeper.RemoveStateInfo(suite.Ctx, rollappId, 1) + }, + }, + }, + blockEnd: blockEnd{ + blockHeight: func() int64 { return initialHeight + int64(suite.App.RollappKeeper.DisputePeriodInBlocks(suite.Ctx)) }, + wantQueue: []queue{{rollappsLeft: 1}}, + wantNumFinalized: 1, + }, + }, + }, { + name: "finalize five rollapps, two with failed state", + fields: fields{ + rollappStateUpdates: []rollappStateUpdate{ + { + stateUpdate: stateUpdate{ + blockHeight: initialHeight, + startHeight: 1, + numBlocks: 10, + }, + }, { + stateUpdate: stateUpdate{ + blockHeight: initialHeight, + startHeight: 1, + numBlocks: 10, + }, + }, { + stateUpdate: stateUpdate{ + blockHeight: initialHeight, + startHeight: 1, + numBlocks: 10, + }, + }, { + stateUpdate: stateUpdate{ + blockHeight: initialHeight, + startHeight: 1, + numBlocks: 10, + }, + malleatePostStateUpdate: func(rollappId string) { + suite.App.RollappKeeper.RemoveStateInfo(suite.Ctx, rollappId, 1) + }, + }, { + stateUpdate: stateUpdate{ + blockHeight: initialHeight, + startHeight: 1, + numBlocks: 10, + }, + malleatePostStateUpdate: func(rollappId string) { + suite.App.RollappKeeper.RemoveStateInfo(suite.Ctx, rollappId, 1) + }, + }, + }, + blockEnd: blockEnd{ + blockHeight: func() int64 { return initialHeight + int64(suite.App.RollappKeeper.DisputePeriodInBlocks(suite.Ctx)) }, + wantQueue: []queue{ + { + rollappsLeft: 2, + }, + }, + wantNumFinalized: 3, + }, + }, + }, + } + + for _, tt := range tests { + suite.T().Run(tt.name, func(t *testing.T) { + suite.SetupTest() + ctx := &suite.Ctx + + for _, rf := range tt.fields.rollappStateUpdates { + // Create a rollapp + rollapp := suite.CreateDefaultRollapp() + proposer := suite.CreateDefaultSequencer(*ctx, rollapp) + + // Create state update + su := rf.stateUpdate + suite.Ctx = suite.Ctx.WithBlockHeight(su.blockHeight) + _, err := suite.PostStateUpdate(*ctx, rollapp, proposer, su.startHeight, su.numBlocks) + suite.Require().Nil(err) + + if rf.malleatePostStateUpdate != nil { + rf.malleatePostStateUpdate(rollapp) + } + } + // End block and check if finalized + be := tt.fields.blockEnd + suite.Ctx = suite.Ctx.WithBlockHeight(be.blockHeight()) + response := suite.App.EndBlocker(suite.Ctx, abci.RequestEndBlock{Height: suite.Ctx.BlockHeight()}) + + heightQueue := suite.App.RollappKeeper.GetAllBlockHeightToFinalizationQueue(*ctx) + suite.Require().Len(heightQueue, len(be.wantQueue)) + + for i, q := range be.wantQueue { + suite.Require().Len(heightQueue[i].FinalizationQueue, q.rollappsLeft) + } + + numFinalized := countFinalized(response) + suite.Assert().Equal(be.wantNumFinalized, numFinalized) + }) + } +} + // TODO: Test FinalizeQueue function with failed states func (suite *RollappTestSuite) TestFinalize() { suite.SetupTest() @@ -87,7 +263,7 @@ func (suite *RollappTestSuite) TestFinalize() { suite.Ctx = suite.Ctx.WithBlockHeight(int64(initialheight)) ctx := &suite.Ctx - keeper := suite.App.RollappKeeper + k := suite.App.RollappKeeper // Create a rollapp rollapp := suite.CreateDefaultRollapp() @@ -103,19 +279,19 @@ func (suite *RollappTestSuite) TestFinalize() { // Finalize pending queues and check response := suite.App.EndBlocker(suite.Ctx, abci.RequestEndBlock{Height: suite.Ctx.BlockHeight()}) - suite.Require().Len(keeper.GetAllBlockHeightToFinalizationQueue(*ctx), 2) + suite.Require().Len(k.GetAllBlockHeightToFinalizationQueue(*ctx), 2) suite.False(findEvent(response, types.EventTypeStatusChange)) // Finalize pending queues and check - suite.Ctx = suite.Ctx.WithBlockHeight(int64(initialheight + keeper.DisputePeriodInBlocks(*ctx))) + suite.Ctx = suite.Ctx.WithBlockHeight(int64(initialheight + k.DisputePeriodInBlocks(*ctx))) response = suite.App.EndBlocker(suite.Ctx, abci.RequestEndBlock{Height: suite.Ctx.BlockHeight()}) - suite.Require().Len(keeper.GetAllBlockHeightToFinalizationQueue(*ctx), 1) + suite.Require().Len(k.GetAllBlockHeightToFinalizationQueue(*ctx), 1) suite.True(findEvent(response, types.EventTypeStatusChange)) // Finalize pending queues and check - suite.Ctx = suite.Ctx.WithBlockHeight(int64(initialheight + keeper.DisputePeriodInBlocks(*ctx) + 1)) + suite.Ctx = suite.Ctx.WithBlockHeight(int64(initialheight + k.DisputePeriodInBlocks(*ctx) + 1)) response = suite.App.EndBlocker(suite.Ctx, abci.RequestEndBlock{Height: suite.Ctx.BlockHeight()}) - suite.Require().Len(keeper.GetAllBlockHeightToFinalizationQueue(*ctx), 0) + suite.Require().Len(k.GetAllBlockHeightToFinalizationQueue(*ctx), 0) suite.True(findEvent(response, types.EventTypeStatusChange)) } @@ -129,6 +305,227 @@ func createNBlockHeightToFinalizationQueue(keeper *keeper.Keeper, ctx sdk.Contex return items } +func countFinalized(response abci.ResponseEndBlock) int { + count := 0 + for _, event := range response.Events { + if event.Type == types.EventTypeStatusChange { + count++ + } + } + return count +} + func findEvent(response abci.ResponseEndBlock, eventType string) bool { return slices.ContainsFunc(response.Events, func(e abci.Event) bool { return e.Type == eventType }) } + +//nolint:govet +func (suite *RollappTestSuite) TestKeeperFinalizePending() { + tests := []struct { + name string + pendingFinalizationQueue []types.BlockHeightToFinalizationQueue + errFinalizeIndices []types.StateInfoIndex + expectQueueAfter []types.BlockHeightToFinalizationQueue + }{ + { + name: "finalize pending: all rollapps successfully finalized", + pendingFinalizationQueue: []types.BlockHeightToFinalizationQueue{ + { + CreationHeight: 1, + FinalizationQueue: []types.StateInfoIndex{ + {RollappId: "rollapp1", Index: 1}, + {RollappId: "rollapp2", Index: 2}, + {RollappId: "rollapp1", Index: 2}, + {RollappId: "rollapp3", Index: 1}, + {RollappId: "rollapp2", Index: 3}, + {RollappId: "rollapp3", Index: 2}, + }, + }, { + CreationHeight: 2, + FinalizationQueue: []types.StateInfoIndex{ + {RollappId: "rollapp1", Index: 3}, + {RollappId: "rollapp2", Index: 4}, + {RollappId: "rollapp1", Index: 4}, + {RollappId: "rollapp3", Index: 3}, + {RollappId: "rollapp2", Index: 5}, + {RollappId: "rollapp3", Index: 4}, + }, + }, + }, + errFinalizeIndices: []types.StateInfoIndex{}, + expectQueueAfter: nil, + }, { + name: "finalize pending: 2 rollapps failed at 1 height", + pendingFinalizationQueue: []types.BlockHeightToFinalizationQueue{ + { + CreationHeight: 1, + FinalizationQueue: []types.StateInfoIndex{ + {RollappId: "rollapp1", Index: 1}, + {RollappId: "rollapp2", Index: 2}, + {RollappId: "rollapp1", Index: 2}, + {RollappId: "rollapp3", Index: 1}, + {RollappId: "rollapp1", Index: 3}, + {RollappId: "rollapp3", Index: 2}, + }, + }, + }, + errFinalizeIndices: []types.StateInfoIndex{{"rollapp1", 2}, {"rollapp3", 2}}, + expectQueueAfter: []types.BlockHeightToFinalizationQueue{ + { + CreationHeight: 1, + FinalizationQueue: []types.StateInfoIndex{ + {RollappId: "rollapp1", Index: 2}, + {RollappId: "rollapp1", Index: 3}, + {RollappId: "rollapp3", Index: 2}, + }, + }, + }, + }, { + name: "finalize pending: 2 rollapps failed at 2 heights", + pendingFinalizationQueue: []types.BlockHeightToFinalizationQueue{ + { + CreationHeight: 1, + FinalizationQueue: []types.StateInfoIndex{ + {RollappId: "rollapp1", Index: 1}, + {RollappId: "rollapp2", Index: 1}, + }, + }, { + CreationHeight: 2, + FinalizationQueue: []types.StateInfoIndex{ + {RollappId: "rollapp1", Index: 2}, + {RollappId: "rollapp2", Index: 2}, + }, + }, + }, + errFinalizeIndices: []types.StateInfoIndex{{"rollapp1", 1}, {"rollapp2", 2}}, + expectQueueAfter: []types.BlockHeightToFinalizationQueue{ + { + CreationHeight: 1, + FinalizationQueue: []types.StateInfoIndex{ + {RollappId: "rollapp1", Index: 1}, + }, + }, { + CreationHeight: 2, + FinalizationQueue: []types.StateInfoIndex{ + {RollappId: "rollapp1", Index: 2}, + {RollappId: "rollapp2", Index: 2}, + }, + }, + }, + }, { + name: "finalize pending: 2 rollapps failed at 2 heights, one in each height", + pendingFinalizationQueue: []types.BlockHeightToFinalizationQueue{ + { + CreationHeight: 1, + FinalizationQueue: []types.StateInfoIndex{ + {RollappId: "rollapp1", Index: 1}, + {RollappId: "rollapp2", Index: 2}, + {RollappId: "rollapp1", Index: 2}, + {RollappId: "rollapp3", Index: 1}, + {RollappId: "rollapp2", Index: 3}, + {RollappId: "rollapp3", Index: 2}, + }, + }, { + CreationHeight: 2, + FinalizationQueue: []types.StateInfoIndex{ + {RollappId: "rollapp1", Index: 3}, + {RollappId: "rollapp2", Index: 4}, + {RollappId: "rollapp1", Index: 4}, + {RollappId: "rollapp3", Index: 3}, + {RollappId: "rollapp2", Index: 5}, + {RollappId: "rollapp3", Index: 4}, + }, + }, + }, + errFinalizeIndices: []types.StateInfoIndex{{"rollapp1", 2}, {"rollapp3", 4}}, + expectQueueAfter: []types.BlockHeightToFinalizationQueue{ + { + CreationHeight: 1, + FinalizationQueue: []types.StateInfoIndex{ + {RollappId: "rollapp1", Index: 2}, + }, + }, { + CreationHeight: 2, + FinalizationQueue: []types.StateInfoIndex{ + {RollappId: "rollapp1", Index: 3}, + {RollappId: "rollapp1", Index: 4}, + {RollappId: "rollapp3", Index: 4}, + }, + }, + }, + }, { + name: "finalize pending: all rollapps failed to finalize", + pendingFinalizationQueue: []types.BlockHeightToFinalizationQueue{ + { + CreationHeight: 1, + FinalizationQueue: []types.StateInfoIndex{ + {RollappId: "rollapp1", Index: 1}, + {RollappId: "rollapp2", Index: 2}, + {RollappId: "rollapp1", Index: 2}, + {RollappId: "rollapp3", Index: 1}, + {RollappId: "rollapp2", Index: 3}, + {RollappId: "rollapp3", Index: 2}, + }, + }, { + CreationHeight: 2, + FinalizationQueue: []types.StateInfoIndex{ + {RollappId: "rollapp1", Index: 3}, + {RollappId: "rollapp2", Index: 4}, + {RollappId: "rollapp1", Index: 4}, + {RollappId: "rollapp3", Index: 3}, + {RollappId: "rollapp2", Index: 5}, + {RollappId: "rollapp3", Index: 4}, + }, + }, + }, + errFinalizeIndices: []types.StateInfoIndex{ + {RollappId: "rollapp1", Index: 1}, + {RollappId: "rollapp2", Index: 2}, + {RollappId: "rollapp3", Index: 1}, + }, + expectQueueAfter: []types.BlockHeightToFinalizationQueue{ + { + CreationHeight: 1, + FinalizationQueue: []types.StateInfoIndex{ + {RollappId: "rollapp1", Index: 1}, + {RollappId: "rollapp2", Index: 2}, + {RollappId: "rollapp1", Index: 2}, + {RollappId: "rollapp3", Index: 1}, + {RollappId: "rollapp2", Index: 3}, + {RollappId: "rollapp3", Index: 2}, + }, + }, { + CreationHeight: 2, + FinalizationQueue: []types.StateInfoIndex{ + {RollappId: "rollapp1", Index: 3}, + {RollappId: "rollapp2", Index: 4}, + {RollappId: "rollapp1", Index: 4}, + {RollappId: "rollapp3", Index: 3}, + {RollappId: "rollapp2", Index: 5}, + {RollappId: "rollapp3", Index: 4}, + }, + }, + }, + }, + } + for _, tt := range tests { + suite.T().Run(tt.name, func(t *testing.T) { + suite.SetupTest() + + k := suite.App.RollappKeeper + k.SetFinalizePendingFn(MockFinalizePending(tt.errFinalizeIndices)) + k.FinalizeAllPending(suite.Ctx, tt.pendingFinalizationQueue) + + suite.Require().Equal(tt.expectQueueAfter, k.GetAllBlockHeightToFinalizationQueue(suite.Ctx)) + }) + } +} + +func MockFinalizePending(errFinalizedIndices []types.StateInfoIndex) func(ctx sdk.Context, stateInfoIndex types.StateInfoIndex) error { + return func(ctx sdk.Context, stateInfoIndex types.StateInfoIndex) error { + if slices.Contains(errFinalizedIndices, stateInfoIndex) { + return errors.New("error") + } + return nil + } +} diff --git a/x/rollapp/keeper/fraud_handler_test.go b/x/rollapp/keeper/fraud_handler_test.go index 1793236be..320500396 100644 --- a/x/rollapp/keeper/fraud_handler_test.go +++ b/x/rollapp/keeper/fraud_handler_test.go @@ -49,8 +49,7 @@ func (suite *RollappTestSuite) TestHandleFraud() { } // finalize some of the states - err = suite.App.RollappKeeper.FinalizeRollappStates(suite.Ctx.WithBlockHeight(20)) - suite.Require().Nil(err) + suite.App.RollappKeeper.FinalizeRollappStates(suite.Ctx.WithBlockHeight(20)) // assert before fraud submission suite.assertBeforeFraud(rollapp, fraudHeight) @@ -165,8 +164,7 @@ func (suite *RollappTestSuite) TestHandleFraud_AlreadyFinalized() { // finalize state suite.Ctx = suite.Ctx.WithBlockHeight(ctx.BlockHeight() + int64(keeper.DisputePeriodInBlocks(*ctx))) - err = suite.App.RollappKeeper.FinalizeRollappStates(suite.Ctx) - suite.Require().Nil(err) + suite.App.RollappKeeper.FinalizeRollappStates(suite.Ctx) stateInfo, err := suite.App.RollappKeeper.FindStateInfoByHeight(suite.Ctx, rollapp, 1) suite.Require().Nil(err) suite.Require().Equal(common.Status_FINALIZED, stateInfo.Status) diff --git a/x/rollapp/keeper/invariants_test.go b/x/rollapp/keeper/invariants_test.go index d11853637..fe189a13c 100644 --- a/x/rollapp/keeper/invariants_test.go +++ b/x/rollapp/keeper/invariants_test.go @@ -52,12 +52,11 @@ func (suite *RollappTestSuite) TestInvariants() { // progress finalization queue suite.Ctx = suite.Ctx.WithBlockHeight(initialheight + 2) - err := suite.App.RollappKeeper.FinalizeRollappStates(suite.Ctx) - suite.Require().Nil(err) + suite.App.RollappKeeper.FinalizeRollappStates(suite.Ctx) // check invariant - msg, bool := keeper.AllInvariants(suite.App.RollappKeeper)(suite.Ctx) - suite.Require().False(bool, msg) + msg, ok := keeper.AllInvariants(*suite.App.RollappKeeper)(suite.Ctx) + suite.Require().False(ok, msg) } func (suite *RollappTestSuite) TestRollappFinalizedStateInvariant() { @@ -166,7 +165,7 @@ func (suite *RollappTestSuite) TestRollappFinalizedStateInvariant() { Index: tc.latestStateInfoIndex.GetIndex().Index, }) // check invariant - _, isBroken := keeper.RollappFinalizedStateInvariant(suite.App.RollappKeeper)(ctx) + _, isBroken := keeper.RollappFinalizedStateInvariant(*suite.App.RollappKeeper)(ctx) suite.Require().Equal(tc.expectedIsBroken, isBroken) }) } diff --git a/x/rollapp/keeper/keeper.go b/x/rollapp/keeper/keeper.go index d8a291062..7eab7de2a 100644 --- a/x/rollapp/keeper/keeper.go +++ b/x/rollapp/keeper/keeper.go @@ -21,15 +21,23 @@ type Keeper struct { ibcClientKeeper types.IBCClientKeeper channelKeeper types.ChannelKeeper + + finalizePending func(ctx sdk.Context, stateInfoIndex types.StateInfoIndex) error } -func NewKeeper(cdc codec.BinaryCodec, storeKey storetypes.StoreKey, ps paramtypes.Subspace, channelKeeper types.ChannelKeeper, ibcclientKeeper types.IBCClientKeeper) *Keeper { +func NewKeeper( + cdc codec.BinaryCodec, + storeKey storetypes.StoreKey, + ps paramtypes.Subspace, + channelKeeper types.ChannelKeeper, + ibcclientKeeper types.IBCClientKeeper, +) *Keeper { // set KeyTable if it has not already been set if !ps.HasKeyTable() { ps = ps.WithKeyTable(types.ParamKeyTable()) } - return &Keeper{ + k := &Keeper{ cdc: cdc, storeKey: storeKey, paramstore: ps, @@ -37,6 +45,12 @@ func NewKeeper(cdc codec.BinaryCodec, storeKey storetypes.StoreKey, ps paramtype channelKeeper: channelKeeper, ibcClientKeeper: ibcclientKeeper, } + k.SetFinalizePendingFn(k.finalizePendingState) + return k +} + +func (k *Keeper) SetFinalizePendingFn(fn func(ctx sdk.Context, stateInfoIndex types.StateInfoIndex) error) { + k.finalizePending = fn } func (k Keeper) Logger(ctx sdk.Context) log.Logger { diff --git a/x/rollapp/keeper/rollapp_suite_test.go b/x/rollapp/keeper/rollapp_suite_test.go index 858cb3c02..1234cebab 100644 --- a/x/rollapp/keeper/rollapp_suite_test.go +++ b/x/rollapp/keeper/rollapp_suite_test.go @@ -65,7 +65,7 @@ func (suite *RollappTestSuite) SetupTest(deployerWhitelist ...types.DeployerPara queryClient := types.NewQueryClient(queryHelper) suite.App = app - suite.msgServer = keeper.NewMsgServerImpl(app.RollappKeeper) + suite.msgServer = keeper.NewMsgServerImpl(*app.RollappKeeper) suite.Ctx = ctx suite.queryClient = queryClient } diff --git a/x/rollapp/module.go b/x/rollapp/module.go index 582b466e4..db1d53105 100644 --- a/x/rollapp/module.go +++ b/x/rollapp/module.go @@ -5,19 +5,18 @@ import ( "encoding/json" "fmt" - simulationtypes "github.com/dymensionxyz/dymension/v3/simulation/types" - - "github.com/gorilla/mux" - "github.com/grpc-ecosystem/grpc-gateway/runtime" - "github.com/spf13/cobra" - abci "github.com/cometbft/cometbft/abci/types" - "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/codec" cdctypes "github.com/cosmos/cosmos-sdk/codec/types" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/types/module" + "github.com/gorilla/mux" + "github.com/grpc-ecosystem/grpc-gateway/runtime" + "github.com/spf13/cobra" + + simulationtypes "github.com/dymensionxyz/dymension/v3/simulation/types" + "github.com/dymensionxyz/dymension/v3/x/rollapp/client/cli" "github.com/dymensionxyz/dymension/v3/x/rollapp/keeper" "github.com/dymensionxyz/dymension/v3/x/rollapp/types" @@ -168,14 +167,6 @@ func (am AppModule) GetHooks() []types.RollappHooks { // EndBlock executes all ABCI EndBlock logic respective to the capability module. It // returns no validator updates. func (am AppModule) EndBlock(ctx sdk.Context, _ abci.RequestEndBlock) []abci.ValidatorUpdate { - err := am.keeper.FinalizeRollappStates(ctx) - if err != nil { - // we failed finalizing the queue for one or more rollapps. - // we choose not to panic as it's not invariant breaking and the consequences are - // that soft confirmation will need to be relayed upon until this is resolved. - // TODO: Future iteration should finalize per rollapp (i.e not collectively punish) - // but for the first few rollapps we can handle this way. - ctx.Logger().Error("finalize queue", "error", err.Error()) - } + am.keeper.FinalizeRollappStates(ctx) return []abci.ValidatorUpdate{} } diff --git a/x/rollapp/module_simulation.go b/x/rollapp/module_simulation.go index efedc6a80..bc54c1db3 100644 --- a/x/rollapp/module_simulation.go +++ b/x/rollapp/module_simulation.go @@ -8,6 +8,7 @@ import ( "github.com/cosmos/cosmos-sdk/types/module" simtypes "github.com/cosmos/cosmos-sdk/types/simulation" "github.com/cosmos/cosmos-sdk/x/simulation" + "github.com/dymensionxyz/dymension/v3/app/params" "github.com/dymensionxyz/dymension/v3/testutil/sample" rollappsimulation "github.com/dymensionxyz/dymension/v3/x/rollapp/simulation" @@ -32,8 +33,6 @@ const ( opWeightMsgUpdateState = "op_weight_msg_update_state" // TODO: Determine the simulation weight value defaultWeightMsgUpdateState int = 100 - - // this line is used by starport scaffolding # simapp/module/const ) // GenerateGenesisState creates a randomized GenState of the module @@ -44,7 +43,6 @@ func (AppModule) GenerateGenesisState(simState *module.SimulationState) { } rollappGenesis := types.GenesisState{ Params: types.DefaultParams(), - // this line is used by starport scaffolding # simapp/module/genesisState } simState.GenState[types.ModuleName] = simState.Cdc.MustMarshalJSON(&rollappGenesis) } @@ -83,7 +81,5 @@ func (am AppModule) WeightedOperations(simState module.SimulationState) []simtyp rollappsimulation.SimulateMsgUpdateState(am.accountKeeper, am.bankKeeper), )) - // this line is used by starport scaffolding # simapp/module/operation - return operations }