Skip to content

Commit

Permalink
addressed carlos comments 2
Browse files Browse the repository at this point in the history
  • Loading branch information
stackman27 committed Feb 8, 2023
1 parent f2aa9e0 commit dc1a91c
Show file tree
Hide file tree
Showing 13 changed files with 231 additions and 185 deletions.
12 changes: 6 additions & 6 deletions modules/apps/transfer/client/cli/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -171,10 +171,10 @@ func GetCmdQueryDenomHash() *cobra.Command {
// GetCmdQueryDenomHash defines the command to query a denomination hash from a given trace.
func GetCmdQueryTotalEscrowForDenom() *cobra.Command {
cmd := &cobra.Command{
Use: "token-out [denom]",
Short: "Query the total source chain tokens that has been ibc'd out",
Long: "Query the total source chain tokens that has been ibc'd out",
Example: fmt.Sprintf("%s query ibc-transfer token-out uosmo", version.AppName),
Use: "token-escrow [denom]",
Short: "Query the total amount of tokens of a native denom in escrow",
Long: "Query the total amount of tokens of a native denom in escrow",
Example: fmt.Sprintf("%s query ibc-transfer token-escrow uosmo", version.AppName),
Args: cobra.ExactArgs(1),
RunE: func(cmd *cobra.Command, args []string) error {
clientCtx, err := client.GetClientQueryContext(cmd)
Expand All @@ -184,11 +184,11 @@ func GetCmdQueryTotalEscrowForDenom() *cobra.Command {

queryClient := types.NewQueryClient(clientCtx)

req := &types.QueryTotalNativeIBCOutRequest{
req := &types.QueryTotalEscrowFormDenomRequest{
Denom: args[0],
}

res, err := queryClient.TotalNativeIBCOut(cmd.Context(), req)
res, err := queryClient.TotalEscrowForDenom(cmd.Context(), req)
if err != nil {
return err
}
Expand Down
6 changes: 3 additions & 3 deletions modules/apps/transfer/keeper/grpc_query.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,16 +122,16 @@ func (q Keeper) EscrowAddress(c context.Context, req *types.QueryEscrowAddressRe
}, nil
}

// TotalNativeIBCOut implements the TotalNativeIBCOut gRPC method.
func (q Keeper) TotalNativeIBCOut(c context.Context, req *types.QueryTotalNativeIBCOutRequest) (*types.QueryTotalNativeIBCOutResponse, error) {
// TotalEscrowForDenom implements the TotalEscrowForDenom gRPC method.
func (q Keeper) TotalEscrowForDenom(c context.Context, req *types.QueryTotalEscrowFormDenomRequest) (*types.QueryTotalEscrowForDenomResponse, error) {
if req == nil {
return nil, status.Error(codes.InvalidArgument, "empty request")
}

ctx := sdk.UnwrapSDKContext(c)
denomAmount := q.GetIBCOutDenomAmount(ctx, req.Denom)

return &types.QueryTotalNativeIBCOutResponse{
return &types.QueryTotalEscrowForDenomResponse{
Amount: denomAmount.Int64(),
}, nil
}
6 changes: 2 additions & 4 deletions modules/apps/transfer/keeper/keeper.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
package keeper

import (
"fmt"

"github.com/cosmos/cosmos-sdk/codec"
"github.com/cosmos/cosmos-sdk/store/prefix"
storetypes "github.com/cosmos/cosmos-sdk/store/types"
Expand Down Expand Up @@ -171,8 +169,8 @@ func (k Keeper) GetIBCOutDenomAmount(ctx sdk.Context, denom string) sdk.Int {

// SetIBCOutDenomAmount stores the source tokens about to get IBC'd out.
func (k Keeper) SetIBCOutDenomAmount(ctx sdk.Context, denom string, amount sdk.Int) error {
if amount.LTE(sdk.ZeroInt()) {
return fmt.Errorf("amount cannot be negative.")
if amount.LT(sdk.ZeroInt()) {
panic("amount cannot be negative.")
}

store := ctx.KVStore(k.storeKey)
Expand Down
29 changes: 17 additions & 12 deletions modules/apps/transfer/keeper/migrations.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,29 +56,34 @@ func (m Migrator) MigrateTraces(ctx sdk.Context) error {

// MigrateTotalEscrowOut migrates the total escrow amount to calculate total IBC'd out.
func (m Migrator) MigrateTotalEscrowOut(ctx sdk.Context) error {
getExistingChannels := m.keeper.channelKeeper.GetAllChannelsWithPortPrefix(ctx, types.PortID)
var nativeTokens = make(map[string]int64)

var nativeDenom string
escrowAmount := sdk.ZeroInt()

for _, channel := range getExistingChannels {
transferChannels := m.keeper.channelKeeper.GetAllChannelsWithPortPrefix(ctx, types.PortID)
for _, channel := range transferChannels {
escrowAddress := types.GetEscrowAddress(types.PortID, channel.ChannelId)
getEscrowBalances := m.keeper.bankKeeper.GetAllBalances(ctx, escrowAddress)

for _, escrowBalance := range getEscrowBalances {
if !strings.Contains(escrowBalance.Denom, "ibc") {
// native tokens
nativeDenom = escrowBalance.Denom
escrowAmount = escrowAmount.Add(escrowBalance.Amount)
// Denom possibilities:
// - "atom" = native denom
// - "ibc/atom" = non native denom
// - "atom/ibc/osmo" = native denom

denomSplit := strings.SplitN(escrowBalance.Denom, "/", 2)
if denomSplit[0] != "ibc" || len(denomSplit) == 1 {
// native denom
escrowAmount := sdk.NewInt(nativeTokens[escrowBalance.Denom]).Add(escrowBalance.Amount).Int64()
nativeTokens[escrowBalance.Denom] = escrowAmount
}
}
}

if len(nativeDenom) == 0 {
return fmt.Errorf("invalid native denom")
if len(nativeTokens) != 0 {
for denom, amount := range nativeTokens {
m.keeper.SetIBCOutDenomAmount(ctx, denom, sdk.NewInt(amount))
}
}

m.keeper.SetIBCOutDenomAmount(ctx, nativeDenom, escrowAmount)
return nil
}

Expand Down
44 changes: 32 additions & 12 deletions modules/apps/transfer/keeper/migrations_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@ import (
"fmt"

sdk "github.com/cosmos/cosmos-sdk/types"
banktestutil "github.com/cosmos/cosmos-sdk/x/bank/testutil"
transferkeeper "github.com/cosmos/ibc-go/v6/modules/apps/transfer/keeper"
"github.com/cosmos/ibc-go/v6/modules/apps/transfer/types"
transfertypes "github.com/cosmos/ibc-go/v6/modules/apps/transfer/types"
)

Expand Down Expand Up @@ -128,26 +130,44 @@ func (suite *KeeperTestSuite) TestMigrateTotalEscrowOut() {
expectedCoin sdk.Coin
}{
{
msg: "Success: account contains native denom",
msg: "Success: chain contains native denom",
malleate: func() {
suite.chainA.GetSimApp().TransferKeeper.SetIBCOutDenomAmount(
suite.chainA.GetContext(),
sdk.DefaultBondDenom,
sdk.NewInt(100_000_000),
path := NewTransferPath(suite.chainA, suite.chainB)
suite.coordinator.Setup(path)
coin := sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(100))
msg := types.NewMsgTransfer(
path.EndpointA.ChannelConfig.PortID,
path.EndpointA.ChannelID,
coin, suite.chainA.SenderAccount.GetAddress().String(), suite.chainB.SenderAccount.GetAddress().String(),
suite.chainB.GetTimeoutHeight(), 0, "memo",
)

ctx := suite.chainA.GetContext()
_, err := suite.chainA.GetSimApp().TransferKeeper.Transfer(sdk.WrapSDKContext(ctx), msg)
suite.Require().NoError(err)
},
expectedCoin: sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(100_000_000)),
expectedCoin: sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(100)),
},
{
msg: "failure: amount doesnot contain native denom",
msg: "Success: chain contains non native denom",
malleate: func() {
suite.chainA.GetSimApp().TransferKeeper.SetIBCOutDenomAmount(
suite.chainA.GetContext(),
"ibc/stake",
sdk.NewInt(100_000_000),
path := NewTransferPath(suite.chainA, suite.chainB)
suite.coordinator.Setup(path)
coin := sdk.NewCoin("IBC/27394FB092D2ECCD56123C74F36E4C1F926001CEADA9CA97EA622B25F41E5EB2", sdk.NewInt(100))
banktestutil.FundAccount(suite.chainA.GetSimApp().BankKeeper, suite.chainA.GetContext(), suite.chainA.SenderAccount.GetAddress(), sdk.NewCoins(coin))

msg := types.NewMsgTransfer(
path.EndpointA.ChannelConfig.PortID,
path.EndpointA.ChannelID,
coin, suite.chainA.SenderAccount.GetAddress().String(), suite.chainB.SenderAccount.GetAddress().String(),
suite.chainB.GetTimeoutHeight(), 0, "memo",
)

ctx := suite.chainA.GetContext()
_, err := suite.chainA.GetSimApp().TransferKeeper.Transfer(sdk.WrapSDKContext(ctx), msg)
suite.Require().NoError(err)
},
expectedCoin: sdk.NewCoin(sdk.DefaultBondDenom, sdk.ZeroInt()),
expectedCoin: sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(0)),
},
}

Expand Down
2 changes: 1 addition & 1 deletion modules/apps/transfer/keeper/relay.go
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ func (k Keeper) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet, data t
existingToken := k.GetIBCOutDenomAmount(ctx, token.Denom)
existingTokenWithNewTokens := existingToken.Sub(token.Amount)

// store the token about to be IBC'd Out here
// store the token about to be IBC'd Out
k.SetIBCOutDenomAmount(ctx, token.GetDenom(), existingTokenWithNewTokens)

defer func() {
Expand Down
41 changes: 31 additions & 10 deletions modules/apps/transfer/keeper/relay_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,10 +155,11 @@ func (suite *KeeperTestSuite) TestSendTransfer() {
// malleate function allows for testing invalid cases.
func (suite *KeeperTestSuite) TestOnRecvPacket() {
var (
trace types.DenomTrace
amount math.Int
receiver string
memo string
trace types.DenomTrace
amount math.Int
receiver string
expectedEscrowAmt sdk.Int
memo string
)

testCases := []struct {
Expand All @@ -167,11 +168,15 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() {
recvIsSource bool // the receiving chain is the source of the coin originally
expPass bool
}{
{"success receive on source chain", func() {}, true, true},
{"success receive on source chain", func() {
expectedEscrowAmt = sdk.ZeroInt()
}, true, true},
{"success receive on source chain with memo", func() {
memo = "memo"
expectedEscrowAmt = sdk.ZeroInt()
}, true, true},
{"success receive with coin from another chain as source", func() {}, false, true},
{"success receive with coin from another chain as source", func() {
}, false, true},
{"success receive with coin from another chain as source with memo", func() {
memo = "memo"
}, false, true},
Expand Down Expand Up @@ -254,6 +259,10 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() {

if tc.expPass {
suite.Require().NoError(err)
if tc.recvIsSource {
existingToken := suite.chainA.GetSimApp().TransferKeeper.GetIBCOutDenomAmount(suite.chainA.GetContext(), sdk.DefaultBondDenom)
suite.Require().Equal(expectedEscrowAmt, existingToken)
}
} else {
suite.Require().Error(err)
}
Expand Down Expand Up @@ -350,10 +359,11 @@ func (suite *KeeperTestSuite) TestOnAcknowledgementPacket() {
// so the refunds are occurring on chainA.
func (suite *KeeperTestSuite) TestOnTimeoutPacket() {
var (
trace types.DenomTrace
path *ibctesting.Path
amount math.Int
sender string
trace types.DenomTrace
path *ibctesting.Path
amount math.Int
expectedEscrowAmt sdk.Int
sender string
)

testCases := []struct {
Expand All @@ -367,8 +377,12 @@ func (suite *KeeperTestSuite) TestOnTimeoutPacket() {
escrow := types.GetEscrowAddress(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)
trace = types.ParseDenomTrace(sdk.DefaultBondDenom)
coin := sdk.NewCoin(trace.IBCDenom(), amount)
expectedEscrowAmt = sdk.ZeroInt()

// funds the escrow account to have balance
suite.Require().NoError(banktestutil.FundAccount(suite.chainA.GetSimApp().BankKeeper, suite.chainA.GetContext(), escrow, sdk.NewCoins(coin)))
// store the source token thats about to get ibc'd out
suite.chainA.GetSimApp().TransferKeeper.SetIBCOutDenomAmount(suite.chainA.GetContext(), coin.Denom, coin.Amount)
}, true,
},
{
Expand All @@ -377,8 +391,12 @@ func (suite *KeeperTestSuite) TestOnTimeoutPacket() {
escrow := types.GetEscrowAddress(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)
trace = types.ParseDenomTrace(types.GetPrefixedDenom(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, sdk.DefaultBondDenom))
coin := sdk.NewCoin(trace.IBCDenom(), amount)
expectedEscrowAmt = sdk.ZeroInt()

// funds the escrow account to have balance
suite.Require().NoError(banktestutil.FundAccount(suite.chainA.GetSimApp().BankKeeper, suite.chainA.GetContext(), escrow, sdk.NewCoins(coin)))
// store the source token thats about to get ibc'd out
suite.chainA.GetSimApp().TransferKeeper.SetIBCOutDenomAmount(suite.chainA.GetContext(), coin.Denom, coin.Amount)
}, true,
},
{
Expand Down Expand Up @@ -429,6 +447,9 @@ func (suite *KeeperTestSuite) TestOnTimeoutPacket() {
if tc.expPass {
suite.Require().NoError(err)
suite.Require().Equal(amount.Int64(), deltaAmount.Int64(), "successful timeout did not trigger refund")

existingToken := suite.chainA.GetSimApp().TransferKeeper.GetIBCOutDenomAmount(suite.chainA.GetContext(), sdk.DefaultBondDenom)
suite.Require().Equal(expectedEscrowAmt, existingToken)
} else {
suite.Require().Error(err)
}
Expand Down
4 changes: 4 additions & 0 deletions modules/apps/transfer/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,10 @@ func (am AppModule) RegisterServices(cfg module.Configurator) {
if err := cfg.RegisterMigration(types.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.MigrateTotalEscrowOut); err != nil {
panic(fmt.Sprintf("failed to migrate total escrow amount: %v", err))
}
}

// InitGenesis performs genesis initialization for the ibc-transfer module. It returns
Expand Down
2 changes: 1 addition & 1 deletion modules/apps/transfer/types/keys.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,5 +56,5 @@ func GetEscrowAddress(portID, channelID string) sdk.AccAddress {

// GetIBCOutDenom holds the total ibcout for native tokens per transfer.
func GetTotalEscrowForDenomKey(denom string) []byte {
return []byte(fmt.Sprintf("total_native_ibc_out/%s", denom))
return []byte(fmt.Sprintf("total_escrow/%s", denom))
}
Loading

0 comments on commit dc1a91c

Please sign in to comment.