From 94cf34357c1ceca162ca617ea9d322bbaedcfff2 Mon Sep 17 00:00:00 2001 From: Sammy Date: Fri, 11 Oct 2024 01:11:21 +0800 Subject: [PATCH 1/2] fix(nexus): coin type should be ICS20 if the coin is from external cosmos chain --- x/nexus/keeper/lockable_asset.go | 26 +++++++++++++------- x/nexus/keeper/lockable_asset_test.go | 34 +++++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 8 deletions(-) diff --git a/x/nexus/keeper/lockable_asset.go b/x/nexus/keeper/lockable_asset.go index 02c246142..ffb3567c6 100644 --- a/x/nexus/keeper/lockable_asset.go +++ b/x/nexus/keeper/lockable_asset.go @@ -31,14 +31,14 @@ type lockableAsset struct { func newLockableAsset(ctx sdk.Context, nexus types.Nexus, ibc types.IBCKeeper, bank types.BankKeeper, coin sdk.Coin) (lockableAsset, error) { denom := coin.GetDenom() - coinType, err := getCoinType(ctx, nexus, denom) + coinType, err := getCoinType(ctx, nexus, ibc, denom) if err != nil { return lockableAsset{}, err } // If coin type is ICS20, we need to normalize it to convert from 'ibc/{hash}' // to native asset denom so that nexus could recognize it - if coinType == types.ICS20 { + if coinType == types.ICS20 && isIBCDenom(denom) { denomTrace, err := ibc.ParseIBCDenom(ctx, denom) if err != nil { return lockableAsset{}, err @@ -55,13 +55,9 @@ func newLockableAsset(ctx sdk.Context, nexus types.Nexus, ibc types.IBCKeeper, b bank: bank, } - originalCoin, err := c.getCoin(ctx) - if err != nil { + if _, err := c.getCoin(ctx); err != nil { return lockableAsset{}, err } - if originalCoin.GetDenom() != denom { - return lockableAsset{}, fmt.Errorf("denom mismatch, expected %s, got %s", denom, originalCoin.GetDenom()) - } return c, nil } @@ -176,11 +172,13 @@ func mint(ctx sdk.Context, bank types.BankKeeper, toAddr sdk.AccAddress, coin sd return nil } -func getCoinType(ctx sdk.Context, nexus types.Nexus, denom string) (types.CoinType, error) { +func getCoinType(ctx sdk.Context, nexus types.Nexus, ibc types.IBCKeeper, denom string) (types.CoinType, error) { switch { // check if the format of token denomination is 'ibc/{hash}' case isIBCDenom(denom): return types.ICS20, nil + case isFromExternalCosmosChain(ctx, nexus, ibc, denom): + return types.ICS20, nil case isNativeAssetOnAxelarnet(ctx, nexus, denom): return types.Native, nil case nexus.IsAssetRegistered(ctx, axelarnet.Axelarnet, denom): @@ -190,6 +188,18 @@ func getCoinType(ctx sdk.Context, nexus types.Nexus, denom string) (types.CoinTy } } +// isFromExternalCosmosChain returns true if the asset origins from cosmos chains +func isFromExternalCosmosChain(ctx sdk.Context, nexus types.Nexus, ibc types.IBCKeeper, denom string) bool { + chain, ok := nexus.GetChainByNativeAsset(ctx, denom) + if !ok { + return false + } + + _, ok = ibc.GetIBCPath(ctx, chain.Name) + + return ok +} + // isIBCDenom validates that the given denomination is a valid ICS token representation (ibc/{hash}) func isIBCDenom(denom string) bool { if err := sdk.ValidateDenom(denom); err != nil { diff --git a/x/nexus/keeper/lockable_asset_test.go b/x/nexus/keeper/lockable_asset_test.go index d8a074ee0..4805765f3 100644 --- a/x/nexus/keeper/lockable_asset_test.go +++ b/x/nexus/keeper/lockable_asset_test.go @@ -49,6 +49,7 @@ func TestLockableAsset(t *testing.T) { return exported.Chain{}, false } + ibc.GetIBCPathFunc = func(ctx sdk.Context, chain exported.ChainName) (string, bool) { return "", false } }) whenCoinIsExternal := When("coin is external", func() { @@ -61,6 +62,27 @@ func TestLockableAsset(t *testing.T) { } }) + whenCoinIsICS20FromExternalCosmosChain := When("coin is ICS20 from external cosmos chain", func() { + path := testutils.RandomIBCPath() + trace = ibctypes.DenomTrace{ + Path: path, + BaseDenom: rand.Denom(5, 10), + } + + nexus.GetChainByNativeAssetFunc = func(ctx sdk.Context, asset string) (exported.Chain, bool) { + if asset == trace.GetBaseDenom() { + return axelarnet.Axelarnet, true + } + + return exported.Chain{}, false + } + ibc.GetIBCPathFunc = func(ctx sdk.Context, chain exported.ChainName) (string, bool) { + return path, chain == axelarnet.Axelarnet.Name + } + + coin = sdk.NewCoin(trace.GetBaseDenom(), sdk.NewInt(rand.PosI64())) + }) + whenCoinIsICS20 := When("coin is ICS20", func() { path := testutils.RandomIBCPath() trace = ibctypes.DenomTrace{ @@ -106,6 +128,18 @@ func TestLockableAsset(t *testing.T) { }). Run(t) + givenKeeper. + When2(whenCoinIsICS20FromExternalCosmosChain). + Then("should create a new lockable asset of type ICS20", func(t *testing.T) { + lockableAsset, err := newLockableAsset(ctx, nexus, ibc, bank, coin) + + assert.NoError(t, err) + assert.Equal(t, types.CoinType(types.ICS20), lockableAsset.coinType) + assert.Equal(t, coin, lockableAsset.GetAsset()) + assert.Equal(t, sdk.NewCoin(trace.IBCDenom(), coin.Amount), lockableAsset.GetCoin(ctx)) + }). + Run(t) + givenKeeper. When2(whenCoinIsNative). Then("should create a new lockable asset of type native", func(t *testing.T) { From da162d5460e632549851d5aabd6611757f053b4a Mon Sep 17 00:00:00 2001 From: Sammy Date: Mon, 14 Oct 2024 09:53:47 +0800 Subject: [PATCH 2/2] improve comments --- x/nexus/keeper/lockable_asset.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/x/nexus/keeper/lockable_asset.go b/x/nexus/keeper/lockable_asset.go index ffb3567c6..3bdd508f4 100644 --- a/x/nexus/keeper/lockable_asset.go +++ b/x/nexus/keeper/lockable_asset.go @@ -13,7 +13,9 @@ import ( "github.com/axelarnetwork/utils/funcs" ) -// NewLockableAsset creates a new lockable asset +// NewLockableAsset creates a new lockable asset. +// The coin denom can either be an actual bank Coin (e.g. uaxl, ICS20 coin) , +// or the registered asset name (e.g. base denom for an ICS20 coin) func (k Keeper) NewLockableAsset(ctx sdk.Context, ibc types.IBCKeeper, bank types.BankKeeper, coin sdk.Coin) (exported.LockableAsset, error) { return newLockableAsset(ctx, k, ibc, bank, coin) } @@ -177,6 +179,7 @@ func getCoinType(ctx sdk.Context, nexus types.Nexus, ibc types.IBCKeeper, denom // check if the format of token denomination is 'ibc/{hash}' case isIBCDenom(denom): return types.ICS20, nil + // check if the denom is the registered asset name for an ICS20 coin from a cosmos chain case isFromExternalCosmosChain(ctx, nexus, ibc, denom): return types.ICS20, nil case isNativeAssetOnAxelarnet(ctx, nexus, denom): @@ -188,7 +191,8 @@ func getCoinType(ctx sdk.Context, nexus types.Nexus, ibc types.IBCKeeper, denom } } -// isFromExternalCosmosChain returns true if the asset origins from cosmos chains +// isFromExternalCosmosChain returns true if the denom is a nexus-registered +// asset name for an ICS20 coin originating from a cosmos chain func isFromExternalCosmosChain(ctx sdk.Context, nexus types.Nexus, ibc types.IBCKeeper, denom string) bool { chain, ok := nexus.GetChainByNativeAsset(ctx, denom) if !ok {