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

fix(permit): add support for usdc mainnet #3231

Merged
merged 8 commits into from
Oct 18, 2023
Merged

Conversation

alfetopito
Copy link
Collaborator

@alfetopito alfetopito commented Oct 16, 2023

Summary

Fixes #3148

Make USDC on mainnet permittable \o/

(And any other token that has a different version)

TODO DONE

There are 3 pending issues:

  • USDC not permittable when loading FIXED
  1. Clear permit cache
  2. Load the app with USDC already selected as sell token (https://swap-dev-git-permit-3148usdc-mainnet-cowswap.vercel.app/#/1/swap/USDC)
  3. Whatever USDC token instance found, it comes with the token name as USD//C, different from contract's USD Coin
  4. That leads to an invalid signature
  • DAI no longer permittable FIXED
  1. Clear permit cache
  2. Pick DAI as sell token
  3. Not recognized as permittable

The version fetched is 2
image

While the value when looking at etherescan is 1
image

Not sure this issue is only for DAI or for any DAI like token

  • Not able to place order due to low fee FIXED

Estimated fee for the quote is much lower than what is calculated for the real user account.
This was an expected edge case which should have been addressed with increased defaults.
See this thread for more context

It was an issue with the local cache. Succeeded now: https://dev.explorer.cow.fi/orders/0xf087fab4df05bb0f6bb1e96d1915106312248cf900d410a2465d231b5d63ad2a5b0abe214ab7875562adee331deff0fe1912fe42652e9ae9?tab=overview

To Test

  1. On mainnet, select USDC as the sell token
  2. Check the localStorage key permittableTokens:v1
  • USDC (0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48) should contain:
{"type":"eip-2612","version":"2"}
  1. Make sure there's no allowance for USDC mainnet
  2. Place a trade with USDC mainnet
  • Should trade and set allowance to infinite
  1. Do the same with DAI
  • Should trade and set permit as usual
  1. Do the same with other kind of permittable token such as COW
  • Should trade and set permit as usual

@vercel
Copy link

vercel bot commented Oct 16, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
swap-dev ✅ Ready (Inspect) Visit Preview Oct 18, 2023 8:05am

🌃 Cosmos ↗︎

@alfetopito alfetopito changed the base branch from develop to hotfix/1.48.1 October 16, 2023 17:14
@shoom3301
Copy link
Collaborator

image

USDC detected as eip2612

Copy link
Collaborator

@shoom3301 shoom3301 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't tested the full scenario, but USDC is detected as permittable

Base automatically changed from hotfix/1.48.1 to main October 17, 2023 09:47
Comment on lines -26 to +33
'USD//C'
'USD Coin'
)
export const USDC_GOERLI = new Token(
SupportedChainId.GOERLI,
'0x07865c6e87b9f70255377e024ace6630c1eaa37f',
6,
'USDC',
'USD//C'
'USD Coin'
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works on a fresh page, doesn't seem to if the website was used before...

image

I'm guessing this is because of cached token lists.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, the bad cached name is on my favourite tokens
image

Copy link
Collaborator Author

@alfetopito alfetopito Oct 17, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolved hackishly by mapping token name USD//C to USD Coin 😬

@alfetopito alfetopito added RELEASE Included in the release that is being closed permit labels Oct 17, 2023
@alfetopito alfetopito self-assigned this Oct 17, 2023
@alfetopito alfetopito requested review from a team and shoom3301 October 17, 2023 14:55
@alfetopito alfetopito marked this pull request as ready for review October 17, 2023 14:56
Copy link
Contributor

@anxolin anxolin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great!

// For now, this is a problem for favourite tokens cached locally with the hardcoded name for USDC token
// Using the wrong name breaks the signature.
const [permitParams, chainId, _tokenName, ...rest] = callDataParams
const tokenName = _tokenName === 'USD//C' ? 'USD Coin' : _tokenName
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😅

@alfetopito alfetopito changed the title feat(permit): add support for usdc mainnet fix(permit): add support for usdc mainnet Oct 17, 2023
@@ -19,6 +26,7 @@ export async function buildDaiLikePermitCallData({
callDataParams,
}: BuildDaiLikePermitCallDataParams): Promise<string> {
const callData = await eip2162Utils.buildDaiLikePermitCallData(...callDataParams)
console.log(`[buildDaiLikePermitCallData]`, callDataParams, callData)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, don't forget to remove console.log

@@ -31,3 +32,10 @@ export const ORDER_TYPE_SUPPORTS_PERMIT: Record<TradeType, boolean> = {
}

export const PENDING_ORDER_PERMIT_CHECK_INTERVAL = ms`1min`

// DAI's mainnet contract (https://etherscan.io/address/0x6b175474e89094c44da98b954eedeac495271d0f#readContract) returns
// `1` for the version, while when calling the contract method returns `2`.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I didn't get the problem. I see DAI contract returns version=1, where do we get version=2?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know where it comes from!
But that's what we get when calling the version method 🤷

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fetch("https://rpc.ankr.com/eth", {
  "headers": {
    "accept": "*/*",
    "content-type": "application/json",
  },
  "referrer": "http://localhost:3000/",
  "referrerPolicy": "strict-origin-when-cross-origin",
  "body": "{\"method\":\"eth_call\",\"params\":[{\"to\":\"0x6b175474e89094c44da98b954eedeac495271d0f\",\"data\":\"0x54fd4d50\"},\"latest\"],\"id\":49,\"jsonrpc\":\"2.0\"}",
  "method": "POST",
  "mode": "cors",
  "credentials": "omit"
}).then(res => res.json()).then(console.log)

It's very odd. I really see that version=2 in the JSON rpc response

@alfetopito alfetopito merged commit 8de7cfc into main Oct 18, 2023
7 checks passed
@alfetopito alfetopito deleted the permit/3148_usdc-mainnet branch October 18, 2023 08:07
@github-actions github-actions bot locked and limited conversation to collaborators Oct 18, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
permit RELEASE Included in the release that is being closed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Permit: USDC mainnet
3 participants