fix(rollapp): change finalization end blocker logic to not leave brok…

…en invariants (#669)

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
 

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

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

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)

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

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

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