diff --git a/app/app.go b/app/app.go index 56d1cc177a..bbea01fa15 100644 --- a/app/app.go +++ b/app/app.go @@ -333,7 +333,7 @@ func NewStrideApp( appCodec, keys[authtypes.StoreKey], app.GetSubspace(authtypes.ModuleName), authtypes.ProtoBaseAccount, maccPerms, ) app.BankKeeper = bankkeeper.NewBaseKeeper( - appCodec, keys[banktypes.StoreKey], app.AccountKeeper, app.GetSubspace(banktypes.ModuleName), app.ModuleAccountAddrs(), + appCodec, keys[banktypes.StoreKey], app.AccountKeeper, app.GetSubspace(banktypes.ModuleName), app.ModuleAccountAddrsToInd(), ) stakingKeeper := stakingkeeper.NewKeeper( appCodec, keys[stakingtypes.StoreKey], app.AccountKeeper, app.BankKeeper, app.GetSubspace(stakingtypes.ModuleName), @@ -343,7 +343,7 @@ func NewStrideApp( ) app.DistrKeeper = distrkeeper.NewKeeper( appCodec, keys[distrtypes.StoreKey], app.GetSubspace(distrtypes.ModuleName), app.AccountKeeper, app.BankKeeper, - &stakingKeeper, authtypes.FeeCollectorName, app.ModuleAccountAddrs(), + &stakingKeeper, authtypes.FeeCollectorName, app.ModuleAccountAddrsToInd(), ) app.SlashingKeeper = slashingkeeper.NewKeeper( appCodec, keys[slashingtypes.StoreKey], &stakingKeeper, app.GetSubspace(slashingtypes.ModuleName), @@ -426,12 +426,13 @@ func NewStrideApp( app.GetSubspace(recordsmoduletypes.ModuleName), scopedRecordsKeeper, app.AccountKeeper, + app.BankKeeper, ) recordsModule := recordsmodule.NewAppModule(appCodec, app.RecordsKeeper, app.AccountKeeper, app.BankKeeper) // create IBC stacks by combining middleware with base application // recordsStack contains records -> transfer - recordsStack := recordsmodule.NewIBCModule(app.RecordsKeeper, transferIBCModule) + recordsStack := recordsmodule.NewIBCModule(app.RecordsKeeper, transferIBCModule, app.ModuleAccountNameToAddrs()) app.ICAControllerKeeper = icacontrollerkeeper.NewKeeper( appCodec, keys[icacontrollertypes.StoreKey], app.GetSubspace(icacontrollertypes.SubModuleName), @@ -761,8 +762,18 @@ func (app *StrideApp) LoadHeight(height int64) error { return app.LoadVersion(height) } +// ModuleAccountNameToAddrs returns a mapping of each module's name to address +func (app *StrideApp) ModuleAccountNameToAddrs() map[string]string { + modAccAddrs := make(map[string]string) + for _, moduleName := range utils.StringToStringSliceMapKeys(maccPerms) { + modAccAddrs[moduleName] = authtypes.NewModuleAddress(moduleName).String() + } + + return modAccAddrs +} + // ModuleAccountAddrs returns all the app's module account addresses. -func (app *StrideApp) ModuleAccountAddrs() map[string]bool { +func (app *StrideApp) ModuleAccountAddrsToInd() map[string]bool { modAccAddrs := make(map[string]bool) for _, acc := range utils.StringToStringSliceMapKeys(maccPerms) { modAccAddrs[authtypes.NewModuleAddress(acc).String()] = true diff --git a/scripts/tests/1.sh b/scripts/tests/1.sh index 0d85179412..5c3b1e8df7 100755 --- a/scripts/tests/1.sh +++ b/scripts/tests/1.sh @@ -6,6 +6,6 @@ STRIDE_ADDRESS="stride1uk4ze0x4nvh4fk0xm4jdud58eqn4yxhrt52vv7" # ibc over atoms to stride $GAIA1_EXEC tx ibc-transfer transfer transfer channel-0 $STRIDE_ADDRESS 100000uatom --from gval1 --chain-id GAIA -y --keyring-backend test -SLEEP 5 +SLEEP 20 $STR1_EXEC q bank balances $STRIDE_ADDRESS diff --git a/x/records/keeper/keeper.go b/x/records/keeper/keeper.go index 2edd297059..5562f9b241 100644 --- a/x/records/keeper/keeper.go +++ b/x/records/keeper/keeper.go @@ -5,23 +5,26 @@ import ( "github.com/tendermint/tendermint/libs/log" - "github.com/Stride-Labs/stride/x/records/types" "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" + bankkeeper "github.com/cosmos/cosmos-sdk/x/bank/keeper" capabilitykeeper "github.com/cosmos/cosmos-sdk/x/capability/keeper" capabilitytypes "github.com/cosmos/cosmos-sdk/x/capability/types" paramtypes "github.com/cosmos/cosmos-sdk/x/params/types" + + "github.com/Stride-Labs/stride/x/records/types" ) type ( Keeper struct { // *cosmosibckeeper.Keeper - Cdc codec.BinaryCodec - storeKey sdk.StoreKey - memKey sdk.StoreKey - paramstore paramtypes.Subspace - scopedKeeper capabilitykeeper.ScopedKeeper + Cdc codec.BinaryCodec + storeKey sdk.StoreKey + memKey sdk.StoreKey + paramstore paramtypes.Subspace + scopedKeeper capabilitykeeper.ScopedKeeper AccountKeeper types.AccountKeeper + BankKeeper bankkeeper.Keeper } ) @@ -32,7 +35,7 @@ func NewKeeper( ps paramtypes.Subspace, scopedKeeper capabilitykeeper.ScopedKeeper, AccountKeeper types.AccountKeeper, - + BankKeeper bankkeeper.Keeper, ) *Keeper { // set KeyTable if it has not already been set if !ps.HasKeyTable() { @@ -40,12 +43,13 @@ func NewKeeper( } return &Keeper{ - Cdc: Cdc, - storeKey: storeKey, - memKey: memKey, - paramstore: ps, - scopedKeeper: scopedKeeper, + Cdc: Cdc, + storeKey: storeKey, + memKey: memKey, + paramstore: ps, + scopedKeeper: scopedKeeper, AccountKeeper: AccountKeeper, + BankKeeper: BankKeeper, } } @@ -56,4 +60,4 @@ func (k Keeper) Logger(ctx sdk.Context) log.Logger { // ClaimCapability claims the channel capability passed via the OnOpenChanInit callback func (k *Keeper) ClaimCapability(ctx sdk.Context, cap *capabilitytypes.Capability, name string) error { return k.scopedKeeper.ClaimCapability(ctx, cap, name) -} \ No newline at end of file +} diff --git a/x/records/module_ibc.go b/x/records/module_ibc.go index b356507e02..6eff2b91c7 100644 --- a/x/records/module_ibc.go +++ b/x/records/module_ibc.go @@ -11,6 +11,7 @@ import ( channeltypes "github.com/cosmos/ibc-go/v3/modules/core/04-channel/types" porttypes "github.com/cosmos/ibc-go/v3/modules/core/05-port/types" + "github.com/Stride-Labs/stride/utils" "github.com/Stride-Labs/stride/x/records/keeper" "github.com/Stride-Labs/stride/x/records/types" stakeibctypes "github.com/Stride-Labs/stride/x/stakeibc/types" @@ -23,15 +24,17 @@ import ( // IBC MODULE IMPLEMENTATION // IBCModule implements the ICS26 interface for transfer given the transfer keeper. type IBCModule struct { - keeper keeper.Keeper - app porttypes.IBCModule + keeper keeper.Keeper + app porttypes.IBCModule + moduleAddresses map[string]string } // NewIBCModule creates a new IBCModule given the keeper -func NewIBCModule(k keeper.Keeper, app porttypes.IBCModule) IBCModule { +func NewIBCModule(k keeper.Keeper, app porttypes.IBCModule, moduleAddresses map[string]string) IBCModule { return IBCModule{ - keeper: k, - app: app, + keeper: k, + app: app, + moduleAddresses: moduleAddresses, } } @@ -220,7 +223,104 @@ func (im IBCModule) OnTimeoutPacket( relayer sdk.AccAddress, ) error { // doCustomLogic(packet) - return im.app.OnTimeoutPacket(ctx, packet, relayer) + return im.ICS_20_module_patch_OnTimeoutPacket(ctx, packet, relayer) +} + +// Wrapper for the refundPacketToken patch +// (Identical to the transfer module's `OnTimeoutPacket` except for the `refundPacketToken` call which calls the patched function) +func (im IBCModule) ICS_20_module_patch_OnTimeoutPacket( + ctx sdk.Context, + packet channeltypes.Packet, + relayer sdk.AccAddress, +) error { + var data ibctransfertypes.FungibleTokenPacketData + if err := types.ModuleCdc.UnmarshalJSON(packet.GetData(), &data); err != nil { + return sdkerrors.Wrapf(sdkerrors.ErrUnknownRequest, "cannot unmarshal ICS-20 transfer packet data: %s", err.Error()) + } + // refund tokens + if err := im.ICS_20_module_patch_refundPacketToken(ctx, packet, data); err != nil { + return err + } + + ctx.EventManager().EmitEvent( + sdk.NewEvent( + types.EventTypeTimeout, + sdk.NewAttribute(sdk.AttributeKeyModule, types.ModuleName), + sdk.NewAttribute(ibctransfertypes.AttributeKeyRefundReceiver, data.Sender), + sdk.NewAttribute(ibctransfertypes.AttributeKeyRefundDenom, data.Denom), + sdk.NewAttribute(ibctransfertypes.AttributeKeyRefundAmount, data.Amount), + ), + ) + + return nil +} + +// The refundPacketToken function in the IBC transfer module will panic if it attempts to refund to a module address +// The module's address is blacklisted in the bankKeeper's `blockedAddr`, which is referenced in `SendCoinsFromModuleToAccount`` +// This must be patched, otherwise, if a transfer from a module account fails, it will not be refunded +func (im IBCModule) ICS_20_module_patch_refundPacketToken( + ctx sdk.Context, + packet channeltypes.Packet, + data ibctransfertypes.FungibleTokenPacketData, +) error { + // NOTE: packet data type already checked in handler.go + + // parse the denomination from the full denom path + trace := ibctransfertypes.ParseDenomTrace(data.Denom) + + // parse the transfer amount + transferAmount, ok := sdk.NewIntFromString(data.Amount) + if !ok { + return sdkerrors.Wrapf(ibctransfertypes.ErrInvalidAmount, "unable to parse transfer amount (%s) into math.Int", data.Amount) + } + token := sdk.NewCoin(trace.IBCDenom(), transferAmount) + + // decode the sender address + sender, err := sdk.AccAddressFromBech32(data.Sender) + if err != nil { + return err + } + + if ibctransfertypes.SenderChainIsSource(packet.GetSourcePort(), packet.GetSourceChannel(), data.Denom) { + // unescrow tokens back to sender + escrowAddress := ibctransfertypes.GetEscrowAddress(packet.GetSourcePort(), packet.GetSourceChannel()) + if err := im.keeper.BankKeeper.SendCoins(ctx, escrowAddress, sender, sdk.NewCoins(token)); err != nil { + // NOTE: this error is only expected to occur given an unexpected bug or a malicious + // counterparty module. The bug may occur in bank or any part of the code that allows + // the escrow address to be drained. A malicious counterparty module could drain the + // escrow address by allowing more tokens to be sent back then were escrowed. + return sdkerrors.Wrap(err, "unable to unescrow tokens, this may be caused by a malicious counterparty module or a bug: please open an issue on counterparty module") + } + + return nil + } + + // mint vouchers back to sender + if err := im.keeper.BankKeeper.MintCoins(ctx, ibctransfertypes.ModuleName, sdk.NewCoins(token)); err != nil { + return err + } + + // If the recipient is a module address, use SendCoinsFromModuleToModule + for _, moduleName := range utils.StringToStringMapKeys(im.moduleAddresses) { + moduleAddress := im.moduleAddresses[moduleName] + if sender.String() == moduleAddress { + if err := im.keeper.BankKeeper.SendCoinsFromModuleToModule(ctx, ibctransfertypes.ModuleName, moduleName, sdk.NewCoins(token)); err != nil { + errMsg := fmt.Sprintf("unable to send coins from module (%s) to module (%s) ", types.ModuleName, moduleName) + errMsg += fmt.Sprintf("despite previously minting coins to module account: %v", err) + panic(errMsg) + } + return nil + } + } + + // Otherwise, if the recipient is not a module, use SendCoinsFromModuleToAccount + if err := im.keeper.BankKeeper.SendCoinsFromModuleToAccount(ctx, ibctransfertypes.ModuleName, sender, sdk.NewCoins(token)); err != nil { + errMsg := fmt.Sprintf("unable to send coins from module (%s) to account (%s) ", types.ModuleName, sender.String()) + errMsg += fmt.Sprintf("despite previously minting coins to module account: %v", err) + panic(errMsg) + } + + return nil } // This is implemented by ICS4 and all middleware that are wrapping base application.