gaia icon indicating copy to clipboard operation
gaia copied to clipboard

EPIC: Add IBC rate limiting mechanism

Open adizere opened this issue 3 years ago • 8 comments

Summary

A rate limiting mechanism ensures that, intuitively, only a limited amount of value can flow between an IBC channel on the Hub and a counterparty network. By value we can mean different things: token value, number of transfers, number of transfers per signer, number of transfers or value per destination, for example.

Problem Definition

Why do we need this feature? What problems may be addressed by introducing this feature?

In conjunction with a circuit breaker mechanism (SDK feature), rate limiting can help temper the impact of certain security problems by preventing anomalous transfers of funds across networks. The BNB hack is an example of a problem that could have been alleviated by a rate limiter.

Are there any disadvantages of including this feature?

Development time, testing, configuration, and maintenance of the rate limiter module is not for free. Another disadvantage is that a misconfiguring a rate limiter can mean that legitimate user transfers might be prevented (i.e., the rate limiter could have false positives) and also means that not all types of attacks will be addressed (can have false negatives).

Context

Osmosis team already adopted a rate limiter implemented as a wasm contract and had a few iterations on refining/bug fixes (here, here for example) their solution.

I think the Hub team has 2 options:

  1. Adopt the Osmosis solution, either partly or entirely.

    • Since it is CosmWasm-based, this implies the Wasm runtime or at least in a limited capacity. Maybe a shim layer (as the Strangelove team is trying to do with Wasm light clients) is possible towards deploying the Wasm rate limiter on the Hub, but needs investigation (cc @jackzampolin).
  2. Re-implement the Osmosis solution as a Go module.

    • It seems the Stride team is planning to implement rate limiting in Go (cc @asalzmann) also, so a potential cross-team collaboration there is possible.
    • Update: Stride WIP PR kicked-off here: https://github.com/Stride-Labs/stride/pull/492
    • Update: Stride PR implementing the ratelimit module https://github.com/Stride-Labs/stride/pull/556

Resources

  • The Informal team is preparing a specification of the Osmosis solution: https://github.com/informalsystems/interchain/blob/main/security/rate-limiter.md
  • The Osmosis team did a very thorough job of documenting their module, including the rationale and a design discussion centered on configuration/parametrization: https://github.com/osmosis-labs/osmosis/tree/main/x/ibc-rate-limit

This issue is merely a signal. Most likely, it will need to be broken down into multiple sub-issues and tracked accordingly.

Task list

### Must have
- [x] decide on params for the rate limit module
- [ ] put out a signaling proposal
- [ ] align with Stride on rate limit module implementation https://github.com/Stride-Labs/stride/pull/556
- [ ] integrate rate limit module
### Nice to have

adizere avatar Dec 07 '22 15:12 adizere

Our golang solution (mostly implemented using the Osmosis spec) will be done in a few weeks. I think it might make sense to upstream it into ibc-go, because 1) it's an ibc middleware 2) I imagine lots of teams might want to use this. Thoughts @crodriguezvega ?

asalzmann avatar Dec 20 '22 12:12 asalzmann

Theres also an axelar implementation of ours (or a similar) rate limiter in golang as well.

ValarDragon avatar Jan 06 '23 08:01 ValarDragon

@ValarDragon can you point me to the code?

asalzmann avatar Jan 06 '23 13:01 asalzmann

@adizere our rate limiter is probably going to be merged in the next week or so. Please feel free to review

asalzmann avatar Jan 06 '23 13:01 asalzmann

Thanks Aidan! Note the Informal team also reviewed the Osmosis solution and wrote an English spec for it (https://github.com/informalsystems/interchain/pull/4).

Our golang solution (mostly implemented using the Osmosis spec) will be done in a few weeks. I think it might make sense to upstream it into ibc-go, because 1) it's an ibc middleware 2) I imagine lots of teams might want to use this. Thoughts @crodriguezvega ?

This is a good idea. Will also discuss this with the IBC-go team. In the short term, the Gaia team needs this the most, so it makes sense to keep the conversation here for the moment.

adizere avatar Jan 09 '23 10:01 adizere

@adizere our rate limiter is probably going to be merged in the next week or so. Please feel free to review

As soon as it gets merged we can add a link to it in ibc-go's registry.

crodriguezvega avatar Jan 09 '23 11:01 crodriguezvega

We should first put out a signaling proposal.

mpoke avatar May 26 '23 09:05 mpoke

Doc in review, after ok, will put on forum.

mmulji-ic avatar Jun 14 '23 15:06 mmulji-ic

Closing this as completed. Stride's implementation of the IBC rate limit module will be part of Gaia v16.

mpoke avatar Apr 10 '24 12:04 mpoke