Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Evaluate Nested Safe Calling Convention with ERC-4337 #251

Closed
nlordell opened this issue Feb 5, 2024 · 4 comments · Fixed by #273
Closed

Evaluate Nested Safe Calling Convention with ERC-4337 #251

nlordell opened this issue Feb 5, 2024 · 4 comments · Fixed by #273
Assignees

Comments

@nlordell
Copy link
Collaborator

nlordell commented Feb 5, 2024

Context / issue

Nested Safe ownership structures are problematic in the context of 4337 user operations because of storage access restrictions. However, it should technically possible to accomplish at a UX cost, by creating a signing scheme that allows for a "leaf" Safe to execute the user operation, instead of the "root" Safe.

Proposed solutions

Specifically, imagine an acyclic Safe ownership tree, where we want to execute a transaction with the root. The idea would be to pick a path to a "leaf" Safe (that is, a Safe with only EOA owners). Since we assume an acyclic Safe ownership tree, this must always be possible. The idea would then be to have the "leaf" Safe sign a user operation to execute parent.execTransaction(parent.execTransaction(...(root.execTransaction(payload)))) on its parent and make use of "approved hash from executor" signatures (i.e. v = 1). All other Safes/owners sign regular SafeTransactions (i.e. NOT 4337 user operation signature) for executing the root transaction.

The problem with this scheme is that the UX is not great:

  • You only know what signatures are needed once you know the signers, so signature collection is super messy (for example, imagine a 2 of 3 Safe where two owners are EOAs and one is a Safe, you can't know what kind of signatures to collect without knowing who the signers will be).
  • The user operation would appears as if it was from the "leaf" Safe in "4337-user-operation-explorers".

Alternatives

Expected Outcome

A PoC for nested Safe user operation execution using the proposed solution.

@nlordell nlordell changed the title Support Nested Safe Structure with ERC-4337 Evaluate Nested Safe Structure with ERC-4337 Feb 15, 2024
@nlordell nlordell changed the title Evaluate Nested Safe Structure with ERC-4337 Evaluate Nested Safe Calling Convention with ERC-4337 Feb 15, 2024
@remedcu
Copy link
Member

remedcu commented Feb 16, 2024

Is the evaluation based on v0.7?

@mmv08
Copy link
Member

mmv08 commented Feb 16, 2024

Image

@mmv08 mmv08 self-assigned this Feb 16, 2024
@mmv08
Copy link
Member

mmv08 commented Feb 19, 2024

Is the evaluation based on v0.7?

Not necessarily. Why? Are there some significant differences we should take into account?

@nlordell
Copy link
Collaborator Author

I don't think it matter much. Might make sense to evaluate it on 0.6 only because the bundler is available there - and then port it to 0.7 once it releases.

@mmv08 mmv08 linked a pull request Feb 29, 2024 that will close this issue
@mmv08 mmv08 closed this as completed in #273 Mar 1, 2024
mmv08 added a commit that referenced this issue Mar 1, 2024
This PR adds an E2E test case for nested Safe transaction execution in
the 4337 Context. The test uses the following ownership structure:

![image](https://github.com/safe-global/safe-modules/assets/16622558/3e420068-3d3b-4db4-91f6-3da4febc350e)

I implemented helper classes representing the Safe ownership tree with
two node types: Safe and EOA. The tests also required functions to find
an execution path and build the user operation from the execution path.
The alternative would've been to do everything imperatively, but using
those functions, we could theoretically represent arbitrary ownership
structure, find an execution path (currently, the function doesn't find
the most optimal one) and generate a user operation.

The current algorithm works as follows (thanks to @nlordell for the
help):
1. **Finding an execution path:** It does a DFS on the tree until it
finds the first Executor node. We consider an executor node a leaf safe
owned by an EOA. In the context of the ownership structure on the
screenshot, the executor would be the leftmost Leaf Safe. The execution
path is represented by an array ordered from the root node to the
executor node.
2. **Building the user operation:** Using the execution path and our
desired `SafeTransaction` (the one executed with `execTransaction`), we
recursively sign the transaction with all the node owners in the
execution path. After that, we re-encode the `SafeTransaction` for the
next node with a call to the preceding node in the execution path and do
the signing again. It stops when we reach the starting node (the node
before the executor 4337 Safe). After that, we encode and send the
SafeTransaction in a User Operation.

- Implements #251 
- I also had to bring quite some code for executing regular Safe
transactions from the `safe-smart-account` repo

I tried to document the code as much as possible, but somehow, I feel it
still wasn't enough - please feel free to point me to the parts that are
unclear, and I'll do my best to explain them with comments.

I'll quote the Slack message from @nlordell that describes the idea
implemented in this PR:

> As you may already be aware, nested Safe ownership structures are
problematic in the context of 4337 user operations because of storage
access restrictions. However, I think it is technically possible to
accomplish at a UX cost.
> Specifically, imagine an acyclic Safe ownership tree, where we want to
execute a transaction with the root. The idea would be to pick a path to
a "leaf" Safe (that is, a Safe with only EOA owners). Since we assume an
acyclic Safe ownership tree, this must always be possible. The idea
would then be to have the "leaf" Safe sign a user operation to execute
parent.execTransaction(parent.execTransaction(...(root.execTransaction(payload))))
on its parent and make use of "approved hash from executor" signatures
(i.e. v = 1). All other Safes/owners sign regular SafeTransactions (i.e.
NOT 4337 user operation signature) for executing the root transaction.
> The problem with this scheme is that the UX is not great:
> - You only know what signatures are needed once you know the signers,
so signature collection is super messy (for example, imagine a 2 of 3
Safe where two owners are EOAs and one is a Safe, you can't know what
kind of signatures to collect without knowing who the signers will be).
There may be a way to work around this by expanding the Safe4337Module
to allow SafeUserOperation signatures to be used for calling
4337-enabled parent Safes, but I would need to think about it a bit
more...
> - The user operation would appears as if it was from the "leaf" Safe
in 4337-user-operation-explorers.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants