IBC Rate Limit - integration and tests

[nicolaslara]

Closes: #2305

What is the purpose of the change

Integrates IBC rate limiting with the chain.

Brief Changelog

• An IBC middleware is now provided and registered with the IBC transfer module.
• The level of rate limiting is controlled by a rate limiting contract that can be controlled by an address specified by governance (this could be the gov module itself)
• The "governance address" of the rate limiting contract can reset the quota when a rate limit is hit (this could be the gov module itself)

Testing and Verifying

This change added tests and can be verified as follows:

• Added integration tests for the middleware (IBC Message <-> IBC transfer module <-> RateLimitingMiddlware <-> Relayer <-> Counterparty chain) under x/ibc-rate-limit
• Added tests to the contract (see https://github.com/osmosis-labs/osmosis/pull/2408)

Documentation and Release Note

This PR depends on @2408 and @2409. It also requires a change to the osmosis fork of the cosmos-sdk (https://github.com/osmosis-labs/osmosis/issues/2307)

• Does this pull request introduce a new feature or user-facing behavior changes? (yes)
  • If the rate limit is reached, IBC transactions will be blocked for users.
• Is a relevant changelog entry added to the Unreleased section in CHANGELOG.md? (no)
  • Should I add this?
• How is the feature or change documented? (not applicable / specification (x/<module>/spec/) / Osmosis docs repo / not documented)
  • Currently undocummented. Should I add docs for this? what is the right place for that?

[mergify[bot]]

:warning: The sha of the head commit of this PR conflicts with #2339. Mergify cannot evaluate rules on this PR. :warning:

[mergify[bot]]

:warning: The sha of the head commit of this PR conflicts with #2339. Mergify cannot evaluate rules on this PR. :warning:

[mergify[bot]]

:warning: The sha of the head commit of this PR conflicts with #2339. Mergify cannot evaluate rules on this PR. :warning:

[mergify[bot]]

:warning: The sha of the head commit of this PR conflicts with #2339. Mergify cannot evaluate rules on this PR. :warning:

[ValarDragon]

wdyt about splitting out:

• tests/e2e/configurer/
• tests/e2e/container/

• some of the other small changes into another PR that we can merge pretty quickly?

[mergify[bot]]

:warning: The sha of the head commit of this PR conflicts with #2339. Mergify cannot evaluate rules on this PR. :warning:

[mergify[bot]]

:warning: The sha of the head commit of this PR conflicts with #2339. Mergify cannot evaluate rules on this PR. :warning:

[nicolaslara]

Created https://github.com/osmosis-labs/osmosis/pull/2941 to add the testing helpers needed by this branch. That one should be a quick merge and make this PR smaller

[ancazamfir]

Hi, i was looking at the osmosis rate limiter code and have some questions. Maybe this PR is not the best to ask but will do anyway and please redirect. On https://github.com/osmosis-labs/osmosis/blob/4fed7db03b4d41aef68af3669f99e2cab0e32ace/x/ibc-rate-limit/ibc_module.go#L117-L121 GetFundsFromPacket gets the denom as specified by the sender. On receive side (in the code above) shouldn't there be un-prefexing or prefexing performed depending on wether the receiving chain was the one originally sending the packet or not? Similar thing is done in the transfer module https://github.com/cosmos/ibc-go/blob/f693f6c11053192f369b5908c9186bd494c3a33d/modules/apps/transfer/keeper/relay.go#L165

The way it is the channel value is computed for a denom of the counterparty/ sender chain. Or maybe I am missing something important here.

[nicolaslara]

Modified this implementation to use the escrowed amount as the channel value for native tokens. This introduces a bootstrapping problem: channels need to have tokens on escrow before we initialize their quotas (otherwise the quota will always be lower than the sent amount). This can be seen in the tests.

I don't think this is a problem for our use case as we will only use rate limiting for existing channels, but it may become important if someone wants all channels to have rate limiting, or to auto-initialize rate-limiting for channels on creation

[ancazamfir]

Modified this implementation to use the escrowed amount as the channel value for native tokens. This introduces a bootstrapping problem: channels need to have tokens on escrow before we initialize their quotas (otherwise the quota will always be lower than the sent amount). This can be seen in the tests.

I don't think this is a problem for our use case as we will only use rate limiting for existing channels, but it may become important if someone wants all channels to have rate limiting, or to auto-initialize rate-limiting for channels on creation

Right. Another possibility is to set some threshold for which the ratelimiter activates/ deactivates.