Add Permit and Permit2 signature docs #498
Conversation
README.md
Outdated
| ### Permit2 Helper | ||
|
|
||
| Balancer v3 handles token approval through Pemit2 and this helper facilitates Permit2 signature generation. | ||
|
|
||
| Each operation (e.g. addLiquidity, swap, ...) has its own method that leverages the same input type of the operation itself in order to simplify signature generation. | ||
|
|
||
| **Example** | ||
|
|
||
| See [addLiquidityWithPermit2Signature example](/examples/addLiquidity/addLiquidityWithPermit2Signature.ts). | ||
|
|
||
| **Function** | ||
|
|
||
| Helper function to create a Permit2 signature for an addLiquidity operation: | ||
|
|
||
| ```typescript | ||
| static async signAddLiquidityApproval( | ||
| input: AddLiquidityBaseBuildCallInput & { | ||
| client: PublicWalletClient; | ||
| owner: Address; | ||
| nonces?: number[]; | ||
| expirations?: number[]; | ||
| }, | ||
| ): Promise<Permit2> | ||
| ``` | ||
|
|
||
| **Parameters** | ||
|
|
||
| | Name | Type | Description | | ||
| | ------------- | ------------- | ------------ | | ||
| | input | [AddLiquidityBaseBuildCallInput](./src/entities/addLiquidity/types.ts#L62) | Add Liquidity Input | | ||
| | client | [PublicWalletClient](./src/utils/types.ts#L3) | Viem's wallet client with public actions | | ||
| | owner | Address | User address | | ||
| | nonces (optional) | number[] | Nonces for each token | | ||
| | expirations (optional) | number[] | Expirations for each token | | ||
|
|
||
| **Returns** | ||
|
|
||
| ```typescript | ||
| Promise<Permit2>; | ||
| ``` | ||
|
|
||
| [Permit2](./src/entities/permit2Helper/index.ts#L35) - Permit2 object with metadata and encoded signature |
There was a problem hiding this comment.
I like this! Good balance between brevity and detail
If the plan is to detail each of the Permit2Helper methods, this is a great blueprint 💪
There was a problem hiding this comment.
Since different operations are quite similar, maybe this is enough to guide how to use them all?
Still trying to figure out of how much info is too much info 😅
What do you think?
There was a problem hiding this comment.
i'm on the fence as well. might not be a bad idea to hope for the best and only add more explicit docs for each method if we get complaints
| // Skip Permit2 approval during fork setup so we can test approvals through signatures | ||
| const approveOnPermit2 = false; | ||
|
|
||
| // Make the tx against the local fork and print the result | ||
| await makeForkTx( | ||
| call, | ||
| { | ||
| rpcUrl, | ||
| chainId, | ||
| impersonateAccount: userAccount, | ||
| forkTokens: amountsIn.map((a) => ({ | ||
| address: a.address, | ||
| slot: getSlot(chainId, a.address), | ||
| rawBalance: a.rawAmount, | ||
| })), | ||
| client, | ||
| }, | ||
| [...amountsIn.map((a) => a.address), pool.address], | ||
| call.protocolVersion, | ||
| approveOnPermit2, | ||
| ); |
There was a problem hiding this comment.
Nice move adding the flag that prevents the default cascade of approvals by makeForkTx so that we can know the example is working properly!
I do have a larger suggestion for team to consider that I will bring up during standup:
makeForkTx may confuse less experienced devs who might be distracted by token slots or not fully grasp how the call object the SDK creates is used to send the tx.
This may be a stretch that requires a fair bit of refactor, but perhaps rolling a simplified fork setup for examples could be an option where we use the default anvil account #0 to
- wrap ETH into wETH
- swap wETH into BAL
- add liquidity to wETH/BAL pool
- any necessary setup for boosted or nested joins / removes
From there, we should be able to focus examples on just the code needed for each operation and it would be easier to explain a simplified fork setup in some detail
Not suggesting this as part of this PR and not too attached to this idea, just a suggestion that could enable examples to be a little more straightforward
P.S. John had this idea for examples a while back. Not sure if it applies to add / remove, but wanted to raise awareness
There was a problem hiding this comment.
I'll keep as is for now, since we're evaluating different approach for examples on a separate PR.
Closes #369