[BIP-667] Roles V2 Migration & New Treasury Permissions

General Chat Karpatkey Updates
1

PR with Payload

Abstract

This proposal aims to roll out an updated version of the Zodiac Roles Modifier module. The new version improves usability and transparency of treasury management operations. Upon approval, the Roles Modifier v2 module will be activated.

Furthermore, this proposal requests authorization from the DAO to revise the permissions policy. A notable change includes enabling swapping actions on CoW Swap while the other permissions primarily focus on eliminating obsolete actions and protocols, and refining parameters within the existing permissions.

Roles v2 Migration

Motivation

The Zodiac Roles Modifier facilitates karpatkey’s proxy management of the treasury by ensuring that only pre-approved transactions, defined by the permissions policy voted on by the DAO, can be executed. In collaboration with karpatkey, the Gnosis Guild team has significantly upgraded the Zodiac Roles Modifier module and the Zodiac Roles app. These enhancements have resulted in a more powerful and robust on-chain permissions infrastructure with the following improvements:

  • Introduction of Allowances: Implementation of spending limits within permissions.
  • Enhanced Call Data Scoping Toolset: This toolset considerably broadens the range of functions that can have permissions set, increasing flexibility.
  • Advanced Logical Conditions: Allows for the creation of complex permissions structures to accommodate sophisticated operational needs.
  • Compatibility with DeFi Kit: This feature integrates with karpatkey’s permissions policy building module, facilitating the straightforward crafting of protocol actions.
  • Improved Visualisation and User Interface: the new Zodiac Roles app UI not only displays permissions clearly but also provides a user-friendly interface to compare changes in each policy update, enhancing transparency and simplifying audits.

For more detailed information, please refer to the following resources:

Contract Audits

The Zodiac Roles Modifier v2 contract has been rigorously audited by G0 Group, the internal auditing team of Gnosis DAO, and by Omniscia. The detailed audit reports are available for review here.

Changes to the Permissions Policy

This proposal outlines the following modifications to the permissions policy:

  • Token Arrays for Swapping:
    • Considering the tokens involved in the existing permissions, we have updated the token arrays to ensure they can be seamlessly swapped across the various whitelisted protocols and aggregators.
    • Token IN Allowlist: [WBTC, DAI, USDT, stkAAVE, AAVE, COMP, rETH, SWISE, wstETH, WETH, USDC, stETH].
    • Token OUT Allowlist: [WBTC, DAI, USDT, rETH, wstETH, WETH, USDC, stETH].
    • The above arrays are to be utilized for swapping on CoW Swap, with equivalent lists replicated for Uniswap v3 and Balancer.
  • Introduction of CoW Swap(diff):
    • Addition of a CoW Swap Order signer to enable gas-minimized and MEV-protected swaps. This includes an extensive set of aggregated exchange routes, improving the efficiency and effectiveness of required swaps.
    • Tokens will be swapped on CoW Swap according to the token IN/OUT allowlists mentioned above.
  • Deprecations and Removals:
    • Uniswap v2(diff): Removed due to insufficient liquidity in v2 pools.
    • Sushiswap(diff): Removed due to insufficient liquidity in Sushiswap pools.
    • Stakewise v2: Deprecated functions related to deposit (diff) and claim (diff) functions in light of the recent launch of Stakewise v3.
    • Compound v2 (diff): Discontinued all actions targeting v2 contracts and v2 cTokens (cUSDC, cDAI and cAAVE) due to the ongoing transition of the protocol to its v3.
    • Revocation of Existing/Obsolete Allowances: All existing and outdated allowances previously set by the treasury are revoked (set to zero). The ability to call the corresponding approve functions is included in the newly proposed policy. Accordingly, the payload contains a bundle of transactions to revoke these allowances.
  • Updates:
    • AAVE Delegation: Delegation function update for AAVE (diff) and stkAAVE (diff) tokens. Very similar to the delegate function, with the added option of delegation type.
    • Uniswap v3 (diff) and Balancer (diff): Adjusted to allow the mentioned token IN/OUT allowlists.
    • Lido Withdrawals (diff): Enhanced to include new withdrawal methods using permits for both wstETH and stETH; methods include “requestWithdrawalsWstETHWithPermit” and “requestWithdrawalsWithPermit”.
    • Uniswap v3 WBTC / ETH LP (NFT Id 430246) actions updated (diff):
      • Increase Liquidity
      • Refund ETH when Increasing Liquidity.

Audit Considerations

We highly value the community’s involvement in reviewing and providing feedback on this proposal. We encourage members with the necessary technical expertise to examine the content carefully (including this payload) and share their insights with us.

For effective testing of the permissions policy configuration, we have utilized a Testing Avatar Safe. This safe mirrors the current state of the permissions policy v3 granted by the DAO treasury (or Avatar SAFE) to the Manager SAFE operated by karpatkey. The enhancements in the Zodiac Roles app interface now allow for a clear visualization of all existing permissions, accessible here.

The updated interface also simplifies the process of identifying changes by displaying the current permissions policy on the left side and the newly proposed policy on the right side. To further aid in the adoption and understanding of these tools for audit purposes, we have detailed the proposed changes in version 4 documentation, following our standard Policy Update Request (PUR) format.

Additional considerations

Roles Modifier v1 and Enabling of v2

The existing Roles Modifier v1 module will remain active to ensure a smooth transition and prevent any unexpected disruptions in operational execution. Ownership of the deployed Roles Modifier v2 module, equipped with the new proposed permissions policy, has been transferred to the treasury’s Avatar Safe. The payload will only activate this module, marking the first phase of the migration process. A subsequent policy update proposal will seek to disable the v1 module.

Policy Visualization in Terms of DeFi Kit Actions

The “show annotations” button, located at the top-right here, provides a visualization of the proposed permissions policy expressed through the DeFi Kit Protocol Actions. This feature offers a more abstract and simplified description of the policy, enhancing understanding and accessibility.

2

Auditing Steps

Current Permissions Policy

  • Link 1: Current Permissions Policy v3 (PP v3).
  • Link 2: PP v3 currently deployed in the Roles v1 module as displayed in the Zodiac app (navigate to “edit roles”, toggle the roles, review target contracts).
  • Link 3: Same policy as Link 2, but deployed in a Roles v2 module.

First auditing step: Verify that the permissions in Link 3 match those in Link 2.

Comparison of Current Permissions Policy vs New Proposed Policy

  • Link 4: Comparison of the current policy (left) vs. the new proposed policy (right).

Second auditing step:

  • Assess the newly added permissions in the proposed policy (highlighted in green on the right).
  • Review the revoked permissions from the old policy (highlighted in red on the left).
  • Examine the updated or modified permissions (indicated in blue on both sides).

Detailed descriptions of the changes in the permissions policy are available here (Link: “Changes in the Permission Policy”).

Enabling Roles v2 by the DAO Treasury

  • Ownership of the Roles Modifier v2 module, equipped with the new proposed permissions policy, has been transferred to the DAO Treasury.
  • Link 5: The new permissions policy deployed in the Roles v2 module.

Third auditing step: Verify that the new proposed policy is consistent with the policy in Roles v1 (as per Link 4, right side) and with the policy in Roles v2 (Link 5).

Future Audits

  • Future audits will require only the “Current Permissions Policy vs. New Proposed Policy” step.
  • Changes in policies will be assessed by identifying which DeFi Kit actions are added or removed, streamlining the process by bypassing the analysis of individual permissions.
2 Likes
3

while waiting on some clarifications for the next auditing steps, here is the first:

rolesv2 migration: old style policy vs new style policy

i compared 0x0efccbb9e2c09ea29551879bd9da32362b32fc89 using https://zodiac.gnosisguild.org/ ui versus Zodiac Roles (aka link2 vs link3)

  • CometRewards.claim(comet, src): comet is no longer scoped. i dont see any security issues with that

  • CompoundV2Unitroller.claimComp(holder, cTokens): cTokens is no longer scoped. makes sense, no issue

  • StakewiseMerkleDistributor.claim(index, account, tokens, amounts, merkleProof): tokens is no longer scoped. no issue

  • stETH.submit(referral): referral is no longer scoped. no sec issue

  • Balancer.Vault.swap(): tighter scoped looks like, due to not every permutation not being possible anymore. no issue

  • UniswapV3.NFPManager.mint(): idem, less permutations possible. no issue

  • StakewisePool.0x3a4b66f1: this selector was present in the old policy (stake()), but it is no longer available after this proxy upgrade. no issue (as long as they dont reupgrade, ha!), plus it looks like this permission is actually getting removed in your proposal :+1:

2 Likes
4

permissions policy

tldr

  • adds:

    • direct approvals for a list of tokens to cow, uni and balancer
    • $aave: set delegate
    • aave: enable user reserve as collateral on pools
    • lido: request and claim withdrawals
    • univ3: request eth refund, unwrap weth, sweep token
    • univ3: collect a specific tokenid (430246) recipient address(0) (?)

  • removes:

    • compound: v2 components
    • sushi: remove router
    • stakewise: merkle distributor claims, pool stake
    • univ3: swapExactTokensForTokens, mint weth/stakewise, weth/wbtc lp (?)

questions

  • im guessing that if there is no entry in the zodiac roles ui for a given parameter, then it is basically callable without any restrictions? because in the case of signOrder for cowswap, i dont see any restrictions on neither feeAmountBP nor order.feeAmount. i still think that having both completely unbound opens the door to possible unwanted behaviour. not a show stopper per se, but would be nice to see a limit of for example 10% or even 20% total fee. i did mention this in the previous review already as well (ref)

  • why remove swapExactTokensForTokens?

  • why recipient address(0) for univ3 collect?

feedback

  • quite a heavy migration, but i am sure next iterations will go a lot easier with this new zodiac roles ui!

  • overall the ui is great help, would be nice to have an address book connected to it

  • note that a lot of function signatures in the ui (identifiers, parameters) are not decoding at all

  • ui can only display a diff of two policies by comparing an actual policy versus a hash (“saved version” of a policy?). how that hash is generated and how it relates to the actual policy is completely opaque

payload json

your payload json has raw calldata, opposed to actual method names:

{
    "to": "0xae7ab96520DE3A18E5e111B5EaAb095312D7fE84",
    "data": "0x095ea7b3000000000000000000000000dc24316b9ae028f1497c275eb9192a3ea0f670220000000000000000000000000000000000000000000000000000000000000000",
    "value": "0"
}

vs

{
    "to": "0xae7ab96520DE3A18E5e111B5EaAb095312D7fE84",
    "value": "0",
    "data": null,
    "contractMethod": {
    "inputs": [
        { "name": "_spender", "type": "address" },
        { "name": "_amount", "type": "uint256" }
    ],
    "name": "approve",
    "payable": false
    },
    "contractInputsValues": {
    "_spender": "0xDC24316b9AE028F1497c275EB9192a3Ea0f67022",
    "_amount": "0"
    }
}

i also noticed some data type mismatches in the payload you suggested on github, but we can discuss that there :slight_smile:

5

Hey @gosuto ,

To answer your doubts;

  1. Restricting fees in CoW Swap orders.

We’ve not included any caps to the order.feeAmount because the maximum protocol fee is capped at 1% and eligible only when a swap receives a better price than it was quoted.
“50% of the quote improvement or 1% of volume, whichever is lower” - CoW Swap docs.

Nevertheless, this is a great point. If you think it’s necessary to add an additional limit in the permissions polity for added safety / peace of mind, we’re happy to do so.

  1. Removal of swapExactTokensForTokens.

As per BIP-273, we have migrated to exactInputSingle in Uniswap V3 because it does not require us to pass a specific path, instead providing the most efficient route to tokenOut.

  1. Why add a null address as recipient on the univ3 collect function?

This gives us the possibility to collect & remove liquidity in ETH instead of WETH. All V3 NFT Positions hold ERC-20 tokens, to remove WETH we simply pass in the Avatar Safe, but to remove ETH we need to pass in the zero address instead.

You can verify this logic in the Uniswap V3: Position NFT function here (image below). Line 318 checks if the recipient is set to the null address, then sends the WETH to the position NFT contract, unwraps it, and later sends it to the from address, which in this case is the Avatar Safe.

Screenshot 2024-07-17 141458
Screenshot 2024-07-17 1414581191×291 39.1 KB

Thank you for your time and thorough review.

1 Like
6

Find we have updated the permissions payload to a final version which includes:

  • Whitelisting of the Cow Swap Action including:
    • Cow unsignOrder().
    • A2% cap for the feeAmountBP parameter.

Permissions page: https://roles.gnosisguild.org/permissions/eth/FrBx1C1A4GSbQNBrMXcw6F5y3u0P4ksKQEXxoj99A
Permissions diff page: https://roles.gnosisguild.org/eth:0x13c61a25DB73e7a94a244bD2205aDba8b4a60F4a/roles/MANAGER/diff/FrBx1C1A4GSbQNBrMXcw6F5y3u0P4ksKQEXxoj99A

1 Like
7

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