safe-global / safe-modules / 6959417900

85%

push

github

[4337] Separate Signature Fields for `validAfter` and `validUntil` (#167)

Follow up to #143 and #156

This PR refactors signature validation to use the immutable
`SUPPORTED_ENTRYPOINT` value instead of passing it into various
functions. This has the benefit of freeing up a stack slot for an
additional variable. This means that we can now sign `validAfter` and
`validUntil` as separate fields in our `SafeOp` struct (which should
provide a nicer user experience for signing and be less cryptic than
using a “signatureTimestamps” field that combines both values).

The caveat is that `getOperationHash` is tied to the module’s supported
`EntryPoint` value, but IMO this makes more sense that what we had
before, as signatures for other `EntryPoint`s are useless to it anyway.

Additionally, in theory, we don’t need to include the `entryPoint` in
the signature, since it is implied by the `verifyingContract` address of
the ERC-4337 module, seeing as `SUPPORTED_ENTRYPOINT` is an immutable.
However, I decided to leave it in, the rationale being that it adds
additional information to the user signature and improves UX around
collecting EIP-712 signatures.

I added validity timestamps to the user operation in one of the E2E
tests for better coverage.

22 of 22 branches covered (100.0%)

Branch coverage included in aggregate %.

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

29 of 29 relevant lines covered (100.0%)

19.83 hits per line