Plugin design

[JohnGuilding]

There are a few issues with are current Safe plugin design. There are also some small inconsistencies between different plugins. And our design also differs from the canonical Safe 4337 plugin.

The purpose of this issue to is to:

1. Document the issues with the current design and inconsistencies between our plugins
2. Document differences between our plugins and the canonical Safe 4337 module
3. Based on the above findings, outline requirements for plugins and next steps

[JohnGuilding]

Safe plugin design

Issues with the current design

1. Cannot deploy account and send the first userOp in a single step. There are a few things going on here.

On deployment, the plugin appears to be treated as an unstated-entity. Here is a test that verifies this by trying to deploy a Safe account and send a user op via the ecdsa plugin in a single step:

wax/account-integrations/safe/test/hardhat/SafeECDSAPlugin.test.ts

Line 130 in f05ebe5

it("should fail to deploy account via initcode & send a user operation at the same time.", async () => {

The relevant lines are:

• Generating initcode before sending userOp:

  wax/account-integrations/safe/test/hardhat/SafeECDSAPlugin.test.ts

  Line 158 in f05ebe5

  const { initCode, deployedAddress } = await generateInitCodeAndAddress(

• Error message:

  wax/account-integrations/safe/test/hardhat/SafeECDSAPlugin.test.ts

  Line 195 in f05ebe5

  "unstaked factory accessed",

According the ERC, unstaked entities CAN be used if:

If the UserOp doesn’t create a new account (that is initCode is empty), or the UserOp creates a new account using a staked factory contract, then the entity may also use storage associated with the sender.

So because the userOp is creating a new account, we break the above rule. The rule does state another way around this is to use a staked factory contract. This could be one option for overcoming the issue. Although it’s probably too cost-prohibitive as the minimum stake value is meant to be equivalent to ~$1000 in native tokens. If the account is already deployed, then we can access state no problem. Note: when we say account here, we mean the Safe account. We are also assuming the plugin has already been deployed.

The canonical Safe 4337 module gets around this problem because it does not rely on module state during deployment via initCode. It instead uses the state on the Safe account itself and just calls the required functions from the module. Here is a test showing this:

wax/account-integrations/safe/test/hardhat/Safe4337Module.test.ts

Line 31 in a284336

it("should pass the ERC4337 validation with original Safe4337Module.", async () => {

The relevant solidity line of code where the module calls into the Safe when validating signatures:

wax/account-integrations/safe/src/Safe4337Module.sol

Line 189 in a284336

try ISafe(payable(userOp.sender)).checkSignatures(keccak256(operationData), operationData, signatures) {

Here is a test showing that deploying a Safe account and enabling a module with state will fail in doing so in a single userOp:

wax/account-integrations/safe/test/hardhat/Safe4337Module.test.ts

Line 222 in a284336

it("should fail to deploy account via initcode & send a user operation at the same time while accessing state. With Safe4337ModuleWithStorage.", async () => {

The relevant lines are:

• Line of solidity code that causes the test to fail (this function is called when initialising the module):
  

  wax/account-integrations/safe/src/Safe4337ModuleWithStorage.sol

  Line 102 in a284336

  ecdsaOwnerStorage[msg.sender].owner = owner;

• Error message:
  

  wax/account-integrations/safe/test/hardhat/Safe4337Module.test.ts

  Line 297 in a284336

  "unstaked factory accessed",

Here is a test showing that you can use state in the plugin/module if it is used when an account isn’t being deployed (initCode is empty):

wax/account-integrations/safe/test/hardhat/Safe4337Module.test.ts

Line 305 in a284336

it("should deploy account outside of 4337 & send a user operation with no initCode while accessing state. With Safe4337ModuleWithStorage.", async () => {

This is basically the same code as the above example but the Safe is deployed outside of 4337 via the Safe proxy factory. This flow is pretty similar to how are current plugins work (deploy account via regular transaction, then send userOp with no initCode), but with a modified Safe 4337 module.

You might see the following comment in some of the plugin tests:

  // Note: initCode is not used because we need to create both the saf
  // proxy and the plugin, and 4337 currently only allows one contract
  // creation in this step. Since we need an extra step anyway, it's simpler
  // to do the whole create outside of 4337.

I wasn’t sure exactly what rules were broken here so wrote some tests to verify this behaviour:

• Multiple CREATE2 opcodes
  

  wax/account-integrations/safe/test/hardhat/SafeECDSAPluginStateless.test.ts

  Line 100 in a284336

  it("should fail ERC4337 validation because there are multiple 'CREATE2' opcodes in initcode", async () => {

  • 4337 rule: “If op.initcode.length != 0 , allow only one CREATE2 opcode call (in the first (deployment) block), otherwise forbid CREATE2.”
• Use of banned CREATE opcode

  wax/account-integrations/safe/test/hardhat/SafeECDSAPluginStateless.test.ts

  Line 180 in a284336

  it("should fail ERC4337 validation because the banned opcode 'CREATE' is used", async () => {

  • 4337 rule: CREATE opcode is banned

2. Cannot easily use multiple 4337-compatible modules with the current design

This is because the 4337 plugins are also fallback handlers - also an issue with canonical Safe 4447 module. This limitation can be overcome by a routing fallback handler. See https://ethereum.stackexchange.com/questions/157043/is-there-a-recommended-pattern-to-add-multiple-4337-compatible-modules-to-a-safe

Inconsistencies between WAX plugins

• Some of the plugins use factories and some don't. If we are using singletons for the plugin contracts, factories probably aren't justified. If the plugin contracts are not singletons, then factories make more sense. Account creation should still involve a factory, but that is at the level of the Safe account (not the plugin).

Design requirements

Ideal functional requirements for design

There are many requirements for the design of the Safe modules. The following section summarises the key functional requirements in order for the safe account with plugins to behave like regular erc 4337 accounts.

1. Can generate an address locally and start accepting funds
2. Account creation is done by a “factory” contract using create2
3. Can deploy account and send user op in one step. i.e. it's possible to use initCode

WAX Safe plugin design vs official safe 4337 plugin design

The canonical Safe 4337 module is an audited 4337-compatible Safe module that can be used with any Safe using V1.4.1 or newer. The key differences bewteen this module and out plugins are:

1. The Safe 4337 module is a singleton AND it is “stateless”. I’m using stateless in quotation marks here as there is state, but it only consists of constant and immutable variables. This is relevant for difference number 2.
2. The Safe 4337 module can be used so that a user can deploy a Safe account, set the 4337 module up, and send the first user operation all in one step. If the module contained state such as an owner address, this would not be possible as it breaks a 4337 rule.
3. Has an additional execute function that returns an error reason - executeUserOpWithErrorString()
4. Uses _packValidationData with validAfter and validUntil.
5. Inherites CompatibilityFallbackHandler which provides compatibility between pre 1.3.0 and 1.3.0+ Safe Smart Account contracts, and also has callback handlers for receiving tokens e.g. ERC721. Our plugins should probably have this too.
6. Computes its own “Safe operation” hash for signature verification. This is used instead of the userOp hash generated by the EntryPoint. This is because apparently, the 4337 userOp hash is not compatible with EIP-712. This then means that users have to trust dapps to correctly show the user operation information that they should sign with their Safe. So this is done to ensure Safe users can always see what they are signing. Source https://safe.mirror.xyz/4lLUIWD30hfnLcobt28O60Eh0xbggkXwaf-I-VWv-JA

	WAX Safe plugins	Canonical Safe 4337 module
Can generate an address locally and start accepting funds	✅	✅
Account creation is done by a “factory” contract using create2	✅	✅
Can deploy account and send user op in one step (can use initCode)	❌	✅
Can use multiple 4337-compatible modules	❌	❌

Next steps/Questions

• Decide how we want to approach handling multiple 4337-compatible plugins. Most likely option is we write a routing fallback handler. We can take inspiration from CoW swaps’ extensible fallback handler
  • Based off of this, validateUserOp should be supported in each 4337-compatible module.
• Add functionality from CompatibilityFallbackHandler
• Consider _packValidationData to pack validAfter and validUntil
• Decide on using singletons or non-singletons for plugins. Delete/add factories based on this decision
• Decide whether to use EIP712 typed hash for signature verification
• Add disable module functionality
• Add Natspec comments
• Add interfaces
• Should/can we support batched transactions? e.g. executeBatch function as well as an execute function
• A few helper functions we may want to consider that other module contracts implement:
  • Helper function to check if module has been enabled for a specific wallet
  • ERC-1271 support. Safe supports ERC-1271, but how would things work if you wanted to sign something with a signer on one of your modules? Seen 1271 supported in some validator modules
• The enable() function was pretty much copied from kernel, we should double check we’re happy with this code
• Ensure events and errors are sufficient
• Once feature complete, we should ensure the code is well tested. Regarding security, here is a helpful resource with a 4337-specific audit checklist and links to project audits https://github.com/aviggiano/security/blob/main/audit-checklists/ERC-4337.md
• Do we distinguish between validator modules and executor modules?
• Consider ERC-165

ERC rules reference

Checks the account must do:

Under section on specification https://eips.ethereum.org/EIPS/eip-4337#specification

• MUST validate the caller is a trusted EntryPoint
• If the account does not support signature aggregation, it MUST validate the signature is a valid signature of the userOpHash, and SHOULD return SIG_VALIDATION_FAILED (and not revert) on signature mismatch. Any other error should revert.
• MUST pay the entryPoint (caller) at least the “missingAccountFunds” (which might be zero, in case current account’s deposit is high enough)
• The return value MUST be packed of authorizer, validUntil and validAfter timestamps.
  • authorizer - 0 for valid signature, 1 to mark signature failure. Otherwise, an address of an authorizer contract. This ERC defines “signature aggregator” as authorizer.
  • validUntil is 6-byte timestamp value, or zero for “infinite”. The UserOp is valid only up to this time.
  • validAfter is 6-byte timestamp. The UserOp is valid only after this time.

Rules relevant for signature aggregation:

Under section on specification https://eips.ethereum.org/EIPS/eip-4337#specification

• An account that works with aggregated signature, should return its signature aggregator address in the “sigAuthorizer” return value of validateUserOp. It MAY ignore the signature field
• If an account uses an aggregator (returns it from validateUserOp), then its address is returned by simulateValidation() reverting with ValidationResultWithAggregator instead of ValidationResult
• To accept the UserOp, the bundler must call validateUserOpSignature() to validate the userOp’s signature.
• aggregateSignatures() must aggregate all UserOp signature into a single value.
• Note that the above methods are helper method for the bundler. The bundler MAY use a native library to perform the same validation and aggregation logic.
• validateSignatures() MUST validate the aggregated signature matches for all UserOperations in the array, and revert otherwise. This method is called on-chain by handleOps()
• Signature aggregator SHOULD stake just like a paymaster, unless it is exempt due to not accessing global storage

Storage access rules under simulation:

Under section on simulation rationale https://eips.ethereum.org/EIPS/eip-4337#specification-1

Storage access is limited as follows:

1. self storage (of factory/paymaster, respectively) is allowed, but only if self entity is staked
2. account storage access is allowed (see Storage access by Slots, below),
3. 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)

When un-staked entities can be used

Under section un-staked entities https://eips.ethereum.org/EIPS/eip-4337#un-staked-entities

Under the following special conditions, unstaked entities still can be used:

• An entity that doesn’t use any storage at all, or only the senders’s storage (not the entity’s storage - that does require a stake)
• If the UserOp doesn’t create a new account (that is initCode is empty), or the UserOp creates a new account using a staked factory contract, then the entity may also use storage associated with the sender)
• A paymaster that has a “postOp()” method (that is, validatePaymasterUserOp returns “context”) must be staked

Behaviour and rules for first-time account creation

Under section on first-time account creation https://eips.ethereum.org/EIPS/eip-4337#first-time-account-creation

• It is an important design goal of this proposal to replicate the key property of EOAs that users do not need to perform some custom action or rely on an existing user to create their wallet; they can simply generate an address locally and immediately start accepting funds.
• The wallet creation itself is done by a “factory” contract, with wallet-specific data. The factory is expected to use CREATE2 (not CREATE) to create the wallet, so that the order of creation of wallets doesn’t interfere with the generated addresses. The initCode field (if non-zero length) is parsed as a 20-byte address, followed by “calldata” to pass to this address.

[JohnGuilding]

Follow up issue that captures next steps/questions #208

Here is the branch where the aforementioned testing was done https://github.com/getwax/wax/tree/safe-plugin-design-spike