implement feedback on motivation, change structure, iterate on refere…

…nce implementation

EIPS/eip-7484.md

@@ -1,6 +1,6 @@
 ---
 eip: 7484
-title: Registry Adapters for Smart Accounts
+title: Module Registries and Adapters for Smart Accounts
 description: Adapters that allow modular smart contract accounts to verify the security of modules using a Module Registry
 author: Konrad Kopp (@kopy-kat), zeroknots (@zeroknots)
 discussions-to: https://ethereum-magicians.org/t/erc-7484-registry-adapters-for-smart-accounts/15434
@@ -13,15 +13,18 @@ requires: 4337
 
 ## Abstract
 
-This proposal standardises a Registry Adapter for modular smart contract accounts. This Adapter allows the account to query and verify security attestations about a module through an Attestation Registry. The adapter is responsible for querying the registry and correctly handling the return values.
+This proposal standardises the interface and functionality of Module Registries, allowing modular smart contract accounts to verify the security of modules using a Registry Adapter. It also provides a reference implementation of a Singleton Module Registry.
 
 ## Motivation
 
-[ERC-4337](./eip-4337.md) standardises the execution flow of contract accounts and [ERC-6900](./eip-6900.md) aims to standardise the modular implementation of these accounts, allowing any developer to build modules for these modular accounts (hereafter smart accounts). However, adding third-party modules into smart accounts unchecked opens up a wide range of attack vectors on these accounts.
+[ERC-4337](./eip-4337.md) standardises the execution flow of contract accounts and other efforts aim to standardise the modular implementation of these accounts, allowing any developer to build modules for these modular accounts (hereafter Smart Accounts). However, adding third-party modules into Smart Accounts unchecked opens up a wide range of attack vectors.
 
-A proposed solution to these security considerations is a permission framework that allows module developers to define the permissions that their modules require. The counterpiece of these permissions are attestations that assert statements about the security of modules and can be queried onchain.
+One solution to this security issue is to create a Module Registry that stores security attestations on Modules and allows Smart Accounts to query these attestations before using a module. This standard aims to achieve two things:
 
-This proposal is independent of the exact implementation of the permissioning system and could even be used across different kinds of permissioning systems. Instead, the goal of this proposal is to outline a standard, but flexible, way to create onchain attestations and standardise how these attestations are queried by the smart account during module installation or execution.
+1. Standardise the interface and required functionality of Module Registries.
+2. Standardise the functionality of Adapters that allow Smart Accounts to query Module Registries.
+
+This ensures that Smart Accounts can securely query Module Registries and handle the Registry behavior correctly, irrespective of their architecture, execution flows and security assumptions. This standard also provides a reference implementation of a Singleton Module Registry that is ownerless and can be used by any Smart Account. While we see many benefits of the entire ecosystem using this single Module Registry (see `Rationale`), we acknowledge that there are tradeoffs to using a singleton and thus this standard does not require Smart Accounts to use the reference implementation. Hence, this standard ensures that Smart Accounts can query any Module Registry that implements the required interface and functionality, reducing integration overhead and ensuring interoperability for Smart Accounts.
 
 ## Specification
 
@@ -32,91 +35,76 @@ The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT", "SHOULD", "S
 - **Smart account** - An ERC-4337 compliant smart contract account that has a modular architecture.
 - **Module** - Self-contained smart account functionality.
 - **Attestation** - Onchain assertions made about the security of a module.
-- **Registry** - An onchain list of attestations about modules.
+- **Attester** - The entity that makes an attestation about a module.
+- **(Module) Registry** - A contract that stores an onchain list of attestations about modules.
 - **Adapter** - Smart account functionality that handles the fetching and validation of attestations from the Registry.
-- **Attester** - An entity that makes an attestation about a module.
 
-### Overview
+### Required Registry functionality
 
-An Adapter is a piece of functionality that allows smart accounts to query the Module Registry before using a module. In order to be compatible with this proposal, a smart account MUST implement the adapter either natively in the account or as a default module that is installed in the same transaction as the account deployment.
+There are 2 separate required Registry methods: `check` and `checkN`
 
-The Adapter:
+- `check` is used to check the attestation on a module by a single attester.
+- `checkN` is used to check the attestation on a module by multiple attesters.
 
-- MUST query the Registry about module `A` at least once before or during the transaction in which `A` is called for the first time.
-- MUST treat the Registry reverting as a security risk and SHOULD revert the transaction.
-- SHOULD query the Registry about module `A` on installation of `A`.
-- SHOULD query the Registry about module `A` on execution of `A`.
+The core interface for a Registry is as follows:
 
-Example: Adapter flow using `check`
-![Adapter flow using check()](../assets/eip-7484/check-sequence.jpg)
+```solidity
+interface IRegistry {
+  function check(address module, address attester) public view returns (uint256);
 
-### Interfaces
+  function checkN(address module, address[] memory attesters, uint256 threshold) external view returns (uint256[] memory);
+}
+```
 
-#### IRegistry.sol
+The Registry MUST implement the following functionality:
 
-##### `check`
+- Verify that an attester is the creator of an attestation, for example by checking `msg.sender` or by using signatures, before storing it.
+- Allow attesters to revoke attestations that they have made.
+- Store either the attestation data or a reference to the attestation data.
+- Implement `check` and `checkN` as specified below.
 
-Checks the attestation of a given module address and attestations made by a single attester.
+The Registry SHOULD implement the following additional functionality:
 
-Returns two timestamps: `listedAt` is a `uint48` timestamp of when the attestation was made and `revokedAt` is a `uint48` timestamp of when the attestation was revoked.
-Revocations MUST be permanent, meaning that the value of `revokedAt` can only be changed once, from 0 to a value greater than 0.
+- Allow attesters to specify an expiry date for their attestations and revert during `check` or `checkN` if an attestation is expired.
+- Implement a view function that allows an adapter or offchain client to read the data for a specific attestation.
 
-**NOTE**:
+#### `check`
 
-The Registry:
+Takes two arguments: `module` and `attester`.
 
-- MUST revert if `listedAt == 0` (meaning that the queried attester has not made an attestation on the queried module).
-- MUST revert if `revokedAt > 0`.
+- The Registry MUST revert if the `attester` has not made an attestation on the `module`.
+- The Registry MUST revert if the `attester` has revoked their attestation on the `module`.
 
-The Adapter:
+Returns a `uint256` of the timestamp at which the attestation was created.
 
-- MAY evaluate `listedAt` for vendor specific attestation age criteria.
+#### `checkN`
 
-```solidity
-function check(
-        address module,
-        address attester
-    )
-        public
-        view
-        returns (uint48 listedAt, uint48 revokedAt);
-```
+Takes three arguments: `module`, `attesters` and `threshold`.
 
-Note: Timestamps in solidity are natively `uint256`, but storing the attestation timestamp in `uint48` allows for gas savings (by packing variables) while still supporting sufficiently long timestamps.
+Note: `threshold` may be 0.
 
-##### `verify`
+- The Registry MUST revert if the number of `attesters` that have made an attestation on the `module` is smaller than the `threshold`.
+- The Registry MUST revert if any `attester` has revoked their attestation on the `module`.
 
-Verifies the attestations of a given module and multiple attesters. Additionally a `threshold` can be provided.
+Returns an array of `uint256` of the timestamps at which the attestation by each queried attester was created.
 
-The `verify` function returns true only if `N >= threshold`, where `N` is the number of attesters that attested to the queried module, and no provided attester has revoked an attestation on the module.
+Note: The return values of `check` and `checkN` might change in the future, based on community feedback and further exploration of Registries and Adapters.
 
-More concisely, the three conditions for `verify` returning are:
+### Adapter behavior
 
-- for `N` attesters `listedAt > 0`,
-- `N >= threshold` and
-- for all attesters `revokedAt == 0`.
+A Smart Account MUST implement the following Adapter functionality either natively in the account or as a module. This Adapter functionality MUST ensure that:
 
-**NOTE**:
+- The Registry is queried about module `A` at least once before or during the transaction in which `A` is called for the first time.
+- The Registry reverting is treated as a security risk.
 
-The Registry:
+Additionally, the Adapter SHOULD implement the following functionality:
 
-- MUST revert if `N < theshold`.
-- MUST revert if any of the provided `attesters` has revoked their attestation.
+- Revert the transaction flow when the Registry reverts.
+- Query the Registry about module `A` on installation of `A`.
+- Query the Registry about module `A` on execution of `A`.
 
-```solidity
-function verify(
-        address module,
-        address[] memory attesters,
-        uint256 threshold
-    )
-        external
-        view
-        returns;
-```
-
-### Additional registries
-
-The Reference Implementation Registry below is designed to be a singleton that is a public good, maximally flexible and gas efficient (see `Rationale`). While it is NOT RECOMMENDED to use a custom Registry, there might still exist cases in which the benefits of creating a custom Registry outweigh the downsides. In this case, the Registry MUST implement the interface above in order to be compatible with Adapters. Further, the alternative registry SHOULD also exposes a function that allows an Adapter to read the entire data of an attestation (see `findAttestation` in `Reference Implementation`).
+Example: Adapter flow using `check`
+![Adapter flow using check()](../assets/eip-7484/check-sequence.jpg)
 
 ## Rationale
 
@@ -126,11 +114,11 @@ Attestations are onchain assertions made about a module. These assertions could
 
 One example of this would be determining what storage slots a specific module can write to, which might be useful if a smart account uses DELEGATECALL to invoke the module. This assertion is practically infeasible to verify onchain, but can easily be verified off-chain. Thus, an attester could perform this check off-chain and publish an attestation onchain that attests to the fact that a given module can only write to its designated storage slots.
 
-While attestations are always certain kinds of assertions made about a module, this proposal purposefully allows the attestation data to be any kind of data, packed into a `bytes` object (see `Reference Implementation`). This ensures that any kind of data can be used as an assertion, from a simple boolean flag specifying that a module is secure to a complex proof of runtime module behaviour.
+While attestations are always certain kinds of assertions made about a module, this proposal purposefully allows the attestation data to be any kind of data or pointer to data. This ensures that any kind of data can be used as an assertion, from a simple boolean flag specifying that a module is secure to a complex proof of runtime module behaviour.
 
-### Registry
+### Singleton Registry
 
-In order for attestations to be queryable onchain, they need to be stored in some sort of list in a smart contract. This proposal includes the reference implementation of a Singleton Registry that functions as the source of truth for attestations. This proposed Registry is a public good that is permissionless, ownerless and immutable.
+In order for attestations to be queryable onchain, they need to be stored in some sort of list in a smart contract. This proposal includes the reference implementation of an ownerless Singleton Registry that functions as the source of truth for attestations.
 
 The reasons for proposing a Singleton Registry are the following:
 
@@ -142,13 +130,9 @@ However, there are obviously tradeoffs for using a singleton. A Singleton Regist
 
 Due to being a singleton, the Registry needs to be very flexible and thus likely less computationally efficient in comparison to a narrow, optimised Registry. This means that querying a Singleton Registry is likely to be more computationally (and by extension gas) intensive than querying a more narrow Registry. The tradeoff here is that a singleton makes it cheaper to query attestations from multiple parties simultaneously. So, depending on the Registry architectures, there is an amount of attestations to query (N) after which using a flexible singleton is actually computationally cheaper than querying N narrow registries. However, the reference implementation has also been designed with gas usage in mind and it is unlikely that specialised registries will be able to significantly decrease gas beyond the reference implementations benchmarks.
 
-### Adapter
-
-In order for smart accounts to increase security guarantees when adding modules, they must be able to securely query the Module Registry and handle the return data correctly. In order to achieve this, this proposal aims to provide a standardised interface that may be implemented by smart accounts irrespective of their architecture, execution flows and security assumptions.
-
 ### Related work
 
-The reference implementation of the Registry is heavily inspired by the Ethereum Attestation Service. The specific use-case of this proposal, however, required some custom modifications and additions to EAS, meaning that using the existing EAS contracts as the Module Registry was sub-optimal.
+The reference implementation of the Registry is heavily inspired by the Ethereum Attestation Service. The specific use-case of this proposal, however, required some custom modifications and additions to EAS, meaning that using the existing EAS contracts as the Module Registry was sub-optimal. However, it would be possible to use EAS as a Module Registry with some modifications.
 
 ## Backwards Compatibility
 
@@ -163,19 +147,13 @@ contract Adapter {
     IRegistry registry;
 
     function checkModule(address module, address trustedAttester) internal {
-       // check module implementation address on registry
-        (uint48 listedAt, uint48 flaggedAt) = registry.check(module, trustedAttester);
-
-        // revert if module was ever flagged or was never attested to
-        require(listedAt != 0 && flaggedAt == 0, "Module is insecure");
+        // Check module attestation on Registry
+        registry.check(module, trustedAttester);
     }
 
-    function verifyModule(address module, address[] memory attesters, uint256 threshold) internal {
-        // check module implementation address on registry
-        bool verified = registry.verify(module, attesters, threshold);
-
-        // revert if module is not verified by enough attesters
-        require(verified, "Module is insecure");
+    function checkNModule(address module, address[] memory attesters, uint256 threshold) internal {
+        // Check list of module attestations on Registry
+        registry.checkN(module, attesters, threshold);
     }
 }
 ```
@@ -196,7 +174,7 @@ contract Account is Adapter {
 
     // executes a module
     function executeTransactionFromModule(address module, address[] memory attesters, uint256 threshold) public {
-        verifyModule(module, attesters, threshold);
+        checkNModule(module, attesters, threshold);
         ...
     }
 
@@ -206,7 +184,84 @@ contract Account is Adapter {
 
 ### Registry
 
-See `https://github.com/rhinestonewtf/registry/`
+```solidity
+contract Registry {
+    ...
+
+    function check(
+        address module,
+        address attester
+    )
+        public
+        view
+        returns (uint256)
+    {
+        AttestationRecord storage attestation = _getAttestation(module, attester);
+
+        uint48 expirationTime = attestation.expirationTime;
+        uint48 attestedAt =
+            expirationTime != 0 && expirationTime < block.timestamp ? 0 : attestation.time;
+        if (attestedAt == 0) revert AttestationNotFound();
+
+        uint48 revokedAt = attestation.revocationTime;
+        if (revokedAt != 0) revert RevokedAttestation(attestation.attester);
+
+        return uint256(attestedAt);
+    }
+
+    function checkN(
+        address module,
+        address[] calldata attesters,
+        uint256 threshold
+    )
+        external
+        view
+        returns (uint256[] memory)
+    {
+        uint256 attestersLength = attesters.length;
+        if (attestersLength < threshold || threshold == 0) {
+            threshold = attestersLength;
+        }
+
+        uint256 timeNow = block.timestamp;
+        uint256[] memory attestedAtArray = new uint256[](attestersLength);
+
+        for (uint256 i; i < attestersLength; i = uncheckedInc(i)) {
+            AttestationRecord storage attestation = _getAttestation(module, attesters[i]);
+            if (attestation.revocationTime != 0) {
+                revert RevokedAttestation(attestation.attester);
+            }
+
+            uint48 expirationTime = attestation.expirationTime;
+            if (expirationTime != 0 && expirationTime < timeNow) {
+                revert AttestationNotFound();
+            }
+
+            attestedAtArray[i] = uint256(attestation.time);
+
+            if (attestation.time == 0) continue;
+
+            if (threshold != 0) --threshold;
+        }
+        if (threshold == 0) return attestedAtArray;
+        revert InsufficientAttestations();
+    }
+
+    function _getAttestation(
+        address module,
+        address attester
+    )
+        internal
+        view
+        virtual
+        returns (AttestationRecord storage)
+    {
+        return _moduleToAttesterToAttestations[module][attester];
+    }
+
+    ...
+}
+```
 
 ## Security Considerations