Skip to content

Commit d874d6d

Browse files
Progress on IBC rate limit spec (osmosis-labs#3190)
* Progress on IBC rate limit spec * Fix CI * Fix lints * More updates * fix mdlinter * More notes * one more todo * More README update * Update x/ibc-rate-limit/README.md Co-authored-by: Adam Tucker <[email protected]> Co-authored-by: Adam Tucker <[email protected]>
1 parent 2620afd commit d874d6d

File tree

7 files changed

+174
-48
lines changed

7 files changed

+174
-48
lines changed

.github/workflows/contracts.yml

+2-2
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,11 @@ on:
1212

1313
jobs:
1414
test:
15-
name: Test Suite
15+
name: Test Cosmwasm Contracts
1616
runs-on: ubuntu-latest
1717
strategy:
1818
matrix:
19-
contract: [{workdir: ./x/ibc-rate-limit/, output: testdata/rate_limiter.wasm, build: artifacts/rate_limiter-x86_64.wasm, name: rate_limiter}]
19+
contract: [{workdir: ./x/ibc-rate-limit/, output: bytecode/rate_limiter.wasm, build: artifacts/rate_limiter-x86_64.wasm, name: rate_limiter}]
2020

2121
steps:
2222
- name: Checkout sources

.github/workflows/lint.yml

+1-6
Original file line numberDiff line numberDiff line change
@@ -54,9 +54,4 @@ jobs:
5454
# within `super-linter`.
5555
fetch-depth: 0
5656
- name: Run documentation linter
57-
uses: github/super-linter@v4
58-
env:
59-
VALIDATE_ALL_CODEBASE: false
60-
VALIDATE_MARKDOWN: true
61-
DEFAULT_BRANCH: main
62-
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
57+
run: make mdlint

.markdownlint.yml

-2
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,6 @@ MD042: true
1212
MD048: true
1313
MD051: true
1414
# MD004: false
15-
# # Can't disable MD007 :/
16-
# MD007: false
1715
# MD009: false
1816
# MD010:
1917
# code_blocks: false

tests/e2e/e2e_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ func (s *IntegrationTestSuite) TestIBCTokenTransferRateLimiting() {
162162
// co up two levels
163163
projectDir := filepath.Dir(filepath.Dir(wd))
164164
fmt.Println(wd, projectDir)
165-
err = copyFile(projectDir+"/x/ibc-rate-limit/testdata/rate_limiter.wasm", wd+"/scripts/rate_limiter.wasm")
165+
err = copyFile(projectDir+"/x/ibc-rate-limit/bytecode/rate_limiter.wasm", wd+"/scripts/rate_limiter.wasm")
166166
s.NoError(err)
167167
node.StoreWasmCode("rate_limiter.wasm", initialization.ValidatorWalletName)
168168
chainA.LatestCodeId += 1

x/ibc-rate-limit/README.md

+169-36
Original file line numberDiff line numberDiff line change
@@ -1,50 +1,134 @@
1-
# # IBC Rate Limit
1+
# IBC Rate Limit
22

3-
The ``IBC Rate Limit`` middleware implements an [IBC Middleware](https://github.com/cosmos/ibc-go/blob/f57170b1d4dd202a3c6c1c61dcf302b6a9546405/docs/ibc/middleware/develop.md)
4-
that wraps a [transfer](https://ibc.cosmos.network/main/apps/transfer/overview.html) app to regulate how much value can
5-
flow in and out of the chain for a specific denom and channel.
3+
The IBC Rate Limit module is responsible for adding a governance-configurable rate limit to IBC transfers.
4+
This is a safety control, intended to protect assets on osmosis in event of:
65

7-
## Contents
6+
* a bug/hack on osmosis
7+
* a bug/hack on the counter-party chain
8+
* a bug/hack in IBC itself
89

9-
1. **[Concepts](#concepts)**
10-
2. **[Parameters](#parameters)**
11-
3. **[Contract](#contract)**
12-
4. **[Integration](#integration)**
10+
This is done in exchange for a potential (one-way) bridge liveness tradeoff, in periods of high deposits or withdrawals.
1311

14-
## Concepts
12+
The architecture of this package is a minimal go package which implements an [IBC Middleware](https://github.com/cosmos/ibc-go/blob/f57170b1d4dd202a3c6c1c61dcf302b6a9546405/docs/ibc/middleware/develop.md) that wraps the [ICS20 transfer](https://ibc.cosmos.network/main/apps/transfer/overview.html) app, and calls into a cosmwasm contract.
13+
The cosmwasm contract then has all of the actual IBC rate limiting logic.
14+
The Cosmwasm code can be found in the [`contracts`](./contracts/) package, with bytecode findable in the [`bytecode`](./bytecode/) folder. The cosmwasm VM usage allows Osmosis chain governance to choose to change this safety control with no hard forks, via a parameter change proposal, a great mitigation for faster threat adaptavity.
1515

16-
### Overview
16+
The status of the module is being in a state suitable for some initial governance settable rate limits for high value bridged assets.
17+
Its not in its long term / end state for all channels by any means, but does act as a strong protection we
18+
can instantiate today for high value IBC connections.
1719

18-
The `x/ibc-rate-limit` module implements an IBC middleware and a transfer app wrapper. The middleware checks if the
19-
amount of value of a specific denom transferred through a channel has exceeded a quota defined by governance for
20-
that channel/denom. These checks are handled through a CosmWasm contract. The contract to be used for this is
21-
configured via a parameter.
20+
## Motivation
2221

23-
### Middleware
22+
The motivation of IBC-rate-limit comes from the empirical observations of blockchain bridge hacks that a rate limit would have massively reduced the stolen amount of assets in:
23+
24+
- [Polynetwork Bridge Hack ($611 million)](https://rekt.news/polynetwork-rekt/)
25+
- [BNB Bridge Hack ($586 million)](https://rekt.news/bnb-bridge-rekt/)
26+
- [Wormhole Bridge Hack ($326 million)](https://rekt.news/wormhole-rekt/)
27+
- [Nomad Bridge Hack ($190 million)](https://rekt.news/nomad-rekt/)
28+
- [Harmony Bridge Hack ($100 million)](https://rekt.news/harmony-rekt/) - (Would require rate limit + monitoring)
29+
- [Dragonberry IBC bug](https://forum.cosmos.network/t/ibc-security-advisory-dragonberry/7702) (can't yet disclose amount at risk, but was saved due to being found first by altruistic Osmosis core developers)
30+
31+
In the presence of a software bug on Osmosis, IBC itself, or on a counterparty chain, we would like to prevent the bridge from being fully depegged.
32+
This stems from the idea that a 30% asset depeg is ~infinitely better than a 100% depeg.
33+
Its _crazy_ that today these complex bridged assets can instantly go to 0 in event of bug.
34+
The goal of a rate limit is to raise an alert that something has potentially gone wrong, allowing validators and developers to have time to analyze, react, and protect larger portions of user funds.
35+
36+
The thesis of this is that, it is worthwile to sacrifice liveness in the case of legitimate demand to send extreme amounts of funds, to prevent the terrible long-tail full fund risks.
37+
Rate limits aren't the end-all of safety controls, they're merely the simplest automated one. More should be explored and added onto IBC!
38+
39+
## Rate limit types
40+
41+
We express rate limits in time-based periods.
42+
This means, we set rate limits for (say) 6-hour, daily, and weekly intervals.
43+
The rate limit for a given time period stores the relevant amount of assets at the start of the rate limit.
44+
Rate limits are then defined on percentage terms of the asset.
45+
The time windows for rate limits are currently _not_ rolling, they have discrete start/end times.
46+
47+
We allow setting separate rate limits for the inflow and outflow of assets.
48+
We do all of our rate limits based on the _net flow_ of assets on a channel pair. This prevents DOS issues, of someone repeatedly sending assets back and forth, to trigger rate limits and break liveness.
49+
50+
We currently envision creating two kinds of rate limits:
51+
52+
* Per denomination rate limits
53+
- allows safety statements like "Only 30% of Stars on Osmosis can flow out in one day" or "The amount of Atom on Osmosis can at most double per day".
54+
* Per channel rate limits
55+
- Limit the total inflow and outflow on a given IBC channel, based on "USDC" equivalent, using Osmosis as the price oracle.
56+
57+
We currently only implement per denomination rate limits for non-native assets. We do not yet implement channel based rate limits.
58+
59+
Currently these rate limits automatically "expire" at the end of the quota duration. TODO: Think of better designs here. E.g. can we have a constant number of subsequent quotas start filled? Or perhaps harmonically decreasing amounts of next few quotas pre-filled? Halted until DAO override seems not-great.
60+
61+
## Instantiating rate limits
62+
63+
Today all rate limit quotas must be set manually by governance.
64+
In the future, we should design towards some conservative rate limit to add as a safety-backstop automatically for channels.
65+
Ideas for how this could look:
66+
67+
* One month after a channel has been created, automatically add in some USDC-based rate limit
68+
* One month after governance incentivizes an asset, add on a per-denomination rate limit.
69+
70+
Definitely needs far more ideation and iteration!
71+
72+
## Parameterizing the rate limit
73+
74+
One element is we don't want any rate limit timespan thats too short, e.g. not enough time for humans to react to. So we wouldn't want a 1 hour rate limit, unless we think that if its hit, it could be assessed within an hour.
75+
76+
### Handling rate limit boundaries
77+
78+
We want to be safe against the case where say we have a daily rate limit ending at a given time, and an adversary attempts to attack near the boundary window.
79+
We would not like them to be able to "double extract funds" by timing their extraction near a window boundary.
80+
81+
Admittedly, not a lot of thought has been put into how to deal with this well.
82+
Right now we envision simply handling this by saying if you want a quota of duration D, instead include two quotas of duration D, but offset by `D/2` from each other.
83+
84+
Ideally we can change windows to be more 'rolling' in the future, to avoid this overhead and more cleanly handle the problem. (Perhaps rolling ~1 hour at a time)
85+
86+
### Inflow parameterization
87+
88+
The "Inflow" side of a rate limit is essentially protection against unforeseen bug on a counterparty chain.
89+
This can be quite conservative (e.g. bridged amount doubling in one week). This covers a few cases:
90+
91+
* Counter-party chain B having a token theft attack
92+
- TODO: description of how this looks
93+
* Counter-party chain B runaway mint
94+
- TODO: description of how this looks
95+
* IBC theft
96+
- TODO: description of how this looks
97+
98+
It does get more complex when the counterparty chain is itself a DEX, but this is still much more protection than nothing.
99+
100+
### Outflow parameterization
101+
102+
The "Outflow" side of a rate limit is protection against a bug on Osmosis OR IBC.
103+
This has potential for much more user-frustrating issues, if set too low.
104+
E.g. if theres some event that causes many people to suddenly withdraw many STARS or many USDC.
105+
106+
So this parameterization has to contend with being a tradeoff of withdrawal liveness in high volatility periods vs being a crucial safety rail, in event of on-Osmosis bug.
107+
108+
TODO: Better fill out
109+
110+
### Example suggested parameterization
111+
112+
## Code structure
113+
114+
As mentioned at the beginning of the README, the go code is a relatively minimal ICS 20 wrapper, that dispatches relevant calls to a cosmwasm contract that implements the rate limiting functionality.
115+
116+
### Go Middleware
24117

25118
To achieve this, the middleware needs to implement the `porttypes.Middleware` interface and the
26119
`porttypes.ICS4Wrapper` interface. This allows the middleware to send and receive IBC messages by wrapping
27120
any IBC module, and be used as an ICS4 wrapper by a transfer module (for sending packets or writing acknowledgements).
28121

29122
Of those interfaces, just the following methods have custom logic:
30123

31-
* `ICS4Wrapper.SendPacket` adds tracking of value sent via an ibc channel
32-
* `Middleware.OnRecvPacket` adds tracking of value received via an ibc channel
33-
* `Middleware.OnAcknowledgementPacket` undos the tracking of a sent packet if the acknowledgment is not a success
34-
* `OnTimeoutPacket` undos the tracking of a sent packet if the packet times out (is not relayed)
124+
* `ICS4Wrapper.SendPacket` forwards to contract, with intent of tracking of value sent via an ibc channel
125+
* `Middleware.OnRecvPacket` forwards to contract, with intent of tracking of value received via an ibc channel
126+
* `Middleware.OnAcknowledgementPacket` forwards to contract, with intent of undoing the tracking of a sent packet if the acknowledgment is not a success
127+
* `OnTimeoutPacket` forwards to contract, with intent of undoing the tracking of a sent packet if the packet times out (is not relayed)
35128

36129
All other methods from those interfaces are passthroughs to the underlying implementations.
37130

38-
### Contract Concepts
39-
40-
The tracking contract uses the following concepts
41-
42-
1. **RateLimit** - tracks the value flow transferred and the quota for a path.
43-
2. **Path** - is a (denom, channel) pair.
44-
3. **Flow** - tracks the value that has moved through a path during the current time window.
45-
4. **Quota** - is the percentage of the denom's total value that can be transferred through the path in a given period of time (duration)
46-
47-
## Parameters
131+
#### Parameters
48132

49133
The middleware uses the following parameters:
50134

@@ -55,35 +139,84 @@ The middleware uses the following parameters:
55139
1. **ContractAddress** -
56140
The contract address is the address of an instantiated version of the contract provided under `./contracts/`
57141

58-
## Contract
142+
### Cosmwasm Contract Concepts
143+
144+
Something to keep in mind with all of the code, is that we have to reason separately about every item in the following matrix:
145+
146+
| Native Token | Non-Native Token |
147+
|----------------------|--------------------------|
148+
| Send Native Token | Send Non-Native Token |
149+
| Receive Native Token | Receive Non-Native Token |
150+
| Timeout Native Send | Timeout Non-native Send |
59151

60-
### Messages
152+
(Error ACK can reuse the same code as timeout)
153+
154+
TODO: Spend more time on sudo messages in the following description. We need to better describe how we map the quota concepts onto the code.
155+
Need to describe how we get the quota beginning balance, and that its different for sends and receives.
156+
Explain intracacies of tracking that a timeout and/or ErrorAck must appear from the same quota, else we ignore its update to the quotas.
157+
158+
159+
The tracking contract uses the following concepts
160+
161+
1. **RateLimit** - tracks the value flow transferred and the quota for a path.
162+
2. **Path** - is a (denom, channel) pair.
163+
3. **Flow** - tracks the value that has moved through a path during the current time window.
164+
4. **Quota** - is the percentage of the denom's total value that can be transferred through the path in a given period of time (duration)
165+
166+
#### Messages
61167

62168
The contract specifies the following messages:
63169

64-
#### Query
170+
##### Query
65171

66172
* GetQuotas - Returns the quotas for a path
67173

68-
#### Exec
174+
##### Exec
69175

70176
* AddPath - Adds a list of quotas for a path
71177
* RemovePath - Removes a path
72178
* ResetPathQuota - If a rate limit has been reached, the contract's governance address can reset the quota so that transfers are allowed again
73179

74-
#### Sudo
180+
##### Sudo
75181

76182
Sudo messages can only be executed by the chain.
77183

78184
* SendPacket - Increments the amount used out of the send quota and checks that the send is allowed. If it isn't, it will return a RateLimitExceeded error
79185
* RecvPacket - Increments the amount used out of the receive quota and checks that the receive is allowed. If it isn't, it will return a RateLimitExceeded error
80186
* UndoSend - If a send has failed, the undo message is used to remove its cost from the send quota
81187

82-
## Integration
188+
### Integration
83189

84190
The rate limit middleware wraps the `transferIBCModule` and is added as the entry route for IBC transfers.
85191

86192
The module is also provided to the underlying `transferIBCModule` as its `ICS4Wrapper`; previously, this would have
87193
pointed to a channel, which also implements the `ICS4Wrapper` interface.
88194

89195
This integration can be seen in [osmosis/app/keepers/keepers.go](https://github.com/osmosis-labs/osmosis/blob/main/app/keepers/keepers.go)
196+
197+
## Testing strategy
198+
199+
TODO: Fill this out
200+
201+
## Known Future work
202+
203+
Items that've been highlighted above:
204+
205+
* Making automated rate limits get added for channels, instead of manual configuration only
206+
* Improving parameterization strategies / data analysis
207+
* Adding the USDC based rate limits
208+
* We need better strategies for how rate limits "expire".
209+
210+
Not yet highlighted
211+
212+
* Making monitoring tooling to know when approaching rate limiting and when they're hit
213+
* Making tooling to easily give us summaries we can use, to reason about "bug or not bug" in event of rate limit being hit
214+
* Enabling ways to pre-declare large transfers so as to not hit rate limits.
215+
* Perhaps you can on-chain declare intent to send these assets with a large delay, that raises monitoring but bypasses rate limits?
216+
* Maybe contract-based tooling to split up the transfer suffices?
217+
* Strategies to account for high volatility periods without hitting rate limits
218+
* Can imagine "Hop network" style markets emerging
219+
* Could imagine tieng it into looking at AMM volatility, or off-chain oracles
220+
* but these are both things we should be wary of security bugs in.
221+
* Maybe [constraint based programming with tracking of provenance](https://youtu.be/HB5TrK7A4pI?t=2852) as a solution
222+
* Analyze changing denom-based rate limits, to just overall withdrawal amount for Osmosis

x/ibc-rate-limit/testutil/wasm.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ func (chain *TestChain) StoreContractCode(suite *suite.Suite) {
1919
osmosisApp := chain.GetOsmosisApp()
2020

2121
govKeeper := osmosisApp.GovKeeper
22-
wasmCode, err := ioutil.ReadFile("./testdata/rate_limiter.wasm")
22+
wasmCode, err := ioutil.ReadFile("./bytecode/rate_limiter.wasm")
2323
suite.Require().NoError(err)
2424

2525
addr := osmosisApp.AccountKeeper.GetModuleAddress(govtypes.ModuleName)

0 commit comments

Comments
 (0)