safe-global / safe-modules / 7045450570

85%

push

github

Sign Full User Operation Data (#177)

This PR adjusts the Safe module to sign **all** of the `UserOperation`
struct data instead of the reduced set of fields it was originally
signing. Namely, this means that the `SafeOp` EIP-712 struct needs to
also include the fields:

* `initCode`: ensures that initialisation works exactly as the user
specified
* `paymasterAndData`: ensures that user operations are tied to specific
paymaster calls

Unfortunately, this required some non-trivial changes to the module code
in order to accommodate all of these fields. Namely:
- `SafeOp` field order was changed to match that of the `UserOperation`.
This was done for two reasons:
1. Make it clearer and easier to verify that all `UserOperation` fields
are accounted for
2. Allows for more efficient encoding of `SafeOp` for computing the
signing message when copying from a `UserOperation calldata`. While we
don’t make use of this in the code, I believe it is a nice property to
have.
- `getOperationHash` now takes a `UserOperation calldata` instead of all
user operation fields (because of the Solidity “stack too deep”
limitation).


https://github.com/safe-global/safe-modules/pull/177/commits/<a class=hub.com/safe-global/safe-modules/commit/b3e1785548477343f6c2a9f048585d6bf20d6100">b3e178554
contains other versions of possible implementations for computing the
signing message for the Safe operation, namely:
1. Using assembly and `calldatacopy` to setup the `SafeOp` struct in
memory for hashing, this is the most efficient by around 310 gas
compared to the current implementation, but has a lot of magic and is
not easy to understand, hence it was not used.
([code](https://github.com/safe-global/safe-modules/blob/b3e1785548477343f6c2a9f048585d6bf20d6100/4337/contracts/Safe4337Module.sol#L168-L214))
2. Encoding values into a `uint256[14] memory`, while this requires
slightly less boiler plate, it is not as easy to understand (IMO) and
was less gas-efficient.
([code](https://github.com/safe-global/safe-m... (continued)

22 of 22 branches covered (100.0%)

Branch coverage included in aggregate %.

14 of 14 new or added lines in 1 file covered. (100.0%)

33 of 33 relevant lines covered (100.0%)

25.0 hits per line