feat: Add SMA Replay-Safe Hash #155
Conversation
Zer0dot
force-pushed
the
zer0dot/sma-replay-safe-hash
branch
from
3400f3d to
bccc3b2
Compare
Zer0dot
force-pushed
the
zer0dot/sma-replay-safe-hash
branch
from
bccc3b2 to
6faa4b6
Compare
adamegyed
left a comment
adamegyed
left a comment
There was a problem hiding this comment.
Looks good! 1 small style nit.
Side note: it would be possible to implement EIP-5267 for SMA, because we do know all of the fields in the EIP712Domain struct. However, I don't think we need to, because there's already account impl-specific behavior we will need to do for signing, and it will increase codesize.
| uint8 v; | ||
| bytes32 r; | ||
| bytes32 s; | ||
|
|
||
| if (vm.envOr("SMA_TEST", false)) { | ||
| // todo: implement replay-safe hashing for SMA | ||
| (v, r, s) = vm.sign(owner1Key, message); | ||
| } else { | ||
| bytes32 replaySafeHash = singleSignerValidationModule.replaySafeHash(address(account1), message); | ||
| (v, r, s) = vm.sign(owner1Key, replaySafeHash); | ||
| } | ||
| (v, r, s) = vm.sign(owner1Key, replaySafeHash); |
There was a problem hiding this comment.
Style nit: can inline declare vars now as:
(uint8 v, bytes32 r, bytes32 s)
Interesting point, hadn't thought of it. I agree we're better off as it is for now, it's probably not beneficial to expose this on SMA when it can't be done on a standard MA. |
Motivation
Currently, two SMAs with the same fallback signer could be used to verify messages. This is problematic as signatures must be unique on a per-account basis.
This has been addressed for modules already, and this PR addresses this for SMAs.
Solution
Add a
replaySafeHash()function and related EIP712 functionality to theSemiModularAccount, which is computed upon ERC1271 signature validation. This means the fallback signer must sign the replay-safe hash, but the inner hash must be submitted in the transaction.