From bd7b756452dc31d2fbd305a071093818412ee64c Mon Sep 17 00:00:00 2001 From: Mikhail <16622558+mmv08@users.noreply.github.com> Date: Fri, 1 Mar 2024 10:17:18 +0100 Subject: [PATCH] Add an E2E test for nested Safes in 4337 context (#273) 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. --- modules/4337/contracts/test/SafeMock.sol | 5 +- modules/4337/src/utils/execution.ts | 114 ++++- modules/4337/src/utils/safe.ts | 61 ++- modules/4337/test/e2e/4337NestedSafe.spec.ts | 428 ++++++++++++++++++ .../4337/test/e2e/SingletonSigners.spec.ts | 5 +- .../test/e2e/WebAuthnSingletonSigner.spec.ts | 5 +- .../4337/test/erc4337/ERC4337WebAuthn.spec.ts | 5 +- .../test/erc4337/ReferenceEntryPoint.spec.ts | 5 +- modules/4337/test/utils/e2e.ts | 4 +- 9 files changed, 593 insertions(+), 39 deletions(-) create mode 100644 modules/4337/test/e2e/4337NestedSafe.spec.ts diff --git a/modules/4337/contracts/test/SafeMock.sol b/modules/4337/contracts/test/SafeMock.sol index 0ea4c0d90..69500072e 100644 --- a/modules/4337/contracts/test/SafeMock.sol +++ b/modules/4337/contracts/test/SafeMock.sol @@ -39,7 +39,10 @@ contract SafeMock { bytes32 s; (v, r, s) = _signatureSplit(signature); require( - owner == ecrecover(keccak256(abi.encodePacked("\x19Ethereum Signed Message:\n32", dataHash)), v, r, s), + // The original safe contract requires the V value to be > 30 for EIP-191 signed messages + // The canonical eth V value is 27 or 28, so by adding 4 to it we get a value > 30 + // And when we verify the signature we subtract 4 from it + owner == ecrecover(keccak256(abi.encodePacked("\x19Ethereum Signed Message:\n32", dataHash)), v - 4, r, s), "Invalid signature" ); } diff --git a/modules/4337/src/utils/execution.ts b/modules/4337/src/utils/execution.ts index 6164e6042..945acfc6c 100644 --- a/modules/4337/src/utils/execution.ts +++ b/modules/4337/src/utils/execution.ts @@ -7,6 +7,22 @@ export const EIP_DOMAIN = { ], } +export const EIP712_SAFE_TX_TYPE = { + // "SafeTx(address to,uint256 value,bytes data,uint8 operation,uint256 safeTxGas,uint256 baseGas,uint256 gasPrice,address gasToken,address refundReceiver,uint256 nonce)" + SafeTx: [ + { type: 'address', name: 'to' }, + { type: 'uint256', name: 'value' }, + { type: 'bytes', name: 'data' }, + { type: 'uint8', name: 'operation' }, + { type: 'uint256', name: 'safeTxGas' }, + { type: 'uint256', name: 'baseGas' }, + { type: 'uint256', name: 'gasPrice' }, + { type: 'address', name: 'gasToken' }, + { type: 'address', name: 'refundReceiver' }, + { type: 'uint256', name: 'nonce' }, + ], +} + export const EIP712_SAFE_MESSAGE_TYPE = { // "SafeMessage(bytes message)" SafeMessage: [{ type: 'bytes', name: 'message' }], @@ -28,44 +44,94 @@ export interface SafeTransaction extends MetaTransaction { nonce: string | number } +export interface SignedSafeTransaction extends SafeTransaction { + signatures: SafeSignature[] +} + export interface SafeSignature { signer: string data: string + // a flag to indicate if the signature is a contract signature and the data has to be appended to the dynamic part of signature bytes + dynamic?: true +} + +export const calculateSafeDomainSeparator = (safeAddress: string, chainId: BigNumberish): string => { + return ethers.TypedDataEncoder.hashDomain({ verifyingContract: safeAddress, chainId }) +} + +export const preimageSafeTransactionHash = (safeAddress: string, safeTx: SafeTransaction, chainId: BigNumberish): string => { + return ethers.TypedDataEncoder.encode({ verifyingContract: safeAddress, chainId }, EIP712_SAFE_TX_TYPE, safeTx) +} + +export const calculateSafeTransactionHash = (safeAddress: string, safeTx: SafeTransaction, chainId: BigNumberish): string => { + return ethers.TypedDataEncoder.hash({ verifyingContract: safeAddress, chainId }, EIP712_SAFE_TX_TYPE, safeTx) +} + +export const preimageSafeMessageHash = (safeAddress: string, message: string, chainId: BigNumberish): string => { + return ethers.TypedDataEncoder.encode({ verifyingContract: safeAddress, chainId }, EIP712_SAFE_MESSAGE_TYPE, { message }) +} + +export const calculateSafeMessageHash = (safeAddress: string, message: string, chainId: BigNumberish): string => { + return ethers.TypedDataEncoder.hash({ verifyingContract: safeAddress, chainId }, EIP712_SAFE_MESSAGE_TYPE, { message }) } export const signHash = async (signer: Signer, hash: string): Promise => { const typedDataHash = ethers.getBytes(hash) + const signerAddress = await signer.getAddress() + const signature = await signer.signMessage(typedDataHash) + return { - signer: await signer.getAddress(), - data: await signer.signMessage(typedDataHash), + signer: signerAddress, + data: signature.replace(/1b$/, '1f').replace(/1c$/, '20'), } } -const sortSignatures = (signatures: SafeSignature[]) => { - signatures.sort((left, right) => left.signer.toLowerCase().localeCompare(right.signer.toLowerCase())) +export const getPrevalidatedSignature = (signerAddress: string): SafeSignature => { + return { + signer: signerAddress, + data: '0x000000000000000000000000' + signerAddress.slice(2) + '0000000000000000000000000000000000000000000000000000000000000000' + '01', + } +} + +export const buildContractSignature = (signerAddress: string, signature: string): SafeSignature => { + return { + signer: signerAddress, + data: signature, + dynamic: true, + } } export const buildSignatureBytes = (signatures: SafeSignature[]): string => { - sortSignatures(signatures) - return ethers.concat(signatures.map((signature) => signature.data)) -} - -export const buildContractSignatureBytes = (signatures: SafeSignature[]): string => { - sortSignatures(signatures) - const start = 65 * signatures.length - const { segments } = signatures.reduce( - ({ segments, offset }, { signer, data }) => { - return { - segments: [...segments, ethers.solidityPacked(['uint256', 'uint256', 'uint8'], [signer, start + offset, 0])], - offset: offset + 32 + ethers.dataLength(data), - } - }, - { segments: [] as string[], offset: 0 }, - ) - return ethers.concat([ - ...segments, - ...signatures.map(({ data }) => ethers.solidityPacked(['uint256', 'bytes'], [ethers.dataLength(data), data])), - ]) + const SIGNATURE_LENGTH_BYTES = 65 + signatures.sort((left, right) => left.signer.toLowerCase().localeCompare(right.signer.toLowerCase())) + + let signatureBytes = '0x' + let dynamicBytes = '' + for (const sig of signatures) { + if (sig.dynamic) { + /* + A contract signature has a static part of 65 bytes and the dynamic part that needs to be appended + at the end of signature bytes. + The signature format is + Signature type == 0 + Constant part: 65 bytes + {32-bytes signature verifier}{32-bytes dynamic data position}{1-byte signature type} + Dynamic part (solidity bytes): 32 bytes + signature data length + {32-bytes signature length}{bytes signature data} + */ + const dynamicPartPosition = (signatures.length * SIGNATURE_LENGTH_BYTES + dynamicBytes.length / 2).toString(16).padStart(64, '0') + const dynamicPartLength = (sig.data.slice(2).length / 2).toString(16).padStart(64, '0') + const staticSignature = `${sig.signer.slice(2).padStart(64, '0')}${dynamicPartPosition}00` + const dynamicPartWithLength = `${dynamicPartLength}${sig.data.slice(2)}` + + signatureBytes += staticSignature + dynamicBytes += dynamicPartWithLength + } else { + signatureBytes += sig.data.slice(2) + } + } + + return signatureBytes + dynamicBytes } export const logGas = async (message: string, tx: Promise, skip?: boolean): Promise => { diff --git a/modules/4337/src/utils/safe.ts b/modules/4337/src/utils/safe.ts index dbcda09ca..2b61e2349 100644 --- a/modules/4337/src/utils/safe.ts +++ b/modules/4337/src/utils/safe.ts @@ -1,7 +1,7 @@ import { AddressLike, JsonRpcProvider, Provider, Signer, ethers } from 'ethers' -// Import from Safe contracts repo once fixed -import { MetaTransaction, SafeSignature, buildSignatureBytes } from './execution' +// Import from Safe contracts repo once it is upgraded to ethers v6 and can be installed via npm +import { MetaTransaction, SafeSignature, SignedSafeTransaction, buildSignatureBytes } from './execution' import { UserOperation, EIP712_SAFE_OPERATION_TYPE } from './userOp' const AddressOne = '0x0000000000000000000000000000000000000001' @@ -13,6 +13,7 @@ const INTERFACES = new ethers.Interface([ 'function proxyCreationCode() returns (bytes)', 'function enableModules(address[])', 'function execTransactionFromModule(address to, uint256 value, bytes calldata data, uint8 operation) external payable returns (bool success)', + 'function execTransaction(address to, uint256 value, bytes calldata data, uint8 operation, uint256 safeTxGas, uint256 baseGas, uint256 gasPrice, address gasToken, address payable refundReceiver, bytes memory signatures) external payable returns (bool success)', 'function executeUserOp(address to, uint256 value, bytes calldata data, uint8 operation)', 'function getNonce(address,uint192) returns (uint256 nonce)', 'function supportedEntryPoint() returns (address)', @@ -326,17 +327,69 @@ export class Safe4337 { return Safe4337Operation.build(this.provider, this, action, this.globalConfig) } - static async withSigner(signer: string, globalConfig: GlobalConfig): Promise { + /** + * Creates a new instance of Safe4337 with the specified signer and global configuration. + * + * @param {string} signer - A signer address. + * @param {GlobalConfig} globalConfig - The global configuration for the Safe4337 instance. + * @returns {Safe4337} A new instance of Safe4337. + */ + static withSigner(signer: string, globalConfig: GlobalConfig): Safe4337 { const safeConfig: SafeConfig = { signers: [signer], threshold: 1, nonce: 0, } + return Safe4337.withConfigs(safeConfig, globalConfig) } - static async withConfigs(safeConfig: SafeConfig, globalConfig: GlobalConfig): Promise { + /** + * Creates a new instance of Safe4337 with the specified signers, threshold, and global configuration. + * + * @param {string[]} signers - An array of signer addresses. + * @param {number} threshold - The number of signatures required to execute a transaction. + * @param {GlobalConfig} globalConfig - The global configuration for the Safe4337 instance. + * @returns {Safe4337} A new instance of Safe4337. + */ + static withSigners(signers: string[], threshold: number, globalConfig: GlobalConfig): Safe4337 { + const safeConfig: SafeConfig = { + signers, + threshold, + nonce: 0, + } + + return Safe4337.withConfigs(safeConfig, globalConfig) + } + + /** + * Creates a new instance of Safe4337 with the provided SafeConfig and GlobalConfig. + * @param {SafeConfig} safeConfig - The SafeConfig object containing the configuration for the Safe4337 instance. + * @param {GlobalConfig} globalConfig - The GlobalConfig object containing the global configuration for the Safe4337 instance. + * @returns {Safe4337} A new instance of Safe4337. + */ + static withConfigs(safeConfig: SafeConfig, globalConfig: GlobalConfig): Safe4337 { const initParams = buildInitParamsForConfig(safeConfig, globalConfig) return new Safe4337(initParams.safeAddress, globalConfig, safeConfig) } + + /** + * Encodes the transaction data for executing a safe transaction. + * @param {SignedSafeTransaction} transaction - The signed safe transaction object. + * @returns {string} The encoded transaction data. + */ + static getExecTransactionData(transaction: SignedSafeTransaction): string { + return INTERFACES.encodeFunctionData('execTransaction', [ + transaction.to, + transaction.value, + transaction.data, + transaction.operation, + transaction.safeTxGas, + transaction.baseGas, + transaction.gasPrice, + transaction.gasToken, + transaction.refundReceiver, + buildSignatureBytes(transaction.signatures), + ]) + } } diff --git a/modules/4337/test/e2e/4337NestedSafe.spec.ts b/modules/4337/test/e2e/4337NestedSafe.spec.ts new file mode 100644 index 000000000..87c692638 --- /dev/null +++ b/modules/4337/test/e2e/4337NestedSafe.spec.ts @@ -0,0 +1,428 @@ +import { expect } from 'chai' +import { deployments, ethers, network } from 'hardhat' +import { + SafeSignature, + SafeTransaction, + SignedSafeTransaction, + buildSignatureBytes, + calculateSafeMessageHash, + calculateSafeTransactionHash, + getPrevalidatedSignature, + preimageSafeMessageHash, + preimageSafeTransactionHash, + signHash, +} from '../../src/utils/execution' +import { buildUserOperationFromSafeUserOperation, buildSafeUserOpTransaction, signSafeOp, SafeUserOperation } from '../../src/utils/userOp' +import { chainId } from '../utils/encoding' +import { Safe4337 } from '../../src/utils/safe' +import { BUNDLER_MNEMONIC, bundlerRpc, prepareAccounts, waitForUserOp } from '../utils/e2e' +import { BigNumberish, Signer } from 'ethers' +import { assert } from 'console' + +// Set to true to enable global debug logs +const ENABLE_GLOBAL_DEBUG = false + +/** + * Represents a node in the ownership tree of a Safe4337. + */ +class SafeOwnershipTreeNode { + /** + * Creates an instance of SafeOwnershipTreeNode. + * @param value - The Safe4337 value associated with this node. + * @param owners - An array of owners of this node, which can be either SafeOwnershipTreeNode or SafeOwnershipTreeEoaNode instances. + */ + constructor( + public value: Safe4337, + public owners: (SafeOwnershipTreeNode | SafeOwnershipTreeEoaNode)[], + ) {} +} + +/** + * Represents a node in the Safe Ownership Tree for an externally owned account (EOA). + */ +class SafeOwnershipTreeEoaNode { + constructor(public value: Signer) {} +} + +/** + * Represents a tree structure that holds ownership information of a safe. + */ +class SafeOwnershipTree { + constructor(public root: SafeOwnershipTreeNode) {} +} + +/** + * Recursively walks the ownership tree and deploys all nodes. + * A utility function for tests that require all nodes in the ownership tree to be deployed. + * + * @param {SafeOwnershipTreeNode} node - The root node of the ownership tree. + * @param {Signer} deployer - The signer used for deployment. + */ +const walkTreeAndDeployAll = async (node: SafeOwnershipTreeNode, deployer: Signer): Promise => { + await node.value.deploy(deployer) + + for (const owner of node.owners) { + if (owner instanceof SafeOwnershipTreeNode) { + await walkTreeAndDeployAll(owner, deployer) + } + } +} + +/** + * Prints the tree structure of SafeOwnershipTreeNode recursively. + * + * @param {SafeOwnershipTreeNode} node - The root node of the tree. + * @param {number} depth - The current depth of the node in the tree. + */ +const printTree = async (node: SafeOwnershipTreeNode, depth: number): Promise => { + console.log(' '.repeat(depth) + node.value.address) + for (const owner of node.owners) { + if (owner instanceof SafeOwnershipTreeNode) { + await printTree(owner, depth + 1) + } else { + console.log(' '.repeat(depth + 1) + (await owner.value.getAddress())) + } + } +} + +/** + * Prints the execution path of a SafeOwnershipTreeNode array. + * + * @param {SafeOwnershipTreeNode[]} executionPath - The array of SafeOwnershipTreeNode representing the execution path. + */ +const printExecutionPath = (executionPath: SafeOwnershipTreeNode[]): void => { + const executorAddress = executionPath[executionPath.length - 1].value.address + const path = executionPath + .slice(1, -1) + .map((node) => node.value.address) + .join(' -> ') + const rootAddress = executionPath[0].value.address + + console.log(`${executorAddress} (4337 Executor) -> ${path} -> ${rootAddress} (Root)`) +} + +/** + * Checks if a given node is owned by EOAs only. Such nodes are considered leaf nodes in the ownership tree. + * @param node - The SafeOwnershipTreeNode to check. + * @returns {boolean} Returns true if the node is owned by EOAs only, false otherwise. + */ +const isOwnedByEoasOnly = (node: SafeOwnershipTreeNode): boolean => { + return node.owners.every((owner) => owner instanceof SafeOwnershipTreeEoaNode) +} + +/** + * Recursively signs a hash using the owners of a SafeOwnershipTreeNode. Should only be called if the signer is a Safe. + * + * @param {SafeOwnershipTreeNode} safe - The SafeOwnershipTreeNode representing the Safe. + * @param {string} hash - The hash to be signed. + * @param {BigNumberish} chainId - The chain ID. + * @returns {Promise} A Promise that resolves to a SafeSignature object. + */ +const recursivelySignHashWithASafe = async (safe: SafeOwnershipTreeNode, hash: string, chainId: BigNumberish): Promise => { + const signatures = [] + + for (const owner of safe.owners) { + if (owner instanceof SafeOwnershipTreeEoaNode) { + const safeMsgHash = calculateSafeMessageHash(safe.value.address, hash, chainId) + signatures.push(await signHash(owner.value, safeMsgHash)) + } else { + // When Safe contract validates the signatures, it uses the pre-image of the hash to validate the signature (legacy EIP-1271 behaviour), + // hence we're computing the pre-image of the hash here. + const preImageSafeMsgHash = preimageSafeMessageHash(safe.value.address, hash, chainId) + signatures.push(await recursivelySignHashWithASafe(owner, preImageSafeMsgHash, chainId)) + } + } + + return { + signer: safe.value.address, + data: buildSignatureBytes(signatures), + dynamic: true, + } +} + +/** + * Retrieves the execution path from the executor (leaf node) to the root node in a SafeOwnershipTreeNode tree. + * + * @param {SafeOwnershipTreeNode} rootSafe The root node of the SafeOwnershipTreeNode tree. + * @returns {SafeOwnershipTreeNode[]} An array of SafeOwnershipTreeNode objects representing the execution path. The first element is the root node and the last element is the executor. + */ +const getExecutionPath = (rootSafe: SafeOwnershipTreeNode): SafeOwnershipTreeNode[] => { + const executionPath = [] + // We need to build an execution path from the executor (leaf node) to the root node + // To do so, we need to traverse the tree from the root to the executor + // The executor is the first leaf node found or the last node in the execution path + // This algorithm doesn't produce the shortest path, but it's guaranteed to find the executor (if it exists in the tree) + // For further optimization, we can use a breadth-first search algorithm to find the executor + let currentNode = rootSafe + // eslint-disable-next-line no-constant-condition + while (true) { + executionPath.push(currentNode) + if (isOwnedByEoasOnly(currentNode)) { + // We found the executor + break + } else { + const owners = currentNode.owners + for (const owner of owners) { + if (owner instanceof SafeOwnershipTreeNode) { + currentNode = owner + break + } + } + } + } + + return executionPath +} + +/** + * Checks if a node is present in the execution path. Root node is not considered part of the execution path + * because it is rather an execution target. + * @param executionPath - The execution path as an array of SafeOwnershipTreeNode objects. + * @param node - The node to check for presence in the execution path. + * @returns A boolean indicating whether the node is present in the execution path. + */ +const isNodeInExecutionPath = (executionPath: SafeOwnershipTreeNode[], node: SafeOwnershipTreeNode): boolean => { + const index = executionPath.findIndex((n) => n.value.address === node.value.address) + + return index !== -1 && index !== 0 +} + +/** + * Builds a nested safe operation from a SafeTransaction, a root safe, an execution path and an entry point address. + * The algorithm works as follows: + * 1. For each node in the execution path, we create a Safe transaction and sign it with the owners of the node: + * a. If the owner is an EOA, we sign the transaction with the EOA's private key. + * b. If the owner is a Safe, we recursively sign the transaction with the Safe's owners. + * c. If the owner is in the execution path, we sign the transaction with a pre-validated signature (the one that checks that msg.sender is an owner) + * 2. We then build a SafeUserOperation that calls into the starting point of the execution path (a node before the executor 4337 Safe) + * + * @param {SafeTransaction} safeTx - The safe transaction. + * @param {SafeOwnershipTreeNode} rootSafe - The root safe in the ownership tree. + * @param {SafeOwnershipTreeNode[]} executionPath - The execution path of the safe ownership tree nodes. + * @param {string} entryPointAddress - The entry point address. + * @returns {Promise} A promise that resolves to a SafeUserOperation. + */ +const buildNestedSafeOp = async ( + safeTx: SafeTransaction, + rootSafe: SafeOwnershipTreeNode, + executionPath: SafeOwnershipTreeNode[], + entryPointAddress: string, +): Promise => { + const transactions: SignedSafeTransaction[] = [{ ...safeTx, signatures: [] }] + + // We iterate over executionPath.length - 1 because the last node in the execution path is the executor + // And will be handled as a special case after the loop (it needs a UserOperation instead of SafeTransaction) + for (let i = 0; i < executionPath.length - 1; i++) { + if (i === 0) { + assert(executionPath[i].value.address === rootSafe.value.address, 'First node in the execution path should be the root node') + } + + const node = executionPath[i] + // There will be (amount(executionPathNodes) - 1) Safe transactions because the last node in the execution path is the executor + // And will be handled as a special case after the loop (it needs a UserOperation instead of SafeTransaction) + const transaction = transactions[i] + for (const owner of node.owners) { + if (owner instanceof SafeOwnershipTreeEoaNode) { + const safeTxHash = calculateSafeTransactionHash(executionPath[i].value.address, transaction, await chainId()) + transaction.signatures.push(await signHash(owner.value, safeTxHash)) + } else { + // If the node is in the execution path, that means it will be executing an ethereum CALL and will be a msg.sender + // Hence we sign the transaction with a pre-validated signature (the one that checks that msg.sender is an owner) + if (isNodeInExecutionPath(executionPath, owner)) { + transaction.signatures.push(getPrevalidatedSignature(owner.value.address)) + } else { + // If the owner node is not in the execution path and a Safe, we recursively sign the transaction with the Safe's owners. + // When Safe contract validates the signatures, it uses the pre-image of the transaction hash to validate the signature (legacy EIP-1271 behaviour), + // hence we're computing the pre-image of the transaction hash here. + const preimageSafeTxHash = preimageSafeTransactionHash(executionPath[i].value.address, transaction, await chainId()) + transaction.signatures.push(await recursivelySignHashWithASafe(owner, preimageSafeTxHash, await chainId())) + } + } + } + + // After we signed the transaction, we need to generate the next Safe transaction for the next node in the execution path + // We do not need to generate it for the last two nodes in the execution path because the last node will be the executor + // And the preceeding node will be the starting point of the execution path and both will be handled as special cases after the loop + if (i < executionPath.length - 2) { + const newTransaction: SignedSafeTransaction = { + to: node.value.address, + value: 0, + data: Safe4337.getExecTransactionData(transaction), + operation: 0, + safeTxGas: 0, + baseGas: 0, + gasPrice: 0, + gasToken: ethers.ZeroAddress, + refundReceiver: ethers.ZeroAddress, + nonce: 0, + signatures: [], + } + transactions.push(newTransaction) + } + } + + if (ENABLE_GLOBAL_DEBUG) { + console.dir({ transactions }, { depth: null }) + } + + const executor = executionPath[executionPath.length - 1] + const executionStartPoint = executionPath[executionPath.length - 2] + const entryTransaction = transactions[transactions.length - 1] + const entryTransactionCopy = { + ...entryTransaction, + signatures: [getPrevalidatedSignature(executor.value.address)], + } + + return buildSafeUserOpTransaction( + executor.value.address, + executionStartPoint.value.address, + 0, + Safe4337.getExecTransactionData(entryTransactionCopy), + 0, + entryPointAddress, + false, + true, + ) +} + +describe('E2E - Nested Safes With An Execution Initiated by a Leaf 4337 Safe', () => { + before(function () { + if (network.name !== 'localhost') { + this.skip() + } + }) + + const setupTests = async () => { + const { SafeModuleSetup, EntryPoint, HariWillibaldToken, Safe4337Module, SafeL2, SafeProxyFactory } = await deployments.run() + const [user, user2, user3] = await prepareAccounts(BUNDLER_MNEMONIC, 3) + const bundler = bundlerRpc() + + const entryPoint = new ethers.Contract(EntryPoint.address, EntryPoint.abi, ethers.provider) + const validator = await ethers.getContractAt('Safe4337Module', Safe4337Module.address) + const token = await ethers.getContractAt('HariWillibaldToken', HariWillibaldToken.address) + const proxyFactory = await ethers.getContractAt('SafeProxyFactory', SafeProxyFactory.address) + const proxyCreationCode = await proxyFactory.proxyCreationCode() + + const leafSafe = new SafeOwnershipTreeNode( + Safe4337.withSigner(user.address, { + safeSingleton: SafeL2.address, + entryPoint: EntryPoint.address, + erc4337module: Safe4337Module.address, + proxyFactory: SafeProxyFactory.address, + safeModuleSetup: SafeModuleSetup.address, + proxyCreationCode, + chainId: Number(await chainId()), + }), + [new SafeOwnershipTreeEoaNode(user)], + ) + const leafSafe2 = new SafeOwnershipTreeNode( + Safe4337.withSigner(user2.address, { + safeSingleton: SafeL2.address, + entryPoint: EntryPoint.address, + erc4337module: Safe4337Module.address, + proxyFactory: SafeProxyFactory.address, + safeModuleSetup: SafeModuleSetup.address, + proxyCreationCode, + chainId: Number(await chainId()), + }), + [new SafeOwnershipTreeEoaNode(user2)], + ) + const leafEoa = new SafeOwnershipTreeEoaNode(user3) + const nodeSafe = new SafeOwnershipTreeNode( + Safe4337.withSigner(leafSafe.value.address, { + safeSingleton: SafeL2.address, + entryPoint: EntryPoint.address, + erc4337module: Safe4337Module.address, + proxyFactory: SafeProxyFactory.address, + safeModuleSetup: SafeModuleSetup.address, + proxyCreationCode, + chainId: Number(await chainId()), + }), + [leafSafe], + ) + const rootSafe = new SafeOwnershipTreeNode( + Safe4337.withSigners([nodeSafe.value.address, leafSafe2.value.address, await leafEoa.value.getAddress()], 3, { + safeSingleton: SafeL2.address, + entryPoint: EntryPoint.address, + erc4337module: Safe4337Module.address, + proxyFactory: SafeProxyFactory.address, + safeModuleSetup: SafeModuleSetup.address, + proxyCreationCode, + chainId: Number(await chainId()), + }), + [nodeSafe, leafSafe2, leafEoa], + ) + const tree = new SafeOwnershipTree(rootSafe) + + return { + user, + bundler, + tree, + validator, + entryPoint, + token, + } + } + + it('should execute a transaction for an existing Safe', async () => { + const { user, bundler, tree, validator, entryPoint, token } = await setupTests() + + const executionPath = getExecutionPath(tree.root) + + if (ENABLE_GLOBAL_DEBUG) { + console.log('=== Tree structure: ===') + await printTree(tree.root, 0) + console.log('=== Tree structure end ===') + printExecutionPath(executionPath) + } + + await walkTreeAndDeployAll(tree.root, user) + const executor = executionPath[executionPath.length - 1] + const rootSafe = tree.root.value + + await token.transfer(rootSafe.address, ethers.parseUnits('4.2', 18)).then((tx) => tx.wait()) + await user.sendTransaction({ to: executor.value.address, value: ethers.parseEther('1') }).then((tx) => tx.wait()) + + expect(ethers.dataLength(await ethers.provider.getCode(rootSafe.address))).to.not.equal(0) + expect(await token.balanceOf(rootSafe.address)).to.equal(ethers.parseUnits('4.2', 18)) + + const safeTransaction: SafeTransaction = { + to: await token.getAddress(), + value: 0, + data: token.interface.encodeFunctionData('transfer', [user.address, await token.balanceOf(rootSafe.address)]), + operation: 0, + safeTxGas: 0, + baseGas: 0, + gasPrice: 0, + gasToken: ethers.ZeroAddress, + refundReceiver: ethers.ZeroAddress, + nonce: 0, + } + + assert(executor.owners[0] instanceof SafeOwnershipTreeEoaNode, 'Executor should be an EOA') + const executorSigner = executor.owners[0].value as Signer + const safeOp = await buildNestedSafeOp(safeTransaction, tree.root, executionPath, await entryPoint.getAddress()) + const signature = buildSignatureBytes([await signSafeOp(executorSigner, await validator.getAddress(), safeOp, await chainId())]) + const userOp = buildUserOperationFromSafeUserOperation({ + safeOp, + signature, + }) + + if (ENABLE_GLOBAL_DEBUG) { + console.dir({ userOp }, { depth: null }) + } + + const receiptHash = await bundler.sendUserOperation(userOp, await entryPoint.getAddress()) + await waitForUserOp(userOp) + + if (ENABLE_GLOBAL_DEBUG) { + console.dir(await bundler.send('eth_getUserOperationReceipt', [receiptHash]), { depth: null }) + } + expect(await token.balanceOf(rootSafe.address)).to.equal(0n) + }) + + it.skip('should deploy a new Safe and execute a transaction', async () => { + // TODO: Implement this test. This test will take more effort because the above algorithm will need to be adjusted + // to use the CreateCall contract from the Safe Contract suite. + }) +}) diff --git a/modules/4337/test/e2e/SingletonSigners.spec.ts b/modules/4337/test/e2e/SingletonSigners.spec.ts index 0dce336f1..8bdaf2e8b 100644 --- a/modules/4337/test/e2e/SingletonSigners.spec.ts +++ b/modules/4337/test/e2e/SingletonSigners.spec.ts @@ -1,6 +1,6 @@ import { expect } from 'chai' import { deployments, ethers, network } from 'hardhat' -import { buildContractSignatureBytes } from '../../src/utils/execution' +import { buildSignatureBytes } from '../../src/utils/execution' import { buildUserOperationFromSafeUserOperation, buildSafeUserOpTransaction } from '../../src/utils/userOp' import { bundlerRpc, encodeMultiSendTransactions, prepareAccounts, waitForUserOp } from '../utils/e2e' @@ -110,10 +110,11 @@ describe('E2E - Singleton Signers', () => { signature: '0x', }), ) - const signature = buildContractSignatureBytes( + const signature = buildSignatureBytes( customSigners.map(({ signer, key }) => ({ signer: signer.target as string, data: ethers.toBeHex(BigInt(opHash) ^ key, 32), + dynamic: true, })), ) const userOp = buildUserOperationFromSafeUserOperation({ diff --git a/modules/4337/test/e2e/WebAuthnSingletonSigner.spec.ts b/modules/4337/test/e2e/WebAuthnSingletonSigner.spec.ts index 2d144c26e..ba83ade93 100644 --- a/modules/4337/test/e2e/WebAuthnSingletonSigner.spec.ts +++ b/modules/4337/test/e2e/WebAuthnSingletonSigner.spec.ts @@ -9,7 +9,7 @@ import { extractSignature, } from '../utils/webauthn' import { buildSafeUserOpTransaction, buildUserOperationFromSafeUserOperation } from '../../src/utils/userOp' -import { buildContractSignatureBytes } from '../../src/utils/execution' +import { buildSignatureBytes } from '../../src/utils/execution' describe('E2E - WebAuthn Singleton Signers', () => { before(function () { @@ -152,7 +152,7 @@ describe('E2E - WebAuthn Singleton Signers', () => { userVerification: UserVerificationRequirement.required, }, }) - const signature = buildContractSignatureBytes([ + const signature = buildSignatureBytes([ { signer: signer.target as string, data: ethers.AbiCoder.defaultAbiCoder().encode( @@ -163,6 +163,7 @@ describe('E2E - WebAuthn Singleton Signers', () => { extractSignature(assertion.response), ], ), + dynamic: true, }, ]) const userOp = buildUserOperationFromSafeUserOperation({ diff --git a/modules/4337/test/erc4337/ERC4337WebAuthn.spec.ts b/modules/4337/test/erc4337/ERC4337WebAuthn.spec.ts index bc484241a..cb656b812 100644 --- a/modules/4337/test/erc4337/ERC4337WebAuthn.spec.ts +++ b/modules/4337/test/erc4337/ERC4337WebAuthn.spec.ts @@ -1,7 +1,7 @@ import { expect } from 'chai' import { deployments, ethers } from 'hardhat' import { getEntryPoint } from '../utils/setup' -import { buildContractSignatureBytes, logGas } from '../../src/utils/execution' +import { buildSignatureBytes, logGas } from '../../src/utils/execution' import { buildSafeUserOpTransaction, buildUserOperationFromSafeUserOperation, calculateSafeOperationHash } from '../../src/utils/userOp' import { chainId } from '../utils/encoding' import { @@ -274,7 +274,7 @@ describe('Safe4337Module - WebAuthn Owner', () => { userVerification: UserVerificationRequirement.required, }, }) - const signature = buildContractSignatureBytes([ + const signature = buildSignatureBytes([ { signer: signer.target as string, data: ethers.AbiCoder.defaultAbiCoder().encode( @@ -285,6 +285,7 @@ describe('Safe4337Module - WebAuthn Owner', () => { extractSignature(assertion.response), ], ), + dynamic: true, }, ]) diff --git a/modules/4337/test/erc4337/ReferenceEntryPoint.spec.ts b/modules/4337/test/erc4337/ReferenceEntryPoint.spec.ts index 81a9e242b..afc9af4a8 100644 --- a/modules/4337/test/erc4337/ReferenceEntryPoint.spec.ts +++ b/modules/4337/test/erc4337/ReferenceEntryPoint.spec.ts @@ -3,7 +3,7 @@ import { deployments, ethers } from 'hardhat' import { time } from '@nomicfoundation/hardhat-network-helpers' import { EventLog, Log } from 'ethers' import { getEntryPoint, getFactory, getSafeModuleSetup } from '../utils/setup' -import { buildContractSignatureBytes, buildSignatureBytes, logGas } from '../../src/utils/execution' +import { buildSignatureBytes, logGas } from '../../src/utils/execution' import { buildSafeUserOpTransaction, buildUserOperationFromSafeUserOperation, @@ -168,7 +168,7 @@ describe('Safe4337Module - Reference EntryPoint', () => { }, ) const opData = calculateSafeOperationData(await validator.getAddress(), safeOp, await chainId()) - const signature = buildContractSignatureBytes([ + const signature = buildSignatureBytes([ { signer: parentSafe.address, data: await user.signTypedData( @@ -183,6 +183,7 @@ describe('Safe4337Module - Reference EntryPoint', () => { message: opData, }, ), + dynamic: true, }, ]) const userOp = buildUserOperationFromSafeUserOperation({ diff --git a/modules/4337/test/utils/e2e.ts b/modules/4337/test/utils/e2e.ts index 2b92267c1..445deae4a 100644 --- a/modules/4337/test/utils/e2e.ts +++ b/modules/4337/test/utils/e2e.ts @@ -1,12 +1,12 @@ import { deployments, ethers } from 'hardhat' import { MultiProvider4337 } from '../../src/utils/safe' import { UserOperation } from '../../src/utils/userOp' -import { AddressLike, BigNumberish, BytesLike } from 'ethers' +import { AddressLike, BigNumberish, BytesLike, HDNodeWallet } from 'ethers' export const BUNDLER_URL = process.env.TEST_BUNLDER_URL || 'http://localhost:3000/rpc' export const BUNDLER_MNEMONIC = process.env.TEST_BUNDLER_MNEMONIC || 'test test test test test test test test test test test junk' -export async function prepareAccounts(mnemonic = BUNDLER_MNEMONIC, count = 1) { +export async function prepareAccounts(mnemonic = BUNDLER_MNEMONIC, count = 1): Promise { const bundler = ethers.HDNodeWallet.fromPhrase(mnemonic).connect(ethers.provider) const accounts = [...Array(count)].map(() => ethers.Wallet.createRandom(ethers.provider))