Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

introduce refund logic for ibc transfer if tx failed by timeout. #498

Merged
merged 3 commits into from
Apr 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion app/ibctesting/simapp/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,8 @@ import (
simappupgrades "github.com/notional-labs/composable/v6/app/ibctesting/simapp/upgrades"
v6 "github.com/notional-labs/composable/v6/app/ibctesting/simapp/upgrades/v6"
v7 "github.com/notional-labs/composable/v6/app/ibctesting/simapp/upgrades/v7"
ibctransfermiddleware "github.com/notional-labs/composable/v6/x/ibctransfermiddleware/keeper"
ibctransfermiddlewaretypes "github.com/notional-labs/composable/v6/x/ibctransfermiddleware/types"
transfermiddleware "github.com/notional-labs/composable/v6/x/transfermiddleware"
transfermiddlewarekeeper "github.com/notional-labs/composable/v6/x/transfermiddleware/keeper"
transfermiddlewaretypes "github.com/notional-labs/composable/v6/x/transfermiddleware/types"
Expand Down Expand Up @@ -258,7 +260,8 @@ type SimApp struct {
ICAAuthModule ibcmock.IBCModule
FeeMockModule ibcmock.IBCModule

TransferMiddlewarekeeper transfermiddlewarekeeper.Keeper
TransferMiddlewarekeeper transfermiddlewarekeeper.Keeper
IbcTransferMiddlewareKeeper ibctransfermiddleware.Keeper

// the module manager
mm *module.Manager
Expand Down Expand Up @@ -378,13 +381,21 @@ func NewSimApp(

app.FeeGrantKeeper = feegrantkeeper.NewKeeper(appCodec, keys[feegrant.StoreKey], app.AccountKeeper)
app.UpgradeKeeper = upgradekeeper.NewKeeper(skipUpgradeHeights, keys[upgradetypes.StoreKey], appCodec, homePath, app.BaseApp, authtypes.NewModuleAddress(govtypes.ModuleName).String())
app.IbcTransferMiddlewareKeeper = ibctransfermiddleware.NewKeeper(appCodec, keys[ibctransfermiddlewaretypes.StoreKey], authtypes.NewModuleAddress(govtypes.ModuleName).String(),
[]string{

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's this?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think this is for setting fees. this is for testnet

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so it's an unrelated change?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this needed to make tests build success.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we introduce keeper to ibc hook
so we need to pass it now in tests.
this i why i added a IbcTransferMiddlewareKeeper to pass into ibc hook

"pica1ay9y5uns9khw2kzaqr3r33v2pkuptfnnunlt5x",
"pica14lz7gaw92valqjearnye4shex7zg2p05yfguqm",
"pica1r2zlh2xn85v8ljmwymnfrnsmdzjl7k6w9f2ja8",
"pica10556m38z4x6pqalr9rl5ytf3cff8q46nf36090",
})
app.TransferMiddlewarekeeper = transfermiddlewarekeeper.NewKeeper(
keys[transfermiddlewaretypes.StoreKey],
app.GetSubspace(transfermiddlewaretypes.ModuleName),
appCodec,
app.IBCKeeper.ChannelKeeper,
app.TransferKeeper,
app.BankKeeper,
&app.IbcTransferMiddlewareKeeper,
authtypes.NewModuleAddress(govtypes.ModuleName).String(),
)
app.AuthzKeeper = authzkeeper.NewKeeper(keys[authzkeeper.StoreKey], appCodec, app.MsgServiceRouter(), app.AccountKeeper)
Expand Down
1 change: 1 addition & 0 deletions app/keepers/keepers.go
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,7 @@ func (appKeepers *AppKeepers) InitNormalKeepers(
&appKeepers.RatelimitKeeper,
&appKeepers.TransferKeeper,
appKeepers.BankKeeper,
&appKeepers.IbcTransferMiddlewareKeeper,
authorityAddress,
)

Expand Down
2 changes: 1 addition & 1 deletion custom/ibc-transfer/keeper/msg_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ func (k msgServer) Transfer(goCtx context.Context, msg *types.MsgTransfer) (*typ

newAmount := msg.Token.Amount.Sub(charge)

if newAmount.IsPositive() {
if newAmount.IsPositive() && coin.Percentage != 0 {
percentageCharge := newAmount.QuoRaw(coin.Percentage)
newAmount = newAmount.Sub(percentageCharge)
charge = charge.Add(percentageCharge)
Expand Down
18 changes: 9 additions & 9 deletions scripts/testnode.sh
Original file line number Diff line number Diff line change
Expand Up @@ -17,24 +17,24 @@ DENOM=${2:-ppica}
# remove existing daemon
rm -rf ~/.banksy*

centaurid config keyring-backend $KEYRING
centaurid config chain-id $CHAINID
picad config keyring-backend $KEYRING

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why change this?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

prefix change

picad config chain-id $CHAINID

# if $KEY exists it should be deleted
echo "decorate bright ozone fork gallery riot bus exhaust worth way bone indoor calm squirrel merry zero scheme cotton until shop any excess stage laundry" | centaurid keys add $KEY --keyring-backend $KEYRING --algo $KEYALGO --recover
echo "decorate bright ozone fork gallery riot bus exhaust worth way bone indoor calm squirrel merry zero scheme cotton until shop any excess stage laundry" | picad keys add $KEY --keyring-backend $KEYRING --algo $KEYALGO --recover

centaurid init $MONIKER --chain-id $CHAINID
picad init $MONIKER --chain-id $CHAINID

update_test_genesis () {
# update_test_genesis '.consensus_params["block"]["max_gas"]="100000000"'
cat $HOME_DIR/config/genesis.json | jq "$1" > $HOME_DIR/config/tmp_genesis.json && mv $HOME_DIR/config/tmp_genesis.json $HOME_DIR/config/genesis.json
}

# Allocate genesis accounts (cosmos formatted addresses)
centaurid add-genesis-account $KEY 100000000000000000000000000ppica --keyring-backend $KEYRING
picad add-genesis-account $KEY 100000000000000000000000000ppica --keyring-backend $KEYRING

# Sign genesis transaction
centaurid gentx $KEY 10030009994127689ppica --keyring-backend $KEYRING --chain-id $CHAINID
picad gentx $KEY 10030009994127689ppica --keyring-backend $KEYRING --chain-id $CHAINID

update_test_genesis '.app_state["gov"]["params"]["voting_period"]="20s"'
update_test_genesis '.app_state["mint"]["params"]["mint_denom"]="'$DENOM'"'
Expand All @@ -43,10 +43,10 @@ update_test_genesis '.app_state["crisis"]["constant_fee"]={"denom":"'$DENOM'","a
update_test_genesis '.app_state["staking"]["params"]["bond_denom"]="'$DENOM'"'

# Collect genesis tx
centaurid collect-gentxs
picad collect-gentxs

# Run this to ensure everything worked and that the genesis file is setup correctly
centaurid validate-genesis
picad validate-genesis

if [[ $1 == "pending" ]]; then
echo "pending mode is on, please wait for the first block committed."
Expand All @@ -57,4 +57,4 @@ fi
sed -i'' -e 's/max_body_bytes = /max_body_bytes = 1/g' ~/.banksy/config/config.toml

# Start the node (remove the --pruning=nothing flag if historical queries are not needed)
# centaurid start --pruning=nothing --minimum-gas-prices=0.0001ppica --rpc.laddr tcp://0.0.0.0:26657
picad start --pruning=nothing --minimum-gas-prices=0.0001ppica --rpc.laddr tcp://0.0.0.0:26657
18 changes: 18 additions & 0 deletions x/ibctransfermiddleware/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,3 +64,21 @@ func (k Keeper) GetParams(ctx sdk.Context) (p types.Params) {
k.cdc.MustUnmarshal(bz, &p)
return p
}

func (k Keeper) GetCoin(ctx sdk.Context, targetChannelID, denom string) *types.CoinItem {
params := k.GetParams(ctx)
channelFee := findChannelParams(params.ChannelFees, targetChannelID)
if channelFee == nil {
return nil
}
return findCoinByDenom(channelFee.AllowedTokens, denom)
}

func (k Keeper) GetChannelFeeAddress(ctx sdk.Context, targetChannelID string) string {
params := k.GetParams(ctx)
channelFee := findChannelParams(params.ChannelFees, targetChannelID)
if channelFee == nil {
return ""
}
return channelFee.FeeAddress
}
35 changes: 35 additions & 0 deletions x/transfermiddleware/ibc_middleware.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,41 @@ func (im IBCMiddleware) OnTimeoutPacket(ctx sdk.Context, packet channeltypes.Pac
return err
}

denom := data.Denom
coin := im.keeper.IbcTransfermiddleware.GetCoin(ctx, packet.SourceChannel, denom)
сhannelFeeAddress := im.keeper.IbcTransfermiddleware.GetChannelFeeAddress(ctx, packet.SourceChannel)
if coin != nil {
amount := data.Amount
transferAmount, ok := sdk.NewIntFromString(amount)
if !ok {
return errors.Wrapf(transfertypes.ErrInvalidAmount, "unable to parse transfer amount: %s", amount)
}

// calculate the percentage charge
/*
coin.Percentage is 100 that means that we charge 1/100 of the transfer amount

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we charge 1%? Where does this number come from?

Copy link
Collaborator Author

@RustNinja RustNinja Apr 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is example.
we charge depends on config.
I described how it calculates.
To find a percentage fee from a new amount

coin.MinFee.Amount is the minimum fee that we charge
the new amount is the transfer amount minus the percentage charge and the minimum fee
transfer amount is a 99/100 of the original amount
so to get the fee we charge transferAmount.QuoRaw(coin.Percentage - 1) + coin.MinFee.Amount
*/

percentageCharge := sdk.NewInt(0)
if coin.Percentage > 1 {
percentageCharge = transferAmount.QuoRaw(coin.Percentage - 1)
}
percentageCharge = percentageCharge.Add(coin.MinFee.Amount)

fee_address, err := sdk.AccAddressFromBech32(сhannelFeeAddress)
if err != nil {
return errors.Wrapf(err, "failed to decode receiver address: %s", сhannelFeeAddress)
}

refund_fee := sdk.NewCoin(denom, percentageCharge)
im.keeper.RefundChannelCosmosFee(ctx, fee_address, sdk.AccAddress(data.Sender), []sdk.Coin{refund_fee})

}

return nil
}

Expand Down
39 changes: 25 additions & 14 deletions x/transfermiddleware/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,18 @@ import (
porttypes "github.com/cosmos/ibc-go/v7/modules/core/05-port/types"
"github.com/cosmos/ibc-go/v7/modules/core/exported"

ibctransfermiddleware "github.com/notional-labs/composable/v6/x/ibctransfermiddleware/keeper"
"github.com/notional-labs/composable/v6/x/transfermiddleware/types"
)

type Keeper struct {
cdc codec.BinaryCodec
storeKey storetypes.StoreKey
paramSpace paramtypes.Subspace
ICS4Wrapper porttypes.ICS4Wrapper
bankKeeper types.BankKeeper
transferKeeper types.TransferKeeper

cdc codec.BinaryCodec
storeKey storetypes.StoreKey
paramSpace paramtypes.Subspace
ICS4Wrapper porttypes.ICS4Wrapper
bankKeeper types.BankKeeper
transferKeeper types.TransferKeeper
IbcTransfermiddleware *ibctransfermiddleware.Keeper
// the address capable of executing a AddParachainIBCTokenInfo and RemoveParachainIBCTokenInfo message. Typically, this
// should be the x/gov module account.
authority string
Expand All @@ -39,6 +40,7 @@ func NewKeeper(
ics4Wrapper porttypes.ICS4Wrapper,
transferKeeper types.TransferKeeper,
bankKeeper types.BankKeeper,
ibcTransfermiddleware *ibctransfermiddleware.Keeper,
authority string,
) Keeper {
// set KeyTable if it has not already been set
Expand All @@ -47,13 +49,14 @@ func NewKeeper(
}

return Keeper{
storeKey: storeKey,
paramSpace: paramSpace,
transferKeeper: transferKeeper,
bankKeeper: bankKeeper,
cdc: codec,
ICS4Wrapper: ics4Wrapper,
authority: authority,
storeKey: storeKey,
paramSpace: paramSpace,
transferKeeper: transferKeeper,
bankKeeper: bankKeeper,
cdc: codec,
ICS4Wrapper: ics4Wrapper,
IbcTransfermiddleware: ibcTransfermiddleware,
authority: authority,
}
}

Expand Down Expand Up @@ -233,6 +236,14 @@ func (keeper Keeper) GetTotalEscrowedToken(ctx sdk.Context) (coins sdk.Coins) {
return coins
}

func (keeper Keeper) RefundChannelCosmosFee(ctx sdk.Context, sender, receiver sdk.AccAddress, amount sdk.Coins) error {
if err := keeper.bankKeeper.SendCoins(ctx, sender, receiver, amount); err != nil {
return errorsmod.Wrap(err, "failed to refund channel cosmos fee")
}

return nil
}

func (keeper Keeper) Logger(ctx sdk.Context) log.Logger {
return ctx.Logger().With("module", "x/"+exported.ModuleName+"-"+types.ModuleName)
}
Loading