fix(delayedack): handle finalization errors per rollapp (dymensionxyz…

…#966)

Co-authored-by: Daniel T <[email protected]>

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

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)

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
 }

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
 	}

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(

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),

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.

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
 	}
 }
 

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,29 +70,29 @@ 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
 		return err
 	}
 
 	logger.Debug("finalized IBC rollapp packet", logContext...)
-	return
+	return nil
 }
 
 func (k Keeper) writeRecvAck(rollappPacket commontypes.RollappPacket, ack exported.Acknowledgement) wrappedFunc {

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 {

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"

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
 }