[EP 5.12] Roles Modifier V2 Migration & Updates to Endowment Permissions

🗳️ Meta-Governance Treasury Management
1

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

As previously stated, the Zodiac Roles Modifier facilitates karpatkey’s proxy management of the Endowment 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:

  1. Token Arrays for Swapping:
    1. 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.
    2. Token IN Allowlist: [CRV, DAI, USDT, BAL, AURA, CVX, ETHx, COMP, rETH, SWISE, wstETH, LDO, WETH, ankrETH, USDC, stETH].
    3. Token OUT Allowlist: [DAI, USDT, USDC, rETH, wstETH, WETH, stETH].
    4. The above arrays are to be utilised for swapping on CoW Swap, with equivalent lists replicated for Uniswap v3 and Balancer.
  2. Introduction of CoW Swap (diff):
    5. Addition of a CoW Swap Order signer to enable gas-minimised and MEV-protected swaps. This includes an extensive set of aggregated exchange routes, improving the efficiency and effectiveness of required swaps.
    6. Tokens will be swapped on CoW Swap according to the token IN/OUT allowlists mentioned above.
  3. Deprecations and Removals:
    7. Uniswap v2 Swaps (diff): Removed due to insufficient liquidity in V2 pools.
    8. Stakewise v2: Deprecated functions related to deposit (diff) and claim (diff) functions in light of the recent launch of Stakewise v3. Consequently, permitted actions related to Stakewise v2’s sETH2-ETH Uniswap pool are also removed (diff).
    9. Compound v2 (diff): Discontinued all actions targeting v2 contracts and v2 cTokens (cUSDC and cDAI) due to the ongoing transition of the protocol to its v3.
    10. Revocation of Existing/Obsolete Allowances: All existing and outdated allowances previously set by the Endowment 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.
  4. Updates:
    11. Uniswap v3 (diff) and Balancer (diff): Adjusted to allow the mentioned token IN/OUT allowlists.
    12. Curve Pools: Addition of stETH-ng (factory) pool (diff) and removal of cUSDC + cDAI pool (diff; Compound v2 tokens).
    13. Curve ZAP Deposit Contract (diff): Introduced to allow depositing and staking of LP tokens in a single step.
    14. Convex Staking (diff): Added the CVX/stETH Rewards contract.
    15. Lido Withdrawals (diff): Enhanced to include new withdrawal methods using permits for both wstETH and stETH; methods include requestWithdrawalsWstETHWithPermit and requestWithdrawalsWithPermit.
    16. Spark Rewards Claim (diff): Added functionality to claim wstETH rewards in Spark.

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 utilised a Testing Avatar Safe. This safe mirrors the current state of the permissions policy v4 granted by the Endowment to the Manager SAFE operated by karpatkey. The enhancements in the Zodiac Roles app interface now allow for a clear visualisation 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 5 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 Endowment’s Avatar Safe. These permissions are displayed here and match those shown on the comparison interface here. 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 Visualisation in Terms of DeFi Kit Actions

The “show annotations” button, located at the top-right here, provides a visualisation 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.

5 Likes
2

Thanks Karpatkey! A step-by-step “how to audit this” would be really useful given the new improvements to the roles modifier.

2 Likes
3

Auditing Steps

Current Permissions Policy

  • Link 1: Current Permissions Policy v4 (PP v4).
  • Link 2: PP v4 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.

Enabling Roles v2 by the Endowment

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.
1 Like
4

Hi Team Karpatkey,

Thank you for the detailed auditing steps. I have taken the liberty of auditing the permissions policy updates and have prepared detailed commentary here. For anyone reviewing the PUR as well, I hope that the comments I prepared save time.

I have a few questions:

  1. After verifying that the existing permissions in the v2 module match the permissions listed in the v1 module (according to PP v4) as instructed in the first auditing step, I noticed that the target address listed as 0xc874… describes ‘function not found in ABI’ and ‘no condition set’ in either module version. In PP v4, the signature for this target address is described as ‘stake()’ and is listed as revoked in PP v5. Could you help reconcile this discrepancy?

  2. While performing the third auditing step, I found that several tokens had a difference in the number of parameters listed in either module. For example, the wstETH token with target address 0x7f39… has 2 parameters listed in the v2 module, whereas in the v1 module, there are three additional parameters listed. This discrepancy occurs several more times. Could you shed some light on why there might be a difference in the number of parameters listed?

—

Thanks again for all your work! For anyone interested in the ENS Endowment, its initialization, and how this proposal enhances its administration, check out this ELI5 article I wrote.

3 Likes
5

First and foremost; thank you very much for the thorough auditing, @estmcmxci.

We greatly appreciate the time and effort you put into examining the new version of our treasury management framework. We believe that both internal and external reviews of this module are essential, as it is the foundation for us to execute on behalf of the ENS Endowment properly and securely.

Our responses to your first auditing step are as follows:

  • 0xc874b0…da29 is an old Stakewise contract.
    Stakewise deprecated the possibility of staking ETH in exchange for sETH2, which is why the function is no longer part of the contract’s ABI. In PP v5, we removed all Stakewise v2 permissions, which is why it is listed as revoked.

  • 0x5550d…747d is an old Sushiswap route processing contract.
    This permission had been removed in the last PUR update but ended up being added in the payload, which was subsequently fixed on November 3rd.

  • 0x5c0f2…0b1c is Balancer’s wstETH-WETH gauge.
    It appears both in the v1 (“Edit roles” => “Role #1” => “Ctrl+F: 0x5c0f “) and v2 module (as stated it should be on the first auditing step).

Regarding your second point, and third auditing step:

All of these functions are approval signatures whitelisting different protocols and exchanges we need to interact with. The EVM word has a length of 32 bytes, so to complete the word, addresses are padded with leading zeros. The new version of the App removes these leading zeros and converts hexadecimal values to be more human-readable.

For example;

The ‘0x095ea7b3’ signature you mention in the spWETH token contract is the ‘Approve’ function with an allowlisted input:’parameter[0]: 0x000000000000000000000000bd7d6a9ad7865463de44b05f04559f65e3b11704’.

Which can also be formatted as: ‘Spender: 0xBD7D6a9ad7865463DE44B05F04559f65e3B11704’, which is the Spark.fi deployer.

For the other tokens you flagged, and their seemingly missing spenders; the new permissions policy deployed in the Roles v2 module has some of the approved addresses under actions tagged “DeFi kit by karpatkey”. These actions are useful for the DeFi kit we are building.

To make the third auditing step less labor-intensive, we added a “Hide annotations” toggle (top right-hand side) that removes the actions and aggregates all approved spenders in the same format as the new proposed policy (right).

2 Likes
6

Thanks @estmcmxci

And thanks @santinomics.eth and @karpatkey,
Your partnership and support continue to exceed expectations. Thank you for always going above and beyond in your service.

We’d like to help you time this proposal with a couple of other executable proposals that we’re suggesting be posted around the middle of the month - we’re softly targeting around June 19th.

2 Likes
7

The executable version of this proposal is now live onchain. Vote here: Tally | ENS | [EP 5.12] Roles Modifier V2 Migration & Updates to Endowment Permissions

1 Like
8

I had a look at the details of the change. It looks quite interesting and is probably fine but I have not had time to dig dip into it.

I asked around and was told that some others including @estmcmxci, @nick.eth and @alextnetto.eth had had audited it. After talking with them it sounded like everyone sort of looked at it but no thorough audit was made. Or at least that’s how it sounded to me.

I then approached the people of Karpatkey who proposed it and was told they had very carefully audited and they even offered to guide me through it via a call. But since the vote ends in less than 2 days and I have no free time I could not make it.

All the above lead me to vote abstain since I can’t confidently say if it’s a safe change or not.

1 Like
9

The results are in for the [EP 5.12] Roles Modifier V2 Migration & Updates to Endowment Permissions proposal!

See how the community voted and view the detailed analytics here:
⬡ # [EP 5.12] Roles Modifier V2 Migration & Updates to Endowment Permissions | Dhive

1 Like
10

During the last Meta-Gov call, @estmcmxci brought up a very good point related to the interpretation of the criteria needed to be fulfilled in order to establish if an on-chain proposal has passed or failed.

I will use this proposal as a case study to ensure I fully understood the explanation during the call and apply this method to check past and future on-chain proposals.

In the documentation, we have the following mention:

  1. Executable Proposal: This is a proposal for a series of smart contract operations to be executed by accounts the DAO controls. These can include transfers of tokens as well as arbitrary smart contract calls. Examples of this include allocating funding to a workstream multisig wallet, or upgrading an ENS core contract. Executable proposals have a quorum requirement of 1% and require a minimum approval of 50% to pass.

So there are two requirements that need to be met for an on-chain proposal to pass:

1. quorum requirement of 1%

In the smart contract, the counting mode is set to support=bravo&quorum=for,abstain which means that we’re counting the total number of for votes plus abstain votes to establish if the quorum is met.

In this example, the quorum was reached:

666,376 for votes + 709,506 abstain votes > 0.01 * 100,000,000 total supply
=> 1,375,882 votes counted > 1,000,000 quorum threshold :white_check_mark:

So far, everything is clear and anyone can check this information directly in the contract.

2. minimum approval of 50%

Here is where this requirement is subject to interpretation as it’s not specified exactly how the 50% should be calculated, nor can we find this information in the contract.

Although the result of for votes is 666,376 for votes / 1,376,526 total votes * 100 = 48.41%, the 50% requirement should be interpreted and calculated solely in relation to the total of for votes plus against votes, as follows:

666,376 for votes / (666,376 for votes + 643 against votes) * 100 = 99.90%
=> 99.90% approval > 50% minimum approval :white_check_mark:

Could you please confirm if my understanding and method of calculation are correct? Also, is it safe to apply the same method for past and future on-chain proposals to check whether or not a proposal passed?

Thank you very much!

1 Like
11

That sounds correct to me. It would make no sense to count approval as for / total, because that would effectively make votes to abstain into votes against.

1 Like
12

Thanks for pointing this out. Would it make sense to explicitly specify in the Governance Documentation that the 50% minimum approval is defined as the total of for votes plus against votes?

As @alextnetto.eth originally pointed out, the Tally UI had incorrectly shown the proposal as defeated. Illustrating this example in the documentation could help delegates better understand the nuance of how minimum approval for an onchain vote is calculated.

1 Like
13

Hey @snowdot! :grin: Thanks for asking for clarification on this. Yes, your interpretation is correct. That is how the DAO’s governor contract calculates the success or failure of the proposal. As Nick.eth mentioned, the 50% applying to Yes/No votes is the logical interpretation as well (when compared with others).

The language used to describe this in the documentation could be more explicit to help avoid misunderstanding. As @estmcmxci alluded to, we are planning to propose some alternative or additional language in the near future that explains this more clearly.

2 Likes
14

Hi there! ICYMI, ThirdGuard is now providing audit reports. The latest one for PUR #4 can be found here. I tried my best to audit on my own, but it’s a bit over my head :sweat_smile:. I used the audit report as a sort of walkthrough to help me cross-reference permissions updates.

1 Like