Sign Full User Operation Data

[nlordell]

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).

b3e1785 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)
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)
3. Using abi.encode as usual, but using some careful tricks to avoid “stack too deep” errors. Ironically, this was the least gas efficient of all the methods, and when building with instrumentation for generating a coverage report, would run into “stack too deep” errors anyway. (code)

[coveralls]

Pull Request Test Coverage Report for Build 7044495496

• 14 of 14 (100.0%) changed or added relevant lines in 1 file are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage remained the same at 100.0%

---

Totals
Change from base Build 7003971952:	0.0%
Covered Lines:	33
Relevant Lines:	33

---

💛 - Coveralls

[rmeissner]

The solidity implementations looks good.

I could not find test that check that initCode and paymasterData can not be changed.

[nlordell]

I could not find test that check that initCode and paymasterData can not be changed.

The TS library changed to additionally sign initCode and paymasterAndData, so we implicitly check this with the current tests. We don’t explicitly check they cannot change (as that is a property of EIP-712) but I agree this would be a nice test to have (given that we now want to guarantee that all UserOperation fields are signed).

[nlordell]

@rmeissner - added a new unit test in 467df98 that verifies that if any UserOperation field changes, then the operation hash will also change.