Skip to content
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

research(contracts): permit2 instead of vault relayer #9

Open
mfw78 opened this issue Apr 11, 2024 · 8 comments
Open

research(contracts): permit2 instead of vault relayer #9

mfw78 opened this issue Apr 11, 2024 · 8 comments

Comments

@mfw78
Copy link
Contributor

mfw78 commented Apr 11, 2024

Problem

CoW Protocol is heavily into signing messages and using these are authorisations on which to perform actions. A common pain point from the start though is that token allowances must be set - and it has been the function of the GPv2VaultRelayer to hold these allowances, and perform transfers from users.

Acceptance criteria

  • Reduce signatures / transactions required for enhanced UX.
  • Any change with allowances / permits MUST be compliant with ERC-1271.
  • Reduce code (i.e. shrink / eliminate GPv2VaultRelayer).

Details

Currently a new user (in the case of an EOA) placing an order with CoW Protocol requires:

  1. Approve a transaction for ERC20Token.approve(vaultRelayer, type(uint256).max)
  2. Sign via eth_sign, eth_signTypedData_v4 (or, preSign, i.e. a transaction doing GPv2Settlement.setPreSignature)

A new user (in the case of a smart contract) placing an order with CoW Protocol requires:

  1. Set an approval (ERC20Token.approve(vaultRelayer, type(uint256).max))
  2. Sign via preSign (i.e. GPv2Settlement.setPreSignature, or eip1271).

Concretely, in the instance of a Safe, this can be done with a single transaction.

This has become somewhat of a "normal" UX across dapps, however we have the ability to smooth this further with Permit2, eliminating the need for the first approve in the case the user has already .approve() for Permit2.

Possible Solutions

  1. Use Permit2.permitTransferFrom, which takes a signed PermitTransferFrom EIP-712 signature. This has similar UX in that the user would have to sign both for the permit, and then separately for the swap order.
  2. Use Permit2.permitWitnessTransferFrom, which allows for signing a permit to transfer, but also some other arbitrary EIP-712 type. In this case, the order struct type can be appended, allowing for a single signature to verify both the permit and the order. An additional feature here is that the permit can have a signed expiry date, which may also allow for a different type of nullifier to be used for the settlement contract.

NOTE: Permit2 also allows for batch transfers, which would allow for future development into basket trading.

Research track

  • User experience
  • Protocol security
@alfetopito
Copy link

Would this work in conjunction with or replace entirely the need for https://eips.ethereum.org/EIPS/eip-2612 ?

@mfw78
Copy link
Contributor Author

mfw78 commented Apr 11, 2024

Would this work in conjunction with or replace entirely the need for https://eips.ethereum.org/EIPS/eip-2612 ?

This would essentially replace entirely EIP-2612. So the only approve we would have to ask from the user is if they haven't done approvals yet for Permit2 (similar to the GPv2VaultRelayer), but this also allows us to make use of approvals that have already been done for Uniswap.

@MindyCoW
Copy link

Is this the same as the permit we've already implemented? Would it work for all tokens? Would the user have to permit each token individually still? And would there be a way for the user to specify what amount they wanted to permit, and to rescind the permission?

@alfetopito
Copy link

Then, we would - for approvals - depend solely on Permit2, and there would be no other way to do the approvals?
What would be the security implications of relying on a third party permit system?

@mfw78
Copy link
Contributor Author

mfw78 commented Apr 26, 2024

Is this the same as the permit we've already implemented? Would it work for all tokens? Would the user have to permit each token individually still? And would there be a way for the user to specify what amount they wanted to permit, and to rescind the permission?

It isn't quite the same as the permit that we've already implemented. This is a supplemental type that uses a similar pattern as our GPv2VaultRelayer (it's another contract that stores a allowances for a user) - but instead a user can then sign a "Permit2" which is a signed message processed by the Permit2 contract that allows a user to give permission on a signed basis for another entity to spend their tokens.

This is therefore more accurately seen like a "mechanism in which to allow Permit-like functionality for tokens that don't natively support it".

UX wise, if the user didn't have approvals set for the Permit2 contract, we would have to prompt them to set it. Fortunately - everyone using Uniswap now is setting allowances for Permit2, so we benefit from these allowances already being set.

@mfw78
Copy link
Contributor Author

mfw78 commented Apr 26, 2024

Then, we would - for approvals - depend solely on Permit2, and there would be no other way to do the approvals? What would be the security implications of relying on a third party permit system?

Permit2 is now ubiquitous within Uniswap. The contracts are immutable, audited, and heavily battle-tested. Given that this is a relatively solved solution, this allows us to remove a significant portion of code complexity / hairiness from our code base for something that has been built / maintained externally.

@MindyCoW
Copy link

Sounds good to me, then!

@fleupold
Copy link

Note that the current permit (ERC2612) can still be used if an account that hasn't yet set an allowance on Uniswap (or other users if Permit2) would come to CoW and wants to trade.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants