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

EPIC: Add IBC rate limiting mechanism #1945

Closed
5 tasks done
Tracked by #2868
adizere opened this issue Dec 7, 2022 · 9 comments
Closed
5 tasks done
Tracked by #2868

EPIC: Add IBC rate limiting mechanism #1945

adizere opened this issue Dec 7, 2022 · 9 comments
Assignees
Labels
admin: key-result A key result (in the context of OKRs) scope: IBC Integration with IBC (Inter-Blockchain Communication) type: feature-request New feature or request improvement

Comments

@adizere
Copy link

adizere commented Dec 7, 2022

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.

Resources

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

  1. type: feature-request
    mpoke
  2. mpoke

Nice to have

No tasks being tracked yet.
@okwme okwme added scope: IBC Integration with IBC (Inter-Blockchain Communication) release: Lambda labels Dec 13, 2022
@asalzmann
Copy link

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 ?

@ValarDragon
Copy link

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

@asalzmann
Copy link

@ValarDragon can you point me to the code?

@asalzmann
Copy link

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

@adizere
Copy link
Author

adizere commented Jan 9, 2023

Thanks Aidan! Note the Informal team also reviewed the Osmosis solution and wrote an English spec for it (informalsystems/interchain#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.

@crodriguezvega
Copy link

@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.

@mpoke mpoke added type: feature-request New feature or request improvement and removed release: Lambda labels Jan 20, 2023
@mpoke mpoke modified the milestones: Gaia v10.0.0, Update ICS to v1.1 Jan 20, 2023
@mpoke mpoke modified the milestones: Gaia v10.0.0, Next major release Mar 6, 2023
@mpoke mpoke assigned glnro and mpoke Mar 13, 2023
@mpoke mpoke mentioned this issue Apr 14, 2023
@mpoke mpoke removed this from the Gaia v10.0.0 milestone Apr 14, 2023
@mpoke mpoke added the help wanted Open for all. You do not need permission to work on these. label May 26, 2023
@mpoke mpoke unassigned glnro and mpoke May 26, 2023
@mpoke
Copy link
Contributor

mpoke commented May 26, 2023

We should first put out a signaling proposal.

@mmulji-ic mmulji-ic removed their assignment Jun 14, 2023
@mmulji-ic
Copy link
Contributor

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

@mpoke mpoke changed the title Add IBC rate limiting mechanism EPIC: Add IBC rate limiting mechanism Jun 28, 2023
@mpoke mpoke added the admin: key-result A key result (in the context of OKRs) label Jun 28, 2023
@mpoke mpoke added the S: NewThings Work towards your business objectives with new products, features, or integrations label Sep 14, 2023
@mpoke mpoke self-assigned this Nov 16, 2023
@mpoke mpoke removed their assignment Dec 21, 2023
@mpoke mpoke removed help wanted Open for all. You do not need permission to work on these. S: NewThings Work towards your business objectives with new products, features, or integrations labels Dec 21, 2023
@mpoke mpoke self-assigned this Feb 15, 2024
@mpoke
Copy link
Contributor

mpoke commented Apr 10, 2024

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

@mpoke mpoke closed this as completed Apr 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
admin: key-result A key result (in the context of OKRs) scope: IBC Integration with IBC (Inter-Blockchain Communication) type: feature-request New feature or request improvement
Projects
Status: 👍 F4: Assessment
Development

No branches or pull requests

8 participants