[review] MIP-56 : Rate-Limiter for the HTLC-type Native Bridge #56

apenzk · 2024-11-13T12:13:45Z

Summary

The Rate-Limiter expands MIP-50 and is part of the Biarritz Model (MIP-54). It de-escalates concerns regarding liveness and safety of the relayer.

The presented design does not address whether a breach of the bridge happened - which is the role of the informer (no MIP yet) and the security fund (MIP-50).

0xmovses

In general, because of the asymmetry imposed here by only rate-limiting on the L1, I'm wondering if it may make sense to implement rate limiting on both the L1 and L2 to have symmetry in both directions, with separate budgets for each direction.

The only big downside I can see by implementing on the L1 only is that the L1 rate limiter, in the case of a L2->L1 transfer can only block a call on the lockBridgeTransfer call, i.e this is the earliest intervention it can make. However, I think this downside is moot and doesn't effect the rate-limiting mechanism negatively.

What do you think? If you agree, we can with the Rate Limiter on L1 only.

0xmovses · 2024-11-13T12:48:05Z

MIP/MIP-56/README.md

+The **Rate-Limiter** for the Native Bridge is introduced to de-escalate the risk of double-spends through the bridge by limiting the number of tokens that can be transferred during a certain time frame. The rate limit is determined by the reaction time of the bridge operator, called the **Risk Period**, and the value locked in the **Security Fund**, see [MIP-50](https://github.com/movementlabsxyz/MIP/pull/50).
+
+The Native Bridge Rate Limiter is implemented through a contract on L1. This contract is governed by the Aptos governance framework, see [MIP-48](https://github.com/movementlabsxyz/MIP/pull/48/), and can be updated through a governance process.
+


If the Limiter is on the L1, how will the governance framework (which is on the L2 communicate with it?) there is no direct path. Perhaps, for now any arguments passed to it are set via restricted functions and later you could have some kind of proxy governance on the L1. Though I would say this is out of scope here.

If the governance is on L2 the rate limit and other governance related function should be implemented on L2?

is the governance on L2 due to operational (gas) costs?

Good question. I think we just initially thought of governance on the L2 because we are aptos first. But perhaps governance on the L1 makes more sense for us. To align those decisions and execute on the L1.

i think at least initially we should decide on a particular layer, which would require that most things happen on that layer. which would mean that Rate-Limiter, Bridge-Security-Fund, Governance would need to be on the same layer.

Considerations:

Choice for L1 : Ethereum - alignment

Choice for L2 : improved Cost and Latency

Surely L1. We don't care about cost for L1, as governance transactions are infrequent. High Gas could be considered a good thing for governance, i.e: its expensive to submit a proposal, do it thoughtfully.

Also, with that, we can just use already existing protocols such as snapshot https://snapshot.org/#/

Or the MolochDAO contracts (I wouldn't recommend), snapshot better.

For the governance, I think it's because Aptos framework implements one that we wanted to use.

perhaps, for now any arguments passed to it are set via restricted functions

so who sets these values if not governance?

a trusted signer, us. For Bilbao Model, as we wont have any governance implemented for initial mainnet. Only post.

0xmovses · 2024-11-13T12:52:43Z

MIP/MIP-56/README.md

+1. The Rate-Limiter SHOULD be implemented as a contract on L1.
+1. The Rate-Limiter MUST handle both the **transfer directions** L1 -> L2 and L2 -> L1. 
+1. For either transfer direction the transfer value MUST be tracked, i.e. the transferred value for L1->L2 should be recored in `budget_L1L2` and for L2->L1 in `budget_L2L1`.
+1. The Rate-Limiter SHOULD be governed by the Aptos governance framework, see [MIP-48](https://github.com/movementlabsxyz/MIP/pull/48/), and can be updated through a governance process.


Related to my comment above, I see no way (given our current design) the aptos_governance module can communicate directly to the L1.

If you're proposing that decisions be made via aptos_governance on the L2 then those decisions be carried out on the L1 by a trusted party, that will suffice for now. Perhaps you could make that clearer here or elsewhere.

this ties in with the above comment on selecting a main action layer.

0xmovses · 2024-11-13T12:54:49Z

MIP/MIP-56/README.md

+1. For either transfer direction the transfer value MUST be tracked, i.e. the transferred value for L1->L2 should be recored in `budget_L1L2` and for L2->L1 in `budget_L2L1`.
+1. The Rate-Limiter SHOULD be governed by the Aptos governance framework, see [MIP-48](https://github.com/movementlabsxyz/MIP/pull/48/), and can be updated through a governance process.
+1. The security fund contract, see [MIP-50](https://github.com/movementlabsxyz/MIP/pull/50), MUST update `security_fund` in the Rate-Limiter contract if the amount of funds changes in the security fund contract.
+1. The `risk_period` value SHOULD be set by the governance process and updated through the governance process. It estimates the maximum reaction time to check whether transfers have been completed correctly.


At this point it could only be updated manually. Which is fine for Bilbao stage.

We have no way to relay arbitrary actions / messages from one chain to another. This would imply, for full automation and tight governance, a governance relayer, for relayer actions that are outcomes of an accepted proposal from the L2 contract to the L1, which is an interesting idea. Again out of scope though.

0xmovses · 2024-11-13T13:01:38Z

MIP/MIP-56/README.md

+1. The Rate-Limiter SHOULD be governed by the Aptos governance framework, see [MIP-48](https://github.com/movementlabsxyz/MIP/pull/48/), and can be updated through a governance process.
+1. The security fund contract, see [MIP-50](https://github.com/movementlabsxyz/MIP/pull/50), MUST update `security_fund` in the Rate-Limiter contract if the amount of funds changes in the security fund contract.
+1. The `risk_period` value SHOULD be set by the governance process and updated through the governance process. It estimates the maximum reaction time to check whether transfers have been completed correctly.
+1. For a given transfer direction, the Rate-Limiter should disable transfers across the bridge if the rate limit is exceeded for that transfer direction.


the Rate-Limiter should disable transfers across the bridge if the rate limit is exceeded for that transfer direction

For "across the bridge" what is meant exactly, on the L2 and L1? I still fail to see how the L1 contract can do any operations on the L2 (it can use the relayer of course, but I think the idea here is to not rely on relaying a message)?

across means L1->L2 or L2->L1

the L1 contract does not need to perform any actions on L2. the rate limited is set on L1 alone. do you think this is problematic and why? (maybe the L2->L1 is problematic as it might rely on L2timeouts)

0xmovses · 2024-11-13T13:06:25Z

MIP/MIP-56/README.md

+1. The `risk_period` value SHOULD be set by the governance process and updated through the governance process. It estimates the maximum reaction time to check whether transfers have been completed correctly.
+1. For a given transfer direction, the Rate-Limiter should disable transfers across the bridge if the rate limit is exceeded for that transfer direction.
+1. The rate limit MUST be set according to the equation `rate_limit = security_fund / risk_period * 0.5`. This is to ensure that each bridge transfer direction is ensured sufficiently.
+1. The Rate-Limiter MUST reject a transfer request if the rate limit is exceeded for that transfer direction by that transaction. I.e. for L1->L2 the Rate-Limiter MUST reject any transfer that would violate `budget_L1L2 < rate_limit` and for L2->L1 the Rate-Limiter MUST reject any transfer that would violate `budget_L2L1 < rate_limit`.


To drill down a bit. The case :
L1->L2 is fine, as it will just internally block initiateBridgeTransfer
Whereas
L2->L1 the only contract L1 has visibility of in this direction is the AtomicBridgeCounteropartyMOVE.sol, so the only call it can block is the lockBridgeTransfer call.

Is this what you intend?

it seems the function that would apply the rate limit should be lock_bridge_transfer on L1 according to Figure 2 in #39

i assume currently a failed lock on L1 would not be reported to L2? Or is there a way the user on L2 can learn that the transfer request failed due to rate limit on L1?

also see the next code line that i just added (L35 currently: ... The rate limiation should be applied at the earliest ....)

Why we don't apply rater limiting on both side to the initiate_transfer. There's no need to have it only on one chain. They just have to share the same equation.

Would also be possible, but requires two pools : one security pool on L1 and one security pool on L2.

pro

earliest possible reject.

easy to separate different rate limits

con

risk_period has to be set twice (once on L1 and once on L2)

have to implement rate limiter on solidity and on Move

need two security fund contracts, once in solidity and once in Move

increased code surface for bugs?

security funds are separate, weight can only be adjusted by physically sending funds from L1->L2 (L2->L1) between security pools

added it as an optimization suggestion

With the addition of two pools this adds too much complexity for something Minimal that achieves that rate-limit guarantee that we need. I am going to implement on L1 only if we want to add Rate Limiter on the L2 later, this can happen post Bilbao Model. Or, can be considered as part of the Biarritz Model.

MIP/MIP-56/README.md

musitdev · 2024-11-13T13:19:59Z

I have more a practical remark. The rate limit cancels transfers if activated. As it executes on L1, the way to limit it is to reject the Initiate_transfer call on the counterpart contract. For L2, the only way I see is to reject the lock_transfer call on the counterpart L1 contract. If we do that, the fund will be blocked on L2, and the user will have to wait to be refunded.
So if that's the way the bridge UI must inform the user that the rate is limited and block all transfers to avoid too many transfers to refund.

0xmovses · 2024-11-13T13:41:27Z

@musitdev Yes I had the same remark here #56 (comment)

I think this is what @apenzk is suggesting. For this L2 -> L1 and block-lock case, we are forced to simple have the user wait for refund and let them know.

The nice thing about this implementation, is the Front End could read the rate limit from the contract and display that to the user on the front end. Some kind of loader bar to show the rate-limit is approaching, this would incentivise the user to transfer if the rate-limit is almost exceeded.

0xmovses · 2024-11-13T15:50:11Z

MIP/MIP-56/README.md

+1. The `risk_period` value SHOULD be set by the governance process and updated through the governance process. It estimates the maximum reaction time to check whether transfers have been completed correctly.
+1. For a given transfer direction, the Rate-Limiter should disable transfers across the bridge if the rate limit is exceeded for that transfer direction.
+1. The rate limit MUST be set according to the equation `rate_limit = security_fund / risk_period * 0.5`. This is to ensure that each bridge transfer direction is ensured sufficiently.
+1. The Rate-Limiter MUST reject a transfer request if the rate limit is exceeded for that transfer direction by that transaction. I.e. for L1->L2 the Rate-Limiter MUST reject any transfer that would violate `budget_L1L2 < rate_limit` and for L2->L1 the Rate-Limiter MUST reject any transfer that would violate `budget_L2L1 < rate_limit`.


With the addition of two pools this adds too much complexity for something Minimal that achieves that rate-limit guarantee that we need. I am going to implement on L1 only if we want to add Rate Limiter on the L2 later, this can happen post Bilbao Model. Or, can be considered as part of the Biarritz Model.

franck44 · 2024-12-17T20:55:45Z

@apenzk I propose we merge this PR (and the MIP) as we are not going to use an HTLC bridge.
We can start a dedicated MIP with a spec of the rate limiter for a mint/lock bridge.
We may also wait until we have an MIP for the new bridge architecture.
See this issue #68 .

apenzk · 2024-12-18T15:24:05Z

@apenzk I propose we merge this PR (and the MIP) as we are not going to use an HTLC bridge.
We can start a dedicated MIP with a spec of the rate limiter for a mint/lock bridge.
We may also wait until we have an MIP for the new bridge architecture.
See this issue #68 .

@franck44 should we set it to status "withdrawn" instead?

apenzk · 2025-01-14T11:34:06Z

As per MIP-committee decision we are closing MIPs and MDs for the HTLC-based Native-bridge.

apenzk added 5 commits November 13, 2024 11:45

init

4f75ddc

figure, spec and optimization

f71b312

MIP number

a383560

minor fix

3db31bb

limitations, optimisations, verification

5da5f5e

apenzk changed the title ~~[draft] MIP-x : Rate-Limiter for the Native Bridge~~ [review] MIP-56 : Rate-Limiter for the Native Bridge Nov 13, 2024

apenzk marked this pull request as ready for review November 13, 2024 12:40

apenzk requested review from franck44, 0xmovses and l-monninger November 13, 2024 12:40

define weight

28c2b3a

0xmovses requested changes Nov 13, 2024

View reviewed changes

add at which point to apply the rate limit

ed4bcbf

apenzk added 2 commits November 13, 2024 15:10

error message

2541422

optimization of two rate limiters

8d198e4

0xmovses previously approved these changes Nov 13, 2024

View reviewed changes

typo

88c995e

apenzk dismissed 0xmovses’s stale review via 88c995e November 14, 2024 11:26

0xmovses mentioned this pull request Nov 14, 2024

Implement Rate Limiter Contract movementlabsxyz/movement#851

Closed

SA124 assigned apenzk Nov 15, 2024

apenzk changed the title ~~[review] MIP-56 : Rate-Limiter for the Native Bridge~~ [review] MIP-56 : Rate-Limiter for the HTLC-type Native Bridge Nov 25, 2024

apenzk added bridge and removed bridge labels Nov 25, 2024

franck44 added the check_for_outdated label Dec 17, 2024

add HTLC-based

24ded2d

apenzk closed this Jan 14, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[review] MIP-56 : Rate-Limiter for the HTLC-type Native Bridge #56

[review] MIP-56 : Rate-Limiter for the HTLC-type Native Bridge #56

apenzk commented Nov 13, 2024 •

edited

Loading

0xmovses left a comment

0xmovses Nov 13, 2024

musitdev Nov 13, 2024

apenzk Nov 13, 2024

0xmovses Nov 13, 2024

apenzk Nov 13, 2024 •

edited

Loading

0xmovses Nov 13, 2024

0xmovses Nov 13, 2024 •

edited

Loading

musitdev Nov 13, 2024

apenzk Nov 13, 2024

0xmovses Nov 13, 2024

0xmovses Nov 13, 2024

apenzk Nov 13, 2024

0xmovses Nov 13, 2024

0xmovses Nov 13, 2024

apenzk Nov 13, 2024

0xmovses Nov 13, 2024

apenzk Nov 13, 2024

apenzk Nov 13, 2024

musitdev Nov 13, 2024

apenzk Nov 13, 2024 •

edited

Loading

apenzk Nov 13, 2024

0xmovses Nov 13, 2024

musitdev commented Nov 13, 2024

0xmovses commented Nov 13, 2024

0xmovses Nov 13, 2024

franck44 commented Dec 17, 2024

apenzk commented Dec 18, 2024

apenzk commented Jan 14, 2025

		The Rate-Limiter for the Native Bridge is introduced to de-escalate the risk of double-spends through the bridge by limiting the number of tokens that can be transferred during a certain time frame. The rate limit is determined by the reaction time of the bridge operator, called the Risk Period, and the value locked in the Security Fund, see [MIP-50](https://github.com/movementlabsxyz/MIP/pull/50).

		The Native Bridge Rate Limiter is implemented through a contract on L1. This contract is governed by the Aptos governance framework, see [MIP-48](https://github.com/movementlabsxyz/MIP/pull/48/), and can be updated through a governance process.

[review] MIP-56 : Rate-Limiter for the HTLC-type Native Bridge #56

[review] MIP-56 : Rate-Limiter for the HTLC-type Native Bridge #56

Conversation

apenzk commented Nov 13, 2024 • edited Loading

Summary

0xmovses left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

apenzk Nov 13, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

0xmovses Nov 13, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

apenzk Nov 13, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

musitdev commented Nov 13, 2024

0xmovses commented Nov 13, 2024

Choose a reason for hiding this comment

franck44 commented Dec 17, 2024

apenzk commented Dec 18, 2024

apenzk commented Jan 14, 2025

apenzk commented Nov 13, 2024 •

edited

Loading

apenzk Nov 13, 2024 •

edited

Loading

0xmovses Nov 13, 2024 •

edited

Loading

apenzk Nov 13, 2024 •

edited

Loading