Remove templateId and simplify EmailProof validation in ZKEmailUtils
#210
Conversation
ernestognw
changed the title
ernestognw
changed the title
ernestognw
changed the title
ernestognw
changed the title
templateId and simplify EmailProof validation in ZKEmailUtils
ernestognw
requested review from
zkfriendly,
benceharomi and
a team
as code owners
| * NOTE: The returned `emailProof` object should not be accessed if `success` is false. Trying to access the data may | ||
| * cause revert/panic. | ||
| */ | ||
| function tryDecodeEmailProof( |
There was a problem hiding this comment.
Curious to know what the main benefit of these checks is? Looking at the places where this is used, there is always a call to isValidZKEmail, which will reject a malformed EmailProof either by failing to pass the zk-proof checks (including DKIM checks and command matching) or by reverting somewhere. However, it does help to revert early if EmailProof is not a valid struct. So basically would these be redundant checks for valid EmailProofs? or is there some other attack vector if these checks are not in place?
There was a problem hiding this comment.
Great observation! In summary, yes, the tryDecodeEmailProof is redundand if the bytes buffer correctly decodes to EmailProof. However, we try to avoid reverting in functions that may be part of an ERC-4337 user op validation flow for 2 reasons:
- It would affect the reputation of the account if it reverts (anyone can send undecodable EmailProofs).
- To force readability into the signature, we recommend ERC-7739. However, it has a detection mechanism that passes an empty buffer to the signer. If the account can't differentiate a valid struct vs an empy bytes buffer it will revert and detection wouldn't be possible
I think using tryDecodeEmailProof is acceptable in both the signer and the verify since it's only performing calldata slices (not memory copies) and simple JUMP/JUMPI operations. The overhead shouldn't be a big deal.
This PR streamlines the ZKEmailUtils by receiving an EmailProof directly instead of an EmailAuthMsg struct. From the original struct, the
templateIdwas unused and not included in the proof so anyone can construct the expected value, so it was removed.On the
SignerZKEmailandERC7913ZKEmailVerifierside, their implementation was simplified following changes in ZKEmailUtils.Key Changes:
templateIdandskippedCommandPrefixfunctionality across all ZKEmail contractstryDecodeEmailProoffunction for safe calldata decoding with comprehensive bounds checkingEmailProoffield encodingThe simplified implementation reduces complexity while preserving all essential security guarantees.