`SignatureVerification.verify` is not documented
Opened this issue · 3 comments
Problem
Given that cryptography is a one-way street, the place where Permit2 users will most frequently encounter errors is the verify
function of the SignatureVerification
library:
permit2/src/libraries/SignatureVerification.sol
Lines 21 to 24 in bbbc92f
The function, unfortunately, lacks any form of guidance, leaving users on their own to source the necessary documentation.
This a problem that is compounded by Uniswap's leading position in DeFi, as the signature verification processes in Permit2 may be the first encounter with cryptography for many developers, especially front-end developers who have just made a career switch to web3.
Suggestions
- Provide a high-level English description of the function's algorithm by using NatSpec comments (
@notice
,@dev
,@param,
and so forth) - Make the implicit knowledge in this function explicit, i.e., explicitly mention the fact that the
signature
param is an ECDSA signature, and that the typical secp256k1 curve is what powers your cryptography - Explain what the benefit of using both standard ECDSA signatures and EIP-2098 signatures is; also, explain that users can use either signing scheme, and that supporting both or just one scheme is up to them
- Explain that using EIP-1271 means signatures can be signers, too
- Document and explain
UPPER_BIT_MASK
(consider adding aEIP1271_
prefix) - For convenience, provide explicit link to EIP-2098 and EIP-1271
- Consider reducing the control flow nesting depth by flipping the
claimedSigner.code.length == 0
check intoclaimedSigner.code.length > 0
, and usingreturn
to stop the function execution early. Here's a code snippet that shows what I mean.
Additional Context
We (cc @gavriliumircea, @razgraf) are currently working on integrating Permit2 into our project (Sablier V2), and have recently spent a lot of time debugging a bug that was being triggered by a revert on line 41 in SignatureVerification
:
The bug was caused by an issue in our front-end; however, it would have helped us during debugging to have a little more guidance in SignatureVerification.verify
.
I do agree with @PaulRBerg and further more I think it would be helpful, for the frontend developers, to create a guideline for frontend integrations as well.( Right now there a literally almost no resources at all regarding this matter.)
create a guideline for frontend integrations
Yep. A front-end template that shows how to use Permit2 with Ethers.js would go a long way.
create a guideline for frontend integrations
Yep. A front-end template that shows how to use Permit2 with Ethers.js would go a long way.
The permit2-sdk library already acts as some sort of a template (esp. when paired with the interfaces repository). What it does lack is explicit documentation of a full end-to-end flow, which is the real bummer here. Suggestions on how to "extend" the functionality at a contract level (calling .permit(), passing structured params) would go a long way as well. It would help avoid certain misconceptions.