-
Notifications
You must be signed in to change notification settings - Fork 80
WIP: move to single signature ERC-3009 #1275
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: Matt Rice <[email protected]>
fusmanii
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good, just one Q
| bytes32 witness = keccak256( | ||
| abi.encodePacked(BRIDGE_AND_SWAP_WITNESS_IDENTIFIER, abi.encode(swapAndDepositData)) | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how do we ensure witness uniqueness given that its now obtained from deposit data?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My understanding is that the token implementing ERC-3009 will revert on the receiveWithAuthorization call if the witness has already been used
Also there's a nonce in SwapAndDepositData thats part of what gets hashed to generate the witness - so this can create uniqueness even if all the other intent data is identical
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, my undertsanding was the same as Taylor's, witness here is essentially the nonce for the ERC-3009 token's permit with auth
From https://eips.ethereum.org/EIPS/eip-3009 reference implementation:
require(
!_authorizationStates[from][nonce],
"EIP3009: authorization is used"
);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed with the above. There is nonce reuse prevention that is per-user. The nonce within the hashed data means that a user should never have an identical witness hash unless they are accidentally reusing the same API response twice.
Signed-off-by: Ihor Farion <[email protected]>
|
@mrice32 one other Q: do we need to explicitly use domainSeparator in our witness? |
I don't think so. I think the EIP-712 related info is only needed in the outermost layer: the 3009 data that's being signed (which contains the witness). |
No description provided.