From 2312066ee4c34f5d7ec789786e84e888ab353bfb Mon Sep 17 00:00:00 2001 From: Andrei Zavgorodnii Date: Thu, 19 Dec 2024 12:26:32 +0000 Subject: [PATCH 01/15] chore: tests refactoring --- app/app.go | 1 + tests/ibc/tokenfactory_transfer_test.go | 90 +++++++++++++++++++++++++ x/tokenfactory/keeper/bankactions.go | 26 +++++++ x/tokenfactory/keeper/keeper.go | 16 +++++ 4 files changed, 133 insertions(+) create mode 100644 tests/ibc/tokenfactory_transfer_test.go diff --git a/app/app.go b/app/app.go index 8f9d2d0ce..30967e9f4 100644 --- a/app/app.go +++ b/app/app.go @@ -694,6 +694,7 @@ func New( &app.BankKeeper, &app.WasmKeeper, authtypes.NewModuleAddress(adminmoduletypes.ModuleName).String(), + app.IBCKeeper.ChannelKeeper, ) app.TokenFactoryKeeper = &tokenFactoryKeeper diff --git a/tests/ibc/tokenfactory_transfer_test.go b/tests/ibc/tokenfactory_transfer_test.go new file mode 100644 index 000000000..610f92a4f --- /dev/null +++ b/tests/ibc/tokenfactory_transfer_test.go @@ -0,0 +1,90 @@ +package ibc_test + +import ( + "fmt" + "testing" + + "cosmossdk.io/math" + sdk "github.com/cosmos/cosmos-sdk/types" + transfertypes "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types" + tftypes "github.com/neutron-org/neutron/v5/x/tokenfactory/types" + "github.com/stretchr/testify/suite" +) + +type TokenfactoryTestSuite struct { + IBCTestSuite +} + +func TestPocTestSuite(t *testing.T) { + suite.Run(t, new(TokenfactoryTestSuite)) +} + +func (s *TokenfactoryTestSuite) TestForceTransferFromIBCEscrow() { + // Create token factory denom + createDenomMsg := tftypes.NewMsgCreateDenom(s.neutronAddr.String(), "testtest") + _, err := s.neutronChain.SendMsgs(createDenomMsg) + s.Assert().NoError(err) + + // Derive full token factory denom + denom := fmt.Sprintf("factory/%s/%s", createDenomMsg.Sender, createDenomMsg.Subdenom) + + // Mint denom to sender + amount := sdk.NewCoin(denom, math.NewInt(10000000)) + mintMsg := tftypes.NewMsgMint(createDenomMsg.Sender, amount) + _, err = s.neutronChain.SendMsgs(mintMsg) + s.Assert().NoError(err) + + // Send IBC transfer + s.IBCTransfer( + s.neutronTransferPath, + s.neutronTransferPath.EndpointA, + s.neutronAddr, + s.providerAddr, + amount.Denom, + amount.Amount, + "", + ) + + // Derive IBC escrow address for channel + escrowAddress := transfertypes.GetEscrowAddress("transfer", s.neutronTransferPath.EndpointA.ChannelID) + + // Transfer tokens out of escrow address + forceTransferMsg := tftypes.NewMsgForceTransfer(s.neutronAddr.String(), sdk.NewCoin(amount.Denom, amount.Amount), escrowAddress.String(), s.neutronAddr.String()) + _, err = s.neutronChain.SendMsgs(forceTransferMsg) + s.Assert().Error(err) +} + +func (s *TokenfactoryTestSuite) TestBurnFromIBCEscrow() { + // Create token factory denom + createDenomMsg := tftypes.NewMsgCreateDenom(s.neutronAddr.String(), "testtest") + _, err := s.neutronChain.SendMsgs(createDenomMsg) + s.Assert().NoError(err) + + // Derive full token factory denom + denom := fmt.Sprintf("factory/%s/%s", createDenomMsg.Sender, createDenomMsg.Subdenom) + + // Mint denom to sender + amount := sdk.NewCoin(denom, math.NewInt(10000000)) + mintMsg := tftypes.NewMsgMint(createDenomMsg.Sender, amount) + _, err = s.neutronChain.SendMsgs(mintMsg) + s.Assert().NoError(err) + + // Send IBC transfer + s.IBCTransfer( + s.neutronTransferPath, + s.neutronTransferPath.EndpointA, + s.neutronAddr, + s.providerAddr, + amount.Denom, + amount.Amount, + "", + ) + + // Derive IBC escrow address for channel + escrowAddress := transfertypes.GetEscrowAddress("transfer", s.neutronTransferPath.EndpointA.ChannelID) + + // Burn tokens from escrow address + burnMsg := tftypes.NewMsgBurnFrom(s.neutronAddr.String(), amount, escrowAddress.String()) + _, err = s.neutronChain.SendMsgs(burnMsg) + s.Assert().Error(err) +} diff --git a/x/tokenfactory/keeper/bankactions.go b/x/tokenfactory/keeper/bankactions.go index ed2f7d565..5f883f63b 100644 --- a/x/tokenfactory/keeper/bankactions.go +++ b/x/tokenfactory/keeper/bankactions.go @@ -50,6 +50,12 @@ func (k Keeper) burnFrom(ctx sdk.Context, amount sdk.Coin, burnFrom string) erro return status.Errorf(codes.Internal, "burning from module accounts is forbidden") } + escrowAccounts := k.GetAllEscrowAccounts(ctx) + + if k.isEscrowAccount(escrowAccounts, burnFromAcc) { + return status.Errorf(codes.Internal, "burning from escrow accounts is forbidden") + } + err = k.bankKeeper.SendCoinsFromAccountToModule(ctx, burnFromAcc, types.ModuleName, @@ -86,6 +92,16 @@ func (k Keeper) forceTransfer(ctx sdk.Context, amount sdk.Coin, fromAddr, toAddr return status.Errorf(codes.Internal, "force transfer to module accounts is forbidden") } + escrowAccounts := k.GetAllEscrowAccounts(ctx) + + if k.isEscrowAccount(escrowAccounts, transferFromAcc) { + return status.Errorf(codes.Internal, "force transfer from IBC escrow accounts is forbidden") + } + + if k.isEscrowAccount(escrowAccounts, transferToAcc) { + return status.Errorf(codes.Internal, "force transfer to IBC escrow accounts is forbidden") + } + return k.bankKeeper.SendCoins(ctx, transferFromAcc, transferToAcc, sdk.NewCoins(amount)) } @@ -103,3 +119,13 @@ func (k Keeper) isModuleAccount(ctx sdk.Context, addr sdk.AccAddress) bool { return false } + +func (k Keeper) isEscrowAccount(escrowAccounts []sdk.AccAddress, addr sdk.AccAddress) bool { + for _, escrowAccount := range escrowAccounts { + if escrowAccount.Equals(addr) { + return true + } + } + + return false +} diff --git a/x/tokenfactory/keeper/keeper.go b/x/tokenfactory/keeper/keeper.go index 241183ca1..0fa517ad8 100644 --- a/x/tokenfactory/keeper/keeper.go +++ b/x/tokenfactory/keeper/keeper.go @@ -10,6 +10,8 @@ import ( storetypes "cosmossdk.io/store/types" "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" + transfertypes "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types" + channelkeeper "github.com/cosmos/ibc-go/v8/modules/core/04-channel/keeper" "github.com/neutron-org/neutron/v5/x/tokenfactory/types" ) @@ -23,6 +25,7 @@ type ( bankKeeper types.BankKeeper contractKeeper types.ContractKeeper authority string + channelKeeper channelkeeper.Keeper } ) @@ -35,6 +38,7 @@ func NewKeeper( bankKeeper types.BankKeeper, contractKeeper types.ContractKeeper, authority string, + channelKeeper channelkeeper.Keeper, ) Keeper { sortedKnownModules := make([]string, 0, len(maccPerms)) for moduleName := range maccPerms { @@ -50,6 +54,7 @@ func NewKeeper( bankKeeper: bankKeeper, contractKeeper: contractKeeper, authority: authority, + channelKeeper: channelKeeper, } } @@ -96,3 +101,14 @@ func (k Keeper) CreateModuleAccount(ctx sdk.Context) { // GetModuleAccount creates new module account if not present under the hood k.accountKeeper.GetModuleAccount(ctx, types.ModuleName) } + +func (k Keeper) GetAllEscrowAccounts(ctx sdk.Context) []sdk.AccAddress { + var escrowAddresses []sdk.AccAddress + + transferChannels := k.channelKeeper.GetAllChannels(ctx) + for _, channel := range transferChannels { + escrowAddresses = append(escrowAddresses, transfertypes.GetEscrowAddress(channel.PortId, channel.ChannelId)) + } + + return escrowAddresses +} From 0935c098dde089f3e0be40f049368ecfc2dc96b5 Mon Sep 17 00:00:00 2001 From: Andrei Zavgorodnii Date: Thu, 19 Dec 2024 13:00:05 +0000 Subject: [PATCH 02/15] better assertions --- ...{tokenfactory_transfer_test.go => tokenfactory_test.go} | 4 ++-- x/tokenfactory/keeper/bankactions.go | 6 +++--- x/tokenfactory/keeper/keeper.go | 7 ++++--- 3 files changed, 9 insertions(+), 8 deletions(-) rename tests/ibc/{tokenfactory_transfer_test.go => tokenfactory_test.go} (94%) diff --git a/tests/ibc/tokenfactory_transfer_test.go b/tests/ibc/tokenfactory_test.go similarity index 94% rename from tests/ibc/tokenfactory_transfer_test.go rename to tests/ibc/tokenfactory_test.go index 610f92a4f..0402bfef6 100644 --- a/tests/ibc/tokenfactory_transfer_test.go +++ b/tests/ibc/tokenfactory_test.go @@ -51,7 +51,7 @@ func (s *TokenfactoryTestSuite) TestForceTransferFromIBCEscrow() { // Transfer tokens out of escrow address forceTransferMsg := tftypes.NewMsgForceTransfer(s.neutronAddr.String(), sdk.NewCoin(amount.Denom, amount.Amount), escrowAddress.String(), s.neutronAddr.String()) _, err = s.neutronChain.SendMsgs(forceTransferMsg) - s.Assert().Error(err) + s.Assert().ErrorContains(err, "force transfer from IBC escrow accounts is forbidden") } func (s *TokenfactoryTestSuite) TestBurnFromIBCEscrow() { @@ -86,5 +86,5 @@ func (s *TokenfactoryTestSuite) TestBurnFromIBCEscrow() { // Burn tokens from escrow address burnMsg := tftypes.NewMsgBurnFrom(s.neutronAddr.String(), amount, escrowAddress.String()) _, err = s.neutronChain.SendMsgs(burnMsg) - s.Assert().Error(err) + s.Assert().ErrorContains(err, "burning from escrow accounts is forbidden") } diff --git a/x/tokenfactory/keeper/bankactions.go b/x/tokenfactory/keeper/bankactions.go index 5f883f63b..d355e323a 100644 --- a/x/tokenfactory/keeper/bankactions.go +++ b/x/tokenfactory/keeper/bankactions.go @@ -52,7 +52,7 @@ func (k Keeper) burnFrom(ctx sdk.Context, amount sdk.Coin, burnFrom string) erro escrowAccounts := k.GetAllEscrowAccounts(ctx) - if k.isEscrowAccount(escrowAccounts, burnFromAcc) { + if _, ok := escrowAccounts[burnFromAcc.String()]; ok { return status.Errorf(codes.Internal, "burning from escrow accounts is forbidden") } @@ -94,11 +94,11 @@ func (k Keeper) forceTransfer(ctx sdk.Context, amount sdk.Coin, fromAddr, toAddr escrowAccounts := k.GetAllEscrowAccounts(ctx) - if k.isEscrowAccount(escrowAccounts, transferFromAcc) { + if _, ok := escrowAccounts[transferFromAcc.String()]; ok { return status.Errorf(codes.Internal, "force transfer from IBC escrow accounts is forbidden") } - if k.isEscrowAccount(escrowAccounts, transferToAcc) { + if _, ok := escrowAccounts[transferToAcc.String()]; ok { return status.Errorf(codes.Internal, "force transfer to IBC escrow accounts is forbidden") } diff --git a/x/tokenfactory/keeper/keeper.go b/x/tokenfactory/keeper/keeper.go index 0fa517ad8..3cdf299b8 100644 --- a/x/tokenfactory/keeper/keeper.go +++ b/x/tokenfactory/keeper/keeper.go @@ -102,12 +102,13 @@ func (k Keeper) CreateModuleAccount(ctx sdk.Context) { k.accountKeeper.GetModuleAccount(ctx, types.ModuleName) } -func (k Keeper) GetAllEscrowAccounts(ctx sdk.Context) []sdk.AccAddress { - var escrowAddresses []sdk.AccAddress +func (k Keeper) GetAllEscrowAccounts(ctx sdk.Context) map[string]bool { + escrowAddresses := make(map[string]bool) transferChannels := k.channelKeeper.GetAllChannels(ctx) for _, channel := range transferChannels { - escrowAddresses = append(escrowAddresses, transfertypes.GetEscrowAddress(channel.PortId, channel.ChannelId)) + escrowAddress := transfertypes.GetEscrowAddress(channel.PortId, channel.ChannelId) + escrowAddresses[escrowAddress.String()] = true } return escrowAddresses From b4bb189cee9433958ae9b73fba1c30d9e1f9f016 Mon Sep 17 00:00:00 2001 From: Andrei Zavgorodnii Date: Thu, 19 Dec 2024 14:30:42 +0000 Subject: [PATCH 03/15] removed unused functions, renamed other functions --- x/tokenfactory/keeper/bankactions.go | 14 ++------------ x/tokenfactory/keeper/keeper.go | 2 +- 2 files changed, 3 insertions(+), 13 deletions(-) diff --git a/x/tokenfactory/keeper/bankactions.go b/x/tokenfactory/keeper/bankactions.go index d355e323a..e0f4ae7de 100644 --- a/x/tokenfactory/keeper/bankactions.go +++ b/x/tokenfactory/keeper/bankactions.go @@ -50,7 +50,7 @@ func (k Keeper) burnFrom(ctx sdk.Context, amount sdk.Coin, burnFrom string) erro return status.Errorf(codes.Internal, "burning from module accounts is forbidden") } - escrowAccounts := k.GetAllEscrowAccounts(ctx) + escrowAccounts := k.GetAllIBCEscrowAccounts(ctx) if _, ok := escrowAccounts[burnFromAcc.String()]; ok { return status.Errorf(codes.Internal, "burning from escrow accounts is forbidden") @@ -92,7 +92,7 @@ func (k Keeper) forceTransfer(ctx sdk.Context, amount sdk.Coin, fromAddr, toAddr return status.Errorf(codes.Internal, "force transfer to module accounts is forbidden") } - escrowAccounts := k.GetAllEscrowAccounts(ctx) + escrowAccounts := k.GetAllIBCEscrowAccounts(ctx) if _, ok := escrowAccounts[transferFromAcc.String()]; ok { return status.Errorf(codes.Internal, "force transfer from IBC escrow accounts is forbidden") @@ -119,13 +119,3 @@ func (k Keeper) isModuleAccount(ctx sdk.Context, addr sdk.AccAddress) bool { return false } - -func (k Keeper) isEscrowAccount(escrowAccounts []sdk.AccAddress, addr sdk.AccAddress) bool { - for _, escrowAccount := range escrowAccounts { - if escrowAccount.Equals(addr) { - return true - } - } - - return false -} diff --git a/x/tokenfactory/keeper/keeper.go b/x/tokenfactory/keeper/keeper.go index 3cdf299b8..841863498 100644 --- a/x/tokenfactory/keeper/keeper.go +++ b/x/tokenfactory/keeper/keeper.go @@ -102,7 +102,7 @@ func (k Keeper) CreateModuleAccount(ctx sdk.Context) { k.accountKeeper.GetModuleAccount(ctx, types.ModuleName) } -func (k Keeper) GetAllEscrowAccounts(ctx sdk.Context) map[string]bool { +func (k Keeper) GetAllIBCEscrowAccounts(ctx sdk.Context) map[string]bool { escrowAddresses := make(map[string]bool) transferChannels := k.channelKeeper.GetAllChannels(ctx) From ab22b9d0119046b91e8284e6fcbe0a5b1bb1229d Mon Sep 17 00:00:00 2001 From: Aleksandr Pismenskiy Date: Thu, 19 Dec 2024 18:07:09 +0300 Subject: [PATCH 04/15] fix msg_server tests --- testutil/tokenfactory/keeper/tokenfactory.go | 2 ++ x/tokenfactory/keeper/keeper.go | 5 ++--- x/tokenfactory/keeper/msg_server_test.go | 18 +++++++++--------- x/tokenfactory/types/expected_keepers.go | 6 ++++++ 4 files changed, 19 insertions(+), 12 deletions(-) diff --git a/testutil/tokenfactory/keeper/tokenfactory.go b/testutil/tokenfactory/keeper/tokenfactory.go index f65b1ab31..67bae4639 100644 --- a/testutil/tokenfactory/keeper/tokenfactory.go +++ b/testutil/tokenfactory/keeper/tokenfactory.go @@ -24,6 +24,7 @@ func TokenFactoryKeeper( accountKeeper types.AccountKeeper, bankKeeper types.BankKeeper, contractKeeper types.ContractKeeper, + channelKeeper types.ChannelKeeper, ) (keeper.Keeper, sdk.Context) { storeKey := storetypes.NewKVStoreKey(types.StoreKey) memStoreKey := storetypes.NewMemoryStoreKey(types.MemStoreKey) @@ -45,6 +46,7 @@ func TokenFactoryKeeper( bankKeeper, contractKeeper, testutil.TestOwnerAddress, + channelKeeper, ) ctx := sdk.NewContext(stateStore, tmproto.Header{}, false, log.NewNopLogger()) diff --git a/x/tokenfactory/keeper/keeper.go b/x/tokenfactory/keeper/keeper.go index 841863498..23aa80c3c 100644 --- a/x/tokenfactory/keeper/keeper.go +++ b/x/tokenfactory/keeper/keeper.go @@ -11,7 +11,6 @@ import ( "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" transfertypes "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types" - channelkeeper "github.com/cosmos/ibc-go/v8/modules/core/04-channel/keeper" "github.com/neutron-org/neutron/v5/x/tokenfactory/types" ) @@ -25,7 +24,7 @@ type ( bankKeeper types.BankKeeper contractKeeper types.ContractKeeper authority string - channelKeeper channelkeeper.Keeper + channelKeeper types.ChannelKeeper } ) @@ -38,7 +37,7 @@ func NewKeeper( bankKeeper types.BankKeeper, contractKeeper types.ContractKeeper, authority string, - channelKeeper channelkeeper.Keeper, + channelKeeper types.ChannelKeeper, ) Keeper { sortedKnownModules := make([]string, 0, len(maccPerms)) for moduleName := range maccPerms { diff --git a/x/tokenfactory/keeper/msg_server_test.go b/x/tokenfactory/keeper/msg_server_test.go index 70bd4158d..e07ba0aa0 100644 --- a/x/tokenfactory/keeper/msg_server_test.go +++ b/x/tokenfactory/keeper/msg_server_test.go @@ -23,7 +23,7 @@ const ( ) func TestMsgCreateDenomValidate(t *testing.T) { - k, ctx := testkeeper.TokenFactoryKeeper(t, nil, nil, nil) + k, ctx := testkeeper.TokenFactoryKeeper(t, nil, nil, nil, nil) msgServer := keeper.NewMsgServerImpl(k) tests := []struct { @@ -68,7 +68,7 @@ func TestMsgCreateDenomValidate(t *testing.T) { } func TestMsgMintValidate(t *testing.T) { - k, ctx := testkeeper.TokenFactoryKeeper(t, nil, nil, nil) + k, ctx := testkeeper.TokenFactoryKeeper(t, nil, nil, nil, nil) msgServer := keeper.NewMsgServerImpl(k) tests := []struct { @@ -162,7 +162,7 @@ func TestMsgMintValidate(t *testing.T) { } func TestMsgBurnValidate(t *testing.T) { - k, ctx := testkeeper.TokenFactoryKeeper(t, nil, nil, nil) + k, ctx := testkeeper.TokenFactoryKeeper(t, nil, nil, nil, nil) msgServer := keeper.NewMsgServerImpl(k) tests := []struct { @@ -256,7 +256,7 @@ func TestMsgBurnValidate(t *testing.T) { } func TestMsgForceTransferValidate(t *testing.T) { - k, ctx := testkeeper.TokenFactoryKeeper(t, nil, nil, nil) + k, ctx := testkeeper.TokenFactoryKeeper(t, nil, nil, nil, nil) msgServer := keeper.NewMsgServerImpl(k) tests := []struct { @@ -375,7 +375,7 @@ func TestMsgForceTransferValidate(t *testing.T) { } func TestMsgChangeAdminValidate(t *testing.T) { - k, ctx := testkeeper.TokenFactoryKeeper(t, nil, nil, nil) + k, ctx := testkeeper.TokenFactoryKeeper(t, nil, nil, nil, nil) msgServer := keeper.NewMsgServerImpl(k) tests := []struct { @@ -450,7 +450,7 @@ func TestMsgChangeAdminValidate(t *testing.T) { } func TestMsgSetDenomMetadataValidate(t *testing.T) { - k, ctx := testkeeper.TokenFactoryKeeper(t, nil, nil, nil) + k, ctx := testkeeper.TokenFactoryKeeper(t, nil, nil, nil, nil) msgServer := keeper.NewMsgServerImpl(k) tests := []struct { @@ -781,7 +781,7 @@ func TestMsgSetDenomMetadataValidate(t *testing.T) { } func TestMsgSetBeforeSendHookValidate(t *testing.T) { - k, ctx := testkeeper.TokenFactoryKeeper(t, nil, nil, nil) + k, ctx := testkeeper.TokenFactoryKeeper(t, nil, nil, nil, nil) msgServer := keeper.NewMsgServerImpl(k) tests := []struct { @@ -847,7 +847,7 @@ func TestMsgSetBeforeSendHookValidate(t *testing.T) { } func TestMsgUpdateParamsValidate(t *testing.T) { - k, ctx := testkeeper.TokenFactoryKeeper(t, nil, nil, nil) + k, ctx := testkeeper.TokenFactoryKeeper(t, nil, nil, nil, nil) tests := []struct { name string @@ -913,7 +913,7 @@ func TestMsgUpdateParamsValidate(t *testing.T) { } func TestMsgUpdateParamsWhitelistedHooks(t *testing.T) { - k, ctx := testkeeper.TokenFactoryKeeper(t, nil, nil, nil) + k, ctx := testkeeper.TokenFactoryKeeper(t, nil, nil, nil, nil) tests := []struct { name string diff --git a/x/tokenfactory/types/expected_keepers.go b/x/tokenfactory/types/expected_keepers.go index 45e7d8b0e..8b95d9189 100644 --- a/x/tokenfactory/types/expected_keepers.go +++ b/x/tokenfactory/types/expected_keepers.go @@ -6,6 +6,7 @@ import ( wasmtypes "github.com/CosmWasm/wasmd/x/wasm/types" sdk "github.com/cosmos/cosmos-sdk/types" banktypes "github.com/cosmos/cosmos-sdk/x/bank/types" + channeltypes "github.com/cosmos/ibc-go/v8/modules/core/04-channel/types" ) type BankKeeper interface { @@ -39,3 +40,8 @@ type ContractKeeper interface { Sudo(ctx context.Context, contractAddress sdk.AccAddress, msg []byte) ([]byte, error) GetContractInfo(ctx context.Context, contractAddress sdk.AccAddress) *wasmtypes.ContractInfo } + +// ChannelKeeper defines the expected IBC channel keeper +type ChannelKeeper interface { + GetAllChannels(ctx sdk.Context) (channels []channeltypes.IdentifiedChannel) +} From 8a13eaccc061256910c76a53d80cb4d8a511c489 Mon Sep 17 00:00:00 2001 From: Aleksandr Pismenskiy Date: Thu, 19 Dec 2024 19:19:17 +0300 Subject: [PATCH 05/15] v5.0.5 upgrade handler --- app/app.go | 2 ++ app/upgrades/v5.0.5/constants.go | 18 ++++++++++++++ app/upgrades/v5.0.5/upgrades.go | 35 ++++++++++++++++++++++++++ app/upgrades/v5.0.5/upgrades_test.go | 37 ++++++++++++++++++++++++++++ 4 files changed, 92 insertions(+) create mode 100644 app/upgrades/v5.0.5/constants.go create mode 100644 app/upgrades/v5.0.5/upgrades.go create mode 100644 app/upgrades/v5.0.5/upgrades_test.go diff --git a/app/app.go b/app/app.go index 30967e9f4..738bc66f9 100644 --- a/app/app.go +++ b/app/app.go @@ -12,6 +12,7 @@ import ( v502 "github.com/neutron-org/neutron/v5/app/upgrades/v5.0.2" v504 "github.com/neutron-org/neutron/v5/app/upgrades/v5.0.4" + v505 "github.com/neutron-org/neutron/v5/app/upgrades/v5.0.5" dynamicfeestypes "github.com/neutron-org/neutron/v5/x/dynamicfees/types" "github.com/skip-mev/feemarket/x/feemarket" @@ -232,6 +233,7 @@ var ( v500.Upgrade, v502.Upgrade, v504.Upgrade, + v505.Upgrade, } // DefaultNodeHome default home directories for the application daemon diff --git a/app/upgrades/v5.0.5/constants.go b/app/upgrades/v5.0.5/constants.go new file mode 100644 index 000000000..5bb7590d9 --- /dev/null +++ b/app/upgrades/v5.0.5/constants.go @@ -0,0 +1,18 @@ +package v505 + +import ( + storetypes "cosmossdk.io/store/types" + + "github.com/neutron-org/neutron/v5/app/upgrades" +) + +const ( + // UpgradeName defines the on-chain upgrade name. + UpgradeName = "v5.0.5" +) + +var Upgrade = upgrades.Upgrade{ + UpgradeName: UpgradeName, + CreateUpgradeHandler: CreateUpgradeHandler, + StoreUpgrades: storetypes.StoreUpgrades{}, +} diff --git a/app/upgrades/v5.0.5/upgrades.go b/app/upgrades/v5.0.5/upgrades.go new file mode 100644 index 000000000..fb3fe9444 --- /dev/null +++ b/app/upgrades/v5.0.5/upgrades.go @@ -0,0 +1,35 @@ +package v505 + +import ( + "context" + "fmt" + + upgradetypes "cosmossdk.io/x/upgrade/types" + "github.com/cosmos/cosmos-sdk/codec" + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/types/module" + + "github.com/neutron-org/neutron/v5/app/upgrades" +) + +func CreateUpgradeHandler( + mm *module.Manager, + configurator module.Configurator, + _ *upgrades.UpgradeKeepers, + _ upgrades.StoreKeys, + _ codec.Codec, +) upgradetypes.UpgradeHandler { + return func(c context.Context, _ upgradetypes.Plan, vm module.VersionMap) (module.VersionMap, error) { + ctx := sdk.UnwrapSDKContext(c) + + ctx.Logger().Info("Starting module migrations...") + + vm, err := mm.RunMigrations(ctx, configurator, vm) + if err != nil { + return vm, err + } + + ctx.Logger().Info(fmt.Sprintf("Migration {%s} applied", UpgradeName)) + return vm, nil + } +} diff --git a/app/upgrades/v5.0.5/upgrades_test.go b/app/upgrades/v5.0.5/upgrades_test.go new file mode 100644 index 000000000..3590362bd --- /dev/null +++ b/app/upgrades/v5.0.5/upgrades_test.go @@ -0,0 +1,37 @@ +package v505_test + +import ( + "testing" + + upgradetypes "cosmossdk.io/x/upgrade/types" + "github.com/stretchr/testify/require" + "github.com/stretchr/testify/suite" + + v505 "github.com/neutron-org/neutron/v5/app/upgrades/v5.0.5" + "github.com/neutron-org/neutron/v5/testutil" +) + +type UpgradeTestSuite struct { + testutil.IBCConnectionTestSuite +} + +func TestKeeperTestSuite(t *testing.T) { + suite.Run(t, new(UpgradeTestSuite)) +} + +func (suite *UpgradeTestSuite) SetupTest() { + suite.IBCConnectionTestSuite.SetupTest() +} + +func (suite *UpgradeTestSuite) TestOracleUpgrade() { + app := suite.GetNeutronZoneApp(suite.ChainA) + ctx := suite.ChainA.GetContext().WithChainID("neutron-1") + t := suite.T() + + upgrade := upgradetypes.Plan{ + Name: v505.UpgradeName, + Info: "some text here", + Height: 100, + } + require.NoError(t, app.UpgradeKeeper.ApplyUpgrade(ctx, upgrade)) +} From 66925ca06df2e34d55638694de251fb87043f5e8 Mon Sep 17 00:00:00 2001 From: Aleksandr Pismenskiy Date: Thu, 19 Dec 2024 19:53:09 +0300 Subject: [PATCH 06/15] add TestForceTransferToIBCEscrow --- tests/ibc/tokenfactory_test.go | 35 ++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/tests/ibc/tokenfactory_test.go b/tests/ibc/tokenfactory_test.go index 0402bfef6..fa3e3f160 100644 --- a/tests/ibc/tokenfactory_test.go +++ b/tests/ibc/tokenfactory_test.go @@ -54,6 +54,41 @@ func (s *TokenfactoryTestSuite) TestForceTransferFromIBCEscrow() { s.Assert().ErrorContains(err, "force transfer from IBC escrow accounts is forbidden") } +func (s *TokenfactoryTestSuite) TestForceTransferToIBCEscrow() { + // Create token factory denom + createDenomMsg := tftypes.NewMsgCreateDenom(s.neutronAddr.String(), "testtest") + _, err := s.neutronChain.SendMsgs(createDenomMsg) + s.Assert().NoError(err) + + // Derive full token factory denom + denom := fmt.Sprintf("factory/%s/%s", createDenomMsg.Sender, createDenomMsg.Subdenom) + + // Mint denom to sender + amount := sdk.NewCoin(denom, math.NewInt(10000000)) + mintMsg := tftypes.NewMsgMint(createDenomMsg.Sender, amount) + _, err = s.neutronChain.SendMsgs(mintMsg) + s.Assert().NoError(err) + + // Send IBC transfer + s.IBCTransfer( + s.neutronTransferPath, + s.neutronTransferPath.EndpointA, + s.neutronAddr, + s.providerAddr, + amount.Denom, + amount.Amount, + "", + ) + + // Derive IBC escrow address for channel + escrowAddress := transfertypes.GetEscrowAddress("transfer", s.neutronTransferPath.EndpointA.ChannelID) + + // Transfer tokens out of escrow address + forceTransferMsg := tftypes.NewMsgForceTransfer(s.neutronAddr.String(), sdk.NewCoin(amount.Denom, amount.Amount), s.neutronAddr.String(), escrowAddress.String()) + _, err = s.neutronChain.SendMsgs(forceTransferMsg) + s.Assert().ErrorContains(err, "force transfer to IBC escrow accounts is forbidden") +} + func (s *TokenfactoryTestSuite) TestBurnFromIBCEscrow() { // Create token factory denom createDenomMsg := tftypes.NewMsgCreateDenom(s.neutronAddr.String(), "testtest") From 3c6f5d6e900951fd41105ab9248d808787462f95 Mon Sep 17 00:00:00 2001 From: Andrei Zavgorodnii Date: Thu, 19 Dec 2024 22:35:53 +0000 Subject: [PATCH 07/15] filter channels by port id --- x/tokenfactory/keeper/keeper.go | 2 +- x/tokenfactory/types/expected_keepers.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/x/tokenfactory/keeper/keeper.go b/x/tokenfactory/keeper/keeper.go index 23aa80c3c..011480f71 100644 --- a/x/tokenfactory/keeper/keeper.go +++ b/x/tokenfactory/keeper/keeper.go @@ -104,7 +104,7 @@ func (k Keeper) CreateModuleAccount(ctx sdk.Context) { func (k Keeper) GetAllIBCEscrowAccounts(ctx sdk.Context) map[string]bool { escrowAddresses := make(map[string]bool) - transferChannels := k.channelKeeper.GetAllChannels(ctx) + transferChannels := k.channelKeeper.GetAllChannelsWithPortPrefix(ctx, transfertypes.PortID) for _, channel := range transferChannels { escrowAddress := transfertypes.GetEscrowAddress(channel.PortId, channel.ChannelId) escrowAddresses[escrowAddress.String()] = true diff --git a/x/tokenfactory/types/expected_keepers.go b/x/tokenfactory/types/expected_keepers.go index 8b95d9189..18e66484b 100644 --- a/x/tokenfactory/types/expected_keepers.go +++ b/x/tokenfactory/types/expected_keepers.go @@ -43,5 +43,5 @@ type ContractKeeper interface { // ChannelKeeper defines the expected IBC channel keeper type ChannelKeeper interface { - GetAllChannels(ctx sdk.Context) (channels []channeltypes.IdentifiedChannel) + GetAllChannelsWithPortPrefix(ctx sdk.Context, portPrefix string) (channels []channeltypes.IdentifiedChannel) } From 13fd9f825a11a2bf9ad9353c425afce9cc7a230b Mon Sep 17 00:00:00 2001 From: Andrei Zavgorodnii Date: Thu, 19 Dec 2024 23:05:31 +0000 Subject: [PATCH 08/15] fixed tests --- app/app.go | 1 + testutil/tokenfactory/keeper/tokenfactory.go | 2 ++ x/tokenfactory/keeper/keeper.go | 8 +++++--- x/tokenfactory/keeper/msg_server_test.go | 18 +++++++++--------- x/tokenfactory/types/expected_keepers.go | 5 +++++ 5 files changed, 22 insertions(+), 12 deletions(-) diff --git a/app/app.go b/app/app.go index 738bc66f9..f4455e490 100644 --- a/app/app.go +++ b/app/app.go @@ -697,6 +697,7 @@ func New( &app.WasmKeeper, authtypes.NewModuleAddress(adminmoduletypes.ModuleName).String(), app.IBCKeeper.ChannelKeeper, + app.TransferKeeper.Keeper, ) app.TokenFactoryKeeper = &tokenFactoryKeeper diff --git a/testutil/tokenfactory/keeper/tokenfactory.go b/testutil/tokenfactory/keeper/tokenfactory.go index 67bae4639..e39125a26 100644 --- a/testutil/tokenfactory/keeper/tokenfactory.go +++ b/testutil/tokenfactory/keeper/tokenfactory.go @@ -25,6 +25,7 @@ func TokenFactoryKeeper( bankKeeper types.BankKeeper, contractKeeper types.ContractKeeper, channelKeeper types.ChannelKeeper, + transferKeeper types.TransferKeeper, ) (keeper.Keeper, sdk.Context) { storeKey := storetypes.NewKVStoreKey(types.StoreKey) memStoreKey := storetypes.NewMemoryStoreKey(types.MemStoreKey) @@ -47,6 +48,7 @@ func TokenFactoryKeeper( contractKeeper, testutil.TestOwnerAddress, channelKeeper, + transferKeeper, ) ctx := sdk.NewContext(stateStore, tmproto.Header{}, false, log.NewNopLogger()) diff --git a/x/tokenfactory/keeper/keeper.go b/x/tokenfactory/keeper/keeper.go index 011480f71..7775b836e 100644 --- a/x/tokenfactory/keeper/keeper.go +++ b/x/tokenfactory/keeper/keeper.go @@ -3,6 +3,7 @@ package keeper import ( "context" "fmt" + transfertypes "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types" "sort" "cosmossdk.io/log" @@ -10,8 +11,6 @@ import ( storetypes "cosmossdk.io/store/types" "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" - transfertypes "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types" - "github.com/neutron-org/neutron/v5/x/tokenfactory/types" ) @@ -25,6 +24,7 @@ type ( contractKeeper types.ContractKeeper authority string channelKeeper types.ChannelKeeper + transferKeeper types.TransferKeeper } ) @@ -38,6 +38,7 @@ func NewKeeper( contractKeeper types.ContractKeeper, authority string, channelKeeper types.ChannelKeeper, + transferKeeper types.TransferKeeper, ) Keeper { sortedKnownModules := make([]string, 0, len(maccPerms)) for moduleName := range maccPerms { @@ -54,6 +55,7 @@ func NewKeeper( contractKeeper: contractKeeper, authority: authority, channelKeeper: channelKeeper, + transferKeeper: transferKeeper, } } @@ -104,7 +106,7 @@ func (k Keeper) CreateModuleAccount(ctx sdk.Context) { func (k Keeper) GetAllIBCEscrowAccounts(ctx sdk.Context) map[string]bool { escrowAddresses := make(map[string]bool) - transferChannels := k.channelKeeper.GetAllChannelsWithPortPrefix(ctx, transfertypes.PortID) + transferChannels := k.channelKeeper.GetAllChannelsWithPortPrefix(ctx, k.transferKeeper.GetPort(ctx)) for _, channel := range transferChannels { escrowAddress := transfertypes.GetEscrowAddress(channel.PortId, channel.ChannelId) escrowAddresses[escrowAddress.String()] = true diff --git a/x/tokenfactory/keeper/msg_server_test.go b/x/tokenfactory/keeper/msg_server_test.go index e07ba0aa0..ef41cec97 100644 --- a/x/tokenfactory/keeper/msg_server_test.go +++ b/x/tokenfactory/keeper/msg_server_test.go @@ -23,7 +23,7 @@ const ( ) func TestMsgCreateDenomValidate(t *testing.T) { - k, ctx := testkeeper.TokenFactoryKeeper(t, nil, nil, nil, nil) + k, ctx := testkeeper.TokenFactoryKeeper(t, nil, nil, nil, nil, nil) msgServer := keeper.NewMsgServerImpl(k) tests := []struct { @@ -68,7 +68,7 @@ func TestMsgCreateDenomValidate(t *testing.T) { } func TestMsgMintValidate(t *testing.T) { - k, ctx := testkeeper.TokenFactoryKeeper(t, nil, nil, nil, nil) + k, ctx := testkeeper.TokenFactoryKeeper(t, nil, nil, nil, nil, nil) msgServer := keeper.NewMsgServerImpl(k) tests := []struct { @@ -162,7 +162,7 @@ func TestMsgMintValidate(t *testing.T) { } func TestMsgBurnValidate(t *testing.T) { - k, ctx := testkeeper.TokenFactoryKeeper(t, nil, nil, nil, nil) + k, ctx := testkeeper.TokenFactoryKeeper(t, nil, nil, nil, nil, nil) msgServer := keeper.NewMsgServerImpl(k) tests := []struct { @@ -256,7 +256,7 @@ func TestMsgBurnValidate(t *testing.T) { } func TestMsgForceTransferValidate(t *testing.T) { - k, ctx := testkeeper.TokenFactoryKeeper(t, nil, nil, nil, nil) + k, ctx := testkeeper.TokenFactoryKeeper(t, nil, nil, nil, nil, nil) msgServer := keeper.NewMsgServerImpl(k) tests := []struct { @@ -375,7 +375,7 @@ func TestMsgForceTransferValidate(t *testing.T) { } func TestMsgChangeAdminValidate(t *testing.T) { - k, ctx := testkeeper.TokenFactoryKeeper(t, nil, nil, nil, nil) + k, ctx := testkeeper.TokenFactoryKeeper(t, nil, nil, nil, nil, nil) msgServer := keeper.NewMsgServerImpl(k) tests := []struct { @@ -450,7 +450,7 @@ func TestMsgChangeAdminValidate(t *testing.T) { } func TestMsgSetDenomMetadataValidate(t *testing.T) { - k, ctx := testkeeper.TokenFactoryKeeper(t, nil, nil, nil, nil) + k, ctx := testkeeper.TokenFactoryKeeper(t, nil, nil, nil, nil, nil) msgServer := keeper.NewMsgServerImpl(k) tests := []struct { @@ -781,7 +781,7 @@ func TestMsgSetDenomMetadataValidate(t *testing.T) { } func TestMsgSetBeforeSendHookValidate(t *testing.T) { - k, ctx := testkeeper.TokenFactoryKeeper(t, nil, nil, nil, nil) + k, ctx := testkeeper.TokenFactoryKeeper(t, nil, nil, nil, nil, nil) msgServer := keeper.NewMsgServerImpl(k) tests := []struct { @@ -847,7 +847,7 @@ func TestMsgSetBeforeSendHookValidate(t *testing.T) { } func TestMsgUpdateParamsValidate(t *testing.T) { - k, ctx := testkeeper.TokenFactoryKeeper(t, nil, nil, nil, nil) + k, ctx := testkeeper.TokenFactoryKeeper(t, nil, nil, nil, nil, nil) tests := []struct { name string @@ -913,7 +913,7 @@ func TestMsgUpdateParamsValidate(t *testing.T) { } func TestMsgUpdateParamsWhitelistedHooks(t *testing.T) { - k, ctx := testkeeper.TokenFactoryKeeper(t, nil, nil, nil, nil) + k, ctx := testkeeper.TokenFactoryKeeper(t, nil, nil, nil, nil, nil) tests := []struct { name string diff --git a/x/tokenfactory/types/expected_keepers.go b/x/tokenfactory/types/expected_keepers.go index 18e66484b..7559ea7ba 100644 --- a/x/tokenfactory/types/expected_keepers.go +++ b/x/tokenfactory/types/expected_keepers.go @@ -45,3 +45,8 @@ type ContractKeeper interface { type ChannelKeeper interface { GetAllChannelsWithPortPrefix(ctx sdk.Context, portPrefix string) (channels []channeltypes.IdentifiedChannel) } + +// TransferKeeper defines the expected IBC Transfer keeper +type TransferKeeper interface { + GetPort(ctx sdk.Context) string +} From b3c00a20832c259d3f861da97c0a56ee2bdfc7a1 Mon Sep 17 00:00:00 2001 From: Andrei Zavgorodnii Date: Fri, 20 Dec 2024 13:53:36 +0000 Subject: [PATCH 09/15] better tests --- app/app.go | 27 ++++----- app/upgrades/types.go | 2 + app/upgrades/v5.0.5/upgrades.go | 9 ++- .../contractmanager/types/expected_keepers.go | 1 - .../mocks/interchainqueries/keeper/verify.go | 1 - .../interchaintxs/types/expected_keepers.go | 1 - .../mocks/transfer/types/expected_keepers.go | 36 +++++++++++- testutil/tokenfactory/keeper/tokenfactory.go | 4 -- x/tokenfactory/keeper/bankactions.go | 10 +--- x/tokenfactory/keeper/escrow_addresses.go | 21 +++++++ x/tokenfactory/keeper/keeper.go | 19 ------- x/tokenfactory/keeper/msg_server_test.go | 18 +++--- x/tokenfactory/types/expected_keepers.go | 11 ---- x/tokenfactory/types/keys.go | 2 + x/transfer/ibc_handlers_test.go | 7 ++- x/transfer/keeper/keeper.go | 2 - x/transfer/module.go | 57 +++++++++++++++---- x/transfer/types/expected_keepers.go | 5 ++ 18 files changed, 151 insertions(+), 82 deletions(-) create mode 100644 x/tokenfactory/keeper/escrow_addresses.go diff --git a/app/app.go b/app/app.go index f4455e490..0772f0f59 100644 --- a/app/app.go +++ b/app/app.go @@ -625,6 +625,17 @@ func New( authtypes.NewModuleAddress(adminmoduletypes.ModuleName).String(), ) + tokenFactoryKeeper := tokenfactorykeeper.NewKeeper( + appCodec, + app.keys[tokenfactorytypes.StoreKey], + maccPerms, + app.AccountKeeper, + &app.BankKeeper, + &app.WasmKeeper, + authtypes.NewModuleAddress(adminmoduletypes.ModuleName).String(), + ) + app.TokenFactoryKeeper = &tokenFactoryKeeper + app.WireICS20PreWasmKeeper(appCodec) app.PFMModule = packetforward.NewAppModule(app.PFMKeeper, app.GetSubspace(pfmtypes.ModuleName)) @@ -688,19 +699,6 @@ func New( app.ConsumerKeeper = *app.ConsumerKeeper.SetHooks(app.SlashingKeeper.Hooks()) consumerModule := ccvconsumer.NewAppModule(app.ConsumerKeeper, app.GetSubspace(ccvconsumertypes.ModuleName)) - tokenFactoryKeeper := tokenfactorykeeper.NewKeeper( - appCodec, - app.keys[tokenfactorytypes.StoreKey], - maccPerms, - app.AccountKeeper, - &app.BankKeeper, - &app.WasmKeeper, - authtypes.NewModuleAddress(adminmoduletypes.ModuleName).String(), - app.IBCKeeper.ChannelKeeper, - app.TransferKeeper.Keeper, - ) - app.TokenFactoryKeeper = &tokenFactoryKeeper - app.BankKeeper.BaseSendKeeper = app.BankKeeper.BaseSendKeeper.SetHooks( banktypes.NewMultiBankHooks( app.TokenFactoryKeeper.Hooks(), @@ -1377,6 +1375,8 @@ func (app *App) setupUpgradeHandlers() { DynamicfeesKeeper: app.DynamicFeesKeeper, DexKeeper: &app.DexKeeper, IbcRateLimitKeeper: app.RateLimitingICS4Wrapper.IbcratelimitKeeper, + ChannelKeeper: &app.IBCKeeper.ChannelKeeper, + TransferKeeper: app.TransferKeeper.Keeper, GlobalFeeSubspace: app.GetSubspace(globalfee.ModuleName), CcvConsumerSubspace: app.GetSubspace(ccvconsumertypes.ModuleName), }, @@ -1726,6 +1726,7 @@ func (app *App) WireICS20PreWasmKeeper( transferSudo.NewIBCModule( app.TransferKeeper, contractmanager.NewSudoLimitWrapper(app.ContractManagerKeeper, &app.WasmKeeper), + app.TokenFactoryKeeper, ), app.PFMKeeper, 0, diff --git a/app/upgrades/types.go b/app/upgrades/types.go index 8c52a43b2..72cf7f709 100644 --- a/app/upgrades/types.go +++ b/app/upgrades/types.go @@ -12,6 +12,7 @@ import ( paramskeeper "github.com/cosmos/cosmos-sdk/x/params/keeper" slashingkeeper "github.com/cosmos/cosmos-sdk/x/slashing/keeper" capabilitykeeper "github.com/cosmos/ibc-go/modules/capability/keeper" + channelkeeper "github.com/cosmos/ibc-go/v8/modules/core/04-channel/keeper" ccvconsumerkeeper "github.com/cosmos/interchain-security/v5/x/ccv/consumer/keeper" auctionkeeper "github.com/skip-mev/block-sdk/v2/x/auction/keeper" feemarketkeeper "github.com/skip-mev/feemarket/x/feemarket/keeper" @@ -69,6 +70,7 @@ type UpgradeKeepers struct { DynamicfeesKeeper *dynamicfeeskeeper.Keeper DexKeeper *dexkeeper.Keeper IbcRateLimitKeeper *ibcratelimitkeeper.Keeper + ChannelKeeper *channelkeeper.Keeper // subspaces GlobalFeeSubspace paramtypes.Subspace CcvConsumerSubspace paramtypes.Subspace diff --git a/app/upgrades/v5.0.5/upgrades.go b/app/upgrades/v5.0.5/upgrades.go index fb3fe9444..0ba93e8f2 100644 --- a/app/upgrades/v5.0.5/upgrades.go +++ b/app/upgrades/v5.0.5/upgrades.go @@ -3,6 +3,7 @@ package v505 import ( "context" "fmt" + transfertypes "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types" upgradetypes "cosmossdk.io/x/upgrade/types" "github.com/cosmos/cosmos-sdk/codec" @@ -15,7 +16,7 @@ import ( func CreateUpgradeHandler( mm *module.Manager, configurator module.Configurator, - _ *upgrades.UpgradeKeepers, + keepers *upgrades.UpgradeKeepers, _ upgrades.StoreKeys, _ codec.Codec, ) upgradetypes.UpgradeHandler { @@ -29,6 +30,12 @@ func CreateUpgradeHandler( return vm, err } + transferChannels := keepers.ChannelKeeper.GetAllChannelsWithPortPrefix(ctx, keepers.TransferKeeper.GetPort(ctx)) + for _, channel := range transferChannels { + escrowAddress := transfertypes.GetEscrowAddress(channel.PortId, channel.ChannelId) + keepers.TokenFactoryKeeper.StoreEscrowAddress(ctx, escrowAddress) + } + ctx.Logger().Info(fmt.Sprintf("Migration {%s} applied", UpgradeName)) return vm, nil } diff --git a/testutil/mocks/contractmanager/types/expected_keepers.go b/testutil/mocks/contractmanager/types/expected_keepers.go index 5d2ef0a4f..7ff82c05c 100644 --- a/testutil/mocks/contractmanager/types/expected_keepers.go +++ b/testutil/mocks/contractmanager/types/expected_keepers.go @@ -11,7 +11,6 @@ import ( types "github.com/CosmWasm/wasmd/x/wasm/types" types0 "github.com/cosmos/cosmos-sdk/types" gomock "github.com/golang/mock/gomock" - types1 "github.com/neutron-org/neutron/v5/x/contractmanager/types" ) diff --git a/testutil/mocks/interchainqueries/keeper/verify.go b/testutil/mocks/interchainqueries/keeper/verify.go index 02f03e7d9..b8cb86c6e 100644 --- a/testutil/mocks/interchainqueries/keeper/verify.go +++ b/testutil/mocks/interchainqueries/keeper/verify.go @@ -13,7 +13,6 @@ import ( exported "github.com/cosmos/ibc-go/v8/modules/core/exported" tendermint "github.com/cosmos/ibc-go/v8/modules/light-clients/07-tendermint" gomock "github.com/golang/mock/gomock" - types1 "github.com/neutron-org/neutron/v5/x/interchainqueries/types" ) diff --git a/testutil/mocks/interchaintxs/types/expected_keepers.go b/testutil/mocks/interchaintxs/types/expected_keepers.go index 0c1d7c3b1..67cc758ab 100644 --- a/testutil/mocks/interchaintxs/types/expected_keepers.go +++ b/testutil/mocks/interchaintxs/types/expected_keepers.go @@ -14,7 +14,6 @@ import ( types2 "github.com/cosmos/ibc-go/v8/modules/core/04-channel/types" exported "github.com/cosmos/ibc-go/v8/modules/core/exported" gomock "github.com/golang/mock/gomock" - types3 "github.com/neutron-org/neutron/v5/x/feerefunder/types" ) diff --git a/testutil/mocks/transfer/types/expected_keepers.go b/testutil/mocks/transfer/types/expected_keepers.go index 9e952f77b..4789cb264 100644 --- a/testutil/mocks/transfer/types/expected_keepers.go +++ b/testutil/mocks/transfer/types/expected_keepers.go @@ -11,7 +11,6 @@ import ( types "github.com/cosmos/cosmos-sdk/types" types0 "github.com/cosmos/ibc-go/v8/modules/core/04-channel/types" gomock "github.com/golang/mock/gomock" - types1 "github.com/neutron-org/neutron/v5/x/feerefunder/types" ) @@ -245,3 +244,38 @@ func (mr *MockAccountKeeperMockRecorder) GetModuleAddress(name interface{}) *gom mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetModuleAddress", reflect.TypeOf((*MockAccountKeeper)(nil).GetModuleAddress), name) } + +// MockTokenfactoryKeeper is a mock of TokenfactoryKeeper interface. +type MockTokenfactoryKeeper struct { + ctrl *gomock.Controller + recorder *MockTokenfactoryKeeperMockRecorder +} + +// MockTokenfactoryKeeperMockRecorder is the mock recorder for MockTokenfactoryKeeper. +type MockTokenfactoryKeeperMockRecorder struct { + mock *MockTokenfactoryKeeper +} + +// NewMockTokenfactoryKeeper creates a new mock instance. +func NewMockTokenfactoryKeeper(ctrl *gomock.Controller) *MockTokenfactoryKeeper { + mock := &MockTokenfactoryKeeper{ctrl: ctrl} + mock.recorder = &MockTokenfactoryKeeperMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockTokenfactoryKeeper) EXPECT() *MockTokenfactoryKeeperMockRecorder { + return m.recorder +} + +// StoreEscrowAddress mocks base method. +func (m *MockTokenfactoryKeeper) StoreEscrowAddress(ctx types.Context, address types.AccAddress) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "StoreEscrowAddress", ctx, address) +} + +// StoreEscrowAddress indicates an expected call of StoreEscrowAddress. +func (mr *MockTokenfactoryKeeperMockRecorder) StoreEscrowAddress(ctx, address interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "StoreEscrowAddress", reflect.TypeOf((*MockTokenfactoryKeeper)(nil).StoreEscrowAddress), ctx, address) +} diff --git a/testutil/tokenfactory/keeper/tokenfactory.go b/testutil/tokenfactory/keeper/tokenfactory.go index e39125a26..f65b1ab31 100644 --- a/testutil/tokenfactory/keeper/tokenfactory.go +++ b/testutil/tokenfactory/keeper/tokenfactory.go @@ -24,8 +24,6 @@ func TokenFactoryKeeper( accountKeeper types.AccountKeeper, bankKeeper types.BankKeeper, contractKeeper types.ContractKeeper, - channelKeeper types.ChannelKeeper, - transferKeeper types.TransferKeeper, ) (keeper.Keeper, sdk.Context) { storeKey := storetypes.NewKVStoreKey(types.StoreKey) memStoreKey := storetypes.NewMemoryStoreKey(types.MemStoreKey) @@ -47,8 +45,6 @@ func TokenFactoryKeeper( bankKeeper, contractKeeper, testutil.TestOwnerAddress, - channelKeeper, - transferKeeper, ) ctx := sdk.NewContext(stateStore, tmproto.Header{}, false, log.NewNopLogger()) diff --git a/x/tokenfactory/keeper/bankactions.go b/x/tokenfactory/keeper/bankactions.go index e0f4ae7de..552bbdb0c 100644 --- a/x/tokenfactory/keeper/bankactions.go +++ b/x/tokenfactory/keeper/bankactions.go @@ -50,9 +50,7 @@ func (k Keeper) burnFrom(ctx sdk.Context, amount sdk.Coin, burnFrom string) erro return status.Errorf(codes.Internal, "burning from module accounts is forbidden") } - escrowAccounts := k.GetAllIBCEscrowAccounts(ctx) - - if _, ok := escrowAccounts[burnFromAcc.String()]; ok { + if k.isEscrowAddress(ctx, burnFromAcc) { return status.Errorf(codes.Internal, "burning from escrow accounts is forbidden") } @@ -92,13 +90,11 @@ func (k Keeper) forceTransfer(ctx sdk.Context, amount sdk.Coin, fromAddr, toAddr return status.Errorf(codes.Internal, "force transfer to module accounts is forbidden") } - escrowAccounts := k.GetAllIBCEscrowAccounts(ctx) - - if _, ok := escrowAccounts[transferFromAcc.String()]; ok { + if k.isEscrowAddress(ctx, transferFromAcc) { return status.Errorf(codes.Internal, "force transfer from IBC escrow accounts is forbidden") } - if _, ok := escrowAccounts[transferToAcc.String()]; ok { + if k.isEscrowAddress(ctx, transferToAcc) { return status.Errorf(codes.Internal, "force transfer to IBC escrow accounts is forbidden") } diff --git a/x/tokenfactory/keeper/escrow_addresses.go b/x/tokenfactory/keeper/escrow_addresses.go new file mode 100644 index 000000000..a4af97a13 --- /dev/null +++ b/x/tokenfactory/keeper/escrow_addresses.go @@ -0,0 +1,21 @@ +package keeper + +import ( + "cosmossdk.io/store/prefix" + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/neutron-org/neutron/v5/x/tokenfactory/types" +) + +// StoreEscrowAddress sets the total set of params. +func (k Keeper) StoreEscrowAddress(ctx sdk.Context, address sdk.AccAddress) { + prefixStore := prefix.NewStore(ctx.KVStore(k.storeKey), types.EscrowAddressKey) + + prefixStore.Set(address.Bytes(), []byte{0}) +} + +func (k Keeper) isEscrowAddress(ctx sdk.Context, address sdk.AccAddress) bool { + prefixStore := prefix.NewStore(ctx.KVStore(k.storeKey), types.EscrowAddressKey) + bz := prefixStore.Get(address.Bytes()) + + return len(bz) != 0 +} diff --git a/x/tokenfactory/keeper/keeper.go b/x/tokenfactory/keeper/keeper.go index 7775b836e..aae912cb2 100644 --- a/x/tokenfactory/keeper/keeper.go +++ b/x/tokenfactory/keeper/keeper.go @@ -3,7 +3,6 @@ package keeper import ( "context" "fmt" - transfertypes "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types" "sort" "cosmossdk.io/log" @@ -23,8 +22,6 @@ type ( bankKeeper types.BankKeeper contractKeeper types.ContractKeeper authority string - channelKeeper types.ChannelKeeper - transferKeeper types.TransferKeeper } ) @@ -37,8 +34,6 @@ func NewKeeper( bankKeeper types.BankKeeper, contractKeeper types.ContractKeeper, authority string, - channelKeeper types.ChannelKeeper, - transferKeeper types.TransferKeeper, ) Keeper { sortedKnownModules := make([]string, 0, len(maccPerms)) for moduleName := range maccPerms { @@ -54,8 +49,6 @@ func NewKeeper( bankKeeper: bankKeeper, contractKeeper: contractKeeper, authority: authority, - channelKeeper: channelKeeper, - transferKeeper: transferKeeper, } } @@ -102,15 +95,3 @@ func (k Keeper) CreateModuleAccount(ctx sdk.Context) { // GetModuleAccount creates new module account if not present under the hood k.accountKeeper.GetModuleAccount(ctx, types.ModuleName) } - -func (k Keeper) GetAllIBCEscrowAccounts(ctx sdk.Context) map[string]bool { - escrowAddresses := make(map[string]bool) - - transferChannels := k.channelKeeper.GetAllChannelsWithPortPrefix(ctx, k.transferKeeper.GetPort(ctx)) - for _, channel := range transferChannels { - escrowAddress := transfertypes.GetEscrowAddress(channel.PortId, channel.ChannelId) - escrowAddresses[escrowAddress.String()] = true - } - - return escrowAddresses -} diff --git a/x/tokenfactory/keeper/msg_server_test.go b/x/tokenfactory/keeper/msg_server_test.go index ef41cec97..70bd4158d 100644 --- a/x/tokenfactory/keeper/msg_server_test.go +++ b/x/tokenfactory/keeper/msg_server_test.go @@ -23,7 +23,7 @@ const ( ) func TestMsgCreateDenomValidate(t *testing.T) { - k, ctx := testkeeper.TokenFactoryKeeper(t, nil, nil, nil, nil, nil) + k, ctx := testkeeper.TokenFactoryKeeper(t, nil, nil, nil) msgServer := keeper.NewMsgServerImpl(k) tests := []struct { @@ -68,7 +68,7 @@ func TestMsgCreateDenomValidate(t *testing.T) { } func TestMsgMintValidate(t *testing.T) { - k, ctx := testkeeper.TokenFactoryKeeper(t, nil, nil, nil, nil, nil) + k, ctx := testkeeper.TokenFactoryKeeper(t, nil, nil, nil) msgServer := keeper.NewMsgServerImpl(k) tests := []struct { @@ -162,7 +162,7 @@ func TestMsgMintValidate(t *testing.T) { } func TestMsgBurnValidate(t *testing.T) { - k, ctx := testkeeper.TokenFactoryKeeper(t, nil, nil, nil, nil, nil) + k, ctx := testkeeper.TokenFactoryKeeper(t, nil, nil, nil) msgServer := keeper.NewMsgServerImpl(k) tests := []struct { @@ -256,7 +256,7 @@ func TestMsgBurnValidate(t *testing.T) { } func TestMsgForceTransferValidate(t *testing.T) { - k, ctx := testkeeper.TokenFactoryKeeper(t, nil, nil, nil, nil, nil) + k, ctx := testkeeper.TokenFactoryKeeper(t, nil, nil, nil) msgServer := keeper.NewMsgServerImpl(k) tests := []struct { @@ -375,7 +375,7 @@ func TestMsgForceTransferValidate(t *testing.T) { } func TestMsgChangeAdminValidate(t *testing.T) { - k, ctx := testkeeper.TokenFactoryKeeper(t, nil, nil, nil, nil, nil) + k, ctx := testkeeper.TokenFactoryKeeper(t, nil, nil, nil) msgServer := keeper.NewMsgServerImpl(k) tests := []struct { @@ -450,7 +450,7 @@ func TestMsgChangeAdminValidate(t *testing.T) { } func TestMsgSetDenomMetadataValidate(t *testing.T) { - k, ctx := testkeeper.TokenFactoryKeeper(t, nil, nil, nil, nil, nil) + k, ctx := testkeeper.TokenFactoryKeeper(t, nil, nil, nil) msgServer := keeper.NewMsgServerImpl(k) tests := []struct { @@ -781,7 +781,7 @@ func TestMsgSetDenomMetadataValidate(t *testing.T) { } func TestMsgSetBeforeSendHookValidate(t *testing.T) { - k, ctx := testkeeper.TokenFactoryKeeper(t, nil, nil, nil, nil, nil) + k, ctx := testkeeper.TokenFactoryKeeper(t, nil, nil, nil) msgServer := keeper.NewMsgServerImpl(k) tests := []struct { @@ -847,7 +847,7 @@ func TestMsgSetBeforeSendHookValidate(t *testing.T) { } func TestMsgUpdateParamsValidate(t *testing.T) { - k, ctx := testkeeper.TokenFactoryKeeper(t, nil, nil, nil, nil, nil) + k, ctx := testkeeper.TokenFactoryKeeper(t, nil, nil, nil) tests := []struct { name string @@ -913,7 +913,7 @@ func TestMsgUpdateParamsValidate(t *testing.T) { } func TestMsgUpdateParamsWhitelistedHooks(t *testing.T) { - k, ctx := testkeeper.TokenFactoryKeeper(t, nil, nil, nil, nil, nil) + k, ctx := testkeeper.TokenFactoryKeeper(t, nil, nil, nil) tests := []struct { name string diff --git a/x/tokenfactory/types/expected_keepers.go b/x/tokenfactory/types/expected_keepers.go index 7559ea7ba..45e7d8b0e 100644 --- a/x/tokenfactory/types/expected_keepers.go +++ b/x/tokenfactory/types/expected_keepers.go @@ -6,7 +6,6 @@ import ( wasmtypes "github.com/CosmWasm/wasmd/x/wasm/types" sdk "github.com/cosmos/cosmos-sdk/types" banktypes "github.com/cosmos/cosmos-sdk/x/bank/types" - channeltypes "github.com/cosmos/ibc-go/v8/modules/core/04-channel/types" ) type BankKeeper interface { @@ -40,13 +39,3 @@ type ContractKeeper interface { Sudo(ctx context.Context, contractAddress sdk.AccAddress, msg []byte) ([]byte, error) GetContractInfo(ctx context.Context, contractAddress sdk.AccAddress) *wasmtypes.ContractInfo } - -// ChannelKeeper defines the expected IBC channel keeper -type ChannelKeeper interface { - GetAllChannelsWithPortPrefix(ctx sdk.Context, portPrefix string) (channels []channeltypes.IdentifiedChannel) -} - -// TransferKeeper defines the expected IBC Transfer keeper -type TransferKeeper interface { - GetPort(ctx sdk.Context) string -} diff --git a/x/tokenfactory/types/keys.go b/x/tokenfactory/types/keys.go index 8bfeb012c..543775fdc 100644 --- a/x/tokenfactory/types/keys.go +++ b/x/tokenfactory/types/keys.go @@ -25,6 +25,7 @@ const ( const ( KeySeparator = "|" prefixParamsKey = iota + 1 + prefixEscrowAddressKey ) var ( @@ -34,6 +35,7 @@ var ( AdminPrefixKey = "admin" BeforeSendHookAddressPrefixKey = "beforesendhook" ParamsKey = []byte{prefixParamsKey} + EscrowAddressKey = []byte{prefixEscrowAddressKey} ) // GetDenomPrefixStore returns the store prefix where all the data associated with a specific denom diff --git a/x/transfer/ibc_handlers_test.go b/x/transfer/ibc_handlers_test.go index e6ccadc1a..e73b99be0 100644 --- a/x/transfer/ibc_handlers_test.go +++ b/x/transfer/ibc_handlers_test.go @@ -31,10 +31,12 @@ func TestHandleAcknowledgement(t *testing.T) { feeKeeper := mock_types.NewMockFeeRefunderKeeper(ctrl) chanKeeper := mock_types.NewMockChannelKeeper(ctrl) authKeeper := mock_types.NewMockAccountKeeper(ctrl) + tokenfactoryKeeper := mock_types.NewMockTokenfactoryKeeper(ctrl) + // required to initialize keeper authKeeper.EXPECT().GetModuleAddress(transfertypes.ModuleName).Return([]byte("address")) txKeeper, infCtx, _ := testkeeper.TransferKeeper(t, wmKeeper, feeKeeper, chanKeeper, authKeeper) - txModule := transfer.NewIBCModule(*txKeeper, wmKeeper) + txModule := transfer.NewIBCModule(*txKeeper, wmKeeper, tokenfactoryKeeper) ctx := infCtx.WithGasMeter(types2.NewGasMeter(1_000_000_000_000)) resACK := channeltypes.Acknowledgement{ @@ -114,10 +116,11 @@ func TestHandleTimeout(t *testing.T) { feeKeeper := mock_types.NewMockFeeRefunderKeeper(ctrl) chanKeeper := mock_types.NewMockChannelKeeper(ctrl) authKeeper := mock_types.NewMockAccountKeeper(ctrl) + tokenfactoryKeeper := mock_types.NewMockTokenfactoryKeeper(ctrl) // required to initialize keeper authKeeper.EXPECT().GetModuleAddress(transfertypes.ModuleName).Return([]byte("address")) txKeeper, infCtx, _ := testkeeper.TransferKeeper(t, wmKeeper, feeKeeper, chanKeeper, authKeeper) - txModule := transfer.NewIBCModule(*txKeeper, wmKeeper) + txModule := transfer.NewIBCModule(*txKeeper, wmKeeper, tokenfactoryKeeper) ctx := infCtx.WithGasMeter(types2.NewGasMeter(1_000_000_000_000)) contractAddress := sdk.MustAccAddressFromBech32(testutil.TestOwnerAddress) relayerBech32 := "neutron1fxudpred77a0grgh69u0j7y84yks5ev4n5050z45kecz792jnd6scqu98z" diff --git a/x/transfer/keeper/keeper.go b/x/transfer/keeper/keeper.go index 191a1ae4f..bd2e8838e 100644 --- a/x/transfer/keeper/keeper.go +++ b/x/transfer/keeper/keeper.go @@ -2,10 +2,8 @@ package transfer import ( "context" - "cosmossdk.io/errors" storetypes "cosmossdk.io/store/types" - "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" diff --git a/x/transfer/module.go b/x/transfer/module.go index 0479b53b5..e925ae92d 100644 --- a/x/transfer/module.go +++ b/x/transfer/module.go @@ -1,10 +1,10 @@ package transfer import ( - "fmt" - "cosmossdk.io/core/appmodule" "cosmossdk.io/errors" + "fmt" + transfertypes "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types" "github.com/cosmos/cosmos-sdk/codec" cdctypes "github.com/cosmos/cosmos-sdk/codec/types" @@ -25,19 +25,21 @@ import ( */ type IBCModule struct { - wrappedKeeper wrapkeeper.KeeperTransferWrapper - keeper keeper.Keeper - sudoKeeper neutrontypes.WasmKeeper + wrappedKeeper wrapkeeper.KeeperTransferWrapper + keeper keeper.Keeper + sudoKeeper neutrontypes.WasmKeeper + tokenfactoryKeeper neutrontypes.TokenfactoryKeeper transfer.IBCModule } // NewIBCModule creates a new IBCModule given the keeper -func NewIBCModule(k wrapkeeper.KeeperTransferWrapper, sudoKeeper neutrontypes.WasmKeeper) IBCModule { +func NewIBCModule(k wrapkeeper.KeeperTransferWrapper, sudoKeeper neutrontypes.WasmKeeper, tokenfactoryKeeper neutrontypes.TokenfactoryKeeper) IBCModule { return IBCModule{ - wrappedKeeper: k, - keeper: k.Keeper, - sudoKeeper: sudoKeeper, - IBCModule: transfer.NewIBCModule(k.Keeper), + wrappedKeeper: k, + keeper: k.Keeper, + sudoKeeper: sudoKeeper, + IBCModule: transfer.NewIBCModule(k.Keeper), + tokenfactoryKeeper: tokenfactoryKeeper, } } @@ -69,6 +71,41 @@ func (im IBCModule) OnTimeoutPacket( return im.HandleTimeout(ctx, packet, relayer) } +func (im IBCModule) OnChanOpenAck( + ctx sdk.Context, + portID, + channelID string, + _ string, + counterpartyVersion string, +) error { + err := im.IBCModule.OnChanOpenAck(ctx, portID, channelID, "", counterpartyVersion) + if err != nil { + return errors.Wrap(err, "failed to process original OnChanOpenAck") + } + + escrowAddress := transfertypes.GetEscrowAddress(portID, channelID) + im.tokenfactoryKeeper.StoreEscrowAddress(ctx, escrowAddress.Bytes()) + + return nil +} + +// OnChanOpenConfirm implements the IBCModule interface +func (im IBCModule) OnChanOpenConfirm( + ctx sdk.Context, + portID, + channelID string, +) error { + err := im.IBCModule.OnChanOpenConfirm(ctx, portID, channelID) + if err != nil { + return errors.Wrap(err, "failed to process original OnChanOpenConfirm") + } + + escrowAddress := transfertypes.GetEscrowAddress(portID, channelID) + im.tokenfactoryKeeper.StoreEscrowAddress(ctx, escrowAddress.Bytes()) + + return nil +} + var _ appmodule.AppModule = AppModule{} type AppModule struct { diff --git a/x/transfer/types/expected_keepers.go b/x/transfer/types/expected_keepers.go index 5eb6f8edc..8642079fc 100644 --- a/x/transfer/types/expected_keepers.go +++ b/x/transfer/types/expected_keepers.go @@ -32,3 +32,8 @@ type AccountKeeper interface { GetModuleAddress(name string) sdk.AccAddress GetModuleAccount(ctx context.Context, name string) sdk.ModuleAccountI } + +// TokenfactoryKeeper defines the tokenfactory keeper. +type TokenfactoryKeeper interface { + StoreEscrowAddress(ctx sdk.Context, address sdk.AccAddress) +} From 6faa789afee0deef7a7074cb890ff7fbe03ddd78 Mon Sep 17 00:00:00 2001 From: Andrei Zavgorodnii Date: Fri, 20 Dec 2024 14:07:32 +0000 Subject: [PATCH 10/15] better logging --- app/upgrades/v5.0.5/upgrades.go | 2 ++ app/upgrades/v5.0.5/upgrades_test.go | 16 +++++++++++++++- x/tokenfactory/keeper/bankactions.go | 6 +++--- x/tokenfactory/keeper/escrow_addresses.go | 2 +- 4 files changed, 21 insertions(+), 5 deletions(-) diff --git a/app/upgrades/v5.0.5/upgrades.go b/app/upgrades/v5.0.5/upgrades.go index 0ba93e8f2..112225a24 100644 --- a/app/upgrades/v5.0.5/upgrades.go +++ b/app/upgrades/v5.0.5/upgrades.go @@ -33,6 +33,8 @@ func CreateUpgradeHandler( transferChannels := keepers.ChannelKeeper.GetAllChannelsWithPortPrefix(ctx, keepers.TransferKeeper.GetPort(ctx)) for _, channel := range transferChannels { escrowAddress := transfertypes.GetEscrowAddress(channel.PortId, channel.ChannelId) + ctx.Logger().Info("Saving escrow address", "port_id", channel.PortId, "channel_id", + channel.ChannelId, "address", escrowAddress.String()) keepers.TokenFactoryKeeper.StoreEscrowAddress(ctx, escrowAddress) } diff --git a/app/upgrades/v5.0.5/upgrades_test.go b/app/upgrades/v5.0.5/upgrades_test.go index 3590362bd..be9cbde86 100644 --- a/app/upgrades/v5.0.5/upgrades_test.go +++ b/app/upgrades/v5.0.5/upgrades_test.go @@ -1,6 +1,8 @@ package v505_test import ( + sdk "github.com/cosmos/cosmos-sdk/types" + transfertypes "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types" "testing" upgradetypes "cosmossdk.io/x/upgrade/types" @@ -23,7 +25,7 @@ func (suite *UpgradeTestSuite) SetupTest() { suite.IBCConnectionTestSuite.SetupTest() } -func (suite *UpgradeTestSuite) TestOracleUpgrade() { +func (suite *UpgradeTestSuite) TestUpgrade() { app := suite.GetNeutronZoneApp(suite.ChainA) ctx := suite.ChainA.GetContext().WithChainID("neutron-1") t := suite.T() @@ -33,5 +35,17 @@ func (suite *UpgradeTestSuite) TestOracleUpgrade() { Info: "some text here", Height: 100, } + + var escrowAddresses []sdk.AccAddress + transferChannels := app.IBCKeeper.ChannelKeeper.GetAllChannelsWithPortPrefix(ctx, app.TransferKeeper.GetPort(ctx)) + for _, channel := range transferChannels { + escrowAddresses = append(escrowAddresses, transfertypes.GetEscrowAddress(channel.PortId, channel.ChannelId)) + } + require.Greater(t, len(escrowAddresses), 0) require.NoError(t, app.UpgradeKeeper.ApplyUpgrade(ctx, upgrade)) + + for _, escrowAddress := range escrowAddresses { + require.True(t, app.TokenFactoryKeeper.IsEscrowAddress(ctx, escrowAddress)) + } + require.False(t, app.TokenFactoryKeeper.IsEscrowAddress(ctx, []byte{1, 2, 3, 4, 5})) } diff --git a/x/tokenfactory/keeper/bankactions.go b/x/tokenfactory/keeper/bankactions.go index 552bbdb0c..e9bc92171 100644 --- a/x/tokenfactory/keeper/bankactions.go +++ b/x/tokenfactory/keeper/bankactions.go @@ -50,7 +50,7 @@ func (k Keeper) burnFrom(ctx sdk.Context, amount sdk.Coin, burnFrom string) erro return status.Errorf(codes.Internal, "burning from module accounts is forbidden") } - if k.isEscrowAddress(ctx, burnFromAcc) { + if k.IsEscrowAddress(ctx, burnFromAcc) { return status.Errorf(codes.Internal, "burning from escrow accounts is forbidden") } @@ -90,11 +90,11 @@ func (k Keeper) forceTransfer(ctx sdk.Context, amount sdk.Coin, fromAddr, toAddr return status.Errorf(codes.Internal, "force transfer to module accounts is forbidden") } - if k.isEscrowAddress(ctx, transferFromAcc) { + if k.IsEscrowAddress(ctx, transferFromAcc) { return status.Errorf(codes.Internal, "force transfer from IBC escrow accounts is forbidden") } - if k.isEscrowAddress(ctx, transferToAcc) { + if k.IsEscrowAddress(ctx, transferToAcc) { return status.Errorf(codes.Internal, "force transfer to IBC escrow accounts is forbidden") } diff --git a/x/tokenfactory/keeper/escrow_addresses.go b/x/tokenfactory/keeper/escrow_addresses.go index a4af97a13..5189a3e7d 100644 --- a/x/tokenfactory/keeper/escrow_addresses.go +++ b/x/tokenfactory/keeper/escrow_addresses.go @@ -13,7 +13,7 @@ func (k Keeper) StoreEscrowAddress(ctx sdk.Context, address sdk.AccAddress) { prefixStore.Set(address.Bytes(), []byte{0}) } -func (k Keeper) isEscrowAddress(ctx sdk.Context, address sdk.AccAddress) bool { +func (k Keeper) IsEscrowAddress(ctx sdk.Context, address sdk.AccAddress) bool { prefixStore := prefix.NewStore(ctx.KVStore(k.storeKey), types.EscrowAddressKey) bz := prefixStore.Get(address.Bytes()) From 5a180b72b291ab358c52cdb41ae591cbe6a8d3b1 Mon Sep 17 00:00:00 2001 From: Andrei Zavgorodnii Date: Fri, 20 Dec 2024 14:16:20 +0000 Subject: [PATCH 11/15] fixed tests --- tests/ibc/tokenfactory_test.go | 2 +- x/tokenfactory/keeper/bankactions.go | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/ibc/tokenfactory_test.go b/tests/ibc/tokenfactory_test.go index fa3e3f160..20dc3c22b 100644 --- a/tests/ibc/tokenfactory_test.go +++ b/tests/ibc/tokenfactory_test.go @@ -121,5 +121,5 @@ func (s *TokenfactoryTestSuite) TestBurnFromIBCEscrow() { // Burn tokens from escrow address burnMsg := tftypes.NewMsgBurnFrom(s.neutronAddr.String(), amount, escrowAddress.String()) _, err = s.neutronChain.SendMsgs(burnMsg) - s.Assert().ErrorContains(err, "burning from escrow accounts is forbidden") + s.Assert().ErrorContains(err, "burning from IBC escrow accounts is forbidden") } diff --git a/x/tokenfactory/keeper/bankactions.go b/x/tokenfactory/keeper/bankactions.go index e9bc92171..7960c42e3 100644 --- a/x/tokenfactory/keeper/bankactions.go +++ b/x/tokenfactory/keeper/bankactions.go @@ -24,6 +24,10 @@ func (k Keeper) mintTo(ctx sdk.Context, amount sdk.Coin, mintTo string) error { return status.Errorf(codes.Internal, "minting to module accounts is forbidden") } + if k.IsEscrowAddress(ctx, mintToAcc) { + return status.Errorf(codes.Internal, "minting to IBC escrow accounts is forbidden") + } + err = k.bankKeeper.MintCoins(ctx, types.ModuleName, sdk.NewCoins(amount)) if err != nil { return err @@ -51,7 +55,7 @@ func (k Keeper) burnFrom(ctx sdk.Context, amount sdk.Coin, burnFrom string) erro } if k.IsEscrowAddress(ctx, burnFromAcc) { - return status.Errorf(codes.Internal, "burning from escrow accounts is forbidden") + return status.Errorf(codes.Internal, "burning from IBC escrow accounts is forbidden") } err = k.bankKeeper.SendCoinsFromAccountToModule(ctx, From f3bd034a118ab00552ae5c064e5f4200e342d1a5 Mon Sep 17 00:00:00 2001 From: Andrei Zavgorodnii Date: Fri, 20 Dec 2024 16:05:30 +0000 Subject: [PATCH 12/15] additional test --- tests/ibc/tokenfactory_test.go | 48 +++++++++++++++++++++++++++++++++- 1 file changed, 47 insertions(+), 1 deletion(-) diff --git a/tests/ibc/tokenfactory_test.go b/tests/ibc/tokenfactory_test.go index 20dc3c22b..22f7cfb6e 100644 --- a/tests/ibc/tokenfactory_test.go +++ b/tests/ibc/tokenfactory_test.go @@ -15,7 +15,7 @@ type TokenfactoryTestSuite struct { IBCTestSuite } -func TestPocTestSuite(t *testing.T) { +func TestTokenfactoryTestSuite(t *testing.T) { suite.Run(t, new(TokenfactoryTestSuite)) } @@ -123,3 +123,49 @@ func (s *TokenfactoryTestSuite) TestBurnFromIBCEscrow() { _, err = s.neutronChain.SendMsgs(burnMsg) s.Assert().ErrorContains(err, "burning from IBC escrow accounts is forbidden") } + +func (s *TokenfactoryTestSuite) TestBurnFromIBCEscrowReverse() { + // Create token factory denom + createDenomMsg := tftypes.NewMsgCreateDenom(s.neutronAddr.String(), "testtest") + _, err := s.neutronChain.SendMsgs(createDenomMsg) + s.Assert().NoError(err) + + // Derive full token factory denom + denom := fmt.Sprintf("factory/%s/%s", createDenomMsg.Sender, createDenomMsg.Subdenom) + + // Mint denom to sender + amount := sdk.NewCoin(denom, math.NewInt(10000000)) + mintMsg := tftypes.NewMsgMint(createDenomMsg.Sender, amount) + _, err = s.neutronChain.SendMsgs(mintMsg) + s.Assert().NoError(err) + + // Send IBC transfer + s.IBCTransfer( + s.neutronChainBPath, + s.neutronChainBPath.EndpointA, + s.neutronAddr, + s.neutronAddr, + amount.Denom, + amount.Amount, + "", + ) + + // Create token factory denom + createDenomMsg = tftypes.NewMsgCreateDenom(s.bundleB.Chain.SenderAccount.GetAddress().String(), "testtest") + _, err = s.bundleB.Chain.SendMsgs(createDenomMsg) + s.Assert().NoError(err) + + // Derive IBC escrow address for channel + escrowAddress := transfertypes.GetEscrowAddress("transfer", s.neutronChainBPath.EndpointB.ChannelID) + + // Derive full token factory denom + denom = fmt.Sprintf("factory/%s/%s", s.bundleB.Chain.SenderAccount.GetAddress().String(), createDenomMsg.Subdenom) + + // Mint denom to sender + amount = sdk.NewCoin(denom, math.NewInt(10000000)) + + // Burn tokens from escrow address + burnMsg := tftypes.NewMsgBurnFrom(s.bundleB.Chain.SenderAccount.GetAddress().String(), amount, escrowAddress.String()) + _, err = s.bundleB.Chain.SendMsgs(burnMsg) + s.Assert().ErrorContains(err, "burning from IBC escrow accounts is forbidden") +} From 58d5ec1768718578c15bce3e8d778cb024456d64 Mon Sep 17 00:00:00 2001 From: sotnikov-s Date: Fri, 20 Dec 2024 19:12:28 +0300 Subject: [PATCH 13/15] rm tf test to move to integration test --- tests/ibc/tokenfactory_test.go | 171 --------------------------------- 1 file changed, 171 deletions(-) delete mode 100644 tests/ibc/tokenfactory_test.go diff --git a/tests/ibc/tokenfactory_test.go b/tests/ibc/tokenfactory_test.go deleted file mode 100644 index 22f7cfb6e..000000000 --- a/tests/ibc/tokenfactory_test.go +++ /dev/null @@ -1,171 +0,0 @@ -package ibc_test - -import ( - "fmt" - "testing" - - "cosmossdk.io/math" - sdk "github.com/cosmos/cosmos-sdk/types" - transfertypes "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types" - tftypes "github.com/neutron-org/neutron/v5/x/tokenfactory/types" - "github.com/stretchr/testify/suite" -) - -type TokenfactoryTestSuite struct { - IBCTestSuite -} - -func TestTokenfactoryTestSuite(t *testing.T) { - suite.Run(t, new(TokenfactoryTestSuite)) -} - -func (s *TokenfactoryTestSuite) TestForceTransferFromIBCEscrow() { - // Create token factory denom - createDenomMsg := tftypes.NewMsgCreateDenom(s.neutronAddr.String(), "testtest") - _, err := s.neutronChain.SendMsgs(createDenomMsg) - s.Assert().NoError(err) - - // Derive full token factory denom - denom := fmt.Sprintf("factory/%s/%s", createDenomMsg.Sender, createDenomMsg.Subdenom) - - // Mint denom to sender - amount := sdk.NewCoin(denom, math.NewInt(10000000)) - mintMsg := tftypes.NewMsgMint(createDenomMsg.Sender, amount) - _, err = s.neutronChain.SendMsgs(mintMsg) - s.Assert().NoError(err) - - // Send IBC transfer - s.IBCTransfer( - s.neutronTransferPath, - s.neutronTransferPath.EndpointA, - s.neutronAddr, - s.providerAddr, - amount.Denom, - amount.Amount, - "", - ) - - // Derive IBC escrow address for channel - escrowAddress := transfertypes.GetEscrowAddress("transfer", s.neutronTransferPath.EndpointA.ChannelID) - - // Transfer tokens out of escrow address - forceTransferMsg := tftypes.NewMsgForceTransfer(s.neutronAddr.String(), sdk.NewCoin(amount.Denom, amount.Amount), escrowAddress.String(), s.neutronAddr.String()) - _, err = s.neutronChain.SendMsgs(forceTransferMsg) - s.Assert().ErrorContains(err, "force transfer from IBC escrow accounts is forbidden") -} - -func (s *TokenfactoryTestSuite) TestForceTransferToIBCEscrow() { - // Create token factory denom - createDenomMsg := tftypes.NewMsgCreateDenom(s.neutronAddr.String(), "testtest") - _, err := s.neutronChain.SendMsgs(createDenomMsg) - s.Assert().NoError(err) - - // Derive full token factory denom - denom := fmt.Sprintf("factory/%s/%s", createDenomMsg.Sender, createDenomMsg.Subdenom) - - // Mint denom to sender - amount := sdk.NewCoin(denom, math.NewInt(10000000)) - mintMsg := tftypes.NewMsgMint(createDenomMsg.Sender, amount) - _, err = s.neutronChain.SendMsgs(mintMsg) - s.Assert().NoError(err) - - // Send IBC transfer - s.IBCTransfer( - s.neutronTransferPath, - s.neutronTransferPath.EndpointA, - s.neutronAddr, - s.providerAddr, - amount.Denom, - amount.Amount, - "", - ) - - // Derive IBC escrow address for channel - escrowAddress := transfertypes.GetEscrowAddress("transfer", s.neutronTransferPath.EndpointA.ChannelID) - - // Transfer tokens out of escrow address - forceTransferMsg := tftypes.NewMsgForceTransfer(s.neutronAddr.String(), sdk.NewCoin(amount.Denom, amount.Amount), s.neutronAddr.String(), escrowAddress.String()) - _, err = s.neutronChain.SendMsgs(forceTransferMsg) - s.Assert().ErrorContains(err, "force transfer to IBC escrow accounts is forbidden") -} - -func (s *TokenfactoryTestSuite) TestBurnFromIBCEscrow() { - // Create token factory denom - createDenomMsg := tftypes.NewMsgCreateDenom(s.neutronAddr.String(), "testtest") - _, err := s.neutronChain.SendMsgs(createDenomMsg) - s.Assert().NoError(err) - - // Derive full token factory denom - denom := fmt.Sprintf("factory/%s/%s", createDenomMsg.Sender, createDenomMsg.Subdenom) - - // Mint denom to sender - amount := sdk.NewCoin(denom, math.NewInt(10000000)) - mintMsg := tftypes.NewMsgMint(createDenomMsg.Sender, amount) - _, err = s.neutronChain.SendMsgs(mintMsg) - s.Assert().NoError(err) - - // Send IBC transfer - s.IBCTransfer( - s.neutronTransferPath, - s.neutronTransferPath.EndpointA, - s.neutronAddr, - s.providerAddr, - amount.Denom, - amount.Amount, - "", - ) - - // Derive IBC escrow address for channel - escrowAddress := transfertypes.GetEscrowAddress("transfer", s.neutronTransferPath.EndpointA.ChannelID) - - // Burn tokens from escrow address - burnMsg := tftypes.NewMsgBurnFrom(s.neutronAddr.String(), amount, escrowAddress.String()) - _, err = s.neutronChain.SendMsgs(burnMsg) - s.Assert().ErrorContains(err, "burning from IBC escrow accounts is forbidden") -} - -func (s *TokenfactoryTestSuite) TestBurnFromIBCEscrowReverse() { - // Create token factory denom - createDenomMsg := tftypes.NewMsgCreateDenom(s.neutronAddr.String(), "testtest") - _, err := s.neutronChain.SendMsgs(createDenomMsg) - s.Assert().NoError(err) - - // Derive full token factory denom - denom := fmt.Sprintf("factory/%s/%s", createDenomMsg.Sender, createDenomMsg.Subdenom) - - // Mint denom to sender - amount := sdk.NewCoin(denom, math.NewInt(10000000)) - mintMsg := tftypes.NewMsgMint(createDenomMsg.Sender, amount) - _, err = s.neutronChain.SendMsgs(mintMsg) - s.Assert().NoError(err) - - // Send IBC transfer - s.IBCTransfer( - s.neutronChainBPath, - s.neutronChainBPath.EndpointA, - s.neutronAddr, - s.neutronAddr, - amount.Denom, - amount.Amount, - "", - ) - - // Create token factory denom - createDenomMsg = tftypes.NewMsgCreateDenom(s.bundleB.Chain.SenderAccount.GetAddress().String(), "testtest") - _, err = s.bundleB.Chain.SendMsgs(createDenomMsg) - s.Assert().NoError(err) - - // Derive IBC escrow address for channel - escrowAddress := transfertypes.GetEscrowAddress("transfer", s.neutronChainBPath.EndpointB.ChannelID) - - // Derive full token factory denom - denom = fmt.Sprintf("factory/%s/%s", s.bundleB.Chain.SenderAccount.GetAddress().String(), createDenomMsg.Subdenom) - - // Mint denom to sender - amount = sdk.NewCoin(denom, math.NewInt(10000000)) - - // Burn tokens from escrow address - burnMsg := tftypes.NewMsgBurnFrom(s.bundleB.Chain.SenderAccount.GetAddress().String(), amount, escrowAddress.String()) - _, err = s.bundleB.Chain.SendMsgs(burnMsg) - s.Assert().ErrorContains(err, "burning from IBC escrow accounts is forbidden") -} From 4f6b9553e4ee45b0020a598d12ade23e7430184b Mon Sep 17 00:00:00 2001 From: sotnikov-s Date: Fri, 20 Dec 2024 19:28:34 +0300 Subject: [PATCH 14/15] fix duplicated import of package --- x/transfer/module.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/x/transfer/module.go b/x/transfer/module.go index e925ae92d..e26810739 100644 --- a/x/transfer/module.go +++ b/x/transfer/module.go @@ -1,9 +1,10 @@ package transfer import ( + "fmt" + "cosmossdk.io/core/appmodule" "cosmossdk.io/errors" - "fmt" transfertypes "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types" "github.com/cosmos/cosmos-sdk/codec" @@ -12,7 +13,6 @@ import ( "github.com/cosmos/cosmos-sdk/types/module" "github.com/cosmos/ibc-go/v8/modules/apps/transfer" "github.com/cosmos/ibc-go/v8/modules/apps/transfer/keeper" - "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types" channeltypes "github.com/cosmos/ibc-go/v8/modules/core/04-channel/types" wrapkeeper "github.com/neutron-org/neutron/v5/x/transfer/keeper" @@ -131,25 +131,25 @@ func (am AppModule) IsAppModule() { // marker // RegisterServices registers module services. func (am AppModule) RegisterServices(cfg module.Configurator) { - types.RegisterQueryServer(cfg.QueryServer(), am.keeper) + transfertypes.RegisterQueryServer(cfg.QueryServer(), am.keeper) neutrontypes.RegisterMsgServer(cfg.MsgServer(), am.keeper) cfg.MsgServer().RegisterService(&neutrontypes.MsgServiceDescOrig, am.keeper) m := keeper.NewMigrator(am.keeper.Keeper) - if err := cfg.RegisterMigration(types.ModuleName, 1, m.MigrateTraces); err != nil { + if err := cfg.RegisterMigration(transfertypes.ModuleName, 1, m.MigrateTraces); err != nil { panic(fmt.Sprintf("failed to migrate transfer app from version 1 to 2: %v", err)) } - if err := cfg.RegisterMigration(types.ModuleName, 2, m.MigrateTotalEscrowForDenom); err != nil { + if err := cfg.RegisterMigration(transfertypes.ModuleName, 2, m.MigrateTotalEscrowForDenom); err != nil { panic(fmt.Sprintf("failed to migrate transfer app from version 2 to 3: %v", err)) } - if err := cfg.RegisterMigration(types.ModuleName, 3, m.MigrateParams); err != nil { + if err := cfg.RegisterMigration(transfertypes.ModuleName, 3, m.MigrateParams); err != nil { panic(fmt.Sprintf("failed to migrate transfer app version 3 to 4: %v", err)) } - if err := cfg.RegisterMigration(types.ModuleName, 4, m.MigrateDenomMetadata); err != nil { + if err := cfg.RegisterMigration(transfertypes.ModuleName, 4, m.MigrateDenomMetadata); err != nil { panic(fmt.Sprintf("failed to migrate transfer app from version 4 to 5: %v", err)) } } From 9c3a5c13a5a189b40673dc6f74e7e9ae91e0d20e Mon Sep 17 00:00:00 2001 From: sotnikov-s Date: Fri, 20 Dec 2024 19:36:13 +0300 Subject: [PATCH 15/15] fix lint errors --- app/upgrades/v5.0.5/upgrades.go | 1 + app/upgrades/v5.0.5/upgrades_test.go | 3 ++- x/transfer/keeper/keeper.go | 1 + 3 files changed, 4 insertions(+), 1 deletion(-) diff --git a/app/upgrades/v5.0.5/upgrades.go b/app/upgrades/v5.0.5/upgrades.go index 112225a24..e90ebc332 100644 --- a/app/upgrades/v5.0.5/upgrades.go +++ b/app/upgrades/v5.0.5/upgrades.go @@ -3,6 +3,7 @@ package v505 import ( "context" "fmt" + transfertypes "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types" upgradetypes "cosmossdk.io/x/upgrade/types" diff --git a/app/upgrades/v5.0.5/upgrades_test.go b/app/upgrades/v5.0.5/upgrades_test.go index be9cbde86..b65e60b3a 100644 --- a/app/upgrades/v5.0.5/upgrades_test.go +++ b/app/upgrades/v5.0.5/upgrades_test.go @@ -1,9 +1,10 @@ package v505_test import ( + "testing" + sdk "github.com/cosmos/cosmos-sdk/types" transfertypes "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types" - "testing" upgradetypes "cosmossdk.io/x/upgrade/types" "github.com/stretchr/testify/require" diff --git a/x/transfer/keeper/keeper.go b/x/transfer/keeper/keeper.go index bd2e8838e..15a70efea 100644 --- a/x/transfer/keeper/keeper.go +++ b/x/transfer/keeper/keeper.go @@ -2,6 +2,7 @@ package transfer import ( "context" + "cosmossdk.io/errors" storetypes "cosmossdk.io/store/types" "github.com/cosmos/cosmos-sdk/codec"