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

[CORE-834] Wire x/ratelimit methods to IBC transfer logic #962

Merged
merged 8 commits into from
Jan 26, 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
43 changes: 24 additions & 19 deletions protocol/app/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -569,11 +569,13 @@ func New(
// Set legacy router for backwards compatibility with gov v1beta1
govKeeper.SetLegacyRouter(govRouter)

// grant capabilities for the ibc, ibc-transfer and ICAHostKeeper modules
// grant capabilities for the ibc, ibc-transfer, ICAHostKeeper and ratelimit modules

scopedIBCKeeper := app.CapabilityKeeper.ScopeToModule(ibcexported.ModuleName)
scopedIBCTransferKeeper := app.CapabilityKeeper.ScopeToModule(ibctransfertypes.ModuleName)
scopedICAHostKeeper := app.CapabilityKeeper.ScopeToModule(icahosttypes.SubModuleName)
// scopedRatelimitKeeper is not used as an input to any other module.
app.CapabilityKeeper.ScopeToModule(ratelimitmoduletypes.ModuleName)
Comment on lines +572 to +578
Copy link
Contributor

Choose a reason for hiding this comment

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

The addition of the ratelimit module to the capabilities grant is noted. However, the scoped keeper for ratelimit is created but not stored or used. This could potentially be an oversight.

Ensure that the scoped keeper for the ratelimit module is stored and used appropriately, or remove it if it's unnecessary.


app.CapabilityKeeper.Seal()

Expand Down Expand Up @@ -602,22 +604,6 @@ func New(
lib.GovModuleAddress.String(), // authority
)

// Create Transfer Keepers
app.TransferKeeper = ibctransferkeeper.NewKeeper(
appCodec,
keys[ibctransfertypes.StoreKey],
app.getSubspace(ibctransfertypes.ModuleName),
app.IBCKeeper.ChannelKeeper,
app.IBCKeeper.ChannelKeeper,
app.IBCKeeper.PortKeeper,
app.AccountKeeper,
app.BankKeeper,
scopedIBCTransferKeeper,
lib.GovModuleAddress.String(),
)
transferModule := transfer.NewAppModule(app.TransferKeeper)
transferIBCModule := transfer.NewIBCModule(app.TransferKeeper)

app.BlockTimeKeeper = *blocktimemodulekeeper.NewKeeper(
appCodec,
keys[blocktimemoduletypes.StoreKey],
Expand All @@ -634,6 +620,7 @@ func New(
keys[ratelimitmoduletypes.StoreKey],
app.BankKeeper,
app.BlockTimeKeeper,
app.IBCKeeper.ChannelKeeper, // ICS4Wrapper
// set the governance and delaymsg module accounts as the authority for conducting upgrades
[]string{
lib.GovModuleAddress.String(),
Expand All @@ -642,13 +629,31 @@ func New(
)
rateLimitModule := ratelimitmodule.NewAppModule(appCodec, app.RatelimitKeeper)

// TODO(CORE-834): Add ratelimitKeeper to the IBC transfer stack.
// Create Transfer Keepers
app.TransferKeeper = ibctransferkeeper.NewKeeper(
appCodec,
keys[ibctransfertypes.StoreKey],
app.getSubspace(ibctransfertypes.ModuleName),
app.RatelimitKeeper, // ICS4Wrapper
app.IBCKeeper.ChannelKeeper,
app.IBCKeeper.PortKeeper,
app.AccountKeeper,
app.BankKeeper,
scopedIBCTransferKeeper,
lib.GovModuleAddress.String(),
)
transferModule := transfer.NewAppModule(app.TransferKeeper)
transferIBCModule := transfer.NewIBCModule(app.TransferKeeper)

// Wrap the x/ratelimit middlware over the IBC Transfer module
var transferStack ibcporttypes.IBCModule = transferIBCModule
transferStack = ratelimitmodule.NewIBCMiddleware(app.RatelimitKeeper, transferStack)

icaHostIBCModule := icahost.NewIBCModule(app.ICAHostKeeper)
// Create static IBC router, add transfer route, then set and seal it
ibcRouter := ibcporttypes.NewRouter()
// Ordering of `AddRoute` does not matter.
ibcRouter.AddRoute(ibctransfertypes.ModuleName, transferIBCModule)
ibcRouter.AddRoute(ibctransfertypes.ModuleName, transferStack)
ibcRouter.AddRoute(icahosttypes.SubModuleName, icaHostIBCModule)

app.IBCKeeper.SetRouter(ibcRouter)
Expand Down
7 changes: 4 additions & 3 deletions protocol/lib/metrics/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -403,9 +403,10 @@ const (
ValidatorVolumeQuoteQuantums = "validator_volume_quote_quantums"

// x/ratelimit
Capacity = "capacity"
RateLimitDenom = "rate_limit_denom"
LimiterIndex = "limiter_index"
Capacity = "capacity"
RateLimitDenom = "rate_limit_denom"
LimiterIndex = "limiter_index"
UndoWithdrawAmount = "undo_withdraw_amount"

// x/authz
MsgExec = "msg_exec"
Expand Down
212 changes: 212 additions & 0 deletions protocol/x/ratelimit/ibc_middleware.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,212 @@
package ratelimit

// TODO(CORE-854): Improve attribution message.
// This file was written with reference to Stride's IBC Rate Limit implementation:
// https://github.com/Stride-Labs/stride/blob/121f2ac5d2e5f8e406f89999410a49ea4277a552/x/ratelimit

import (
"fmt"

"github.com/dydxprotocol/v4-chain/protocol/x/ratelimit/keeper"
"github.com/dydxprotocol/v4-chain/protocol/x/ratelimit/types"
"github.com/dydxprotocol/v4-chain/protocol/x/ratelimit/util"

sdk "github.com/cosmos/cosmos-sdk/types"
capabilitytypes "github.com/cosmos/ibc-go/modules/capability/types"
clienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types" //nolint:staticcheck
channeltypes "github.com/cosmos/ibc-go/v8/modules/core/04-channel/types"
porttypes "github.com/cosmos/ibc-go/v8/modules/core/05-port/types"

"github.com/cosmos/ibc-go/v8/modules/core/exported"
)

var _ porttypes.Middleware = &IBCMiddleware{}

type IBCMiddleware struct {
app porttypes.IBCModule
keeper keeper.Keeper
}

// TODO(CORE-855): Add e2e testing for the x/ratelimit IBC Middlware
func NewIBCMiddleware(k keeper.Keeper, app porttypes.IBCModule) IBCMiddleware {
return IBCMiddleware{
app: app,
keeper: k,
}
}

// OnChanOpenInit implements the IBCMiddleware interface.
// Use default implementation of the underlying IBC application, since rate-limiting logic
// doesn't require customized logic at channel initialization.
func (im IBCMiddleware) OnChanOpenInit(ctx sdk.Context,
order channeltypes.Order,
connectionHops []string,
portID string,
channelID string,
channelCap *capabilitytypes.Capability,
counterparty channeltypes.Counterparty,
version string,
) (string, error) {
return im.app.OnChanOpenInit(
ctx,
order,
connectionHops,
portID,
channelID,
channelCap,
counterparty,
version,
)
}

// OnChanOpenTry implements the IBCMiddleware interface
// Use default implementation of the underlying IBC application, since rate-limiting logic
// doesn't require customized logic at channel opening.
func (im IBCMiddleware) OnChanOpenTry(
ctx sdk.Context,
order channeltypes.Order,
connectionHops []string,
portID,
channelID string,
channelCap *capabilitytypes.Capability,
counterparty channeltypes.Counterparty,
counterpartyVersion string,
) (string, error) {
return im.app.OnChanOpenTry(
ctx, order,
connectionHops,
portID,
channelID,
channelCap,
counterparty,
counterpartyVersion,
)
}

// OnChanOpenAck implements the IBCMiddleware interface
func (im IBCMiddleware) OnChanOpenAck(
ctx sdk.Context,
portID,
channelID string,
counterpartyChannelID string,
counterpartyVersion string,
) error {
return im.app.OnChanOpenAck(ctx, portID, channelID, counterpartyChannelID, counterpartyVersion)
}

// OnChanOpenConfirm implements the IBCMiddleware interface
func (im IBCMiddleware) OnChanOpenConfirm(
ctx sdk.Context,
portID,
channelID string,
) error {
return im.app.OnChanOpenConfirm(ctx, portID, channelID)
}

// OnChanCloseInit implements the IBCMiddleware interface
func (im IBCMiddleware) OnChanCloseInit(
ctx sdk.Context,
portID,
channelID string,
) error {
return im.app.OnChanCloseInit(ctx, portID, channelID)
}

// OnChanCloseConfirm implements the IBCMiddleware interface
func (im IBCMiddleware) OnChanCloseConfirm(
ctx sdk.Context,
portID,
channelID string,
) error {
return im.app.OnChanCloseConfirm(ctx, portID, channelID)
}

// OnRecvPacket implements the IBCMiddleware interface
// Called on the receiver chain when a relayer pick up the `SendPacket` event from sender chain and relayer
// to the receiver chain. On dYdX chain, this signals an inbound IBC transfer.
// Does the following:
// - Call `IncrementCapacitiesForDenom` to update `capacity` for the token received.
// - Invoke `OnRecvPacket` callback on IBC transfer module.
func (im IBCMiddleware) OnRecvPacket(
ctx sdk.Context,
packet channeltypes.Packet,
relayer sdk.AccAddress,
) exported.Acknowledgement {
ibcTransferPacketInfo, err := util.ParsePacketInfo(packet, types.PACKET_RECV)
if err != nil {
im.keeper.Logger(ctx).Error(fmt.Sprintf("ICS20 receive packet could not be parsed: %s", err.Error()))
return channeltypes.NewErrorAcknowledgement(err)
}

// Process deposit of incoming asset.
// If the `Recv` fails on the underlying transfer stack (e.g. Reciver is invalid), the state
// change is not committed.
// TODO(CORE-855): Add an E2E test for this.
im.keeper.IncrementCapacitiesForDenom(ctx, ibcTransferPacketInfo.Denom, ibcTransferPacketInfo.Amount)

return im.app.OnRecvPacket(ctx, packet, relayer)
}

// OnAcknowledgementPacket implements the IBCMiddleware interface
func (im IBCMiddleware) OnAcknowledgementPacket(
ctx sdk.Context,
packet channeltypes.Packet,
acknowledgement []byte,
relayer sdk.AccAddress,
) error {
if err := im.keeper.AcknowledgeIBCTransferPacket(ctx, packet, acknowledgement); err != nil {
im.keeper.Logger(ctx).Error(fmt.Sprintf("ICS20 OnAckPacket failed: %s", err.Error()))
return err
}
return im.app.OnAcknowledgementPacket(ctx, packet, acknowledgement, relayer)
}

// OnTimeoutPacket implements the IBCMiddleware interface
func (im IBCMiddleware) OnTimeoutPacket(
ctx sdk.Context,
packet channeltypes.Packet,
relayer sdk.AccAddress,
) error {
if err := im.keeper.TimeoutIBCTransferPacket(ctx, packet); err != nil {
im.keeper.Logger(ctx).Error(fmt.Sprintf("ICS20 OnTimeoutPacket failed: %s", err.Error()))
return err
}
return im.app.OnTimeoutPacket(ctx, packet, relayer)
}

// SendPacket implements the ICS4 Wrapper interface
// Rate-limited SendPacket found in RateLimit Keeper
func (im IBCMiddleware) SendPacket(
ctx sdk.Context,
chanCap *capabilitytypes.Capability,
sourcePort string,
sourceChannel string,
timeoutHeight clienttypes.Height,
timeoutTimestamp uint64,
data []byte,
) (sequence uint64, err error) {
return im.keeper.SendPacket(
ctx,
chanCap,
sourcePort,
sourceChannel,
timeoutHeight,
timeoutTimestamp,
data,
)
}

// WriteAcknowledgement implements the ICS4 Wrapper interface
func (im IBCMiddleware) WriteAcknowledgement(
ctx sdk.Context,
chanCap *capabilitytypes.Capability,
packet exported.PacketI,
ack exported.Acknowledgement,
) error {
return im.keeper.WriteAcknowledgement(ctx, chanCap, packet, ack)
}

// GetAppVersion returns the application version of the underlying application
func (i IBCMiddleware) GetAppVersion(ctx sdk.Context, portID, channelID string) (string, bool) {
return i.keeper.GetAppVersion(ctx, portID, channelID)
}
27 changes: 25 additions & 2 deletions protocol/x/ratelimit/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ type (
storeKey storetypes.StoreKey
bankKeeper types.BankKeeper
blockTimeKeeper types.BlockTimeKeeper
ics4Wrapper types.ICS4Wrapper

// the addresses capable of executing MsgSetLimitParams message.
authorities map[string]struct{}
Expand All @@ -38,19 +39,23 @@ func NewKeeper(
storeKey storetypes.StoreKey,
bankKeeper types.BankKeeper,
blockTimeKeeper types.BlockTimeKeeper,
ics4Wrapper types.ICS4Wrapper,
authorities []string,
) *Keeper {
return &Keeper{
cdc: cdc,
storeKey: storeKey,
bankKeeper: bankKeeper,
blockTimeKeeper: blockTimeKeeper,
ics4Wrapper: ics4Wrapper,
authorities: lib.UniqueSliceToSet(authorities),
}
}

// ProcessWithdrawal processes an outbound IBC transfer,
// by updating the capacity lists for the denom.
// If any of the capacities are inefficient, returns an error which results in
// transaction failing upstream.
func (k Keeper) ProcessWithdrawal(
ctx sdk.Context,
denom string,
Expand Down Expand Up @@ -93,9 +98,27 @@ func (k Keeper) ProcessWithdrawal(
return nil
}

// ProcessDeposit processes a inbound IBC transfer,
// UndoWithdrawal is a wrapper around `IncrementCapacitiesForDenom`.
// It also emits telemetry for the amount of withdrawal undone.
func (k Keeper) UndoWithdrawal(
ctx sdk.Context,
denom string,
amount *big.Int,
) {
k.IncrementCapacitiesForDenom(ctx, denom, amount)

telemetry.IncrCounterWithLabels(
[]string{types.ModuleName, metrics.UndoWithdrawAmount},
metrics.GetMetricValueFromBigInt(amount),
[]gometrics.Label{
metrics.GetLabelForStringValue(metrics.RateLimitDenom, denom),
},
)
}

// IncrementCapacitiesForDenom processes a inbound IBC transfer,
// by updating the capacity lists for the denom.
func (k Keeper) ProcessDeposit(
func (k Keeper) IncrementCapacitiesForDenom(
ctx sdk.Context,
denom string,
amount *big.Int,
Expand Down
Loading
Loading