gnosisguild / zodiac-modifier-roles

Smart account toolkit for role-based access control
https://roles.gnosisguild.org
GNU Lesser General Public License v3.0
80 stars 39 forks source link

Standardizable conditions storage layout #189

Closed jfschwarz closed 1 year ago

jfschwarz commented 1 year ago

This PR is not meant to be merged

The branch is a POC to demonstrate what's necessary to bring our implementation in compliance with the proposed standard. It also has some renames, leading to a huge diff. Don't bother reviewing.

While I think the changes are completely merge-ready, we can just discard this branch and work in the changes from a fresh base, if we decide we wanna do this.


Let's consider standardizing our design for defining conditions on ABI encoded data as an EIP in the future!

The problem seems to be generic enough to justify a well designed standard solution. In particular, I'd consider our design of the storage-optimized, packed representation of the conditions tree to make a pretty good candidate for standardization.

The storage layout design is already close to perfection, in preparation for standardization we should just apply some final touches:

Boolean expression completeness

While we don't currently see use cases for additional operators, I feel we should be prepared for these to come up in the future.

For the sake of boolean expression completeness, I propose to add two new operators:

enum Operator (former name: Comparison)

We should consider giving an extra bit to operator. We're already using 13 of the currently possible 16 values. The proposed 2 extra boolean operators bring us pretty close to the limit.

Also we should design for extensibility (future requirements for new operators).

enum Operator {
    // 00:    EMPTY EXPRESSION (default, always passes)
    //          paramType: Static / Dynamic
    //          🚫 children
    //          🚫 compValue
    /* 00: */ Whatever,
    // ------------------------------------------------------------
    // 01-04: BOOLEAN EXPRESSIONS
    //          paramType: None
    //          ✅ children
    //          🚫 compValue
    /* 01: */ And,
    /* 02: */ Or,
    /* 03: */ Xor,
    /* 04: */ Not,
    // ------------------------------------------------------------
    // 05-16: COMPLEX TYPE EXPRESSIONS
    //          paramType: AbiEncoded / Tuple / Array,
    //          ✅ children
    //          🚫 compValue
    /* 05: */ Matches,
    /* 06: */ ArraySome,
    /* 07: */ ArrayEvery,
    /* 08: */ ArraySubsetOf,
    /* 09: */ _ComplexPlaceholder09,
    /* 10: */ _ComplexPlaceholder10,
    /* 11: */ _ComplexPlaceholder11,
    /* 12: */ _ComplexPlaceholder12,
    /* 13: */ _ComplexPlaceholder13,
    /* 14: */ _ComplexPlaceholder14,
    /* 15: */ _ComplexPlaceholder15,
    /* 16: */ _ComplexPlaceholder16,
    // ------------------------------------------------------------
    // 17-31: BINARY COMPARISON EXPRESSIONS
    //          paramType: Static / Dynamic / AbiEncoded / Tuple / Array 
    //          🚫 children
    //          ✅ compValue
    /* 17: */ EqualTo,
    /* 18: */ GreaterThan,
    /* 19: */ LessThan,
    /* 20: */ Bitmask,
    /* 21: */ _BinaryPlaceholder21,
    /* 22: */ _BinaryPlaceholder22,
    /* 23: */ _BinaryPlaceholder23,
    /* 24: */ _BinaryPlaceholder24,
    /* 25: */ _BinaryPlaceholder25,
    /* 26: */ _BinaryPlaceholder26,
    /* 27: */ _BinaryPlaceholder27,
    /* 28: */ _BinaryPlaceholder28,
    /* 29: */ WithinAllowance,
    /* 30: */ EthWithinAllowance,
    /* 31: */ CallWithinAllowance
}

The WithinAllowance expressions would be custom to the Roles mod and should live in the binary expression placeholder range 21..31. They don't fit into a standard, because they are not pure functions of the ABI encoded calldata but rely on data from external sources (allowances, ether value).

Packed storage layout

The packed representation of a condition (also taking into account point O6 from #190) would have this form, keeping its original size of 16 bits:

    <8 bits : parent>
    <3 bits : paramType>
    <5 bits : comparison>

The compValue words are following the conditions list in the buffer just like we already have it.

Why standardize?

The benefit of a standardized storage format would be that equivalent condition trees will map to identical create2 addresses. This means any condition tree would only ever have to be stored on chain once and could then be used by anyone.

Benefits for Roles mod

Even if we don't pursue proposing a standard, the design changes will improve extensibility of our conditions implementation:

vercel[bot] commented 1 year ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
zodiac-roles-app ❌ Failed (Inspect) Mar 17, 2023 at 1:28PM (UTC)
jfschwarz commented 1 year ago

proposal has been taken into account