From 9a27004ceee6d2126eb0b2c4115c980435c4247e Mon Sep 17 00:00:00 2001 From: Omri Date: Mon, 18 Mar 2024 16:02:36 +0100 Subject: [PATCH] fix(rollapp): change finalization end blocker logic to not leave broken invariants (#669) --- CHANGELOG.md | 2 +- x/delayedack/keeper/hooks.go | 2 +- .../block_height_to_finalization_queue.go | 70 ++++------ x/rollapp/keeper/fraud_handler_test.go | 6 +- x/rollapp/keeper/invariants.go | 54 +++++++- x/rollapp/keeper/invariants_test.go | 121 +++++++++++++++++- x/rollapp/module.go | 10 +- 7 files changed, 212 insertions(+), 53 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 058bd529b..ea791aaf0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,7 +37,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ## Unreleased -#### Features +### Features - (evm) [#668](https://github.com/dymensionxyz/dymension/issues/668) Integrate virtual frontier bank contract diff --git a/x/delayedack/keeper/hooks.go b/x/delayedack/keeper/hooks.go index dc43a7a9d..0ee2aa95a 100644 --- a/x/delayedack/keeper/hooks.go +++ b/x/delayedack/keeper/hooks.go @@ -62,7 +62,7 @@ func (e epochHooks) AfterEpochEnd(ctx sdk.Context, epochIdentifier string, epoch if epochIdentifier != e.GetParams(ctx).EpochIdentifier { return nil } - // Get all rollapp packets with status FINALIZED and delete them + // Get all rollapp packets with status != PENDING and delete them finalizedRollappPackets := e.ListRollappPacketsByStatus(ctx, commontypes.Status_FINALIZED, 0) revertedRollappPackets := e.ListRollappPacketsByStatus(ctx, commontypes.Status_REVERTED, 0) toDeletePackats := append(finalizedRollappPackets, revertedRollappPackets...) diff --git a/x/rollapp/keeper/block_height_to_finalization_queue.go b/x/rollapp/keeper/block_height_to_finalization_queue.go index 77ec36e7d..b1f0b6b4c 100644 --- a/x/rollapp/keeper/block_height_to_finalization_queue.go +++ b/x/rollapp/keeper/block_height_to_finalization_queue.go @@ -1,71 +1,57 @@ package keeper import ( + "fmt" + "github.com/cosmos/cosmos-sdk/store/prefix" sdk "github.com/cosmos/cosmos-sdk/types" common "github.com/dymensionxyz/dymension/v3/x/common/types" "github.com/dymensionxyz/dymension/v3/x/rollapp/types" - "github.com/osmosis-labs/osmosis/v15/osmoutils" ) // Called every block to finalize states that their dispute period over. -func (k Keeper) FinalizeQueue(ctx sdk.Context) { +func (k Keeper) FinalizeQueue(ctx sdk.Context) error { if uint64(ctx.BlockHeight()) < k.DisputePeriodInBlocks(ctx) { - return + return nil } // check to see if there are pending states to be finalized finalizationHeight := uint64(ctx.BlockHeight() - int64(k.DisputePeriodInBlocks(ctx))) pendingFinalizationQueue := k.GetAllFinalizationQueueUntilHeight(ctx, finalizationHeight) - + // Iterate over all the pending finalization queue + var err error for _, blockHeightToFinalizationQueue := range pendingFinalizationQueue { - // finalize pending states - var failedToFinalizeQueue []types.StateInfoIndex for _, stateInfoIndex := range blockHeightToFinalizationQueue.FinalizationQueue { stateInfo, found := k.GetStateInfo(ctx, stateInfoIndex.RollappId, stateInfoIndex.Index) if !found || stateInfo.Status != common.Status_PENDING { - ctx.Logger().Error("Missing stateInfo data when trying to finalize or alreay finalized", "rollappID", stateInfoIndex.RollappId, "height", ctx.BlockHeight(), "index", stateInfoIndex.Index) - continue - } - - wrappedFunc := func(ctx sdk.Context) error { - stateInfo.Finalize() - // update the status of the stateInfo - k.SetStateInfo(ctx, stateInfo) - // uppdate 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 { - ctx.Logger().Error("Error after state finalized", "rollappID", stateInfoIndex.RollappId, "error", err.Error()) - return err - } - // emit event - ctx.EventManager().EmitEvent( - sdk.NewEvent(types.EventTypeStateUpdate, - stateInfo.GetEvents()..., - ), - ) - return nil + // Invariant breaking + return fmt.Errorf("failed to find state for finalization: rollappId %s, index %d, found %t, status %s", + stateInfoIndex.RollappId, stateInfoIndex.Index, found, stateInfo.Status) } - err := osmoutils.ApplyFuncIfNoError(ctx, wrappedFunc) + stateInfo.Finalize() + // update the status of the stateInfo + k.SetStateInfo(ctx, stateInfo) + // uppdate 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 { - ctx.Logger().Error("Error finalizing state", "height", blockHeightToFinalizationQueue.CreationHeight, "rollappId", stateInfo.StateInfoIndex.RollappId) - failedToFinalizeQueue = append(failedToFinalizeQueue, stateInfoIndex) + // Failed to call finalization dependent event like ibc packet finalization, invariant breaking. can't proceed + return fmt.Errorf("error calling finalized state finalized: rollappID %s, stateInfo: %+v, error %s", + stateInfoIndex.RollappId, stateInfo, err.Error()) } - + // emit event + // TODO: Create an update state keeper method and move this to be called from there + ctx.EventManager().EmitEvent( + sdk.NewEvent(types.EventTypeStateUpdate, + stateInfo.GetEvents()..., + ), + ) } k.RemoveBlockHeightToFinalizationQueue(ctx, blockHeightToFinalizationQueue.CreationHeight) - if len(failedToFinalizeQueue) > 0 { - newBlockHeightToFinalizationQueue := types.BlockHeightToFinalizationQueue{ - CreationHeight: blockHeightToFinalizationQueue.CreationHeight, - FinalizationQueue: failedToFinalizeQueue, - } - - k.SetBlockHeightToFinalizationQueue(ctx, newBlockHeightToFinalizationQueue) - } } + return nil } // SetBlockHeightToFinalizationQueue set a specific blockHeightToFinalizationQueue in the store from its index diff --git a/x/rollapp/keeper/fraud_handler_test.go b/x/rollapp/keeper/fraud_handler_test.go index 4097957eb..da440860a 100644 --- a/x/rollapp/keeper/fraud_handler_test.go +++ b/x/rollapp/keeper/fraud_handler_test.go @@ -49,7 +49,8 @@ func (suite *RollappTestSuite) TestHandleFraud() { } // finalize some of the states - suite.App.RollappKeeper.FinalizeQueue(suite.Ctx.WithBlockHeight(20)) + err = suite.App.RollappKeeper.FinalizeQueue(suite.Ctx.WithBlockHeight(20)) + suite.Require().Nil(err) // assert before fraud submission suite.assertBeforeFraud(rollapp, fraudHeight) @@ -164,7 +165,8 @@ func (suite *RollappTestSuite) TestHandleFraud_AlreadyFinalized() { // finalize state suite.Ctx = suite.Ctx.WithBlockHeight(ctx.BlockHeight() + int64(keeper.DisputePeriodInBlocks(*ctx))) - suite.App.RollappKeeper.FinalizeQueue(suite.Ctx) + err = suite.App.RollappKeeper.FinalizeQueue(suite.Ctx) + suite.Require().Nil(err) 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.go b/x/rollapp/keeper/invariants.go index b9533e4ef..b13e2f11f 100644 --- a/x/rollapp/keeper/invariants.go +++ b/x/rollapp/keeper/invariants.go @@ -4,6 +4,7 @@ import ( "fmt" sdk "github.com/cosmos/cosmos-sdk/types" + commontypes "github.com/dymensionxyz/dymension/v3/x/common/types" "github.com/dymensionxyz/dymension/v3/x/rollapp/types" ) @@ -13,6 +14,7 @@ func RegisterInvariants(ir sdk.InvariantRegistry, k Keeper) { ir.RegisterRoute(types.ModuleName, "rollapp-count", RollappCountInvariant(k)) ir.RegisterRoute(types.ModuleName, "block-height-to-finalization-queue", BlockHeightToFinalizationQueueInvariant(k)) ir.RegisterRoute(types.ModuleName, "rollapp-by-eip155-key", RollappByEIP155KeyInvariant(k)) + ir.RegisterRoute(types.ModuleName, "rollapp-finalized-state", RollappFinalizedStateInvariant(k)) } // AllInvariants runs all invariants of the X/bank module. @@ -34,10 +36,16 @@ func AllInvariants(k Keeper) sdk.Invariant { if stop { return res, stop } + res, stop = RollappFinalizedStateInvariant(k)(ctx) + if stop { + return res, stop + } return "", false } } +// RollappByEIP155KeyInvariant checks that assuming rollapp id is registered with eip155 chain id +// then it should be retrievable by eip155 key func RollappByEIP155KeyInvariant(k Keeper) sdk.Invariant { return func(ctx sdk.Context) (string, bool) { var ( @@ -74,6 +82,7 @@ func RollappByEIP155KeyInvariant(k Keeper) sdk.Invariant { } } +// BlockHeightToFinalizationQueueInvariant checks that all unfinalized states are in the finalization queue func BlockHeightToFinalizationQueueInvariant(k Keeper) sdk.Invariant { return func(ctx sdk.Context) (string, bool) { var ( @@ -99,8 +108,6 @@ func BlockHeightToFinalizationQueueInvariant(k Keeper) sdk.Invariant { continue } creationHeight := stateInfo.CreationHeight - // finalizationTime := creationHeight + k.DisputePeriodInBlocks(ctx) - val, found := k.GetBlockHeightToFinalizationQueue(ctx, creationHeight) if !found { msg += fmt.Sprintf("finalizationQueue (%d) have no block height\n", creationHeight) @@ -184,6 +191,7 @@ func RollappLatestStateIndexInvariant(k Keeper) sdk.Invariant { if !found { msg += fmt.Sprintf("rollapp (%s) have no latestStateIdx\n", rollapp.RollappId) broken = true + break } latestFinalizedStateIdx, _ := k.GetLatestFinalizedStateIndex(ctx, rollapp.RollappId) @@ -201,3 +209,45 @@ func RollappLatestStateIndexInvariant(k Keeper) sdk.Invariant { ), broken } } + +// RollappFinalizedStateInvariant checks that all the states until latest finalized state are finalized +func RollappFinalizedStateInvariant(k Keeper) sdk.Invariant { + return func(ctx sdk.Context) (string, bool) { + var ( + broken bool + msg string + ) + + rollapps := k.GetAllRollapps(ctx) + for _, rollapp := range rollapps { + // If the rollapp is not started, we don't need to check + if !k.IsRollappStarted(ctx, rollapp.RollappId) { + continue + } + + // If we didn't finalize any state yet, we don't need to check + latestFinalizedStateIdx, found := k.GetLatestFinalizedStateIndex(ctx, rollapp.RollappId) + if !found { + continue + } + + for i := uint64(1); i <= latestFinalizedStateIdx.Index; i++ { + stateInfo, found := k.GetStateInfo(ctx, rollapp.RollappId, i) + if !found { + msg += fmt.Sprintf("rollapp (%s) have no stateInfo at index %d\n", rollapp.RollappId, i) + broken = true + } + + if stateInfo.Status != commontypes.Status_FINALIZED { + msg += fmt.Sprintf("rollapp (%s) have stateInfo at index %d not finalized\n", rollapp.RollappId, i) + broken = true + } + } + } + + return sdk.FormatInvariant( + types.ModuleName, "rollapp-finalized-state", + msg, + ), broken + } +} diff --git a/x/rollapp/keeper/invariants_test.go b/x/rollapp/keeper/invariants_test.go index c4f14c057..383a5ea48 100644 --- a/x/rollapp/keeper/invariants_test.go +++ b/x/rollapp/keeper/invariants_test.go @@ -1,9 +1,10 @@ package keeper_test import ( - "github.com/tendermint/tendermint/libs/rand" - + commontypes "github.com/dymensionxyz/dymension/v3/x/common/types" "github.com/dymensionxyz/dymension/v3/x/rollapp/keeper" + "github.com/dymensionxyz/dymension/v3/x/rollapp/types" + "github.com/tendermint/tendermint/libs/rand" ) func (suite *RollappTestSuite) TestInvariants() { @@ -50,11 +51,123 @@ func (suite *RollappTestSuite) TestInvariants() { } // progress finalization queue - // disputePeriod := int64(suite.App.RollappKeeper.GetParams(ctx).DisputePeriodInBlocks) suite.Ctx = suite.Ctx.WithBlockHeight(initialheight + 2) - suite.App.RollappKeeper.FinalizeQueue(suite.Ctx) + err := suite.App.RollappKeeper.FinalizeQueue(suite.Ctx) + suite.Require().Nil(err) // check invariant msg, bool := keeper.AllInvariants(suite.App.RollappKeeper)(suite.Ctx) suite.Require().False(bool, msg) } + +func (suite *RollappTestSuite) TestRollappFinalizedStateInvariant() { + suite.SetupTest() + ctx := suite.Ctx + rollapp1, rollapp2, rollapp3 := "rollapp1", "rollapp2", "rollapp3" + cases := []struct { + name string + rollappId string + stateInfo *types.StateInfo + latestFinalizedStateInfo types.StateInfo + latestStateInfoIndex types.StateInfo + expectedIsBroken bool + }{ + { + "successful invariant check", + "rollapp1", + &types.StateInfo{ + StateInfoIndex: types.StateInfoIndex{ + RollappId: rollapp1, + Index: 1, + }, + Status: commontypes.Status_FINALIZED, + }, + types.StateInfo{ + StateInfoIndex: types.StateInfoIndex{ + RollappId: rollapp1, + Index: 2, + }, + Status: commontypes.Status_FINALIZED, + }, + types.StateInfo{ + StateInfoIndex: types.StateInfoIndex{ + RollappId: rollapp1, + Index: 3, + }, + Status: commontypes.Status_PENDING, + }, + false, + }, + { + "failed invariant check - state not found", + rollapp2, + nil, + types.StateInfo{ + StateInfoIndex: types.StateInfoIndex{ + RollappId: rollapp2, + Index: 2, + }, + Status: commontypes.Status_FINALIZED, + }, + types.StateInfo{ + StateInfoIndex: types.StateInfoIndex{ + RollappId: rollapp2, + Index: 3, + }, + Status: commontypes.Status_PENDING, + }, + true, + }, + { + "failed invariant check - state not finalized", + rollapp3, + &types.StateInfo{ + StateInfoIndex: types.StateInfoIndex{ + RollappId: rollapp3, + Index: 1, + }, + Status: commontypes.Status_PENDING, + }, + types.StateInfo{ + StateInfoIndex: types.StateInfoIndex{ + RollappId: rollapp3, + Index: 2, + }, + Status: commontypes.Status_FINALIZED, + }, + types.StateInfo{ + StateInfoIndex: types.StateInfoIndex{ + RollappId: rollapp3, + Index: 3, + }, + Status: commontypes.Status_PENDING, + }, + true, + }, + } + for _, tc := range cases { + suite.Run(tc.name, func() { + // create rollapp + suite.CreateRollappWithName(tc.rollappId) + // update state infos + if tc.stateInfo != nil { + suite.App.RollappKeeper.SetStateInfo(ctx, *tc.stateInfo) + } + // update latest finalized state info + suite.App.RollappKeeper.SetStateInfo(ctx, tc.latestFinalizedStateInfo) + suite.App.RollappKeeper.SetLatestFinalizedStateIndex(ctx, types.StateInfoIndex{ + RollappId: tc.rollappId, + Index: tc.latestFinalizedStateInfo.GetIndex().Index, + }) + // update latest state info index + suite.App.RollappKeeper.SetStateInfo(ctx, tc.latestStateInfoIndex) + suite.App.RollappKeeper.SetLatestStateInfoIndex(ctx, types.StateInfoIndex{ + RollappId: tc.rollappId, + Index: tc.latestStateInfoIndex.GetIndex().Index, + }) + // check invariant + _, isBroken := keeper.RollappFinalizedStateInvariant(suite.App.RollappKeeper)(ctx) + suite.Require().Equal(tc.expectedIsBroken, isBroken) + }) + } +} diff --git a/x/rollapp/module.go b/x/rollapp/module.go index 43c3aa6bc..2f6a309b9 100644 --- a/x/rollapp/module.go +++ b/x/rollapp/module.go @@ -175,6 +175,14 @@ 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 { - am.keeper.FinalizeQueue(ctx) + err := am.keeper.FinalizeQueue(ctx) + // we failed finalizing the queue for one or more rollapps. + // we choose not to panic as it's not invariant breaking and the consequnces 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. + if err != nil { + ctx.Logger().Error("error finalizing queue", "error", err.Error()) + } return []abci.ValidatorUpdate{} }