feat: add mean bridge

[nchamo]

Description

We are adding a new Aztec bridge that adds support for DCAing with Mean Finance. This bridge supports:

• All pairs supported by Mean Finance
• All swap intervals (weekly, daily, etc)
• Any amount of swaps
• Yield while DCAing

Subsidies

In our case, we will subsidize a combination of:

• From token
• To token
• Amount of swaps
• Swap interval

For example, we will subsidize (DAI => ETH, 7 swaps, daily) or (USDC => AAVE, 10 swaps, weekly)

Now, it's important to understand that (DAI => ETH, 7 swaps, daily) is different from (ETH => DAI, 7 swaps, daily)
We are doing this because if we considered them the same, then they would share the same subsidy funds. If that happened, then depending on what positions were created, we could be subsidizing DAI => ETH twice before doing it for ETH => DAI

Another important detail is that since we are using the from/to token, we can subsidize "DAI with yield on Euler" without subsidizing "DAI" (and viceversa)

Note: the bridge has an owner that can set and modify subsidies

Checklist:

• I have reviewed my diff in github, line by line.
• Every change is related to the PR description.
• There are no unexpected formatting changes, or superfluous debug logs.
• The branch has been rebased against the head of its merge target.
• I'm happy for the PR to be merged at the reviewers next convenience.
• NatSpec documentation of all the non-test functions is present and is complete.
• Continuous integration (CI) passes.
• Command forge coverage --match-contract MyContract returns 100% line coverage.
• All the possible reverts are tested.

Technical details

Wrappers Registry

This bridge supports the yield-while-DCAing feature. Since we can't pass the 4626 wrapper's address as part of the convert function, we introduced the concept of the Wrappers Registry. The idea is to have a registry that would allow us to map id => address. Since the id is smaller (uint16) than an address, we can pass it as part of the auxData

Anyone will be able to register tokens to the registry, since we can easily make sure that there is no wrong-doing

Flow

This is how we expect the data to be passed to the bridge:

• inputAssetA will represent the token that the user will deposit (for example DAI)
• outputAssetA will represent the token that the user will withdraw (for example ETH)
• outputAssetB will be the same as output _inputAssetA (to support withdrawing unswapped funds)
• auxData will encode:
  • The amount of swaps
  • The swap interval
  • The wrapper for the "from" token
  • The wrapper for the "to" token

AuxData

The auxData field is 64bits long:

• First 24 bits: amount of swaps
• Next 8 bits: swap interval code (we map the values between 0 and 7 to a swap interval)
• Next 16 bits: wrapper id for the "from" token
• Last 16 bits: wrapper id for the "to" token

[LHerskind]

Hi, I have added a couple of comments and questions to different parts of the PR. Legend: [i] = info (used for comment or lower severity issues), [?] = question, [!] = issue (loss of user funds).

• Specs:
  • [?] How do you expect the finalise to be called? The user are not actively calling any of these, so if there is not some sort of incentive for someone else to finalise, they could very well
  • [?] What do you plan for gas limits? With different variations for input and outputs with wrapping and unwrapping seems like it can span quite widely.
  • [i] Would be nice with a description of the encoding in here.
• Tests:
  • [i] Helper functions are inconsistently named, _calculateBalance don't calculate anything, but just reads balance of the rollup processor. For mocks useful if mock is consistently part of the name.
  • [i] Use bound instead of assume when dealing with bounded values.
• Code
  • [!] Invalid withdraw amounts, loss of user funds. The _unwrapIfNeeded is returning a value, but it is never used. If finalising a wrapped asset, exiting will leave tokens in the bridge. This is NOT the case for eth output because it is sent to the rollup inside with the proper amount. But for ERC20 you will see this issue, think that is what you run into in the test where you have added // Note: Euler returns some wei less that expected, so we don"t test it here.
  • [i] Convert looks so clean. I like
  • [i] Good outline of cases for finalise
  • [i] Inconsistent ordering of constant, immutable and storage
  • [i] Comment for constructor mentions subsidy-related info, which is handled there for your implementations
  • [i] Unclear why the tokenWrapperRegistry not done with just a mapping to have less
  • [i] In your docs it does not mention Ethereum, should probably also include `TRANSFORMER_REGISTRY
    • [i] TRANSFORMER_REGISTRY behind a proxy, could be updated to steal funds in process of swap.
    • [?] Upgradeable etc. Would assume that this is similar assumptions to Mean finance?.
  • [i] Nice to have would be it all their functions had a small natspec of. Many of the functions are small, but then call other functions further down.
  • [i] What is the reasoning of having THIS_ADDRESS instead of address(this)?
  • [i] Bad practice to overwrite _amountToUnwrap. Seems to be used for the underlying amount (after unwrap) as well. Not a memory value, so the copied value will be overwritten and then returned, see the issue.

Let me know if you have any questions 😄

[nchamo]

@LHerskind wow! That was a really fast review! I'll try to go one by one

Specs

[?] How do you expect the finalise to be called? The user are not actively calling any of these, so if there is not some sort of incentive for someone else to finalise, they could very well

Let me get back to you on that. I was under the impression that when a user initiated the withdraw, then it would do so for all the users on the batch

[?] What do you plan for gas limits? With different variations for input and outputs with wrapping and unwrapping seems like it can span quite widely.

Well, that's a really good question. Just so that I understand it correctly, can the gas limit be updated at a later stage?

[i] Would be nice with a description of the encoding in here.

Done!

Tests

[i] Helper functions are inconsistently named, _calculateBalance don't calculate anything, but just reads balance of the rollup processor. For mocks useful if mock is consistently part of the name.

Let me know if the changes are ok or if I can change something else

[i] Use bound instead of assume when dealing with bounded values.

Nice, didn't know that existed (first time with Foundry). Done!

Code

[!] Invalid withdraw amounts, loss of user funds. The _unwrapIfNeeded is returning a value, but it is never used. If finalising a wrapped asset, exiting will leave tokens in the bridge. This is NOT the case for eth output because it is sent to the rollup inside with the proper amount. But for ERC20 you will see this issue, think that is what you run into in the test where you have added // Note: Euler returns some wei less that expected, so we don"t test it here.

Wow, good catch! Fixed it 😄

[i] Convert looks so clean. I like
[i] Good outline of cases for finalise

Glad you liked it 😄

[i] Inconsistent ordering of constant, immutable and storage

Done!

[i] Comment for constructor mentions subsidy-related info, which is handled there for your implementations

Done!

[i] Unclear why the tokenWrapperRegistry not done with just a mapping to have less

Decided to go with an EnumerableSet because we needed to map id => address, but also avoid duplicates (which would require a mapping of address > something). Could have made it a permissioned function and assume that there would be no duplicates, but wanted to make open so it can continue to work without our intervention. But maybe I missed something? I'm open to suggestions

[i] In your docs it does not mention Ethereum, should probably also include `TRANSFORMER_REGISTRY
[i] TRANSFORMER_REGISTRY behind a proxy, could be updated to steal funds in process of swap.
[?] Upgradeable etc. Would assume that this is similar assumptions to Mean finance?.

Not sure what you mean with the fact that we don't mention Ethereum. But I added some information to the spec regarding immutability. Please let me know if that's enough

[i] Nice to have would be it all their functions had a small natspec of. Many of the functions are small, but then call other functions further down.

Done!

[i] What is the reasoning of having THIS_ADDRESS instead of address(this)?

My bad 😅

[i] Bad practice to overwrite _amountToUnwrap. Seems to be used for the underlying amount (after unwrap) as well. Not a memory value, so the copied value will be overwritten and then returned, see the issue.

I was not trying to modify the reference, just forgot to use the return value. But I did a little refactor, let me know if you like it better

[LHerskind]

Let me get back to you on that. I was under the impression that when a user initiated the withdraw, then it would do so for all the users on the batch

Yes, that batch will be closed at once. But if it cannot be done from within the convert, then it would require an L1 transaction. Someone needs to execute the L1 transaction, and if there is no incentive to do that, then it is most likely one of the users in the batch doing it which leaks information.

Well, that's a really good question. Just so that I understand it correctly, can the gas limit be updated at a later stage?

The same bridge can be listed with multiple gas limits and new can be added over time. Was mainly to hear if you would want to have multiple different so frontends need to figure out which one based on type of call, or going for the higher gas ones where users on average then would pay a bit more gas but it is easy to handle for people building frontends because they don't need to handle it.

Decided to go with an EnumerableSet because we needed to map id => address, but also avoid duplicates (which would require a mapping of address > something). Could have made it a permissioned function and assume that there would be no duplicates, but wanted to make open so it can continue to work without our intervention. But maybe I missed something? I'm open to suggestions

Fine with me that you use the EnumerableSet, was just curious.

Not sure what you mean with the fact that we don't mention Ethereum. But I added some information to the spec regarding immutability. Please let me know if that's enough

My bad on clarity. I was looking at your docs for the Smart Contract Registry where it just don't mention that it is also the addresses on Ethereum 😄

Will look at the code in a moment. The CI is not running, I'm not fully sure why, might be fixed with a rebase though, should be running.

---

• [i] The natspec on your constructor is missing the _transformerRegistry. Just a note on naming, I would expect registry to just be register to fetch data from and not also helping on doing the transforms.

[nchamo]

Yes, that batch will be closed at once. But if it cannot be done from within the convert, then it would require an L1 transaction. Someone needs to execute the L1 transaction, and if there is no incentive to do that, then it is most likely one of the users in the batch doing it which leaks information.

I see. I'm curious about your experience with the current Uniswap DCA bridge. I see that there is a fixed fee. How big is it? Is it enough so that third parties execute the finalise? What is there is a batch with low amount of funds? Just trying to figure out what the best approach would be for us

The same bridge can be listed with multiple gas limits and new can be added over time. Was mainly to hear if you would want to have multiple different so frontends need to figure out which one based on type of call, or going for the higher gas ones where users on average then would pay a bit more gas but it is easy to handle for people building frontends because they don't need to handle it.

We are going to be building our own FE at aztec.mean.finance. After we are done with the measurements, we can see what the initial gas limit should be, but it's nice to know that it can be updated

My bad on clarity. I was looking at your docs for the Smart Contract Registry where it just don't mention that it is also the addresses on Ethereum 😄

Oh, you are right. Just updated it. It's the same address across all chains

[LHerskind]

I see. I'm curious about your experience with the current Uniswap DCA bridge. I see that there is a fixed fee. How big is it? Is it enough so that third parties execute the finalise? What is there is a batch with low amount of funds? Just trying to figure out what the best approach would be for us

To be honest the current Uniswap DCA bridge don't work particularly well with the fee, fee is too small for searchers to be executing it "often". Also have not won interest from many searchers because profit is tiny and need to do a bit of extra logic to see if it makes sense etc. Generalized frontrunners will frontrun finalises if profitable though, just not look for it themselves 😅

We are going to be building our own FE at aztec.mean.finance. After we are done with the measurements, we can see what the initial gas limit should be, but it's nice to know that it can be updated

Cool cool. It can be a good idea to have a couple of options for different flows. For the ERC4626 the bridge is listed 3 times, with 300, 400 and 500K gas. With the more general bridges gas are not as consistent, so it is nice to have something to pick between.

[LHerskind]

Looked at it again:

• Think it would be helpful if you could add a diagram to the spec, for easier getting an overview of the flow. Outline here:
  
• Add a comment to _unswapIfNeeded that it is also doing a transfer of eth to the rollup if _outputAsset is Eth. Otherwise it could easily slip by.
• The constructor still is missing a natspec @param line for the _transformerRegistry

[nchamo]

Thank you for the diagram @LHerskind , it definitely adds a ton of value. Just added it to the bridge spec, and also added the comments you asked for 😄

Based on the experience you've described with the Uniswap DCA bridge, we are currently discussing with the rest of the Mean team potential approaches to solve the finalise issue

[LHerskind]

Based on the experience you've described with the Uniswap DCA bridge, we are currently discussing with the rest of the Mean team potential approaches to solve the finalise issue.

Cool, thanks 👍 Let me know if you need input or have questions related to it 😄. Since this might impact the code, will wait a bit before planning the audit, ok?

[nchamo]

Yeah, 100%. Let's wait a little bit before we set it up

[nchamo]

Hey @LHerskind !


We have a proposal, but I’m not 100% it fits with how Aztec works, so please let us know if we are missing something.

The idea would be to make the bridge synchronous, similarly to the ERC4626 bridge. Users would deposit into Mean, and they would get a virtual asset in response. From what I understood, virtual assets simply have the id of the interaction nonce, so we could always return a fixed amount that would be automatically distributed along all users on the same batch.

By doing this, we would have the following benefits:

• Users would be able to execute both the deposit and withdraw directly from L2, without having to give up their privacy
• With the previous approach, when the position was finalized then funds would be automatically bridged for all users. If they were using yield, then all of them would stop earning yield. With this approach, a user can withdraw their funds but the rest would still be earning yield until they execute their own withdraw (more on this below)
• Our plan was to subsidize some pairs and intervals, but we wanted to let users set up their own custom positions, as long as they paid for the full batch. If we had to pay for the finalize, then it could have gotten very expensive. With this approach, they can pay for everything for themselves

Changes necessary:

• With the previous approach, when the withdraw was executed, all funds were unwrapped and bridged automatically. With the new approach, funds would be withdrawn from Mean (we don’t support partial withdraw), but they would only be unwrapped when the owner wants to bridge them. This would mean that the bridge will be holding funds with the new approach
• To support the above, we’ll probably need to add some extra accounting to the bridge
• I’ll need to revisit the subsidies to make sure we can configure both deposits and withdraws separately

And I guess that’s it. What do you think? Do you see it possible?

[LHerskind]

Hi, sorry for the slow response. Yes, modelling a synchronous bridge using the Virtual assets should be possible.
You can use the virtual assets as shares, simply returning as many as was deposited. Then keep track of the total, and when withdrawals are happening you can use the share to compute how much the individual user should receive.

Your second benefit might not be a full benefit, you also run into have a more difficult time finding other people to batch with. If someone have already exited, you have a pretty small set of users that you could batch together with. But agree on 1 and 3 being clear benefits.

For changes, yes you would probably need to do some accounting. Practically, your bridge was already holding funds in the form of the mean position before as well, so don't see the big difference there.

Deposits and withdrawals should be happening with different ordering of the assets, so you should be able to use the ids to distinguish between the two flows.

[nchamo]

Hey @LHerskind !

Sorry for the delay. Just pushed the changes to implement the approach we discussed. I also updated the diagram, tests and spec.

Please let me know if there is something else I can do from my side