From f53be8e9d0bbfffa53406ee969b3f3506c5b85b0 Mon Sep 17 00:00:00 2001 From: Joe Bowman Date: Mon, 20 May 2024 17:00:57 +0100 Subject: [PATCH] resolve issues with mappAccounts storage (#1622) * resolve issues with mappAccounts storage * lint --- x/interchainstaking/keeper/address_map.go | 12 +- .../keeper/address_map_test.go | 16 +- x/interchainstaking/keeper/grpc_query.go | 2 +- x/interchainstaking/keeper/receipt.go | 4 +- x/interchainstaking/keeper/receipt_test.go | 145 ++++++++++++++++++ .../keeper/withdrawal_record.go | 1 - x/interchainstaking/types/keys.go | 6 +- 7 files changed, 164 insertions(+), 22 deletions(-) diff --git a/x/interchainstaking/keeper/address_map.go b/x/interchainstaking/keeper/address_map.go index f81b2081d..825a9a3a6 100644 --- a/x/interchainstaking/keeper/address_map.go +++ b/x/interchainstaking/keeper/address_map.go @@ -7,7 +7,7 @@ import ( ) // GetRemoteAddressMap retrieves a remote address using a local address. -func (k *Keeper) GetRemoteAddressMap(ctx sdk.Context, localAddress []byte, chainID string) ([]byte, bool) { +func (k *Keeper) GetRemoteAddressMap(ctx sdk.Context, localAddress sdk.AccAddress, chainID string) (sdk.AccAddress, bool) { store := ctx.KVStore(k.storeKey) key := types.GetRemoteAddressKey(localAddress, chainID) value := store.Get(key) @@ -16,7 +16,7 @@ func (k *Keeper) GetRemoteAddressMap(ctx sdk.Context, localAddress []byte, chain } // IterateUserMappedAccounts iterates over all the user mapped accounts. -func (k Keeper) IterateUserMappedAccounts(ctx sdk.Context, localAddress []byte, fn func(index int64, chainID string, remoteAddressBytes []byte) (stop bool)) { +func (k Keeper) IterateUserMappedAccounts(ctx sdk.Context, localAddress sdk.AccAddress, fn func(index int64, chainID string, remoteAddressBytes sdk.AccAddress) (stop bool)) { // noop if fn == nil { return @@ -41,14 +41,14 @@ func (k Keeper) IterateUserMappedAccounts(ctx sdk.Context, localAddress []byte, } // SetRemoteAddressMap sets a remote address using a local address as a map. -func (k *Keeper) SetRemoteAddressMap(ctx sdk.Context, localAddress, remoteAddress []byte, chainID string) { +func (k *Keeper) SetRemoteAddressMap(ctx sdk.Context, localAddress, remoteAddress sdk.AccAddress, chainID string) { store := ctx.KVStore(k.storeKey) key := types.GetRemoteAddressKey(localAddress, chainID) store.Set(key, remoteAddress) } // GetLocalAddressMap retrieves a local address using a remote address. -func (k *Keeper) GetLocalAddressMap(ctx sdk.Context, remoteAddress []byte, chainID string) ([]byte, bool) { +func (k *Keeper) GetLocalAddressMap(ctx sdk.Context, remoteAddress sdk.AccAddress, chainID string) (sdk.AccAddress, bool) { store := ctx.KVStore(k.storeKey) key := types.GetLocalAddressKey(remoteAddress, chainID) value := store.Get(key) @@ -57,14 +57,14 @@ func (k *Keeper) GetLocalAddressMap(ctx sdk.Context, remoteAddress []byte, chain } // SetLocalAddressMap sets a local address using a remote address as a map. -func (k *Keeper) SetLocalAddressMap(ctx sdk.Context, localAddress, remoteAddress []byte, chainID string) { +func (k *Keeper) SetLocalAddressMap(ctx sdk.Context, localAddress, remoteAddress sdk.AccAddress, chainID string) { store := ctx.KVStore(k.storeKey) key := types.GetLocalAddressKey(remoteAddress, chainID) store.Set(key, localAddress) } // SetAddressMapPair sets forward and reverse maps for localAddress => remoteAddress and remoteAddress => localAddress. -func (k *Keeper) SetAddressMapPair(ctx sdk.Context, localAddress, remoteAddress []byte, chainID string) { +func (k *Keeper) SetAddressMapPair(ctx sdk.Context, localAddress, remoteAddress sdk.AccAddress, chainID string) { k.SetLocalAddressMap(ctx, localAddress, remoteAddress, chainID) k.SetRemoteAddressMap(ctx, localAddress, remoteAddress, chainID) } diff --git a/x/interchainstaking/keeper/address_map_test.go b/x/interchainstaking/keeper/address_map_test.go index 17c15ce17..412185258 100644 --- a/x/interchainstaking/keeper/address_map_test.go +++ b/x/interchainstaking/keeper/address_map_test.go @@ -1,6 +1,8 @@ package keeper_test import ( + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/quicksilver-zone/quicksilver/utils/randomutils" ) @@ -8,6 +10,11 @@ const ( testChainID = "test-1" ) +var ( + localAddress = sdk.AccAddress(randomutils.GenerateRandomBytes(20)) + remoteAddress = sdk.AccAddress(randomutils.GenerateRandomBytes(32)) +) + func (suite *KeeperTestSuite) TestKeeper_RemoteAddressStore() { suite.SetupTest() suite.setupTestZones() @@ -15,9 +22,6 @@ func (suite *KeeperTestSuite) TestKeeper_RemoteAddressStore() { icsKeeper := suite.GetQuicksilverApp(suite.chainA).InterchainstakingKeeper ctx := suite.chainA.GetContext() - localAddress := randomutils.GenerateRandomBytes(28) - remoteAddress := randomutils.GenerateRandomBytes(40) - suite.Run("not found", func() { _, found := icsKeeper.GetRemoteAddressMap(ctx, localAddress, testChainID) suite.False(found) @@ -41,9 +45,6 @@ func (suite *KeeperTestSuite) TestKeeper_LocalAddressStore() { icsKeeper := suite.GetQuicksilverApp(suite.chainA).InterchainstakingKeeper ctx := suite.chainA.GetContext() - localAddress := randomutils.GenerateRandomBytes(28) - remoteAddress := randomutils.GenerateRandomBytes(40) - suite.Run("not found", func() { _, found := icsKeeper.GetLocalAddressMap(ctx, remoteAddress, testChainID) suite.False(found) @@ -67,9 +68,6 @@ func (suite *KeeperTestSuite) TestKeeper_AddressMapPair() { icsKeeper := suite.GetQuicksilverApp(suite.chainA).InterchainstakingKeeper ctx := suite.chainA.GetContext() - localAddress := randomutils.GenerateRandomBytes(28) - remoteAddress := randomutils.GenerateRandomBytes(40) - suite.Run("not found", func() { _, found := icsKeeper.GetLocalAddressMap(ctx, remoteAddress, testChainID) suite.False(found) diff --git a/x/interchainstaking/keeper/grpc_query.go b/x/interchainstaking/keeper/grpc_query.go index 7eaccd033..593cd1f51 100644 --- a/x/interchainstaking/keeper/grpc_query.go +++ b/x/interchainstaking/keeper/grpc_query.go @@ -345,7 +345,7 @@ func (k *Keeper) MappedAccounts(c context.Context, req *types.QueryMappedAccount return nil, status.Error(codes.InvalidArgument, "Invalid Address") } - k.IterateUserMappedAccounts(ctx, addrBytes, func(index int64, chainID string, remoteAddressBytes []byte) (stop bool) { + k.IterateUserMappedAccounts(ctx, addrBytes, func(index int64, chainID string, remoteAddressBytes sdk.AccAddress) (stop bool) { remoteAddressMap[chainID] = remoteAddressBytes return false }) diff --git a/x/interchainstaking/keeper/receipt.go b/x/interchainstaking/keeper/receipt.go index 377b8b3e4..367fe0aa2 100644 --- a/x/interchainstaking/keeper/receipt.go +++ b/x/interchainstaking/keeper/receipt.go @@ -179,7 +179,7 @@ func (k *Keeper) SendTokenIBC(ctx sdk.Context, senderAccAddress sdk.AccAddress, // - Mint QAssets, set new mapping for the mapped account in the keeper, and send to corresponding mapped account. // 5. If the zone is 118 and no other flags are set: // - Mint QAssets and transfer to send to msg creator. -func (k *Keeper) MintAndSendQAsset(ctx sdk.Context, sender sdk.AccAddress, senderAddress string, zone *types.Zone, assets sdk.Coins, memoRTS bool, mappedAddress []byte) error { +func (k *Keeper) MintAndSendQAsset(ctx sdk.Context, sender sdk.AccAddress, senderAddress string, zone *types.Zone, assets sdk.Coins, memoRTS bool, mappedAddress sdk.AccAddress) error { if zone.RedemptionRate.IsZero() { return errors.New("zero redemption rate") } @@ -218,7 +218,7 @@ func (k *Keeper) MintAndSendQAsset(ctx sdk.Context, sender sdk.AccAddress, sende case mappedAddress != nil: // set mapped account if setMappedAddress { - k.SetAddressMapPair(ctx, sender, mappedAddress, zone.ChainId) + k.SetAddressMapPair(ctx, mappedAddress, sender, zone.ChainId) } // set send to mapped account diff --git a/x/interchainstaking/keeper/receipt_test.go b/x/interchainstaking/keeper/receipt_test.go index 9719e5fbd..7af2db296 100644 --- a/x/interchainstaking/keeper/receipt_test.go +++ b/x/interchainstaking/keeper/receipt_test.go @@ -380,3 +380,148 @@ func (suite *KeeperTestSuite) TestSendTokenIBC() { suite.NoError(err) }) } + +func (suite *KeeperTestSuite) TestMintAndSendQAsset1RR() { + suite.SetupTest() + suite.setupTestZones() + + quicksilver := suite.GetQuicksilverApp(suite.chainA) + ctx := suite.chainA.GetContext() + + zone, found := quicksilver.InterchainstakingKeeper.GetZone(ctx, suite.chainB.ChainID) + suite.True(found) + + senderAddress := addressutils.GenerateAddressForTestWithPrefix("cosmos") + sender := addressutils.MustAccAddressFromBech32(senderAddress, "") + + amount := sdk.NewCoins(sdk.NewCoin(zone.BaseDenom, sdk.NewInt(5000))) + + // Test sending QAsset + err := quicksilver.InterchainstakingKeeper.MintAndSendQAsset(ctx, sender, senderAddress, &zone, amount, false, nil) + suite.NoError(err) + + // Verify balance of receiver + receiverBalance := quicksilver.BankKeeper.GetBalance(ctx, sender, zone.LocalDenom) + suite.Equal(sdk.NewCoin(zone.LocalDenom, sdk.NewInt(5000)), receiverBalance) +} + +func (suite *KeeperTestSuite) TestMintAndSendQAssetNon1RR() { + suite.SetupTest() + suite.setupTestZones() + + quicksilver := suite.GetQuicksilverApp(suite.chainA) + ctx := suite.chainA.GetContext() + + zone, found := quicksilver.InterchainstakingKeeper.GetZone(ctx, suite.chainB.ChainID) + zone.RedemptionRate = sdk.NewDecWithPrec(110, 2) + suite.True(found) + + senderAddress := addressutils.GenerateAddressForTestWithPrefix("cosmos") + sender := addressutils.MustAccAddressFromBech32(senderAddress, "") + + amount := sdk.NewCoins(sdk.NewCoin(zone.BaseDenom, sdk.NewInt(5000))) + + // Test sending QAsset + err := quicksilver.InterchainstakingKeeper.MintAndSendQAsset(ctx, sender, senderAddress, &zone, amount, false, nil) + suite.NoError(err) + + // Verify balance of receiver + receiverBalance := quicksilver.BankKeeper.GetBalance(ctx, sender, zone.LocalDenom) + suite.Equal(sdk.NewCoin(zone.LocalDenom, sdk.NewInt(4545)), receiverBalance) +} + +func (suite *KeeperTestSuite) TestMintAndSendQAssetSub1RR() { + suite.SetupTest() + suite.setupTestZones() + + quicksilver := suite.GetQuicksilverApp(suite.chainA) + ctx := suite.chainA.GetContext() + + zone, found := quicksilver.InterchainstakingKeeper.GetZone(ctx, suite.chainB.ChainID) + zone.RedemptionRate = sdk.NewDecWithPrec(90, 2) + suite.True(found) + + senderAddress := addressutils.GenerateAddressForTestWithPrefix("cosmos") + sender := addressutils.MustAccAddressFromBech32(senderAddress, "") + + amount := sdk.NewCoins(sdk.NewCoin(zone.BaseDenom, sdk.NewInt(5000))) + + // Test sending QAsset + err := quicksilver.InterchainstakingKeeper.MintAndSendQAsset(ctx, sender, senderAddress, &zone, amount, false, nil) + suite.NoError(err) + + // Verify balance of receiver + receiverBalance := quicksilver.BankKeeper.GetBalance(ctx, sender, zone.LocalDenom) + suite.Equal(sdk.NewCoin(zone.LocalDenom, sdk.NewInt(5555)), receiverBalance) +} + +func (suite *KeeperTestSuite) TestMintAndSendQAssetNon1RRMappedAccount() { + suite.SetupTest() + suite.setupTestZones() + + quicksilver := suite.GetQuicksilverApp(suite.chainA) + ctx := suite.chainA.GetContext() + + zone, found := quicksilver.InterchainstakingKeeper.GetZone(ctx, suite.chainB.ChainID) + zone.RedemptionRate = sdk.NewDecWithPrec(110, 2) + suite.True(found) + + senderAddress := addressutils.GenerateAddressForTestWithPrefix("cosmos") + sender := addressutils.MustAccAddressFromBech32(senderAddress, "") + mappedAccount := addressutils.GenerateAccAddressForTest() + + amount := sdk.NewCoins(sdk.NewCoin(zone.BaseDenom, sdk.NewInt(5000))) + + // Test sending QAsset + err := quicksilver.InterchainstakingKeeper.MintAndSendQAsset(ctx, sender, senderAddress, &zone, amount, false, mappedAccount) + suite.NoError(err) + + // Verify balance of receiver + receiverBalance := quicksilver.BankKeeper.GetBalance(ctx, sender, zone.LocalDenom) + suite.Equal(sdk.NewCoin(zone.LocalDenom, sdk.NewInt(0)), receiverBalance) + + mappedBalance := quicksilver.BankKeeper.GetBalance(ctx, mappedAccount, zone.LocalDenom) + suite.Equal(sdk.NewCoin(zone.LocalDenom, sdk.NewInt(4545)), mappedBalance) + + remoteAddress, found := quicksilver.InterchainstakingKeeper.GetRemoteAddressMap(ctx, mappedAccount, suite.chainB.ChainID) + suite.True(found) + suite.Equal(senderAddress, remoteAddress.String()) + + localAddress, found := quicksilver.InterchainstakingKeeper.GetLocalAddressMap(ctx, sender, suite.chainB.ChainID) + suite.True(found) + suite.Equal(mappedAccount, localAddress) +} + +func (suite *KeeperTestSuite) TestMintAndSendQAssetNon1RTS() { + suite.SetupTest() + // this is required because the ibc-go test suite CreateTransferChannels defaults to a value that causes executing a message to error. + suite.path.EndpointA.ChannelConfig.Version = "ics20-1" + suite.path.EndpointA.Counterparty.ChannelConfig.Version = "ics20-1" + suite.coordinator.CreateTransferChannels(suite.path) + + suite.setupTestZones() + + quicksilver := suite.GetQuicksilverApp(suite.chainA) + ctx := suite.chainA.GetContext() + + zone, found := quicksilver.InterchainstakingKeeper.GetZone(ctx, suite.chainB.ChainID) + zone.RedemptionRate = sdk.NewDecWithPrec(110, 2) + suite.True(found) + + senderAddress := addressutils.GenerateAddressForTestWithPrefix("cosmos") + sender := addressutils.MustAccAddressFromBech32(senderAddress, "") + + amount := sdk.NewCoins(sdk.NewCoin(zone.BaseDenom, sdk.NewInt(5000))) + + // Test sending QAsset + err := quicksilver.InterchainstakingKeeper.MintAndSendQAsset(ctx, sender, senderAddress, &zone, amount, true, nil) + suite.NoError(err) + + // Verify balance of receiver + receiverBalance := quicksilver.BankKeeper.GetBalance(ctx, sender, zone.LocalDenom) + suite.Equal(sdk.NewCoin(zone.LocalDenom, sdk.NewInt(0)), receiverBalance) + + ibcEscrowAddress := transfertypes.GetEscrowAddress("transfer", "channel-0") + ibcEscrowAccountBalance := quicksilver.BankKeeper.GetBalance(ctx, ibcEscrowAddress, zone.LocalDenom) + suite.Equal(sdk.NewCoin(zone.LocalDenom, sdk.NewInt(4545)), ibcEscrowAccountBalance) +} diff --git a/x/interchainstaking/keeper/withdrawal_record.go b/x/interchainstaking/keeper/withdrawal_record.go index c29d4472c..15cb4fadc 100644 --- a/x/interchainstaking/keeper/withdrawal_record.go +++ b/x/interchainstaking/keeper/withdrawal_record.go @@ -185,7 +185,6 @@ func (k *Keeper) GetUserChainRequeuedWithdrawalRecord(ctx sdk.Context, chainID s k.IterateZoneStatusWithdrawalRecords(ctx, chainID, types.WithdrawStatusQueued, func(_ int64, record types.WithdrawalRecord) (stop bool) { if record.Requeued && record.Delegator == address { - fmt.Println("FOUND MATCHING RECORD") toReturn = record return true } diff --git a/x/interchainstaking/types/keys.go b/x/interchainstaking/types/keys.go index ba7287aa7..0739ed451 100644 --- a/x/interchainstaking/types/keys.go +++ b/x/interchainstaking/types/keys.go @@ -93,12 +93,12 @@ func ParseStakingDelegationKey(key []byte) (sdk.AccAddress, sdk.ValAddress, erro } // GetRemoteAddressKey gets the prefix for a remote address mapping. -func GetRemoteAddressKey(localAddress []byte, chainID string) []byte { +func GetRemoteAddressKey(localAddress sdk.AccAddress, chainID string) sdk.AccAddress { return append(append(KeyPrefixRemoteAddress, localAddress...), chainID...) } // GetLocalAddressKey gets the prefix for a local address mapping. -func GetLocalAddressKey(remoteAddress []byte, chainID string) []byte { +func GetLocalAddressKey(remoteAddress sdk.AccAddress, chainID string) sdk.AccAddress { return append(append(KeyPrefixLocalAddress, chainID...), remoteAddress...) } @@ -158,7 +158,7 @@ func GetZoneValidatorsKey(chainID string) []byte { } // GetRemoteAddressPrefix gets the prefix for a remote address mapping. -func GetRemoteAddressPrefix(locaAddress []byte) []byte { +func GetRemoteAddressPrefix(locaAddress sdk.AccAddress) []byte { return append(KeyPrefixRemoteAddress, locaAddress...) }