Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ibcmodulev2 transfer application spike #7524

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
7a257c3
chore: adding ibc module v2 and on recv implementation with hard code…
chatton Oct 23, 2024
6608b93
chore: adding ack callback
chatton Oct 23, 2024
3c23128
chore: adding send packet
chatton Oct 23, 2024
c7a74a6
chore: adding transfer v2 module to app wiring
chatton Oct 24, 2024
4f70fc4
chore: adding happy path and basic failure for send and recv for tran…
chatton Oct 24, 2024
49392cd
chore: adding ack test for transfer
chatton Oct 24, 2024
d3b19c9
chore: fix some linter errors
chatton Oct 24, 2024
615f956
chore: adding timeout test for transfer v2
chatton Oct 24, 2024
c03b8c9
chore: adding test case which ensures tokens can be transfered over b…
chatton Oct 29, 2024
0e72c17
chore: full transfer flow from A - B - C - B - A
chatton Oct 29, 2024
198996b
chore: separated test out into subtests
chatton Oct 29, 2024
973f2f7
chore: add sequence as argument to OnRecvPacket
bznein Oct 29, 2024
f53d865
chore: adding TODOs for next steps
chatton Oct 31, 2024
cec9d52
chore: adding transferv2 module to wasm simapp
chatton Nov 4, 2024
8d6f0a8
chore: refactor OnTimeout to accept sequence
chatton Nov 4, 2024
756c52b
chore: refactor OnAck to accept sequence
chatton Nov 4, 2024
20bae59
chore: switch argument order
chatton Nov 4, 2024
16700f9
wip: mid merge with feature branch, build will be broken
chatton Nov 4, 2024
c2f6937
Merge branch 'feat/ibc-eureka' into cian/issue#7510-ibcmodulev2-trans…
bznein Nov 4, 2024
b2a1514
Fix timeoutTimestamp for tests
Nov 4, 2024
20afb28
linter
bznein Nov 4, 2024
29f1151
chore: removing duplicate imports in wasm simapp
chatton Nov 4, 2024
a6c801a
chore: adding channelkeeperv2 query server
chatton Nov 6, 2024
6d68fb6
Merge branch 'feat/ibc-eureka' into cian/issue#7510-ibcmodulev2-trans…
bznein Nov 6, 2024
dbf6f2c
register grpc gateway routes
gjermundgaraba Nov 6, 2024
739af65
Merge branch 'feat/ibc-eureka' into cian/issue#7510-ibcmodulev2-trans…
bznein Nov 15, 2024
d7c9437
fix ack structure for v2 transfer tests
bznein Nov 15, 2024
8a4fb46
Merge branch 'feat/ibc-eureka' into cian/issue#7510-ibcmodulev2-trans…
gjermundgaraba Nov 15, 2024
468035e
lint
gjermundgaraba Nov 15, 2024
1992fde
make signature consistent
bznein Nov 19, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions modules/apps/transfer/keeper/export_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
10 changes: 5 additions & 5 deletions modules/apps/transfer/keeper/forwarding.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand All @@ -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
}
}
Expand All @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion modules/apps/transfer/keeper/genesis.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion modules/apps/transfer/keeper/invariants.go
Original file line number Diff line number Diff line change
Expand Up @@ -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...)
}
Expand Down
20 changes: 10 additions & 10 deletions modules/apps/transfer/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -68,8 +68,8 @@ func NewKeeper(
legacySubspace: legacySubspace,
ics4Wrapper: ics4Wrapper,
channelKeeper: channelKeeper,
authKeeper: authKeeper,
bankKeeper: bankKeeper,
AuthKeeper: authKeeper,
BankKeeper: bankKeeper,
authority: authority,
}
}
Expand Down Expand Up @@ -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{
Expand All @@ -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
Expand Down Expand Up @@ -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)
}
6 changes: 3 additions & 3 deletions modules/apps/transfer/keeper/migrations.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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...)
}
Expand Down Expand Up @@ -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)
}
4 changes: 2 additions & 2 deletions modules/apps/transfer/keeper/msg_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
44 changes: 22 additions & 22 deletions modules/apps/transfer/keeper/relay.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
Expand All @@ -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
}
}
Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -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
}

Expand All @@ -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())
Expand Down Expand Up @@ -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 {
Expand All @@ -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
}
}
Expand All @@ -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
}
Expand All @@ -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
Expand Down
2 changes: 2 additions & 0 deletions modules/apps/transfer/types/packet.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have an issue created for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually have the commit ready to be added to this PR (if we want to!) or I can add it in a separate one! I just wanted to go over it w/ Cian before

// encoding exist here based on the ics20 version passed in.
switch ics20Version {
case V1:
var datav1 FungibleTokenPacketData
Expand Down
Loading
Loading