[BIP-473] Permissions Preset Update Request #2 (Revised version of BIP-442)

General Chat Karpatkey Updates
1

PR with Payload

Abstract

In this revised iteration of BIP-442, we’ve made a significant adjustment by excluding all Cow Protocol permissions from the preset. This decision arises from a recent protocol update, which rendered the Zodiac Role Modifier incompatible with the Cow Protocol. To ensure the Balancer Maxis do not execute the outdated payload included in BIP-442, we preemptively halted its implementation to avoid introducing unnecessary permissions.

Having seized the opportunity to fine-tune our proposal, we’ve chosen to seamlessly integrate new permission requests into this framework. This approach streamlines all permissions into a unified structure, eliminating the need for another BIP in the coming weeks.

Motivation

Permissions granted to karpatkey to manage the BalancerDAO treasury are not static, and require regular updates to effectively respond to evolving market conditions and to capitalise on emerging opportunities.

As it’s customary, any changes to the preset permissions must be executed by the DAO.

Specification

This proposal encompasses the addition of the following permissions:

  1. Deposit USDC on Compound v3
  2. Deposit DAI on AAVE v3
  3. Deposit USDC on AAVE v3
  4. Deposit DAI on MakerDAO’s Dai Savings Rate (DSR) Module
  5. Unstake stETH on Lido
  6. Stake (and unstake) ETH on Rocket Pool (rETH)
  7. Swap various tokens on SushiSwap
  8. Swap rETH <> WETH on Balancer
  9. Swap rETH <> WETH on Uniswap v3
  10. Swap stETH <> WETH on Curve
  11. Restake AAVE in the Safety Module
  12. Delegate AAVE/stkAAVE to karpatkey
  13. Wrap stETH

Audit Considerations

We propose an updated version of the Preset permissions - Balancer Treasury document that shows all the permissions granted to karpatkey, including the ones that are being requested now, highlighted in green.

Reviewing and providing feedback on the preset update payload would be a great service to the Balancer community. We welcome anyone who is willing to take the time to verify the content and encourage community members to share their feedback with us.

2 Likes
2

Proposed Actions Overview

1. Deposit USDC on Compound v3

Compound v3 is a streamlined protocol version emphasising security, capital efficiency, and user experience. Following a proposal executed on September 18th, v2 is being phased out, with measures in place to transition users to v3. OpenZeppelin and ChainSecurity have audited v3. As of the day of this writing, $315M USDC is deposited in v3, offering an APR of 4.7%, inclusive of COMP token rewards.

2. Deposit DAI on AAVE v3

This strategy aims to diversify stablecoin holdings via AAVE v3, a leading decentralised money market with a strong link to the Balancer DAO. AAVE v3 was officially deployed on mainnet in January 2023 after being live across six networks since March 2022. This version introduces features for enhanced capital efficiency and decentralisation, coupled with new risk management tools. Comprehensive audits by firms like Trail of Bits, ABDK, Peckshield, OpenZeppelin, SigmaPrime, and formal verification by Certora. Currently, AAVE v3 holds $69M DAI with an expected APY of 4.07%.

3. Deposit USDC on AAVE v3

Same logic as above but for USDC. As of the day of this writing, the total USDC supplied on AAVE v3 is $332M, and the expected APY is 2.85%.

4. Deposit DAI on MakerDAO’s DAI Savings Rate (DSR) module

MakerDAO, renowned for creating DAI, is a leading DAO in the DeFi sector. A recent DSR rate hike in May and the introduction of eDSR (enhanced DSR) in August make it an attractive strategy from a risk/reward perspective. Currently, eDSR offers an APR of 5%. By diversifying the Balancer treasury’s stablecoin allocation and capitalising on the higher yield offered by one of the most battle-tested protocols, the investment approach can be optimised.

5. Unstake stETH on Lido

Post the Shapella fork in April, users can now unstake or withdraw staked ETH from the different LST providers. This action was not part of the previous preset update request to the Balancer DAO due to its timing.

6. Stake (and unstake) ETH on Rocket Pool (rETH)

To diversify LST holdings, we suggest adding RocketPool to the whitelist for ETH staking and unstaking. RocketPool ranks third in the liquid staking market, holding about 805,000 ETH, or 5.5% of the total market. In market share, Rocket Pool’s rETH trails Lido’s stETH (74%) and Coinbase’s cbETH (11%). Despite slightly lower liquidity for rETH to ETH swaps on major DEXes, Balancer DAO treasury’s allocated amount should face minimal slippage. For example, DeFi Llama indicates a swap of ~5,900 rETH for ETH would have under 0.01% slippage. Currently, rETH offers an APR of 3.03%.

7. Swap Various Tokens on SushiSwap

Recognizing the dynamic nature of token liquidity, we propose the whitelisting of additional swapping routes to enhance the efficiency of swaps for the Balancer DAO. We seek permission to swap COMP, DAI, USDC, USDT, and WETH on SushiSwap:

  • Token In List: [COMP, DAI, USDC, USDT, WETH]
  • Token Out List: [DAI, USDC, USDT, WETH]

8. Swap rETH <> WETH on Balancer

Considering the inclusion of RocketPool in strategy #6, it is desirable to add alternative swapping options for rETH <> WETH, ensuring flexibility when adjusting this strategy. Balancer, with a pool TVL of $58M and a daily trading volume of $60k at the time of writing, stands as a top choice to fulfil this objective.

9. Swap rETH <> WETH on Uniswap v3

This mirrors the rationale behind the previous action but extends to Uniswap v3. With a combined TVL of nearly $6M and a daily trading volume of $3.2M, factoring in the rETH<>ETH (0.05%) and rETH<>ETH (0.01%) pools, Uniswap v3 presents an additional viable avenue for executing the desired swaps, should the need arise.

10. Swap stETH <> ETH on Curve

This action echoes the aforementioned strategy but pertains to a different liquid staking token (stETH) and operates on the Curve platform. With a current TVL of approximately $196M and a daily trading volume of $4.7M at the time of writing, the stETH <> ETH pool on Curve represents a compelling alternative when modifying the Stake ETH on Lido strategy.

11. Restake AAVE on the Safety Module

Although this action was not initially requested in the strategy due to the absence of compounding in the Stake AAVE on the Safety Module strategy, it is now deemed desirable. This move is driven by the strong partnership between AAVE and Balancer, aiming to compound the results and refrain from selling the resulting AAVE rewards. The current preset permits compounding through two different actions: Claim and Stake. Nevertheless, the Restake action allows a more efficient approach to achieving the same result.

12. Delegate AAVE/stkAAVE to karpatkey

As per BIP-452, the Balancer DAO has made the decision to delegate voting power in the AAVE DAO, which is granted by AAVE and stkAAVE, to karpatkey. Given that any changes to this strategy, such as unstaking or staking, necessitate the reinstatement of delegation, it is prudent to include this action in the preset. Doing so alleviates the Balancer DAO’s burden by enabling karpatkey to execute it directly through the Manager Safe. As long as BIP-452 remains valid, this permission will remain active.

13. Wrap stETH

While conducting our routine examination of the permissions granted to karpatkey within the preset, we identified an inconsistency. Specifically, we observed that the route wstETH → stETH is currently whitelisted, whereas the reverse path, stETH → wstETH, is not. Consequently, we are submitting a request to rectify this omission by adding the latter route to the whitelist.

3

Due Diligence: Revised Version of BIP-442

Karpatkey did a self audit of this payload using my methodology of the initial due diligence report of BIP-442.

I went through their self audit and can confirm it matches their simulation of the proposed payload.

However, the only event that I cannot read properly is id 59. The function signature only has 12 parameters, but 14 are given. This probably has to do with the fact that some of the parameters are packed in a struct/tuple, but this then throws off the order of the parameters:

swap(
    (
        bytes32,
        uint8,
        address,
        address,
        uint256,
        bytes
    ) singleSwap,
    (
        address,
        bool,
        address,
        bool
    ) funds,
    uint256 limit,
    uint256 deadline
)

0:"0x00000000000000000000000000000000000000000000000000000000000000e0"
1:"0x0000000000000000000000000efccbb9e2c09ea29551879bd9da32362b32fc89"
2:"0x0000000000000000000000000000000000000000000000000000000000000000"
3:"0x0000000000000000000000000efccbb9e2c09ea29551879bd9da32362b32fc89"
4:"0x0000000000000000000000000000000000000000000000000000000000000000"
5:"0x"
6:"0x"
7:"0x"
8:"0x0000000000000000000000000000000000000000000000000000000000000000"
9:"0x"
10:"0x"
11:"0x"
12:"0x00000000000000000000000000000000000000000000000000000000000000c0"
13:"0x0000000000000000000000000000000000000000000000000000000000000000"

E.g. I am guessing that [1-4] are the values of the funds struct ((address,bool,address,bool)), but they are clearly not in the same place as in the swap signature. @karpatkey can you elaborate on how to match the parameter indices to the signature? Same then goes for events with id 60-62.

Conclusion

Events [59-62] need some more explanation from Karpatkey, given I cannot match indices to the corresponding function signature.

I would also be curious as to your reasoning behind making infinite approvals for the ERC20s, versus making them atomically? E.g. it would be possible to scope the approval functions on these ERC20s, so that you could sandwich your actions on the avatar with the necessary approve(inf) and approve(0) (or just approve(n)). This would minimise the risk vector of leaving inifite approvals open for an unforseen time (the SushiSwap RouterProcessor2 approval bug costing $3.3MM comes to mind for example).

Recommendations

Although the self audit you provide makes the payload a lot more readable and verifiable, it is still very time consuming on both sides (since the self audit has to be audited…). A programmatic and open source way of turning the payload into a report would be the solution here. I would be happy to discuss initiating or assisting with such a project, either as a pilot here with Balancer or elsewhere.

4

ah i forget to mention, i noticed the karpatkey delegate multisig (0x8787FC2De4De95c53e5E3a4e5459247D9773ea52) only has a signing threshold of 1/7. now i know that karpatkey is not a dao, but normally such setups are considered a risk (eg one compromised key could have quite some unintended consequences).

5

Hi @gosuto,

We are grateful for your diligence and expertise in auditing the payload, and for sharing your insights with the community. As highlighted, payload auditability remains a paramount concern for us, and we are committed to enhancing its ease of inspection. Below you can find our elaboration and further commentary in response to your request for clarification.

Balancer: Vault swap scoping

In our effort to accurately scope the parameters of the function, certain considerations are imperative. The swap function within the Balancer:Vault is delineated as follows:

swap(
   (bytes32 poolId,
   uint8 kind,
   address assetIn,
   address assetOut,
   uint256 amount,
   bytes userData
   ),
   (address sender,
   bool fromInternalBalance,
   address recipient,
   bool toInternalBalance
   ),
   uint256 limit,
   uint256 deadline
)

The first tuple (bytes32,uint8,address,address,uint256,bytes), encapsulating a variable-length bytes parameter, necessitates a preliminary parameter that denotes the offset in hexadecimal of bytes where the tuple is positioned within the call data; in this instance, offset = 7. This tuple is subsequently parsed at offset 7, at the end of the parameters.

Upon reaching the tuple, the fixed-length parameters are conventionally constrained, with the 12th 32-byte chunk indicating again the offset from the tuple where the authentic data commences — offset = 6 in this scenario — thus enabling the constraint to be passed.

Here’s a representation of the constraints passed and their corresponding values:

0:"0x00000000000000000000000000000000000000000000000000000000000000e0" - tuple offset
1:"0x0000000000000000000000000efccbb9e2c09ea29551879bd9da32362b32fc89" - sender
2:"0x0000000000000000000000000000000000000000000000000000000000000000" - fromInternalBalance
3:"0x0000000000000000000000000efccbb9e2c09ea29551879bd9da32362b32fc89" - recipient
4:"0x0000000000000000000000000000000000000000000000000000000000000000" - toInternalBalance
5:"0x" - limit
6:"0x" - deadline
7:"0x" - poolId
8:"0x0000000000000000000000000000000000000000000000000000000000000000" - kind
9:"0x" - assetIn
10:"0x" - assetOut
11:"0x" - amount
12:"0x00000000000000000000000000000000000000000000000000000000000000c0" - bytes offset
13:"0x0000000000000000000000000000000000000000000000000000000000000000" - userData

Values for each of the constraints passed above can be seen here:

0: 224 = 32 * 7
1: Avatar
2: ZERO_ADDRESS
3: Avatar
4: ZERO_ADDRESS
5: NULL
6: NULL
7: NULL
8: ZERO_ADDRESS
9: NULL
10: NULL
11: NULL
12: 192 = 32 * 6
13: ZERO_ADDRESS

Notably, parameters like poolId,assetIn, and assetOut are later constrained under scopeParameterAsOneOf with their respective offsets being 7, 9, and 10. Here’s a clearer depiction of the constraints:

7: poolId (balancer:B_50COMP_50WETH,
   balancer:B_60WETH_40DAI,
   balancer:B_50USDC_50WETH,
   balancer:B_stETH_STABLE,
   balancer:B_rETH_STABLE
   )
9: assetIn (COMP,
   WETH,
   wstETH,
   rETH
   )
10: assetOut (WETH,
   DAI,
   USDC,
   wstETH,
   rETH
   )

Scoping Rationale

Without precise scoping, there’s a potential risk where an ‘approved’ swap initiated at 0xe0 could be followed by an ‘illegal’ swap, with the offset at slot 0 pointing to the illegal swap. While Zodiac might check the parameters of the ‘approved’ swap, the offset directed at the illegal one could bypass the ABI decoder, favoring the illegal swap instead.

Further remarks

Regarding your observation on approvals, we’ve opted to have the Avatar Safe (also known as the Balancer Treasury Safe managed by karpatkey) process all requisite approvals for the initial strategy before transitioning ownership to the Balancer DAO. This approach was aimed at circumventing the necessity of a lengthy list of permissions within the preset. Nevertheless, this aspect had previously undergone internal deliberation and was identified as a potential enhancement to continually fortify the permissions system. Moving forward, we plan to revoke all unlimited permissions and incorporate them within the preset to bolster security and robustness.

3 Likes
6

https://snapshot.org/#/balancer.eth/proposal/0x9e0a4c87103d9858a9e3662993cd864a161b9a259b5a64894194cf9b3cd4e48b

7

@gosuto @karpatkey Following loading the provided json into the safes and doing a final review, it became clear that the review of the simulated transaction occured a week before the payload was uploaded. We still need to verify that the thing the signers will execute does what is documented here.

The payload that was loaded, along with a link to a tenderly from the load can be found here:

and here:

Please advise how you recommend that we verify that the transaction loaded and the tenderly provided match the tenderly from the Karpatkey internal simulation 14 days ago.

We will hold off on asking signers to sign this payload until we can demonstrate that the payload loaded has been fully verified.

8

Hey @Tritium,

We have just finished working on this script to decode each transaction included in the payload. It takes the payload JSON as input and generates a JSON containing each transaction along with its parameters in human-readable form.

The output of the decoder can then be matched with the transactions included in the self audit using the id for each (position in the payload).

By matching decoded transactions with the transactions in the markdown document, you will be able to check that everything included in the payload has been reviewed.

9

my suggestion would be to use the following api endpoint to be able to run the auditing script on a queued up tx: https://safe-transaction-mainnet.safe.global/api/v1/safes/0x0EFcCBb9E2C09Ea29551879bd9Da32362b32fc89/all-transactions/?executed=false&queued=true&trusted=true

a final handy function then would be to confirm that nonce X on safe Y matches payload json file Z; eg through a checksum or something like that.

10

It is still unclear to me that we have managed to verify that the loaded payload is what is described in this BIP and safe.

@gosuto have you been able to make such an assertion yet? If not, we should hold off executing this for another week until it is clear what we are asking signers to sign.

1 Like
11

i took the calldata from the api endpoint i mentioned, and compared it against the calldata which is generated by simulating the payload json linked above.

they match; ie the queued up tx on nonce 47 is a result of loading up the proposed json

Screenshot 2023-11-15 at 12.43.06
Screenshot 2023-11-15 at 12.43.06702×194 4.38 KB

1 Like
12

Thanks for the confirmation. Will request attention to this payload during the signing session next week.

1 Like