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): improve permittable detection #3226

Merged
merged 9 commits into from
Oct 16, 2023
Merged

fix(permit): improve permittable detection #3226

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
4dce01a
chore: return `false` only when it's a known failure
alfetopito Oct 13, 2023
a18d1d3
refactor: add return type
alfetopito Oct 13, 2023
fae25d5
chore: improve permit detection logging
alfetopito Oct 13, 2023
94eefc8
chore: update every package related to @web3-react/walletconnect-v2
alfetopito Oct 13, 2023
c010d4f
chore: add more logging to permit utils creation
alfetopito Oct 13, 2023
abe22f6
fix: lib @walletconnect/ethereum-provider path changed
alfetopito Oct 13, 2023
3feb917
fix: remove aliases no longer needed
alfetopito Oct 13, 2023
520fc29
fix: update async connector provider on network changes from wallet
shoom3301 Oct 16, 2023
2ea49c9
chore: reduce some logging
alfetopito Oct 16, 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
14 changes: 12 additions & 2 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 @@ -61,9 +61,17 @@ async function actuallyCheckTokenIsPermittable(params: CheckIsTokenPermittablePa
try {
nonce = await eip2612PermitUtils.getTokenNonce(tokenAddress, owner)
} catch (e) {
if (e === 'nonce not supported' || e.message === 'nonce is NaN') {
console.debug(`[checkTokenIsPermittable] Not a permittable token ${tokenAddress}`, e?.message || e)
// Here we know it's not supported, return false
// See https://github.com/1inch/permit-signed-approvals-utils/blob/b190197a45c3289867ee4e6da93f10dea51ef276/src/eip-2612-permit.utils.ts#L309
// and https://github.com/1inch/permit-signed-approvals-utils/blob/b190197a45c3289867ee4e6da93f10dea51ef276/src/eip-2612-permit.utils.ts#L325
return false
}
console.debug(`[checkTokenIsPermittable] Failed to get nonce for ${tokenAddress}`, e)

return false
// Otherwise, it might have been a network issue or another temporary failure, return error
return { error: e.message || e.toString() }
}

const baseParams: BaseParams = {
Expand All @@ -81,10 +89,12 @@ async function actuallyCheckTokenIsPermittable(params: CheckIsTokenPermittablePa
return await estimateTokenPermit({ ...baseParams, type: 'eip-2612', provider })
} catch (e) {
// Not eip-2612, try dai-like
console.debug(`[checkTokenIsPermittable] Failed to estimate eip-2612 permit for ${tokenAddress}`, e)
try {
return await estimateTokenPermit({ ...baseParams, type: 'dai-like', provider })
} catch (e) {
// Not dai-like either, return error
console.debug(`[checkTokenIsPermittable] Failed to estimate dai-like permit for ${tokenAddress}`, e)
return { error: e.message || e.toString() }
}
}
Expand All @@ -105,7 +115,7 @@ type EstimateParams = BaseParams & {
provider: Web3Provider
}

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

const getCallDataFn = type === 'eip-2612' ? getEip2612CallData : getDaiLikeCallData
Expand Down
12 changes: 9 additions & 3 deletions apps/cowswap-frontend/src/modules/permit/utils/getPermitUtilsInstance.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ const CHAIN_UTILS_CACHE = new Map<number, Eip2612PermitUtils>()
/**
* Cache by provider. Here we cache per provider as each account should have its own instance
*/
const PROVIDER_UTILS_CACHE = new Map<Web3Provider, Eip2612PermitUtils>()
const PROVIDER_UTILS_CACHE = new Map<string, Eip2612PermitUtils>()

export function getPermitUtilsInstance(
chainId: SupportedChainId,
Expand All @@ -26,7 +26,8 @@ export function getPermitUtilsInstance(
if (!account && chainCache) {
return chainCache
}
const providerCache = PROVIDER_UTILS_CACHE.get(provider)
const providerCacheKey = `${chainId}-${account}`
const providerCache = PROVIDER_UTILS_CACHE.get(providerCacheKey)

if (providerCache) {
return providerCache
Expand All @@ -36,9 +37,14 @@ export function getPermitUtilsInstance(
const eip2612PermitUtils = new Eip2612PermitUtils(web3ProviderConnector)

if (!account) {
console.log(`[getPermitUtilsInstance] Set cached chain utils for chain ${chainId}`, eip2612PermitUtils)
CHAIN_UTILS_CACHE.set(chainId, eip2612PermitUtils)
} else {
PROVIDER_UTILS_CACHE.set(provider, eip2612PermitUtils)
console.log(
`[getPermitUtilsInstance] Set cached provider utils for chain ${chainId}-${account}`,
eip2612PermitUtils
)
PROVIDER_UTILS_CACHE.set(providerCacheKey, eip2612PermitUtils)
}

return eip2612PermitUtils
Expand Down
14 changes: 0 additions & 14 deletions apps/cowswap-frontend/vite.config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@ import { VitePWA } from 'vite-plugin-pwa'
import svgr from 'vite-plugin-svgr'
import viteTsConfigPaths from 'vite-tsconfig-paths'

import * as path from 'path'

import { getReactProcessEnv } from '../../tools/getReactProcessEnv'

// eslint-disable-next-line no-restricted-imports
Expand Down Expand Up @@ -102,18 +100,6 @@ export default defineConfig(({ mode }) => {
resolve: {
alias: {
'node-fetch': 'isomorphic-fetch',
/**
* Temporary fix for walletconnect
* https://github.com/Uniswap/web3-react/issues/861
*/
'@walletconnect/ethereum-provider': path.resolve(
__dirname,
'../../node_modules/@walletconnect/ethereum-provider/dist/umd/index.min.js'
),
'@walletconnect/universal-provider': path.resolve(
__dirname,
'../../node_modules/@walletconnect/universal-provider/dist/index.cjs.js'
),
},
},

Expand Down
5 changes: 5 additions & 0 deletions libs/wallet/src/web3-react/connection/asyncConnector.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,11 @@ function initCustomProvider(self: AsyncConnector, connector: Connector, chainId:
}

self.events.emit(ASYNC_CUSTOM_PROVIDER_EVENT, self.customProvider)

// Update provider when network is changed on wallet side
connector.provider?.on('chainChanged', (chainIdHex: string) => {
initCustomProvider(self, connector, +chainIdHex)
})
Comment on lines +14 to +18
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 is the actual fix!

Thanks Sasha!

Everything else is not necessary but good to have as improvements.

}

/**
Expand Down
22 changes: 11 additions & 11 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,17 +88,16 @@
"@uniswap/sdk-core": "^3.0.1",
"@uniswap/token-lists": "^1.0.0-beta.30",
"@use-gesture/react": "^10.2.23",
"@web3-react/coinbase-wallet": "^8.2.0",
"@web3-react/core": "^8.2.0",
"@web3-react/eip1193": "^8.2.0",
"@web3-react/empty": "^8.2.0",
"@web3-react/gnosis-safe": "^8.2.0",
"@web3-react/metamask": "^8.2.1",
"@web3-react/network": "^8.2.0",
"@web3-react/types": "^8.2.0",
"@web3-react/url": "^8.2.0",
"@web3-react/walletconnect": "^8.2.0",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No need for this anymore, right?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yep

"@web3-react/walletconnect-v2": "^8.3.7",
"@walletconnect/ethereum-provider": "^2.10.2",
"@web3-react/coinbase-wallet": "^8.2.3",
"@web3-react/core": "^8.2.3",
"@web3-react/eip1193": "^8.2.3",
"@web3-react/empty": "^8.2.3",
"@web3-react/gnosis-safe": "^8.2.4",
"@web3-react/metamask": "^8.2.4",
"@web3-react/network": "^8.2.3",
"@web3-react/url": "^8.2.3",
"@web3-react/walletconnect-v2": "^8.5.1",
"bnc-sdk": "^4.6.0",
"buffer": "^6.0.3",
"cids": "^1.0.0",
Expand Down Expand Up @@ -214,6 +213,7 @@
"@vitejs/plugin-react-swc": "^3.3.2",
"@vitest/coverage-c8": "~0.32.0",
"@vitest/ui": "~0.32.0",
"@web3-react/types": "^8.2.3",
"babel-jest": "^29.6.2",
"babel-plugin-styled-components": "2.1.4",
"babel-plugin-transform-import-meta": "^2.2.0",
Expand Down
11 changes: 0 additions & 11 deletions patches/@walletconnect+universal-provider+2.9.2.patch
View file
Open in desktop

This file was deleted.

13 changes: 0 additions & 13 deletions patches/@web3-react+walletconnect-v2+8.3.7.patch
View file
Open in desktop

This file was deleted.

Loading
Loading