diff --git a/app/app.go b/app/app.go index 1ac1367c4..87609f4e4 100644 --- a/app/app.go +++ b/app/app.go @@ -304,6 +304,7 @@ func NewAxelarApp( *GetKeeper[axelarnetKeeper.IBCKeeper](keepers), GetKeeper[nexusKeeper.Keeper](keepers), axelarbankkeeper.NewBankKeeper(GetKeeper[bankkeeper.BaseKeeper](keepers)), + GetKeeper[authkeeper.AccountKeeper](keepers), logger, ), ) diff --git a/x/axelarnet/keeper/migrate.go b/x/axelarnet/keeper/migrate.go index c4e07dc49..c4f83dd6a 100644 --- a/x/axelarnet/keeper/migrate.go +++ b/x/axelarnet/keeper/migrate.go @@ -1,20 +1,75 @@ package keeper import ( + "fmt" + sdk "github.com/cosmos/cosmos-sdk/types" "github.com/axelarnetwork/axelar-core/x/axelarnet/types" + nexus "github.com/axelarnetwork/axelar-core/x/nexus/types" ) -// Migrate5to6 returns the handler that performs in-place store migrations from version 5 to 6 -func Migrate5to6(k Keeper) func(ctx sdk.Context) error { +// Migrate6to7 returns the handler that performs in-place store migrations from version 6 to 7 +func Migrate6to7(k Keeper, bankK types.BankKeeper, accountK types.AccountKeeper, nexusK types.Nexus, ibcK IBCKeeper) func(ctx sdk.Context) error { return func(ctx sdk.Context) error { - addModuleParamCallContractsProposalMinDeposits(ctx, k) + // Failed IBC transfers are held in Axelarnet module account for later retry. + // This migration escrows tokens back to escrow accounts so that we can use the same code path for retry. + err := escrowFundsFromFailedTransfers(ctx, k, bankK, accountK, nexusK, ibcK) + if err != nil { + return err + } + + // All IBC transfer are routed from AxelarIBCAccount after v1.1 + // This migration updates the sender of failed transfers to AxelarIBCAccount for retry + err = migrateFailedTransfersToAxelarIBCAccount(ctx, k) + if err != nil { + return err + } return nil } } -func addModuleParamCallContractsProposalMinDeposits(ctx sdk.Context, k Keeper) { - k.params.Set(ctx, types.KeyCallContractsProposalMinDeposits, types.DefaultParams().CallContractsProposalMinDeposits) +func escrowFundsFromFailedTransfers(ctx sdk.Context, k Keeper, bankK types.BankKeeper, accountK types.AccountKeeper, nexusK types.Nexus, ibcK IBCKeeper) error { + axelarnetModuleAccount := accountK.GetModuleAddress(types.ModuleName) + nexusModuleAccount := accountK.GetModuleAddress(nexus.ModuleName) + + balances := bankK.SpendableCoins(ctx, axelarnetModuleAccount) + for _, coin := range balances { + asset, err := nexusK.NewLockableAsset(ctx, ibcK, bankK, coin) + if err != nil { + k.Logger(ctx).Info(fmt.Sprintf("coin %s is not a lockable asset", coin), "error", err) + continue + } + + // Transfer coins from the Axelarnet module account to the Nexus module account for subsequent locking, + // as the Axelarnet module account is now restricted from sending coins. + err = bankK.SendCoinsFromModuleToModule(ctx, axelarnetModuleAccount.String(), nexusModuleAccount.String(), sdk.NewCoins(asset.GetAsset())) + if err != nil { + return err + } + + err = asset.LockFrom(ctx, nexusModuleAccount) + if err != nil { + return err + } + } + + return nil +} + +func migrateFailedTransfersToAxelarIBCAccount(ctx sdk.Context, k Keeper) error { + transfers := k.getIBCTransfers(ctx) + for _, transfer := range transfers { + if transfer.Status != types.TransferFailed { + continue + } + + transfer.Sender = types.AxelarIBCAccount + if err := k.setTransfer(ctx, transfer); err != nil { + return err + } + } + + return nil } diff --git a/x/axelarnet/keeper/migrate_test.go b/x/axelarnet/keeper/migrate_test.go index 22d63e40c..d3f14b45b 100644 --- a/x/axelarnet/keeper/migrate_test.go +++ b/x/axelarnet/keeper/migrate_test.go @@ -11,44 +11,85 @@ import ( "github.com/axelarnetwork/axelar-core/app" "github.com/axelarnetwork/axelar-core/testutils/fake" + "github.com/axelarnetwork/axelar-core/testutils/rand" "github.com/axelarnetwork/axelar-core/x/axelarnet/keeper" "github.com/axelarnetwork/axelar-core/x/axelarnet/types" "github.com/axelarnetwork/axelar-core/x/axelarnet/types/mock" + axelartestutils "github.com/axelarnetwork/axelar-core/x/axelarnet/types/testutils" + nexus "github.com/axelarnetwork/axelar-core/x/nexus/exported" + nexusmock "github.com/axelarnetwork/axelar-core/x/nexus/exported/mock" + nexustypes "github.com/axelarnetwork/axelar-core/x/nexus/types" + "github.com/axelarnetwork/utils/funcs" . "github.com/axelarnetwork/utils/test" ) -func TestMigrate5to6(t *testing.T) { +func TestMigrate6to7(t *testing.T) { + var ( + bank *mock.BankKeeperMock + account *mock.AccountKeeperMock + nexusK *mock.NexusMock + lockableAsset *nexusmock.LockableAssetMock + transfers []types.IBCTransfer + ) + encCfg := app.MakeEncodingConfig() subspace := params.NewSubspace(encCfg.Codec, encCfg.Amino, sdk.NewKVStoreKey("axelarnetKey"), sdk.NewKVStoreKey("tAxelarnetKey"), "axelarnet") k := keeper.NewKeeper(encCfg.Codec, sdk.NewKVStoreKey("axelarnet"), subspace, &mock.ChannelKeeperMock{}, &mock.FeegrantKeeperMock{}) + ibcK := keeper.NewIBCKeeper(k, &mock.IBCTransferKeeperMock{}) ctx := sdk.NewContext(fake.NewMultiStore(), tmproto.Header{}, false, log.TestingLogger()) - Given("subspace is setup with params before migration", func() { - subspace.Set(ctx, types.KeyRouteTimeoutWindow, types.DefaultParams().RouteTimeoutWindow) - subspace.Set(ctx, types.KeyTransferLimit, types.DefaultParams().TransferLimit) - subspace.Set(ctx, types.KeyEndBlockerLimit, types.DefaultParams().EndBlockerLimit) + Given("keeper setup before migration", func() { + bank = &mock.BankKeeperMock{ + SendCoinsFromModuleToModuleFunc: func(ctx sdk.Context, senderModule, recipientModule string, amt sdk.Coins) error { + return nil + }, + } + account = &mock.AccountKeeperMock{ + GetModuleAddressFunc: func(_ string) sdk.AccAddress { + return rand.AccAddr() + }, + } + lockableAsset = &nexusmock.LockableAssetMock{ + LockFromFunc: func(ctx sdk.Context, fromAddr sdk.AccAddress) error { + return nil + }, + GetAssetFunc: func() sdk.Coin { + return rand.Coin() + }, + } + nexusK = &mock.NexusMock{ + NewLockableAssetFunc: func(ctx sdk.Context, ibc nexustypes.IBCKeeper, bank nexustypes.BankKeeper, coin sdk.Coin) (nexus.LockableAsset, error) { + return lockableAsset, nil + }, + } }). - When("", func() {}). - Then("the migration should add the new param with the default value", func(t *testing.T) { - actual := types.CallContractProposalMinDeposits{} - - assert.PanicsWithError(t, "UnmarshalJSON cannot decode empty bytes", func() { - subspace.Get(ctx, types.KeyCallContractsProposalMinDeposits, &actual) - }) - assert.PanicsWithError(t, "UnmarshalJSON cannot decode empty bytes", func() { - k.GetParams(ctx) - }) - - keeper.Migrate5to6(k)(ctx) - - assert.NotPanics(t, func() { - subspace.Get(ctx, types.KeyCallContractsProposalMinDeposits, &actual) - }) - assert.NotPanics(t, func() { - k.GetParams(ctx) - }) - - assert.Equal(t, types.DefaultParams().CallContractsProposalMinDeposits, actual) + When("there are some failed transfers and Axelarnet module account has balances", func() { + bank.SpendableCoinsFunc = func(ctx sdk.Context, addr sdk.AccAddress) sdk.Coins { + return sdk.NewCoins(rand.Coin(), rand.Coin(), rand.Coin()) + } + + for i := 0; i < 50; i++ { + transfer := axelartestutils.RandomIBCTransfer() + if i%2 == 0 { + transfer.Status = types.TransferFailed + } + transfers = append(transfers, transfer) + assert.NoError(t, k.EnqueueIBCTransfer(ctx, transfer)) + } + }). + Then("the migration should lock back to escrow account and update sender of failed transfers", func(t *testing.T) { + err := keeper.Migrate6to7(k, bank, account, nexusK, ibcK)(ctx) + assert.NoError(t, err) + assert.Len(t, lockableAsset.LockFromCalls(), 3) + + for _, transfer := range transfers { + actual := funcs.MustOk(k.GetTransfer(ctx, transfer.ID)) + if transfer.Status == types.TransferFailed { + assert.Equal(t, types.AxelarIBCAccount, actual.Sender) + } else { + assert.Equal(t, transfer.Sender, actual.Sender) + } + } }). Run(t) } diff --git a/x/axelarnet/keeper/msg_server.go b/x/axelarnet/keeper/msg_server.go index 0d3d65a84..4015ac332 100644 --- a/x/axelarnet/keeper/msg_server.go +++ b/x/axelarnet/keeper/msg_server.go @@ -470,7 +470,17 @@ func (s msgServer) RetryIBCTransfer(c context.Context, req *types.RetryIBCTransf return nil, fmt.Errorf("chain %s is not activated", chain.Name) } - err := s.ibcK.SendIBCTransfer(ctx, t) + lockableAsset, err := s.nexus.NewLockableAsset(ctx, s.ibcK, s.bank, t.Token) + if err != nil { + return nil, err + } + + err = lockableAsset.UnlockTo(ctx, types.AxelarIBCAccount) + if err != nil { + return nil, err + } + + err = s.ibcK.SendIBCTransfer(ctx, t) if err != nil { return nil, err } diff --git a/x/axelarnet/keeper/msg_server_test.go b/x/axelarnet/keeper/msg_server_test.go index 2a8f0871c..29821f00c 100644 --- a/x/axelarnet/keeper/msg_server_test.go +++ b/x/axelarnet/keeper/msg_server_test.go @@ -565,7 +565,17 @@ func TestRetryIBCTransfer(t *testing.T) { EnqueueForTransferFunc: func(sdk.Context, nexus.CrossChainAddress, sdk.Coin) (nexus.TransferID, error) { return nexus.TransferID(rand.I64Between(1, 9999)), nil }, + NewLockableAssetFunc: func(ctx sdk.Context, ibc nexustypes.IBCKeeper, bank nexustypes.BankKeeper, coin sdk.Coin) (nexus.LockableAsset, error) { + lockableAsset := &nexusmock.LockableAssetMock{ + UnlockToFunc: func(ctx sdk.Context, fromAddr sdk.AccAddress) error { + return nil + }, + } + + return lockableAsset, nil + }, } + channelK.GetNextSequenceSendFunc = func(sdk.Context, string, string) (uint64, bool) { return uint64(rand.I64Between(1, 99999)), true } diff --git a/x/axelarnet/module.go b/x/axelarnet/module.go index 1860a5dc8..ebe93c125 100644 --- a/x/axelarnet/module.go +++ b/x/axelarnet/module.go @@ -96,15 +96,16 @@ func (AppModuleBasic) GetQueryCmd() *cobra.Command { // AppModule implements module.AppModule type AppModule struct { AppModuleBasic - logger log.Logger - ibcK keeper.IBCKeeper - keeper keeper.Keeper - nexus types.Nexus - bank types.BankKeeper + logger log.Logger + ibcK keeper.IBCKeeper + keeper keeper.Keeper + nexus types.Nexus + bank types.BankKeeper + account types.AccountKeeper } // NewAppModule creates a new AppModule object -func NewAppModule(ibcK keeper.IBCKeeper, nexus types.Nexus, bank types.BankKeeper, logger log.Logger) AppModule { +func NewAppModule(ibcK keeper.IBCKeeper, nexus types.Nexus, bank types.BankKeeper, account types.AccountKeeper, logger log.Logger) AppModule { return AppModule{ AppModuleBasic: AppModuleBasic{}, logger: logger, @@ -112,6 +113,7 @@ func NewAppModule(ibcK keeper.IBCKeeper, nexus types.Nexus, bank types.BankKeepe keeper: ibcK.Keeper, nexus: nexus, bank: bank, + account: account, } } @@ -160,7 +162,7 @@ func (am AppModule) RegisterServices(cfg module.Configurator) { types.RegisterQueryServiceServer(cfg.QueryServer(), keeper.NewGRPCQuerier(am.keeper, am.nexus)) - err := cfg.RegisterMigration(types.ModuleName, 5, keeper.Migrate5to6(am.keeper)) + err := cfg.RegisterMigration(types.ModuleName, 6, keeper.Migrate6to7(am.keeper, am.bank, am.account, am.nexus, am.ibcK)) if err != nil { panic(err) } @@ -179,7 +181,7 @@ func (am AppModule) EndBlock(ctx sdk.Context, req abci.RequestEndBlock) []abci.V } // ConsensusVersion implements AppModule/ConsensusVersion. -func (AppModule) ConsensusVersion() uint64 { return 6 } +func (AppModule) ConsensusVersion() uint64 { return 7 } // AxelarnetIBCModule is an IBCModule that adds rate limiting and gmp processing to the ibc middleware type AxelarnetIBCModule struct { diff --git a/x/axelarnet/types/expected_keepers.go b/x/axelarnet/types/expected_keepers.go index dcb950e50..f4ca67a12 100644 --- a/x/axelarnet/types/expected_keepers.go +++ b/x/axelarnet/types/expected_keepers.go @@ -105,6 +105,7 @@ type BankKeeper interface { SpendableBalance(ctx sdk.Context, address sdk.AccAddress, denom string) sdk.Coin SendCoinsFromModuleToModule(ctx sdk.Context, senderModule, recipientModule string, amt sdk.Coins) error GetAllBalances(ctx sdk.Context, addr sdk.AccAddress) sdk.Coins + SpendableCoins(ctx sdk.Context, addr sdk.AccAddress) sdk.Coins } // IBCTransferKeeper provides functionality to manage IBC transfers diff --git a/x/axelarnet/types/mock/expected_keepers.go b/x/axelarnet/types/mock/expected_keepers.go index f58afe5a2..767c8c525 100644 --- a/x/axelarnet/types/mock/expected_keepers.go +++ b/x/axelarnet/types/mock/expected_keepers.go @@ -2991,6 +2991,9 @@ var _ axelarnettypes.BankKeeper = &BankKeeperMock{} // SpendableBalanceFunc: func(ctx cosmossdktypes.Context, address cosmossdktypes.AccAddress, denom string) cosmossdktypes.Coin { // panic("mock out the SpendableBalance method") // }, +// SpendableCoinsFunc: func(ctx cosmossdktypes.Context, addr cosmossdktypes.AccAddress) cosmossdktypes.Coins { +// panic("mock out the SpendableCoins method") +// }, // } // // // use mockedBankKeeper in code that requires axelarnettypes.BankKeeper @@ -3031,6 +3034,9 @@ type BankKeeperMock struct { // SpendableBalanceFunc mocks the SpendableBalance method. SpendableBalanceFunc func(ctx cosmossdktypes.Context, address cosmossdktypes.AccAddress, denom string) cosmossdktypes.Coin + // SpendableCoinsFunc mocks the SpendableCoins method. + SpendableCoinsFunc func(ctx cosmossdktypes.Context, addr cosmossdktypes.AccAddress) cosmossdktypes.Coins + // calls tracks calls to the methods. calls struct { // BlockedAddr holds details about calls to the BlockedAddr method. @@ -3130,6 +3136,13 @@ type BankKeeperMock struct { // Denom is the denom argument value. Denom string } + // SpendableCoins holds details about calls to the SpendableCoins method. + SpendableCoins []struct { + // Ctx is the ctx argument value. + Ctx cosmossdktypes.Context + // Addr is the addr argument value. + Addr cosmossdktypes.AccAddress + } } lockBlockedAddr sync.RWMutex lockBurnCoins sync.RWMutex @@ -3142,6 +3155,7 @@ type BankKeeperMock struct { lockSendCoinsFromModuleToAccount sync.RWMutex lockSendCoinsFromModuleToModule sync.RWMutex lockSpendableBalance sync.RWMutex + lockSpendableCoins sync.RWMutex } // BlockedAddr calls BlockedAddrFunc. @@ -3580,6 +3594,42 @@ func (mock *BankKeeperMock) SpendableBalanceCalls() []struct { return calls } +// SpendableCoins calls SpendableCoinsFunc. +func (mock *BankKeeperMock) SpendableCoins(ctx cosmossdktypes.Context, addr cosmossdktypes.AccAddress) cosmossdktypes.Coins { + if mock.SpendableCoinsFunc == nil { + panic("BankKeeperMock.SpendableCoinsFunc: method is nil but BankKeeper.SpendableCoins was just called") + } + callInfo := struct { + Ctx cosmossdktypes.Context + Addr cosmossdktypes.AccAddress + }{ + Ctx: ctx, + Addr: addr, + } + mock.lockSpendableCoins.Lock() + mock.calls.SpendableCoins = append(mock.calls.SpendableCoins, callInfo) + mock.lockSpendableCoins.Unlock() + return mock.SpendableCoinsFunc(ctx, addr) +} + +// SpendableCoinsCalls gets all the calls that were made to SpendableCoins. +// Check the length with: +// +// len(mockedBankKeeper.SpendableCoinsCalls()) +func (mock *BankKeeperMock) SpendableCoinsCalls() []struct { + Ctx cosmossdktypes.Context + Addr cosmossdktypes.AccAddress +} { + var calls []struct { + Ctx cosmossdktypes.Context + Addr cosmossdktypes.AccAddress + } + mock.lockSpendableCoins.RLock() + calls = mock.calls.SpendableCoins + mock.lockSpendableCoins.RUnlock() + return calls +} + // Ensure, that IBCTransferKeeperMock does implement axelarnettypes.IBCTransferKeeper. // If this is not the case, regenerate this file with moq. var _ axelarnettypes.IBCTransferKeeper = &IBCTransferKeeperMock{}