From 1ffa8793ce16b096bed4be20b4370f2d9aa60aed Mon Sep 17 00:00:00 2001 From: Akshay Date: Thu, 26 Oct 2023 13:39:36 +0200 Subject: [PATCH 1/3] Document ERC-4337 specifics and learnings --- manager/README.md | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/manager/README.md b/manager/README.md index 5c45981..e3a456c 100644 --- a/manager/README.md +++ b/manager/README.md @@ -104,3 +104,38 @@ TBD ## Upgradeability It is inevitable that more features will be added to Safe{Core} Protocol (e.g. new modules). As the Manager is the central part of this setup, it is important to consider a path for integrating these new features. Using an upgradeable proxy for the Manager would introduce unacceptable security concerns. Separating too much of the functionality into separate contract to allow reusability (i.e. the list of enabled integration) would increase the gas costs, and so is also not practical. A better pattern is to allow new versions of the Manager to load information from a previous version and thereby facilitate a migration. + +## ERC-4337 compatibility + +In its current state, the Safe{Core} Protocol specification is not compatible with [ERC-4337](https://eips.ethereum.org/EIPS/eip-4337). The reason being that ERC-4337 enforces rules on the the read/write operations that are not compatible with the Safe{Core} Protocol. + +As per ERC-4337 specs state the following when simulating a `UserOp` : + +``` +Storage access is limited as follows: +- self storage (of factory/paymaster, respectively) is allowed, but only if self entity is staked +- account storage access is allowed (see Storage access by Slots, below), +- in any case, may not use storage used by another UserOp sender in the same bundle (that is, paymaster and factory are not allowed as senders + +Storage associated with an account is defined as follows: + +An address A is associated with: +- Slots of contract A address itself. +- Slot A on any other address. +- Slots of type keccak256(A || X) + n on any other address. (to cover mapping(address => value), which is usually used for balance in ERC-20 tokens). n is an offset value up to 128, to allow accessing fields in the format mapping(address => struct) +``` + +Whereas, the Protocol Manager contract specifies following operations that make use of storage slots that are not allowed by ERC-4337: + +- Read the registry address from storage slot not associated with the account + - When executing transaction from Plugin + - When hooks are enabled + - When validating signature +- Registry contract reads the storage associated with module address + +Potential solutions: + +- Avoid registry storage reads +- Adjust data structures to avoid reading storage not associated with account address + +More details on this is available here: https://github.com/safe-global/safe-core-protocol/issues/60#issuecomment-1761296305 \ No newline at end of file From d0dfcd7248e2109a002797239d3135b76e45b850 Mon Sep 17 00:00:00 2001 From: Akshay Date: Thu, 26 Oct 2023 15:51:18 +0200 Subject: [PATCH 2/3] Add note of what developers need to be aware for developing 4337 plugins --- manager/README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/manager/README.md b/manager/README.md index e3a456c..3f76102 100644 --- a/manager/README.md +++ b/manager/README.md @@ -138,4 +138,6 @@ Potential solutions: - Avoid registry storage reads - Adjust data structures to avoid reading storage not associated with account address +Developers aiming to develop plugins that are ERC-4337 compatible should be aware of storage access restrictions, opcode usage restrictions during the simulation step of ERC-4337 specification. + More details on this is available here: https://github.com/safe-global/safe-core-protocol/issues/60#issuecomment-1761296305 \ No newline at end of file From a7ae1ac3d129098accc47b0f03482d01f8e8233d Mon Sep 17 00:00:00 2001 From: Akshay Date: Fri, 27 Oct 2023 10:57:37 +0200 Subject: [PATCH 3/3] Update ERC-4337 compatibility docs --- manager/README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/manager/README.md b/manager/README.md index 3f76102..3203fce 100644 --- a/manager/README.md +++ b/manager/README.md @@ -107,7 +107,7 @@ It is inevitable that more features will be added to Safe{Core} Protocol (e.g. n ## ERC-4337 compatibility -In its current state, the Safe{Core} Protocol specification is not compatible with [ERC-4337](https://eips.ethereum.org/EIPS/eip-4337). The reason being that ERC-4337 enforces rules on the the read/write operations that are not compatible with the Safe{Core} Protocol. +Safe{Core} Protocol specification is meant to be compatible with [ERC-4337](https://eips.ethereum.org/EIPS/eip-4337). ERC-4337 enforces rules on the the read/write operations that should be carefully looked into for the Safe{Core} Protocol implementation to be compatible with ERC-4337. As per ERC-4337 specs state the following when simulating a `UserOp` : @@ -125,7 +125,7 @@ An address A is associated with: - Slots of type keccak256(A || X) + n on any other address. (to cover mapping(address => value), which is usually used for balance in ERC-20 tokens). n is an offset value up to 128, to allow accessing fields in the format mapping(address => struct) ``` -Whereas, the Protocol Manager contract specifies following operations that make use of storage slots that are not allowed by ERC-4337: +The current Protocol Manager implementations specifies following operations that make use of storage slots that are not allowed by ERC-4337: - Read the registry address from storage slot not associated with the account - When executing transaction from Plugin