fix(genesis bridge): better UX for chains without genesis transfers

go.mod

@@ -11,6 +11,7 @@ require (
 	github.com/cosmos/gogoproto v1.4.11
 	github.com/cosmos/ibc-go/v6 v6.2.1
 	github.com/dymensionxyz/dymint v1.1.3-rc04
+	github.com/dymensionxyz/gerr-cosmos v1.0.0
 	github.com/dymensionxyz/sdk-utils v0.1.1
 	github.com/gogo/protobuf v1.3.3
 	github.com/golang/protobuf v1.5.4
@@ -101,7 +102,6 @@ require (
 	github.com/docker/go-units v0.5.0 // indirect
 	github.com/dustin/go-humanize v1.0.1-0.20200219035652-afde56e7acac // indirect
 	github.com/dvsekhvalnov/jose2go v1.5.0 // indirect
-	github.com/dymensionxyz/gerr-cosmos v1.0.0 // indirect
 	github.com/elastic/gosigar v0.14.2 // indirect
 	github.com/ethereum/go-ethereum v1.12.0 // indirect
 	github.com/felixge/httpsnoop v1.0.4 // indirect

proto/hub-genesis/genesis.proto

@@ -12,4 +12,5 @@ message GenesisState {
   // params defines all the parameters of the module.
   Params params = 1 [ (gogoproto.nullable) = false ];
   State state = 2 [(gogoproto.nullable) = false];
+  repeated uint64 unacked_transfer_seq_nums = 3 [(gogoproto.nullable) = false];
 }

proto/hub-genesis/state.proto

@@ -12,11 +12,20 @@ message State {
   reserved 2;
   // accounts on the Hub to fund with some bootstrapping transfers
   repeated GenesisAccount genesis_accounts = 3 [(gogoproto.nullable) = false];
+  // the number of genesis transfers for which an ack has not yet been received
+  uint64  num_unacked_transfers = 5;
+  // are outboundTransfersEnabled? This is only true if the genesis protocol has finished
+  bool outbound_transfers_enabled = 6;
+  // the canonical transfer port and channel for the hub
+  PortAndChannel hub_port_and_channel= 7;
 }
-/*
 
-TODO: the below is copy pasted, clean up
- */
+message PortAndChannel {
+  // port
+  string port = 1;
+  // channel
+  string channel = 2;
+}
 
 // GenesisAccount is a struct for the genesis account for the rollapp
 message GenesisAccount {

testutil/app/app.go

@@ -417,12 +417,21 @@ func NewRollapp(
 		),
 	)
 
+	app.HubGenesisKeeper = hubgenkeeper.NewKeeper(
+		appCodec,
+		keys[hubgentypes.StoreKey],
+		app.GetSubspace(hubgentypes.ModuleName),
+		app.AccountKeeper,
+	)
+
+	genesisTransfersBlocker := hubgenkeeper.NewICS4Wrapper(app.IBCKeeper.ChannelKeeper, app.HubGenesisKeeper)
+
 	// Create Transfer Keepers
 	app.TransferKeeper = ibctransferkeeper.NewKeeper(
 		appCodec,
 		keys[ibctransfertypes.StoreKey],
 		app.GetSubspace(ibctransfertypes.ModuleName),
-		app.IBCKeeper.ChannelKeeper,
+		genesisTransfersBlocker,
 		app.IBCKeeper.ChannelKeeper,
 		&app.IBCKeeper.PortKeeper,
 		app.AccountKeeper,
@@ -438,12 +447,11 @@ func NewRollapp(
 		app.TransferKeeper,
 		denommetadatamoduletypes.NewMultiDenommetadataHooks(),
 	)
-
-	app.HubGenesisKeeper = hubgenkeeper.NewKeeper(
-		appCodec,
-		keys[hubgentypes.StoreKey],
-		app.GetSubspace(hubgentypes.ModuleName),
-		app.AccountKeeper,
+	transferStack = hubgenkeeper.NewIBCModule(
+		transferStack,
+		app.TransferKeeper,
+		app.HubGenesisKeeper,
+		app.BankKeeper,
 	)
 
 	app.GaslessKeeper = gaslesskeeper.NewKeeper(
@@ -738,10 +746,11 @@ func (app *App) ModuleAccountAddrs() map[string]bool {
 }
 
 // BlockedModuleAccountAddrs returns all the app's blocked module account
-// addresses.
+// addresses. A true value means blocked. Absent or false means not blocked.
 func (app *App) BlockedModuleAccountAddrs() map[string]bool {
 	modAccAddrs := app.ModuleAccountAddrs()
 	delete(modAccAddrs, authtypes.NewModuleAddress(govtypes.ModuleName).String())
+	delete(modAccAddrs, authtypes.NewModuleAddress(hubgentypes.ModuleName).String())
 
 	return modAccAddrs
 }

x/hub-genesis/keeper/genesis.go

@@ -9,13 +9,16 @@ import (
 func (k Keeper) InitGenesis(ctx sdk.Context, genState *types.GenesisState) {
 	k.SetParams(ctx, genState.Params)
 	k.SetState(ctx, genState.State)
+	for _, seq := range genState.UnackedTransferSeqNums {
+		k.saveUnackedTransferSeqNum(ctx, seq)
+	}
 }
 
 // ExportGenesis returns a GenesisState for a given context and keeper.
 func (k Keeper) ExportGenesis(ctx sdk.Context) *types.GenesisState {
 	genesis := types.DefaultGenesisState()
 	genesis.Params = k.GetParams(ctx)
 	genesis.State = k.GetState(ctx)
-
+	genesis.UnackedTransferSeqNums = k.getAllUnackedTransferSeqNums(ctx)
 	return genesis
 }

x/hub-genesis/keeper/genesis_test.go

@@ -5,21 +5,25 @@ import (
 	"testing"
 
 	testkeepers "github.com/dymensionxyz/dymension-rdk/testutil/keepers"
-	"github.com/dymensionxyz/dymension-rdk/testutil/nullify"
 	"github.com/dymensionxyz/dymension-rdk/testutil/utils"
 	"github.com/dymensionxyz/dymension-rdk/x/hub-genesis/types"
 	"github.com/stretchr/testify/require"
 )
 
 func TestGenesis(t *testing.T) {
-	genesisState := &types.GenesisState{}
 	app := utils.Setup(t, false)
 	k, ctx := testkeepers.NewTestHubGenesisKeeperFromApp(app)
 
-	k.InitGenesis(ctx, genesisState)
+	expect := &types.GenesisState{
+		State: types.State{
+			NumUnackedTransfers: 2,
+		},
+		UnackedTransferSeqNums: []uint64{42, 43},
+	}
+	k.InitGenesis(ctx, expect)
 	got := k.ExportGenesis(ctx)
 	require.NotNil(t, got)
 
-	nullify.Fill(&genesisState)
-	nullify.Fill(got)
+	require.ElementsMatch(t, expect.UnackedTransferSeqNums, got.UnackedTransferSeqNums)
+	require.Equal(t, expect.State.NumUnackedTransfers, got.State.NumUnackedTransfers)
 }

x/hub-genesis/keeper/ibc_module.go

@@ -1,17 +1,21 @@
 package keeper
 
 import (
+	"context"
+	"errors"
 	"fmt"
 	"time"
 
 	errorsmod "cosmossdk.io/errors"
-
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
 	transfertypes "github.com/cosmos/ibc-go/v6/modules/apps/transfer/types"
 	clienttypes "github.com/cosmos/ibc-go/v6/modules/core/02-client/types"
+	channeltypes "github.com/cosmos/ibc-go/v6/modules/core/04-channel/types"
 	porttypes "github.com/cosmos/ibc-go/v6/modules/core/05-port/types"
 	"github.com/dymensionxyz/dymension-rdk/x/hub-genesis/types"
+	"github.com/dymensionxyz/gerr-cosmos/gerrc"
+	"github.com/tendermint/tendermint/libs/log"
 )
 
 const (
@@ -20,20 +24,26 @@ const (
 
 type IBCModule struct {
 	porttypes.IBCModule
-	k         Keeper
-	transfer  Transfer
-	getDenom  GetDenomMetaData
-	mintCoins MintCoins
+	k        Keeper
+	transfer TransferKeeper
+	bank     BankKeeper
 }
 
-type (
-	Transfer         func(ctx sdk.Context, transfer *transfertypes.MsgTransfer) error
-	GetDenomMetaData func(ctx sdk.Context, denom string) (banktypes.Metadata, bool)
-	MintCoins        func(ctx sdk.Context, moduleName string, amt sdk.Coins) error
-)
+type TransferKeeper interface {
+	Transfer(goCtx context.Context, msg *transfertypes.MsgTransfer) (*transfertypes.MsgTransferResponse, error)
+}
 
-func NewIBCModule(next porttypes.IBCModule, t Transfer, k Keeper, d GetDenomMetaData, m MintCoins) *IBCModule {
-	return &IBCModule{next, k, t, d, m}
+type BankKeeper interface {
+	GetDenomMetaData(ctx sdk.Context, denom string) (banktypes.Metadata, bool)
+	MintCoins(ctx sdk.Context, moduleName string, amt sdk.Coins) error
+}
+
+func NewIBCModule(next porttypes.IBCModule, t TransferKeeper, k Keeper, bank BankKeeper) *IBCModule {
+	return &IBCModule{next, k, t, bank}
+}
+
+func (w IBCModule) logger(ctx sdk.Context) log.Logger {
+	return w.k.Logger(ctx).With("module", types.ModuleName, "component", "ibc module")
 }
 
 // OnChanOpenConfirm will send any unsent genesis account transfers over the channel.
@@ -50,42 +60,49 @@ func (w IBCModule) OnChanOpenConfirm(
 
 	err := w.IBCModule.OnChanOpenConfirm(ctx, portID, channelID)
 	if err != nil {
-		l.Error("Next middleware.", "err", err)
 		return err
 	}
 
 	state := w.k.GetState(ctx)
 
+	if state.CanonicalHubTransferChannelHasBeenSet() {
+		// We only set the canonical channel in this function, so if it's already been set, we don't need
+		// to send the transfers again.
+		return nil
+	}
+
+	state.SetCanonicalTransferChannel(portID, channelID)
+
+	state.NumUnackedTransfers = uint64(len(state.GetGenesisAccounts()))
+
+	w.k.SetState(ctx, state)
+
+	if len(state.GetGenesisAccounts()) == 0 {
+		// we want to handle the case where the rollapp doesn't have genesis transfers
+		// normally we would enable outbound transfers on an ack, but in this case we won't have an ack
+		w.k.enableOutboundTransfers(ctx)
+		return nil
+	}
+
 	srcAccount := w.k.accountKeeper.GetModuleAccount(ctx, types.ModuleName)
 	srcAddr := srcAccount.GetAddress().String()
 
 	for i, a := range state.GetGenesisAccounts() {
-		if err := w.mintAndTransfer(ctx, len(state.GetGenesisAccounts()), a, srcAddr, portID, channelID); err != nil {
+		if err := w.mintAndTransfer(ctx, a, srcAddr, portID, channelID); err != nil {
 			// there is no feasible way to recover
 			panic(fmt.Errorf("mint and transfer: %w", err))
 		}
 		l.Info("Sent genesis transfer.", "index", i, "receiver", a.GetAddress(), "coin", a)
 	}
 
-	state.GenesisAccounts = nil
-
-	w.k.SetState(ctx, state)
-
-	l.Info("Sent all genesis transfers.")
+	l.Info("Sent all genesis transfers.", "n", len(state.GetGenesisAccounts()))
 
 	return nil
 }
 
-func (w IBCModule) mintAndTransfer(
-	ctx sdk.Context,
-	n int,
-	a types.GenesisAccount,
-	srcAddr string,
-	portID string,
-	channelID string,
-) error {
-	coin := a.GetAmount()
-	err := w.mintCoins(ctx, types.ModuleName, sdk.Coins{coin})
+func (w IBCModule) mintAndTransfer(ctx sdk.Context, account types.GenesisAccount, srcAddr string, portID string, channelID string) error {
+	coin := account.GetAmount()
+	err := w.bank.MintCoins(ctx, types.ModuleName, sdk.Coins{coin})
 	if err != nil {
 		return errorsmod.Wrap(err, "mint coins")
 	}
@@ -95,26 +112,63 @@ func (w IBCModule) mintAndTransfer(
 	// or commit anyway, so the packet will be cleared up.
 	// (Actually, since transfers may arrive out of order, we must include the
 	// denom metadata anyway).
-	memo, err := w.createMemo(ctx, a.Amount.Denom, n)
+	memo, err := w.createMemo(ctx, account.Amount.Denom)
 	if err != nil {
 		return errorsmod.Wrap(err, "create memo")
 	}
 
 	m := transfertypes.MsgTransfer{
 		SourcePort:       portID,
 		SourceChannel:    channelID,
-		Token:            a.Amount,
+		Token:            account.Amount,
 		Sender:           srcAddr,
-		Receiver:         a.GetAddress(),
+		Receiver:         account.GetAddress(),
 		TimeoutHeight:    clienttypes.Height{},
 		TimeoutTimestamp: uint64(ctx.BlockTime().Add(transferTimeout).UnixNano()),
 		Memo:             memo,
 	}
 
-	err = w.transfer(skipContext(ctx), &m)
+	res, err := w.transfer.Transfer(sdk.WrapSDKContext(allowSpecialMemoCtx(ctx)), &m)
 	if err != nil {
 		return errorsmod.Wrap(err, "transfer")
 	}
-
+	w.k.saveUnackedTransferSeqNum(ctx, res.Sequence)
 	return nil
 }
+
+func (w IBCModule) OnAcknowledgementPacket(
+	ctx sdk.Context,
+	packet channeltypes.Packet,
+	acknowledgement []byte,
+	relayer sdk.AccAddress,
+) error {
+	l := w.logger(ctx)
+	state := w.k.GetState(ctx)
+
+	if !state.IsCanonicalHubTransferChannel(packet.SourcePort, packet.SourceChannel) {
+		return w.IBCModule.OnAcknowledgementPacket(ctx, packet, acknowledgement, relayer)
+	}
+
+	if state.OutboundTransfersEnabled {
+		return w.IBCModule.OnAcknowledgementPacket(ctx, packet, acknowledgement, relayer)
+	}
+
+	var data transfertypes.FungibleTokenPacketData
+	err := transfertypes.ModuleCdc.UnmarshalJSON(packet.GetData(), &data)
+	if err != nil {
+		return err
+	}
+
+	var ack channeltypes.Acknowledgement
+	err = types.ModuleCdc.UnmarshalJSON(acknowledgement, &ack)
+	if err != nil {
+		return err
+	}
+
+	err = w.k.ackTransferSeqNum(ctx, packet.Sequence, ack)
+	if err != nil {
+		l.Error("Processing ack from transfer.", "err", err)
+		return errorsmod.Wrap(errors.Join(err, gerrc.ErrUnknown), "ack transfer seq num")
+	}
+	return w.IBCModule.OnAcknowledgementPacket(ctx, packet, acknowledgement, relayer)
+}