Ibcmodulev2 transfer application spike (#7524)

* chore: adding ibc module v2 and on recv implementation with hard coded sequence

* chore: adding ack callback

* chore: adding send packet

* chore: adding transfer v2 module to app wiring

* chore: adding happy path and basic failure for send and recv for transfer module

* chore: adding ack test for transfer

* chore: fix some linter errors

* chore: adding timeout test for transfer v2

* chore: adding test case which ensures tokens can be transfered over both v1 and v2

* chore: full transfer flow from A - B - C - B - A

* chore: separated test out into subtests

* chore: add sequence as argument to OnRecvPacket

* chore: adding TODOs for next steps

* chore: adding transferv2 module to wasm simapp

* chore: refactor OnTimeout to accept sequence

* chore: refactor OnAck to accept sequence

* chore: switch argument order

* wip: mid merge with feature branch, build will be broken

* Fix timeoutTimestamp for tests

* linter

* chore: removing duplicate imports in wasm simapp

* chore: adding channelkeeperv2 query server

* register grpc gateway routes

* fix ack structure for v2 transfer tests

* lint

* make signature consistent

---------

Co-authored-by: chatton <[email protected]>
Co-authored-by: bznein <bznein@debian>
Co-authored-by: Gjermund Garaba <[email protected]>

modules/apps/transfer/keeper/export_test.go

@@ -54,11 +54,6 @@ func (k Keeper) GetAllForwardedPackets(ctx sdk.Context) []types.ForwardedPacket
 	return k.getAllForwardedPackets(ctx)
 }
 
-// IsBlockedAddr is a wrapper around isBlockedAddr for testing purposes
-func (k Keeper) IsBlockedAddr(addr sdk.AccAddress) bool {
-	return k.isBlockedAddr(addr)
-}
-
 // CreatePacketDataBytesFromVersion is a wrapper around createPacketDataBytesFromVersion for testing purposes
 func CreatePacketDataBytesFromVersion(appVersion, sender, receiver, memo string, tokens types.Tokens, hops []types.Hop) ([]byte, error) {
 	return createPacketDataBytesFromVersion(appVersion, sender, receiver, memo, tokens, hops)

modules/apps/transfer/keeper/forwarding.go

@@ -22,7 +22,7 @@ func (k Keeper) forwardPacket(ctx context.Context, data types.FungibleTokenPacke
 	}
 
 	// sending from module account (used as a temporary forward escrow) to the original receiver address.
-	sender := k.authKeeper.GetModuleAddress(types.ModuleName)
+	sender := k.AuthKeeper.GetModuleAddress(types.ModuleName)
 
 	msg := types.NewMsgTransfer(
 		data.Forwarding.Hops[0].PortId,
@@ -68,7 +68,7 @@ func (k Keeper) revertForwardedPacket(ctx context.Context, forwardedPacket chann
 			2. Burning voucher tokens if the funds are foreign
 	*/
 
-	forwardingAddr := k.authKeeper.GetModuleAddress(types.ModuleName)
+	forwardingAddr := k.AuthKeeper.GetModuleAddress(types.ModuleName)
 	escrow := types.GetEscrowAddress(forwardedPacket.DestinationPort, forwardedPacket.DestinationChannel)
 
 	// we can iterate over the received tokens of forwardedPacket by iterating over the sent tokens of failedPacketData
@@ -83,12 +83,12 @@ func (k Keeper) revertForwardedPacket(ctx context.Context, forwardedPacket chann
 		// given that the packet is being reversed, we check the DestinationChannel and DestinationPort
 		// of the forwardedPacket to see if a hop was added to the trace during the receive step
 		if token.Denom.HasPrefix(forwardedPacket.DestinationPort, forwardedPacket.DestinationChannel) {
-			if err := k.bankKeeper.BurnCoins(ctx, types.ModuleName, sdk.NewCoins(coin)); err != nil {
+			if err := k.BankKeeper.BurnCoins(ctx, types.ModuleName, sdk.NewCoins(coin)); err != nil {
 				return err
 			}
 		} else {
 			// send it back to the escrow address
-			if err := k.escrowCoin(ctx, forwardingAddr, escrow, coin); err != nil {
+			if err := k.EscrowCoin(ctx, forwardingAddr, escrow, coin); err != nil {
 				return err
 			}
 		}
@@ -101,7 +101,7 @@ func (k Keeper) revertForwardedPacket(ctx context.Context, forwardedPacket chann
 func (k Keeper) getReceiverFromPacketData(data types.FungibleTokenPacketDataV2) (sdk.AccAddress, error) {
 	if data.HasForwarding() {
 		// since data.Receiver can potentially be a non-CosmosSDK AccAddress, we return early if the packet should be forwarded
-		return k.authKeeper.GetModuleAddress(types.ModuleName), nil
+		return k.AuthKeeper.GetModuleAddress(types.ModuleName), nil
 	}
 
 	receiver, err := sdk.AccAddressFromBech32(data.Receiver)

modules/apps/transfer/keeper/genesis.go

@@ -12,7 +12,7 @@ func (k Keeper) InitGenesis(ctx sdk.Context, state types.GenesisState) {
 
 	for _, denom := range state.Denoms {
 		k.SetDenom(ctx, denom)
-		k.setDenomMetadata(ctx, denom)
+		k.SetDenomMetadata(ctx, denom)
 	}
 
 	k.SetParams(ctx, state.Params)

modules/apps/transfer/keeper/invariants.go

@@ -33,7 +33,7 @@ func TotalEscrowPerDenomInvariants(k *Keeper) sdk.Invariant {
 		transferChannels := k.channelKeeper.GetAllChannelsWithPortPrefix(ctx, portID)
 		for _, channel := range transferChannels {
 			escrowAddress := types.GetEscrowAddress(portID, channel.ChannelId)
-			escrowBalances := k.bankKeeper.GetAllBalances(ctx, escrowAddress)
+			escrowBalances := k.BankKeeper.GetAllBalances(ctx, escrowAddress)
 
 			actualTotalEscrowed = actualTotalEscrowed.Add(escrowBalances...)
 		}

modules/apps/transfer/keeper/keeper.go

@@ -34,8 +34,8 @@ type Keeper struct {
 
 	ics4Wrapper   porttypes.ICS4Wrapper
 	channelKeeper types.ChannelKeeper
-	authKeeper    types.AccountKeeper
-	bankKeeper    types.BankKeeper
+	AuthKeeper    types.AccountKeeper
+	BankKeeper    types.BankKeeper
 
 	// the address capable of executing a MsgUpdateParams message. Typically, this
 	// should be the x/gov module account.
@@ -68,8 +68,8 @@ func NewKeeper(
 		legacySubspace: legacySubspace,
 		ics4Wrapper:    ics4Wrapper,
 		channelKeeper:  channelKeeper,
-		authKeeper:     authKeeper,
-		bankKeeper:     bankKeeper,
+		AuthKeeper:     authKeeper,
+		BankKeeper:     bankKeeper,
 		authority:      authority,
 	}
 }
@@ -195,8 +195,8 @@ func (k Keeper) IterateDenoms(ctx context.Context, cb func(denom types.Denom) bo
 	}
 }
 
-// setDenomMetadata sets an IBC token's denomination metadata
-func (k Keeper) setDenomMetadata(ctx context.Context, denom types.Denom) {
+// SetDenomMetadata sets an IBC token's denomination metadata
+func (k Keeper) SetDenomMetadata(ctx context.Context, denom types.Denom) {
 	metadata := banktypes.Metadata{
 		Description: fmt.Sprintf("IBC token from %s", denom.Path()),
 		DenomUnits: []*banktypes.DenomUnit{
@@ -214,7 +214,7 @@ func (k Keeper) setDenomMetadata(ctx context.Context, denom types.Denom) {
 		Symbol:  strings.ToUpper(denom.Base),
 	}
 
-	k.bankKeeper.SetDenomMetaData(ctx, metadata)
+	k.BankKeeper.SetDenomMetaData(ctx, metadata)
 }
 
 // GetTotalEscrowForDenom gets the total amount of source chain tokens that
@@ -385,11 +385,11 @@ func (k Keeper) iterateForwardedPackets(ctx context.Context, cb func(packet type
 
 // IsBlockedAddr checks if the given address is allowed to send or receive tokens.
 // The module account is always allowed to send and receive tokens.
-func (k Keeper) isBlockedAddr(addr sdk.AccAddress) bool {
-	moduleAddr := k.authKeeper.GetModuleAddress(types.ModuleName)
+func (k Keeper) IsBlockedAddr(addr sdk.AccAddress) bool {
+	moduleAddr := k.AuthKeeper.GetModuleAddress(types.ModuleName)
 	if addr.Equals(moduleAddr) {
 		return false
 	}
 
-	return k.bankKeeper.BlockedAddr(addr)
+	return k.BankKeeper.BlockedAddr(addr)
 }

modules/apps/transfer/keeper/migrations.go

@@ -43,7 +43,7 @@ func (m Migrator) MigrateDenomMetadata(ctx sdk.Context) error {
 	m.keeper.iterateDenomTraces(ctx,
 		func(dt internaltypes.DenomTrace) (stop bool) {
 			// check if the metadata for the given denom trace does not already exist
-			if !m.keeper.bankKeeper.HasDenomMetaData(ctx, dt.IBCDenom()) {
+			if !m.keeper.BankKeeper.HasDenomMetaData(ctx, dt.IBCDenom()) {
 				m.keeper.setDenomMetadataWithDenomTrace(ctx, dt)
 			}
 			return false
@@ -61,7 +61,7 @@ func (m Migrator) MigrateTotalEscrowForDenom(ctx sdk.Context) error {
 	transferChannels := m.keeper.channelKeeper.GetAllChannelsWithPortPrefix(ctx, portID)
 	for _, channel := range transferChannels {
 		escrowAddress := types.GetEscrowAddress(portID, channel.ChannelId)
-		escrowBalances := m.keeper.bankKeeper.GetAllBalances(ctx, escrowAddress)
+		escrowBalances := m.keeper.BankKeeper.GetAllBalances(ctx, escrowAddress)
 
 		totalEscrowed = totalEscrowed.Add(escrowBalances...)
 	}
@@ -164,5 +164,5 @@ func (k Keeper) setDenomMetadataWithDenomTrace(ctx sdk.Context, denomTrace inter
 		Symbol:  strings.ToUpper(denomTrace.BaseDenom),
 	}
 
-	k.bankKeeper.SetDenomMetaData(ctx, metadata)
+	k.BankKeeper.SetDenomMetaData(ctx, metadata)
 }

modules/apps/transfer/keeper/msg_server.go

@@ -29,11 +29,11 @@ func (k Keeper) Transfer(goCtx context.Context, msg *types.MsgTransfer) (*types.
 
 	coins := msg.GetCoins()
 
-	if err := k.bankKeeper.IsSendEnabledCoins(ctx, coins...); err != nil {
+	if err := k.BankKeeper.IsSendEnabledCoins(ctx, coins...); err != nil {
 		return nil, errorsmod.Wrapf(types.ErrSendDisabled, err.Error())
 	}
 
-	if k.isBlockedAddr(sender) {
+	if k.IsBlockedAddr(sender) {
 		return nil, errorsmod.Wrapf(ibcerrors.ErrUnauthorized, "%s is not allowed to send funds", sender)
 	}
 

modules/apps/transfer/keeper/relay.go

@@ -96,7 +96,7 @@ func (k Keeper) sendTransfer(
 	for _, coin := range coins {
 		// Using types.UnboundedSpendLimit allows us to send the entire balance of a given denom.
 		if coin.Amount.Equal(types.UnboundedSpendLimit()) {
-			coin.Amount = k.bankKeeper.GetBalance(ctx, sender, coin.Denom).Amount
+			coin.Amount = k.BankKeeper.GetBalance(ctx, sender, coin.Denom).Amount
 		}
 
 		token, err := k.tokenFromCoin(ctx, coin)
@@ -112,13 +112,13 @@ func (k Keeper) sendTransfer(
 		// the token, then we must be returning the token back to the chain they originated from
 		if token.Denom.HasPrefix(sourcePort, sourceChannel) {
 			// transfer the coins to the module account and burn them
-			if err := k.bankKeeper.SendCoinsFromAccountToModule(
+			if err := k.BankKeeper.SendCoinsFromAccountToModule(
 				ctx, sender, types.ModuleName, sdk.NewCoins(coin),
 			); err != nil {
 				return 0, err
 			}
 
-			if err := k.bankKeeper.BurnCoins(
+			if err := k.BankKeeper.BurnCoins(
 				ctx, types.ModuleName, sdk.NewCoins(coin),
 			); err != nil {
 				// NOTE: should not happen as the module account was
@@ -129,7 +129,7 @@ func (k Keeper) sendTransfer(
 		} else {
 			// obtain the escrow address for the source channel end
 			escrowAddress := types.GetEscrowAddress(sourcePort, sourceChannel)
-			if err := k.escrowCoin(ctx, sender, escrowAddress, coin); err != nil {
+			if err := k.EscrowCoin(ctx, sender, escrowAddress, coin); err != nil {
 				return 0, err
 			}
 		}
@@ -178,7 +178,7 @@ func (k Keeper) OnRecvPacket(ctx context.Context, packet channeltypes.Packet, da
 		return err
 	}
 
-	if k.isBlockedAddr(receiver) {
+	if k.IsBlockedAddr(receiver) {
 		return errorsmod.Wrapf(ibcerrors.ErrUnauthorized, "%s is not allowed to receive funds", receiver)
 	}
 
@@ -206,7 +206,7 @@ func (k Keeper) OnRecvPacket(ctx context.Context, packet channeltypes.Packet, da
 			coin := sdk.NewCoin(token.Denom.IBCDenom(), transferAmount)
 
 			escrowAddress := types.GetEscrowAddress(packet.GetDestPort(), packet.GetDestChannel())
-			if err := k.unescrowCoin(ctx, escrowAddress, receiver, coin); err != nil {
+			if err := k.UnescrowCoin(ctx, escrowAddress, receiver, coin); err != nil {
 				return err
 			}
 
@@ -224,24 +224,24 @@ func (k Keeper) OnRecvPacket(ctx context.Context, packet channeltypes.Packet, da
 			}
 
 			voucherDenom := token.Denom.IBCDenom()
-			if !k.bankKeeper.HasDenomMetaData(ctx, voucherDenom) {
-				k.setDenomMetadata(ctx, token.Denom)
+			if !k.BankKeeper.HasDenomMetaData(ctx, voucherDenom) {
+				k.SetDenomMetadata(ctx, token.Denom)
 			}
 
 			events.EmitDenomEvent(ctx, token)
 
 			voucher := sdk.NewCoin(voucherDenom, transferAmount)
 
 			// mint new tokens if the source of the transfer is the same chain
-			if err := k.bankKeeper.MintCoins(
+			if err := k.BankKeeper.MintCoins(
 				ctx, types.ModuleName, sdk.NewCoins(voucher),
 			); err != nil {
 				return errorsmod.Wrap(err, "failed to mint IBC tokens")
 			}
 
 			// send to receiver
-			moduleAddr := k.authKeeper.GetModuleAddress(types.ModuleName)
-			if err := k.bankKeeper.SendCoins(
+			moduleAddr := k.AuthKeeper.GetModuleAddress(types.ModuleName)
+			if err := k.BankKeeper.SendCoins(
 				ctx, moduleAddr, receiver, sdk.NewCoins(voucher),
 			); err != nil {
 				return errorsmod.Wrapf(err, "failed to send coins to receiver %s", receiver.String())
@@ -346,14 +346,14 @@ func (k Keeper) refundPacketTokens(ctx context.Context, packet channeltypes.Pack
 	if err != nil {
 		return err
 	}
-	if k.isBlockedAddr(sender) {
+	if k.IsBlockedAddr(sender) {
 		return errorsmod.Wrapf(ibcerrors.ErrUnauthorized, "%s is not allowed to receive funds", sender)
 	}
 
 	// escrow address for unescrowing tokens back to sender
 	escrowAddress := types.GetEscrowAddress(packet.GetSourcePort(), packet.GetSourceChannel())
 
-	moduleAccountAddr := k.authKeeper.GetModuleAddress(types.ModuleName)
+	moduleAccountAddr := k.AuthKeeper.GetModuleAddress(types.ModuleName)
 	for _, token := range data.Tokens {
 		coin, err := token.ToCoin()
 		if err != nil {
@@ -364,17 +364,17 @@ func (k Keeper) refundPacketTokens(ctx context.Context, packet channeltypes.Pack
 		// then the tokens were burnt when the packet was sent and we must mint new tokens
 		if token.Denom.HasPrefix(packet.GetSourcePort(), packet.GetSourceChannel()) {
 			// mint vouchers back to sender
-			if err := k.bankKeeper.MintCoins(
+			if err := k.BankKeeper.MintCoins(
 				ctx, types.ModuleName, sdk.NewCoins(coin),
 			); err != nil {
 				return err
 			}
 
-			if err := k.bankKeeper.SendCoins(ctx, moduleAccountAddr, sender, sdk.NewCoins(coin)); err != nil {
+			if err := k.BankKeeper.SendCoins(ctx, moduleAccountAddr, sender, sdk.NewCoins(coin)); err != nil {
 				panic(fmt.Errorf("unable to send coins from module to account despite previously minting coins to module account: %v", err))
 			}
 		} else {
-			if err := k.unescrowCoin(ctx, escrowAddress, sender, coin); err != nil {
+			if err := k.UnescrowCoin(ctx, escrowAddress, sender, coin); err != nil {
 				return err
 			}
 		}
@@ -383,10 +383,10 @@ func (k Keeper) refundPacketTokens(ctx context.Context, packet channeltypes.Pack
 	return nil
 }
 
-// escrowCoin will send the given coin from the provided sender to the escrow address. It will also
+// EscrowCoin will send the given coin from the provided sender to the escrow address. It will also
 // update the total escrowed amount by adding the escrowed coin's amount to the current total escrow.
-func (k Keeper) escrowCoin(ctx context.Context, sender, escrowAddress sdk.AccAddress, coin sdk.Coin) error {
-	if err := k.bankKeeper.SendCoins(ctx, sender, escrowAddress, sdk.NewCoins(coin)); err != nil {
+func (k Keeper) EscrowCoin(ctx context.Context, sender, escrowAddress sdk.AccAddress, coin sdk.Coin) error {
+	if err := k.BankKeeper.SendCoins(ctx, sender, escrowAddress, sdk.NewCoins(coin)); err != nil {
 		// failure is expected for insufficient balances
 		return err
 	}
@@ -399,10 +399,10 @@ func (k Keeper) escrowCoin(ctx context.Context, sender, escrowAddress sdk.AccAdd
 	return nil
 }
 
-// unescrowCoin will send the given coin from the escrow address to the provided receiver. It will also
+// UnescrowCoin will send the given coin from the escrow address to the provided receiver. It will also
 // update the total escrow by deducting the unescrowed coin's amount from the current total escrow.
-func (k Keeper) unescrowCoin(ctx context.Context, escrowAddress, receiver sdk.AccAddress, coin sdk.Coin) error {
-	if err := k.bankKeeper.SendCoins(ctx, escrowAddress, receiver, sdk.NewCoins(coin)); err != nil {
+func (k Keeper) UnescrowCoin(ctx context.Context, escrowAddress, receiver sdk.AccAddress, coin sdk.Coin) error {
+	if err := k.BankKeeper.SendCoins(ctx, escrowAddress, receiver, sdk.NewCoins(coin)); err != nil {
 		// NOTE: this error is only expected to occur given an unexpected bug or a malicious
 		// counterparty module. The bug may occur in bank or any part of the code that allows
 		// the escrow address to be drained. A malicious counterparty module could drain the

modules/apps/transfer/types/packet.go

@@ -210,6 +210,8 @@ func (ftpd FungibleTokenPacketDataV2) HasForwarding() bool {
 // UnmarshalPacketData attempts to unmarshal the provided packet data bytes into a FungibleTokenPacketDataV2.
 // The version of ics20 should be provided and should be either ics20-1 or ics20-2.
 func UnmarshalPacketData(bz []byte, ics20Version string) (FungibleTokenPacketDataV2, error) {
+	// TODO: in transfer ibc module V2, we need to respect he encoding value passed via the payload, some hard coded assumptions about
+	// encoding exist here based on the ics20 version passed in.
 	switch ics20Version {
 	case V1:
 		var datav1 FungibleTokenPacketData