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.

Sign up for GitHub

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

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
2290c66
chore: remove unused prop `gasLimit` from SupportedPermitInfo
alfetopito Oct 16, 2023
d9703c4
feat: query and add version to permitInfo
alfetopito Oct 16, 2023
c08c353
feat: bump permittableTokensAtom storage key to `v1`
alfetopito Oct 16, 2023
eb81428
fix: usdc default tokens had the wrong name, causing the permit signa…
alfetopito Oct 17, 2023
e24f03a
chore: temporarily logging permit callData and params
alfetopito Oct 17, 2023
020e7e8
fix: hack to fix USDC token name when creating the permit callData
alfetopito Oct 17, 2023
458d259
feat: do not check for permit version if token cannot handle it
alfetopito Oct 17, 2023
86e48b3
chore: remove console logs
alfetopito Oct 18, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions apps/cowswap-frontend/src/modules/permit/const.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { DAI } from '@cowprotocol/common-const'
import { SupportedChainId } from '@cowprotocol/cow-sdk'
import { MaxUint256 } from '@ethersproject/constants'
import { Wallet } from '@ethersproject/wallet'
Expand Down Expand Up @@ -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

// Also, if we use the version returned by the contract, it simply doesn't work
// Thus, do not call it for DAI.
// TODO: figure out whether more tokens behave the same way
export const TOKENS_TO_SKIP_VERSION = new Set([DAI.address.toLowerCase()])
2 changes: 1 addition & 1 deletion apps/cowswap-frontend/src/modules/permit/state/permittableTokensAtom.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { AddPermitTokenParams, PermittableTokens } from '../types'
* Contains either the permit info with `type` and `gasLimit` when supported or
* `false` when not supported
*/
export const permittableTokensAtom = atomWithStorage<PermittableTokens>('permittableTokens:v0', {
export const permittableTokensAtom = atomWithStorage<PermittableTokens>('permittableTokens:v1', {
[SupportedChainId.MAINNET]: {},
[SupportedChainId.GOERLI]: {},
[SupportedChainId.GNOSIS_CHAIN]: {},
Expand Down
2 changes: 1 addition & 1 deletion apps/cowswap-frontend/src/modules/permit/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ export type PermitType = 'dai-like' | 'eip-2612'

export type SupportedPermitInfo = {
type: PermitType
gasLimit: number
version: string | undefined // Some tokens have it different than `1`, and won't work without it
}
type UnsupportedPermitInfo = false
export type PermitInfo = SupportedPermitInfo | UnsupportedPermitInfo
Expand Down
8 changes: 7 additions & 1 deletion apps/cowswap-frontend/src/modules/permit/utils/buildPermitCallData.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,14 @@ export async function buildEip2162PermitCallData({
eip2162Utils,
callDataParams,
}: BuildEip2162PermitCallDataParams): Promise<string> {
const callData = await eip2162Utils.buildPermitCallData(...callDataParams)
// TODO: this is ugly and I'm not happy with it either
// It'll probably go away when the tokens overhaul is implemented
// 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.

😅


const callData = await eip2162Utils.buildPermitCallData(permitParams, chainId, tokenName, ...rest)
// For some reason, the method above removes the permit selector prefix
// https://github.com/1inch/permit-signed-approvals-utils/blob/master/src/eip-2612-permit.utils.ts#L92
// Adding it back
Expand Down
30 changes: 25 additions & 5 deletions apps/cowswap-frontend/src/modules/permit/utils/checkIsTokenPermittable.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { buildDaiLikePermitCallData, buildEip2162PermitCallData } from './buildP
import { getPermitDeadline } from './getPermitDeadline'
import { getPermitUtilsInstance } from './getPermitUtilsInstance'

import { DEFAULT_PERMIT_VALUE, PERMIT_GAS_LIMIT_MIN, PERMIT_SIGNER } from '../const'
import { DEFAULT_PERMIT_VALUE, PERMIT_GAS_LIMIT_MIN, PERMIT_SIGNER, TOKENS_TO_SKIP_VERSION } from '../const'
import { CheckIsTokenPermittableParams, EstimatePermitResult, PermitType } from '../types'

const EIP_2162_PERMIT_PARAMS = {
Expand Down Expand Up @@ -74,6 +74,22 @@ async function actuallyCheckTokenIsPermittable(params: CheckIsTokenPermittablePa
return { error: e.message || e.toString() }
}

let version: string | undefined = undefined

if (!TOKENS_TO_SKIP_VERSION.has(tokenAddress)) {
// If the token does not outright fails when calling with the `version` value
// returned by the contract, fetch it.

try {
// Required by USDC-mainnet as its version is `2`.
// There might be other tokens that need this as well.
version = await eip2612PermitUtils.getTokenVersion(tokenAddress)
} catch (e) {
// Not a problem, we can (try to) continue without it, and will default to `1` (part of the 1inch lib)
console.debug(`[checkTokenIsPermittable] Failed to get version for ${tokenAddress}`, e)
}
}

const baseParams: BaseParams = {
chainId,
eip2612PermitUtils,
Expand All @@ -82,6 +98,7 @@ async function actuallyCheckTokenIsPermittable(params: CheckIsTokenPermittablePa
tokenAddress,
tokenName,
walletAddress: owner,
version,
}

try {
Expand All @@ -108,6 +125,7 @@ type BaseParams = {
spender: string
eip2612PermitUtils: Eip2612PermitUtils
nonce: number
version: string | undefined
}

type EstimateParams = BaseParams & {
Expand All @@ -116,7 +134,7 @@ type EstimateParams = BaseParams & {
}

async function estimateTokenPermit(params: EstimateParams): Promise<EstimatePermitResult> {
const { provider, chainId, walletAddress, tokenAddress, type } = params
const { provider, chainId, walletAddress, tokenAddress, type, version } = params

const getCallDataFn = type === 'eip-2612' ? getEip2612CallData : getDaiLikeCallData

Expand All @@ -136,14 +154,14 @@ async function estimateTokenPermit(params: EstimateParams): Promise<EstimatePerm

return gasLimit > PERMIT_GAS_LIMIT_MIN[chainId]
? {
gasLimit,
type,
version,
}
: false
}

async function getEip2612CallData(params: BaseParams): Promise<string> {
const { eip2612PermitUtils, walletAddress, spender, nonce, chainId, tokenName, tokenAddress } = params
const { eip2612PermitUtils, walletAddress, spender, nonce, chainId, tokenName, tokenAddress, version } = params
return buildEip2162PermitCallData({
eip2162Utils: eip2612PermitUtils,
callDataParams: [
Expand All @@ -156,12 +174,13 @@ async function getEip2612CallData(params: BaseParams): Promise<string> {
+chainId,
tokenName,
tokenAddress,
version,
],
})
}

async function getDaiLikeCallData(params: BaseParams): Promise<string | false> {
const { eip2612PermitUtils, tokenAddress, walletAddress, spender, nonce, chainId, tokenName } = params
const { eip2612PermitUtils, tokenAddress, walletAddress, spender, nonce, chainId, tokenName, version } = params

const permitTypeHash = await eip2612PermitUtils.getPermitTypeHash(tokenAddress)

Expand All @@ -178,6 +197,7 @@ async function getDaiLikeCallData(params: BaseParams): Promise<string | false> {
chainId as number,
tokenName,
tokenAddress,
version,
],
})
}
Expand Down
2 changes: 2 additions & 0 deletions apps/cowswap-frontend/src/modules/permit/utils/generatePermitHook.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ async function generatePermitHookRaw(params: PermitHookParams): Promise<PermitHo
chainId as number,
tokenName,
tokenAddress,
permitInfo.version,
],
})
: await buildDaiLikePermitCallData({
Expand All @@ -81,6 +82,7 @@ async function generatePermitHookRaw(params: PermitHookParams): Promise<PermitHo
chainId as number,
tokenName,
tokenAddress,
permitInfo.version,
],
})

Expand Down
4 changes: 2 additions & 2 deletions libs/common-const/src/tokens.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,14 @@ export const USDC_MAINNET = new Token(
'0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48',
6,
'USDC',
'USD//C'
'USD Coin'
)
export const USDC_GOERLI = new Token(
SupportedChainId.GOERLI,
'0x07865c6e87b9f70255377e024ace6630c1eaa37f',
6,
'USDC',
'USD//C'
'USD Coin'
Comment on lines -26 to +33
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 •
edited
Loading

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 😬

)
export const DAI = new Token(
SupportedChainId.MAINNET,
Expand Down
Loading