fix(rollapp): properly disambiguate empty/nil genesis accounts (#1560)

ibctesting/genesis_bridge_test.go

@@ -399,21 +399,17 @@ func (s *transferGenesisSuite) genesisBridgePacket(raGenesisInfo rollapptypes.Ge
 	gb.NativeDenom = meta
 
 	// add genesis transfer if needed
-	if raGenesisInfo.GenesisAccounts != nil {
-		total := math.ZeroInt()
-		for _, acc := range raGenesisInfo.GenesisAccounts.Accounts {
-			total = total.Add(acc.Amount)
-		}
+	if raGenesisInfo.RequiresTransfer() {
 
 		gTransfer := transfertypes.NewFungibleTokenPacketData(
 			denom,
-			total.String(),
+			raGenesisInfo.GenesisTransferAmount().String(),
 			s.rollappChain().SenderAccount.GetAddress().String(),
 			genesisbridge.HubRecipient,
 			"",
 		)
 		gb.GenesisTransfer = &gTransfer
-		gb.GenesisInfo.GenesisAccounts = raGenesisInfo.GenesisAccounts.Accounts
+		gb.GenesisInfo.GenesisAccounts = raGenesisInfo.Accounts()
 	}
 
 	bz, err := json.Marshal(gb)

ibctesting/utils_test.go

@@ -174,7 +174,7 @@ func (s *transferGenesisSuite) addGenesisAccounts(genesisAccounts []rollapptypes
 	if rollapp.GenesisInfo.GenesisAccounts == nil {
 		rollapp.GenesisInfo.GenesisAccounts = &rollapptypes.GenesisAccounts{}
 	}
-	rollapp.GenesisInfo.GenesisAccounts.Accounts = append(rollapp.GenesisInfo.GenesisAccounts.Accounts, genesisAccounts...)
+	rollapp.GenesisInfo.GenesisAccounts.Accounts = append(rollapp.GenesisInfo.Accounts(), genesisAccounts...)
 	s.hubApp().RollappKeeper.SetRollapp(s.hubCtx(), rollapp)
 }
 

x/iro/keeper/create_plan.go

@@ -65,12 +65,8 @@ func (m msgServer) CreatePlan(goCtx context.Context, req *types.MsgCreatePlan) (
 		return nil, errors.Join(gerrc.ErrFailedPrecondition, types.ErrPlanExists)
 	}
 
-	// validate the IRO funding genesis transfer is expected in rollapp's genesis info
-	if rollapp.GenesisInfo.GenesisAccounts == nil {
-		return nil, errorsmod.Wrap(gerrc.ErrFailedPrecondition, "genesis accounts not found")
-	}
 	found = false
-	for _, gAcc := range rollapp.GenesisInfo.GenesisAccounts.Accounts {
+	for _, gAcc := range rollapp.GenesisInfo.Accounts() {
 		if gAcc.Address == m.Keeper.GetModuleAccountAddress() {
 			if !gAcc.Amount.Equal(req.AllocatedAmount) {
 				return nil, errorsmod.Wrap(gerrc.ErrFailedPrecondition, "allocated amount mismatch")
@@ -80,7 +76,7 @@ func (m msgServer) CreatePlan(goCtx context.Context, req *types.MsgCreatePlan) (
 		}
 	}
 	if !found {
-		return nil, errorsmod.Wrap(gerrc.ErrFailedPrecondition, "genesis transfer not found for IRO")
+		return nil, errorsmod.Wrap(gerrc.ErrFailedPrecondition, "no genesis account for iro module account")
 	}
 
 	planId, err := m.Keeper.CreatePlan(ctx, req.AllocatedAmount, startTime, preLaunchTime, rollapp, req.BondingCurve, req.IncentivePlanParams)

x/rollapp/genesisbridge/genesis_bridge_data.go

@@ -56,19 +56,15 @@ func (data GenesisBridgeData) ValidateBasic() error {
 	return nil
 }
 
-// ValidateBasic performs basic validation checks on the GenesisInfo.
+// converts to a native type and validates that
 func (info GenesisBridgeInfo) ValidateBasic() error {
 	// wrap the genesis info in a GenesisInfo struct, to reuse the validation logic
 	raGenesisInfo := types.GenesisInfo{
 		GenesisChecksum: info.GenesisChecksum,
 		Bech32Prefix:    info.Bech32Prefix,
 		NativeDenom:     info.NativeDenom,
 		InitialSupply:   info.InitialSupply,
-	}
-	if len(info.GenesisAccounts) > 0 {
-		raGenesisInfo.GenesisAccounts = &types.GenesisAccounts{
-			Accounts: info.GenesisAccounts,
-		}
+		GenesisAccounts: &types.GenesisAccounts{Accounts: info.GenesisAccounts},
 	}
 
 	if !raGenesisInfo.AllSet() {

x/rollapp/genesisbridge/genesis_transfer.go

@@ -18,14 +18,10 @@ const (
 // HandleGenesisTransfer handles the genesis transfer packet, if present, and expected.
 // We assume that genesis info is already validated, and the genesis transfer is expected to fulfill the genesis info.
 func (w IBCModule) handleGenesisTransfer(ctx sdk.Context, ra types.Rollapp, packet channeltypes.Packet, gTransfer *transfertypes.FungibleTokenPacketData) error {
-	// check if required or expected
-	required := ra.GenesisInfo.GenesisAccounts != nil
-	// required but not present
-	if required && gTransfer == nil {
+	if ra.GenesisInfo.RequiresTransfer() && gTransfer == nil {
 		return errorsmod.Wrap(gerrc.ErrFailedPrecondition, "genesis transfer required")
 	}
-	// not required but present
-	if !required && gTransfer != nil {
+	if !ra.GenesisInfo.RequiresTransfer() && gTransfer != nil {
 		return errorsmod.Wrap(gerrc.ErrFailedPrecondition, "genesis transfer not expected")
 	}
 	if gTransfer == nil {
@@ -44,7 +40,7 @@ func (w IBCModule) handleGenesisTransfer(ctx sdk.Context, ra types.Rollapp, pack
 	}
 
 	// split the transfer to the genesis accounts
-	for _, acc := range ra.GenesisInfo.GenesisAccounts.Accounts {
+	for _, acc := range ra.GenesisInfo.Accounts() {
 		// create a new packet for each account
 		data := transfertypes.NewFungibleTokenPacketData(
 			gTransfer.Denom,

x/rollapp/genesisbridge/ibc_module.go

@@ -170,27 +170,20 @@ func (w IBCModule) ValidateGenesisBridge(ra *types.Rollapp, data GenesisBridgeIn
 		return fmt.Errorf("initial supply mismatch: expected: %v, got: %v", raInfo.InitialSupply, data.InitialSupply)
 	}
 
-	err := compareGenesisAccounts(raInfo.GenesisAccounts, data.GenesisAccounts)
+	err := compareGenesisAccounts(raInfo.Accounts(), data.GenesisAccounts)
 	if err != nil {
 		return errorsmod.Wrap(err, "genesis accounts mismatch")
 	}
 
 	return nil
 }
 
-func compareGenesisAccounts(raCommitted *types.GenesisAccounts, gbData []types.GenesisAccount) error {
-	if raCommitted == nil {
-		if len(gbData) == 0 {
-			return nil
-		}
-		return fmt.Errorf("genesis accounts length mismatch: expected 0, got %d", len(gbData))
-	}
-
-	if len(raCommitted.Accounts) != len(gbData) {
-		return fmt.Errorf("genesis accounts length mismatch: expected %d, got %d", len(raCommitted.Accounts), len(gbData))
+func compareGenesisAccounts(raCommitted []types.GenesisAccount, gbData []types.GenesisAccount) error {
+	if len(raCommitted) != len(gbData) {
+		return fmt.Errorf("genesis accounts length mismatch: expected %d, got %d", len(raCommitted), len(gbData))
 	}
 
-	for _, acc := range raCommitted.Accounts {
+	for _, acc := range raCommitted {
 		found := slices.ContainsFunc(gbData, func(dataAcc types.GenesisAccount) bool {
 			return dataAcc.Address == acc.Address && dataAcc.Amount.Equal(acc.Amount)
 		})

x/rollapp/keeper/msg_server_update_rollapp_test.go

@@ -41,6 +41,7 @@ func (s *RollappTestSuite) TestUpdateRollapp() {
 						Base:     "aden",
 						Exponent: 18,
 					},
+					GenesisAccounts: &types.GenesisAccounts{}, // Frontend must specify empty type
 				},
 			},
 			expError: nil,
@@ -269,6 +270,69 @@ func (s *RollappTestSuite) TestUpdateRollapp() {
 	}
 }
 
+// Update rollapp: fail - try to update genesis checksum when sealed
+func (s *RollappTestSuite) TestUpdateRollappRegression() {
+	const (
+		rollappId               = "rollapp_1234-1"
+		initialSequencerAddress = "dym10l6edrf9gjv02um5kp7cmy4zgd26tafz6eqajz"
+	)
+
+	goCtx := sdk.WrapSDKContext(s.Ctx)
+	rollapp := types.Rollapp{
+		RollappId:        rollappId,
+		Owner:            alice,
+		InitialSequencer: "",
+		ChannelId:        "",
+		Launched:         true,
+		VmType:           types.Rollapp_EVM,
+		Metadata: &types.RollappMetadata{
+			Website:     "",
+			Description: "",
+			LogoUrl:     "",
+			Telegram:    "",
+			X:           "",
+		},
+		GenesisInfo: types.GenesisInfo{
+			GenesisChecksum: "old",
+			Bech32Prefix:    "old",
+			NativeDenom: types.DenomMetadata{
+				Display:  "OLD",
+				Base:     "aold",
+				Exponent: 18,
+			},
+			InitialSupply: sdk.NewInt(1000),
+			Sealed:        true,
+			GenesisAccounts: &types.GenesisAccounts{
+				Accounts: []types.GenesisAccount{
+					{
+						Amount:  sdk.NewInt(1000),
+						Address: initialSequencerAddress,
+					},
+				},
+			},
+		},
+	}
+
+	update := &types.MsgUpdateRollappInformation{
+		Owner:            alice,
+		RollappId:        rollappId,
+		InitialSequencer: "",
+		Metadata:         nil,
+		GenesisInfo: &types.GenesisInfo{
+			GenesisAccounts: nil,
+		},
+	}
+
+	s.k().SetRollapp(s.Ctx, rollapp)
+
+	_, err := s.msgServer.UpdateRollappInformation(goCtx, update)
+	s.ErrorIs(err, types.ErrGenesisInfoSealed)
+
+	expect := len(rollapp.GenesisInfo.Accounts())
+	ra := s.k().MustGetRollapp(s.Ctx, rollappId)
+	s.Require().Equal(expect, len(ra.GenesisInfo.Accounts()))
+}
+
 func (s *RollappTestSuite) TestCreateAndUpdateRollapp() {
 	const rollappId = "rollapp_1234-1"
 

x/rollapp/keeper/rollapp.go

@@ -56,11 +56,8 @@ func (k Keeper) CheckAndUpdateRollappFields(ctx sdk.Context, update *types.MsgUp
 			current.GenesisInfo.InitialSupply = update.GenesisInfo.InitialSupply
 		}
 
-		if update.GenesisInfo.GenesisAccounts != nil {
-			current.GenesisInfo.GenesisAccounts = update.GenesisInfo.GenesisAccounts
-		} else if current.GenesisInfo.GenesisAccounts != nil {
-			current.GenesisInfo.GenesisAccounts.Accounts = nil
-		}
+		// Frontend always passes new value
+		current.GenesisInfo.GenesisAccounts = update.GenesisInfo.GenesisAccounts
 	}
 
 	if update.Metadata != nil && !update.Metadata.IsEmpty() {

x/rollapp/types/genesis_info.go

@@ -13,12 +13,21 @@ import (
 // We set the maximum amount of genesis accounts to 100
 const maxAllowedGenesisAccounts = 100
 
-func (gi GenesisInfo) GenesisTransferAmount() math.Int {
-	total := math.ZeroInt()
+// Handling should be based on length and contents, not nil status
+func (gi GenesisInfo) Accounts() []GenesisAccount {
 	if gi.GenesisAccounts == nil {
-		return total
+		return nil
 	}
-	for _, a := range gi.GenesisAccounts.Accounts {
+	return gi.GenesisAccounts.Accounts
+}
+
+func (gi GenesisInfo) RequiresTransfer() bool {
+	return 0 < len(gi.Accounts())
+}
+
+func (gi GenesisInfo) GenesisTransferAmount() math.Int {
+	total := math.ZeroInt()
+	for _, a := range gi.Accounts() {
 		total = total.Add(a.Amount)
 	}
 	return total
@@ -52,22 +61,19 @@ func (gi GenesisInfo) Validate() error {
 		return ErrInvalidInitialSupply
 	}
 
-	// validate max limit of genesis accounts
-	if gi.GenesisAccounts != nil {
-		if len(gi.GenesisAccounts.Accounts) > maxAllowedGenesisAccounts {
-			return fmt.Errorf("too many genesis accounts: %d", len(gi.GenesisAccounts.Accounts))
-		}
+	if l := len(gi.Accounts()); l > maxAllowedGenesisAccounts {
+		return fmt.Errorf("too many genesis accounts: %d", l)
+	}
 
-		accountSet := make(map[string]struct{})
-		for _, a := range gi.GenesisAccounts.Accounts {
-			if err := a.ValidateBasic(); err != nil {
-				return errors.Join(gerrc.ErrInvalidArgument, err)
-			}
-			if _, exists := accountSet[a.Address]; exists {
-				return fmt.Errorf("duplicate genesis account: %s", a.Address)
-			}
-			accountSet[a.Address] = struct{}{}
+	accountSet := make(map[string]struct{})
+	for _, a := range gi.Accounts() {
+		if err := a.ValidateBasic(); err != nil {
+			return errors.Join(gerrc.ErrInvalidArgument, err)
+		}
+		if _, exists := accountSet[a.Address]; exists {
+			return fmt.Errorf("duplicate genesis account: %s", a.Address)
 		}
+		accountSet[a.Address] = struct{}{}
 	}
 	return nil
 }

x/rollapp/types/message_update_rollapp.go

@@ -68,6 +68,8 @@ func (msg *MsgUpdateRollappInformation) ValidateBasic() error {
 	}
 
 	if msg.GenesisInfo != nil {
+		// TODO: impl using .Validate() https://github.com/dymensionxyz/dymension/issues/1559
+
 		if len(msg.GenesisInfo.GenesisChecksum) > maxGenesisChecksumLength {
 			return ErrInvalidGenesisChecksum
 		}
@@ -78,16 +80,14 @@ func (msg *MsgUpdateRollappInformation) ValidateBasic() error {
 			}
 		}
 
-		if msg.GenesisInfo.GenesisAccounts != nil {
-			// validate max limit of genesis accounts
-			if len(msg.GenesisInfo.GenesisAccounts.Accounts) > maxAllowedGenesisAccounts {
-				return fmt.Errorf("too many genesis accounts: %d", len(msg.GenesisInfo.GenesisAccounts.Accounts))
-			}
+		// validate max limit of genesis accounts
+		if l := len(msg.GenesisInfo.Accounts()); l > maxAllowedGenesisAccounts {
+			return fmt.Errorf("too many genesis accounts: %d", l)
+		}
 
-			for _, acc := range msg.GenesisInfo.GenesisAccounts.Accounts {
-				if err := acc.ValidateBasic(); err != nil {
-					return errorsmod.Wrapf(errors.Join(gerrc.ErrInvalidArgument, err), "genesis account: %v", acc)
-				}
+		for _, acc := range msg.GenesisInfo.Accounts() {
+			if err := acc.ValidateBasic(); err != nil {
+				return errorsmod.Wrapf(errors.Join(gerrc.ErrInvalidArgument, err), "genesis account: %v", acc)
 			}
 		}
 	}
@@ -106,12 +106,5 @@ func (msg *MsgUpdateRollappInformation) UpdatingImmutableValues() bool {
 }
 
 func (msg *MsgUpdateRollappInformation) UpdatingGenesisInfo() bool {
-	if msg.GenesisInfo == nil {
-		return false
-	}
-	return msg.GenesisInfo.GenesisChecksum != "" ||
-		msg.GenesisInfo.Bech32Prefix != "" ||
-		msg.GenesisInfo.NativeDenom.Base != "" ||
-		!msg.GenesisInfo.InitialSupply.IsNil() ||
-		msg.GenesisInfo.GenesisAccounts != nil
+	return msg.GenesisInfo != nil
 }