Balancer MetaStablePool Strategy

[sparrowDom]

Adds balancer Meta Stable Pool strategy. Corresponding Notion design document.

Design considerations: since we are planning to potentially also support Composable pool strategy, I've decided to make Meta stable pool contract extend Aura contract. This way Aura implementation can stay the same when/if we add the support for Composable Stable pools.

Things done ✅ :

• able to add/remove liquidity to/from Balancer meta Stable pool with correct BPT (liquidity tokens) calculation
• able to deploy/withdraw liquidity to/from Aura booster
• add tests to confirm deposit/withdrawal slippage configuration works as expected
• add test to confirm read-only re-entrancy protection functions

[codecov]

Codecov Report

Merging #1697 (1c782a5) into master (67fa12a) will decrease coverage by 57.31%.
The diff coverage is 77.77%.

❗ Current head 1c782a5 differs from pull request most recent head 88bfd30. Consider uploading reports for the commit 88bfd30 to get more accurate results

@@             Coverage Diff             @@
##           master    #1697       +/-   ##
===========================================
- Coverage   69.98%   12.67%   -57.31%     
===========================================
  Files          45       47        +2     
  Lines        2382     2556      +174     
  Branches      622      655       +33     
===========================================
- Hits         1667      324     -1343     
- Misses        712     2232     +1520     
+ Partials        3        0        -3     

Files Changed	Coverage Δ
...racts/strategies/balancer/BaseBalancerStrategy.sol	64.06% <64.06%> (ø)
...s/strategies/balancer/BalancerMetaPoolStrategy.sol	76.76% <76.76%> (ø)
...contracts/strategies/balancer/BaseAuraStrategy.sol	100.00% <100.00%> (ø)

... and 37 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[github-actions]

	Warnings
⚠️	👀 This PR needs at least 2 reviewers

Generated by 🚫 dangerJS against 1ad9e71

[DanielVF]

Noting, I don't think the code in this function is ever used, tested, or included in a deployed contract?

[sparrowDom]

This is used by the checkBalance functions here and here and both versions (with asset parameter & without) of checkBalance are used in a few places in the fork test file

[DanielVF]

There appears to be a little behavior that's unexpected to me. It looks like an even deposit can make a unit profit. (as recorded by check balance), (but maybe that has to do with tokens getting more valuable)

Similarly, it's possible for withdrawAll() to be making a tiny unit loss, which is not what I would expect.

Again, when picking different withdraw mixes, it looks like after withdrawing we have a little less money than we expect.

All of these point to the the result of check balance being a tiny bit high somehow. Not sure how. Possibly acceptable. But still a bit of a mismatch to what I would we would expect. I wonder where this comes from.

Gist with charts: https://gist.github.com/DanielVF/885753cd1dad7bc840046f19219ec480

[sparrowDom]

@DanielVF you've asked me about caching the decimals yesterday. I forgot that by removing the Oracles (and relying solely on VaultValueChecker) we no longer query the decimals - so no need for caching. 2 birds 1 stone :)

[sparrowDom]

@DanielVF awesome approach of testing the behaviour of checkBalance and value in vault functions. I've made a couple of adjustments:

• by converting the reth amounts to units when accounting for tilts
• by balancing the pool first before performing any tilts. The original script was probably assuming the pool was balanced to begin with, when in reality it is slightly imbalanced. That is what was causing profitability in some cases.

I've rerun the Jupyter notebook and this is what the deposit profit looks like with adjustments:

Link to PR with all the changes.

[naddison36]

Great work Domen

[DanielVF]

Requirements

We are adding support for a balancer MetaStablePool strategy.

We are doing the same way of splitting checkBalances into fixed equal percentages that we used for the curve pools.

Deployment Considerations

• Monitoring needed for pool
• Strategy needed in analytics

Internal State

config only

Attack

Biggest concern with our strategy is check balance inflation. We've fork tested this, and an attacker was unable to manipulate our balance downwards via tilting the pool.

Balancer has known read-only reentrancy vectors. We have the balancer approved method on our checkBalance, and a test checking that this blocks it.

Tests

I've run my own fork test suite.

Flavor

Still rather whenNotInBalancerVaultContext was an inline function, rather than a modifier, but can live with this.

Overflow

• Never use "+" or "-", always use safe math or have contract compile in solidity version > 0.8
• Check that all for loops use uint256

Proxy

• Make sure proxy implementation contracts don't initialize variable state on variable declaration and do it rather in initialize function.

Black magic

• Does not contain selfdestruct
• Does not use delegatecall outside of proxying
• Address.isContract should be treated as if could return anything at any time, because that's reality.

Dependencies

• Review any new contract dependencies thoroughly (e.g. OpenZeppelin imports) when new dependencies are added or version of dependencies changes.
• If OpenZeppelin ACL roles are use review & enumerate all of them.
• Check OpenZeppelin security vulnerabilities and see if any apply to current PR considering the version of OpenZeppelin contract used.

Deploy

• Check that any deployer permissions are removed after deploy

Authentication

• Never use tx.origin
• Check that every external/public function should actually be external
• Check that every external/public function has the correct authentication

Cryptographic code

No crypto code

Gas problems

• Contracts with for loops must have either:
  • A way to remove items
  • Can be upgraded to get unstuck
  • Size can only controlled by admins
• Contracts with for loops must not allow end users to add unlimited items to a loop that is used by others or admins.

External calls

• Contract addresses passed in are validated
• Reentrancy guards on all state changing functions
• Malicious behaviors
• Could fail from stack depth problems (low level calls must require success)
• No slippage attacks (we need to validate expected tokens received)
  • Check with vault value checker
• Oracles, one of:
  • No oracles
  • Oracles can't be bent
  • If oracle can be bent, it won't hurt us.
• Don't call balanceOf for external contracts to determine what they will do, when they instead use internal accounting?

Ethereum

• Contract does not send or receive Ethereum.
• Contract has no payable methods.

Behavior of read-only reentrancy check