Merge remote-tracking branch 'origin/main' into zale144/942-add-missi…

…ng-genesis-validation

CHANGELOG.md

@@ -88,6 +88,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 ### Bug Fixes
 
 - (eibc,delayedack) [#942](https://github.com/dymensionxyz/dymension/issues/942) Add missing genesis validation
+- (rollapp) [#317](https://github.com/dymensionxyz/research/issues/317) Prevent overflow on rollapp state update
 - (code standards) [#932](https://github.com/dymensionxyz/dymension/issues/932) Dry out existing middlewares to make use of new .GetValidTransfer* functions which take care of parsing and validating the fungible packet, and querying and validating any associated rollapp and finalizations
 - (code standards) [#932](https://github.com/dymensionxyz/dymension/issues/932) Removes the obsolete ValidateRollappId func and sub routines
 - (code standards) [#932](https://github.com/dymensionxyz/dymension/issues/932) Simplify GetAllBlockHeightToFinalizationQueue

app/ante/handlers.go

@@ -4,7 +4,7 @@ import (
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	ante "github.com/cosmos/cosmos-sdk/x/auth/ante"
 	ibcante "github.com/cosmos/ibc-go/v6/modules/core/ante"
-	"github.com/dymensionxyz/dymension/v3/x/rollapp/transfersenabled"
+	"github.com/dymensionxyz/dymension/v3/x/rollapp/transfergenesis"
 	ethante "github.com/evmos/ethermint/app/ante"
 	txfeesante "github.com/osmosis-labs/osmosis/v15/x/txfees/ante"
 
@@ -57,7 +57,7 @@ func newLegacyCosmosAnteHandlerEip712(options HandlerOptions) sdk.AnteHandler {
 		ante.NewValidateBasicDecorator(),
 		ante.NewTxTimeoutHeightDecorator(),
 
-		// Use Mempool Fee Decorator from our txfees module instead of default one from auth
+		// Use Mempool Fee TransferEnabledDecorator from our txfees module instead of default one from auth
 		mempoolFeeDecorator,
 		deductFeeDecorator,
 
@@ -74,7 +74,7 @@ func newLegacyCosmosAnteHandlerEip712(options HandlerOptions) sdk.AnteHandler {
 		ibcante.NewRedundantRelayDecorator(options.IBCKeeper),
 		ethante.NewGasWantedDecorator(options.EvmKeeper, options.FeeMarketKeeper),
 
-		transfersenabled.NewDecorator(options.RollappKeeper.GetRollapp, options.IBCKeeper.ChannelKeeper.GetChannelClientState),
+		transfergenesis.NewTransferEnabledDecorator(options.RollappKeeper.GetRollapp, options.IBCKeeper.ChannelKeeper.GetChannelClientState),
 	)
 }
 
@@ -94,7 +94,7 @@ func newCosmosAnteHandler(options HandlerOptions) sdk.AnteHandler {
 		),
 		ante.NewSetUpContextDecorator(),
 		ante.NewExtensionOptionsDecorator(options.ExtensionOptionChecker),
-		// Use Mempool Fee Decorator from our txfees module instead of default one from auth
+		// Use Mempool Fee TransferEnabledDecorator from our txfees module instead of default one from auth
 		mempoolFeeDecorator,
 		deductFeeDecorator,
 		ante.NewValidateBasicDecorator(),
@@ -110,6 +110,6 @@ func newCosmosAnteHandler(options HandlerOptions) sdk.AnteHandler {
 		ibcante.NewRedundantRelayDecorator(options.IBCKeeper),
 		ethante.NewGasWantedDecorator(options.EvmKeeper, options.FeeMarketKeeper),
 
-		transfersenabled.NewDecorator(options.RollappKeeper.GetRollapp, options.IBCKeeper.ChannelKeeper.GetChannelClientState),
+		transfergenesis.NewTransferEnabledDecorator(options.RollappKeeper.GetRollapp, options.IBCKeeper.ChannelKeeper.GetChannelClientState),
 	)
 }

app/app.go

@@ -9,8 +9,6 @@ import (
 	"os"
 	"path/filepath"
 
-	"github.com/dymensionxyz/dymension/v3/x/rollapp/transfersenabled"
-
 	"github.com/dymensionxyz/dymension/v3/x/rollapp/transfergenesis"
 
 	"github.com/dymensionxyz/dymension/v3/x/bridgingfee"
@@ -742,7 +740,6 @@ func New(
 	delayedAckMiddleware := delayedackmodule.NewIBCMiddleware(transferStack, app.DelayedAckKeeper, app.RollappKeeper)
 	transferStack = delayedAckMiddleware
 	transferStack = transferinject.NewIBCModule(transferStack, app.RollappKeeper)
-	transferStack = transfersenabled.NewIBCModule(transferStack, app.RollappKeeper, app.DelayedAckKeeper)
 	transferStack = transfergenesis.NewIBCModule(transferStack, app.DelayedAckKeeper, app.RollappKeeper, app.TransferKeeper, app.DenomMetadataKeeper)
 	transferStack = transfergenesis.NewIBCModuleCanonicalChannelHack(transferStack, app.RollappKeeper, app.IBCKeeper.ChannelKeeper.GetChannelClientState)
 

ibctesting/genesis_transfer_test.go

@@ -4,8 +4,9 @@ import (
 	"testing"
 
 	"cosmossdk.io/math"
-
 	banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
+	channeltypes "github.com/cosmos/ibc-go/v6/modules/core/04-channel/types"
+	"github.com/dymensionxyz/dymension/v3/x/rollapp/transfergenesis"
 	rollapptypes "github.com/dymensionxyz/dymension/v3/x/rollapp/types"
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
@@ -55,10 +56,10 @@ func (s *transferGenesisSuite) TestHappyPath() {
 
 	denoms := []string{"foo", "bar", "baz"}
 
-	for i, denom := range denoms {
+	for _, denom := range denoms {
 		/* ------------------- move non-registered token from rollapp ------------------- */
 
-		msg := s.transferMsg(amt, denom, i, len(denoms))
+		msg := s.transferMsg(amt, denom, true)
 		apptesting.FundAccount(s.rollappApp(), s.rollappCtx(), s.rollappChain().SenderAccount.GetAddress(), sdk.Coins{msg.Token})
 		res, err := s.rollappChain().SendMsgs(msg)
 		s.Require().NoError(err)
@@ -67,9 +68,9 @@ func (s *transferGenesisSuite) TestHappyPath() {
 
 		err = s.path.RelayPacket(packet)
 		s.Require().NoError(err)
-		// after the last one, it should be OK
+
 		transfersEnabled := s.hubApp().RollappKeeper.MustGetRollapp(s.hubCtx(), rollappChainID()).GenesisState.TransfersEnabled
-		s.Require().Equal(i == len(denoms)-1, transfersEnabled, "transfers enabled check", "i", i)
+		s.Require().False(transfersEnabled, "transfers enabled check")
 	}
 
 	for _, denom := range denoms {
@@ -84,7 +85,36 @@ func (s *transferGenesisSuite) TestHappyPath() {
 	}
 }
 
-func (s *transferGenesisSuite) transferMsg(amt math.Int, denom string, i, nDenomsTotal int) *types.MsgTransfer {
+// In the fault path, a chain tries to do another genesis transfer (to skip eibc) after the genesis phase
+// is already complete. It triggers a fraud.
+func (s *transferGenesisSuite) TestCannotDoGenesisTransferAfterBridgeEnabled() {
+	amt := math.NewIntFromUint64(10000000000000000000)
+
+	denoms := []string{"foo", "bar", "baz"}
+
+	for i, denom := range denoms {
+		/* ------------------- move non-registered token from rollapp ------------------- */
+
+		genesis := i%2 == 0 // genesis then regular then genesis again, last one should fail
+		msg := s.transferMsg(amt, denom, genesis)
+		apptesting.FundAccount(s.rollappApp(), s.rollappCtx(), s.rollappChain().SenderAccount.GetAddress(), sdk.Coins{msg.Token})
+		res, err := s.rollappChain().SendMsgs(msg)
+		s.Require().NoError(err)
+		packet, err := ibctesting.ParsePacketFromEvents(res.GetEvents())
+		s.Require().NoError(err)
+
+		_ = s.path.RelayPacket(packet)
+
+		if i == 2 {
+
+			expect := channeltypes.NewErrorAcknowledgement(transfergenesis.ErrDisabled)
+			bz, _ := s.hubApp().IBCKeeper.ChannelKeeper.GetPacketAcknowledgement(s.hubCtx(), packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence())
+			s.Require().Equal(channeltypes.CommitAcknowledgement(expect.Acknowledgement()), bz)
+		}
+	}
+}
+
+func (s *transferGenesisSuite) transferMsg(amt math.Int, denom string, isGenesis bool) *types.MsgTransfer {
 	meta := banktypes.Metadata{
 		Description: "",
 		DenomUnits: []*banktypes.DenomUnit{
@@ -104,10 +134,7 @@ func (s *transferGenesisSuite) transferMsg(amt math.Int, denom string, i, nDenom
 		URIHash: "",
 	}
 	s.Require().NoError(meta.Validate()) // sanity check the test is written correctly
-	memo := rollapptypes.GenesisTransferMemo{
-		Denom:             meta,
-		TotalNumTransfers: uint64(nDenomsTotal),
-	}.Namespaced().MustString()
+
 	tokens := sdk.NewCoin(meta.Base, amt)
 
 	timeoutHeight := clienttypes.NewHeight(100, 110)
@@ -120,8 +147,14 @@ func (s *transferGenesisSuite) transferMsg(amt math.Int, denom string, i, nDenom
 		s.hubChain().SenderAccount.GetAddress().String(),
 		timeoutHeight,
 		0,
-		memo,
+		"",
 	)
 
+	if isGenesis {
+		msg.Memo = rollapptypes.GenesisTransferMemo{
+			Denom: meta,
+		}.Namespaced().MustString()
+	}
+
 	return msg
 }

ibctesting/transfers_enabled_test.go

@@ -37,52 +37,6 @@ func (s *transfersEnabledSuite) SetupTest() {
 	s.Require().False(s.hubApp().RollappKeeper.MustGetRollapp(s.hubCtx(), rollappChainID()).GenesisState.TransfersEnabled)
 }
 
-// Regular (non genesis) transfers RA->Hub (and Hub->RA) should both be blocked when the bridge is not open
-// Note: we don't explicitly test the 'enabled' case, since this is tested in other tests in this package
-func (s *transfersEnabledSuite) TestRollappToHubDisabled() {
-	amt := math.NewIntFromUint64(10000000000000000000)
-	denom := "foo"
-	tokens := sdk.NewCoin(denom, amt)
-
-	timeoutHeight := clienttypes.NewHeight(100, 110)
-
-	msg := types.NewMsgTransfer(
-		s.path.EndpointB.ChannelConfig.PortID,
-		s.path.EndpointB.ChannelID,
-		tokens,
-		s.rollappChain().SenderAccount.GetAddress().String(),
-		s.hubChain().SenderAccount.GetAddress().String(),
-		timeoutHeight,
-		0,
-		"",
-	)
-
-	receiverBalance := s.hubApp().BankKeeper.GetBalance(s.hubCtx(), s.hubChain().SenderAccount.GetAddress(), denom)
-	s.Require().True(receiverBalance.Amount.IsZero())
-
-	apptesting.FundAccount(s.rollappApp(), s.rollappCtx(), s.rollappChain().SenderAccount.GetAddress(), sdk.Coins{msg.Token})
-	res, err := s.rollappChain().SendMsgs(msg)
-	s.Require().NoError(err)
-	packet, err := ibctesting.ParsePacketFromEvents(res.GetEvents())
-	s.Require().NoError(err)
-
-	// money is escrowed
-	senderBalance := s.rollappApp().BankKeeper.GetBalance(s.rollappCtx(), s.rollappChain().SenderAccount.GetAddress(), denom)
-	s.Require().True(senderBalance.Amount.IsZero())
-
-	// fails and returns err ack
-	err = s.path.RelayPacket(packet)
-	s.Require().NoError(err)
-
-	// money is refunded
-	senderBalance = s.rollappApp().BankKeeper.GetBalance(s.rollappCtx(), s.rollappChain().SenderAccount.GetAddress(), denom)
-	s.Require().True(senderBalance.Amount.IsPositive())
-
-	// no double spend
-	receiverBalance = s.hubApp().BankKeeper.GetBalance(s.hubCtx(), s.hubChain().SenderAccount.GetAddress(), denom)
-	s.Require().True(receiverBalance.Amount.IsZero())
-}
-
 // Regular (non genesis) transfers (RA->Hub) and Hub->RA should both be blocked when the bridge is not open
 func (s *transfersEnabledSuite) TestHubToRollappDisabled() {
 	amt := math.NewIntFromUint64(10000000000000000000)

ibctesting/utils_test.go

@@ -116,20 +116,14 @@ func (s *utilSuite) createRollappWithFinishedGenesis(canonicalChannelID string)
 }
 
 func (s *utilSuite) createRollapp(transfersEnabled bool, channelID *string) {
-	msgCreateRollapp := rollapptypes.NewMsgCreateRollapp(
-		s.hubChain().SenderAccount.GetAddress().String(),
-		rollappChainID(),
-		10,
-		[]string{},
-
-		transfersEnabled,
-	)
+	msgCreateRollapp := rollapptypes.NewMsgCreateRollapp(s.hubChain().SenderAccount.GetAddress().String(), rollappChainID(), 10, []string{})
 	_, err := s.hubChain().SendMsgs(msgCreateRollapp)
 	s.Require().NoError(err) // message committed
 	if channelID != nil {
 		a := s.hubApp()
 		ra := a.RollappKeeper.MustGetRollapp(s.hubCtx(), rollappChainID())
 		ra.ChannelId = *channelID
+		ra.GenesisState.TransfersEnabled = transfersEnabled
 		a.RollappKeeper.SetRollapp(s.hubCtx(), ra)
 	}
 }

proto/dymension/rollapp/genesis.proto

@@ -18,6 +18,5 @@ message GenesisState {
   repeated StateInfoIndex latestStateInfoIndexList = 4 [(gogoproto.nullable) = false];
   repeated StateInfoIndex latestFinalizedStateIndexList = 5 [(gogoproto.nullable) = false];
   repeated BlockHeightToFinalizationQueue blockHeightToFinalizationQueueList = 6 [(gogoproto.nullable) = false];
-  repeated GenesisTransfers genesisTransfers = 7 [(gogoproto.nullable) = false];
   // this line is used by starport scaffolding # genesis/proto/state
 }

proto/dymension/rollapp/tx.proto

@@ -30,9 +30,7 @@ message MsgCreateRollapp {
   repeated string permissionedAddresses = 7;
   reserved 8;
   reserved 9;
-  // enable ibc transfers initially? Must be set false if intending
-  // to send genesis transfers.
-  bool transfersEnabled = 10;
+  reserved 10;
 }
 
 message MsgCreateRollappResponse {

x/rollapp/client/cli/flags.go

@@ -5,15 +5,13 @@ import (
 )
 
 const (
-	// Create rollapp flags
+// Create rollapp flags
 
-	FlagGenesisTransfersEnabled = "transfers-enabled"
 )
 
 // FlagSetCreateRollapp returns flags for creating gauges.
 func FlagSetCreateRollapp() *flag.FlagSet {
 	fs := flag.NewFlagSet("", flag.ContinueOnError)
 
-	fs.Bool(FlagGenesisTransfersEnabled, false, "Enable ibc transfers immediately. Must be false if using genesis transfers (genesis accounts).")
 	return fs
 }

x/rollapp/client/cli/tx_create_rollapp.go

@@ -43,12 +43,7 @@ func CmdCreateRollapp() *cobra.Command {
 				return err
 			}
 
-			transfersEnabled, err := cmd.Flags().GetBool(FlagGenesisTransfersEnabled)
-			if err != nil {
-				return err
-			}
-
-			msg := types.NewMsgCreateRollapp(clientCtx.GetFromAddress().String(), argRollappId, argMaxSequencers, argPermissionedAddresses.Addresses, transfersEnabled)
+			msg := types.NewMsgCreateRollapp(clientCtx.GetFromAddress().String(), argRollappId, argMaxSequencers, argPermissionedAddresses.Addresses)
 
 			return tx.GenerateOrBroadcastTxCLI(clientCtx, cmd.Flags(), msg)
 		},

x/rollapp/genesis.go

@@ -30,7 +30,6 @@ func InitGenesis(ctx sdk.Context, k keeper.Keeper, genState types.GenesisState)
 		k.SetBlockHeightToFinalizationQueue(ctx, elem)
 	}
 	k.SetParams(ctx, genState.Params)
-	k.SetGenesisTransfers(ctx, genState.GenesisTransfers)
 	// this line is used by starport scaffolding # genesis/module/init
 }
 
@@ -44,7 +43,6 @@ func ExportGenesis(ctx sdk.Context, k keeper.Keeper) *types.GenesisState {
 	genesis.LatestStateInfoIndexList = k.GetAllLatestStateInfoIndex(ctx)
 	genesis.LatestFinalizedStateIndexList = k.GetAllLatestFinalizedStateIndex(ctx)
 	genesis.BlockHeightToFinalizationQueueList = k.GetAllBlockHeightToFinalizationQueue(ctx)
-	genesis.GenesisTransfers = k.GetAllGenesisTransfers(ctx)
 
 	return genesis
 }

x/rollapp/genesis_test.go

@@ -1,11 +1,8 @@
 package rollapp_test
 
 import (
-	"strings"
 	"testing"
 
-	"golang.org/x/exp/slices"
-
 	keepertest "github.com/dymensionxyz/dymension/v3/testutil/keeper"
 	"github.com/dymensionxyz/dymension/v3/testutil/nullify"
 	"github.com/dymensionxyz/dymension/v3/x/rollapp"
@@ -55,18 +52,6 @@ func TestInitExportGenesis(t *testing.T) {
 				CreationHeight: 1,
 			},
 		},
-		GenesisTransfers: []types.GenesisTransfers{
-			{
-				RollappID:   "0",
-				NumTotal:    3,
-				NumReceived: 3,
-			},
-			{
-				RollappID:   "1",
-				NumTotal:    3,
-				NumReceived: 3,
-			},
-		},
 		// this line is used by starport scaffolding # genesis/test/state
 	}
 
@@ -78,43 +63,9 @@ func TestInitExportGenesis(t *testing.T) {
 	nullify.Fill(genesisState)
 	nullify.Fill(*got)
 
-	require.True(t, GenesisTransfersAreEquivalent(genesisState.GetGenesisTransfers(), got.GetGenesisTransfers()))
-	require.ElementsMatch(t, genesisState.GenesisTransfers, got.GenesisTransfers)
 	require.ElementsMatch(t, genesisState.RollappList, got.RollappList)
 	require.ElementsMatch(t, genesisState.StateInfoList, got.StateInfoList)
 	require.ElementsMatch(t, genesisState.LatestStateInfoIndexList, got.LatestStateInfoIndexList)
 	require.ElementsMatch(t, genesisState.BlockHeightToFinalizationQueueList, got.BlockHeightToFinalizationQueueList)
 	// this line is used by starport scaffolding # genesis/test/assert
 }
-
-// GenesisTransfersAreEquivalent returns if a,b are the same, in terms of containing
-// the same semantic content. Intended for use in tests.
-func GenesisTransfersAreEquivalent(x, y []types.GenesisTransfers) bool {
-	if len(x) != len(y) {
-		return false
-	}
-	sort := func(l []types.GenesisTransfers) {
-		slices.SortStableFunc(l, func(a, b types.GenesisTransfers) bool {
-			return strings.Compare(a.GetRollappID(), b.GetRollappID()) <= 0
-		})
-	}
-
-	sort(x)
-	sort(y)
-	for i := range len(x) {
-		a := x[i]
-		b := y[i]
-		if a.NumTotal != b.NumTotal {
-			return false
-		}
-		if a.NumReceived != b.NumReceived {
-			return false
-		}
-		if a.RollappID != b.RollappID {
-			return false
-		}
-
-	}
-
-	return true
-}