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

Forward compatible with ERC20 which isfunction symbol() view returns (bytes32) #47

Open
hujw77 opened this issue Dec 29, 2021 · 2 comments
Assignees
Labels
bug Something isn't working p0

Comments

@hujw77
Copy link

hujw77 commented Dec 29, 2021

  • I'm submitting a bug report and feature request
    When I trade with RING/ETH pair at app.uniswap.org. An HTTP 500 error occur.
~ curl 'https://api.uniswap.org/v1/quote?protocols=v2%2Cv3&tokenInAddress=0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2&tokenInChainId=1&tokenOutAddress=0x9469D013805bFfB7D3DEBe5E7839237e535ec483&tokenOutChainId=1&amount=11613710000000&type=exactIn' \
        -H 'authority: api.uniswap.org' \
        -H 'pragma: no-cache' \
        -H 'cache-control: no-cache' \
        -H 'accept: */*' \
        -H 'origin: https://app.uniswap.org' \
        -H 'sec-fetch-site: same-site' \
        -H 'sec-fetch-mode: cors' \
        -H 'sec-fetch-dest: empty' \
        -H 'referer: https://app.uniswap.org/' \
        --compressed

{"errorCode":"INTERNAL_ERROR","detail":"Unexpected error","id":"1e381"}

I figure out that is a smart-order-router API query, and then I try it by smart-order-router CLI.

~ ./bin/cli quote --tokenIn 0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2 --tokenOut 0x9469d013805bffb7d3debe5e7839237e535ec483 --amount 1000 --exactIn --recipient 0xAb5801a7D398351b8bE11C439e05C5B3259aeC9B --protocols v2,v3 --debug
INFO:  [Metric]: TokenListLoad: 1 | Milliseconds
  key: "TokenListLoad"
  value: 1
  unit: "Milliseconds"
INFO:  Found 1 out of 2 tokens in local cache. Checking primary token provider for 1 tokens

  addressesToFindInPrimary: ["0x9469d013805bffb7d3debe5e7839237e535ec483"]
INFO:  Found 0 tokens in primary. Checking secondary token provider for 1 tokens
  addressesToFindInSecondary: ["0x9469d013805bffb7d3debe5e7839237e535ec483"]
DEBUG: About to multicall for symbol across 1 addresses
  calls: [{"target":"0x9469d013805bffb7d3debe5e7839237e535ec483","callData":"0x95d89b41...
DEBUG: About to multicall for decimals across 1 addresses
  calls: [{"target":"0x9469d013805bffb7d3debe5e7839237e535ec483","callData":"0x313ce567...
DEBUG: Results for multicall on decimals across 1 addresses as of block 13900877
  results: [{"success":true,"result":[18]}]
    Error: call revert exception (method="symbol()", errorArgs=null, errorName=null,
    errorSignature=null, reason=null, code=CALL_EXCEPTION, version=abi/5.5.0)
    Code: CALL_EXCEPTION

A call revert error occurred, I think the reason is RING's symbol type is bytes32 not the string. This is a historical reason. dapphub/ds-token#28
And there is a lot of token like MKR have the same issue, but the MKR token is in the @uniswap/default-token-list, so it has no effect on MRK.
But other tokens like RING got hit. And I file an issue at Uniswap/default-token-list#1031.
To verify my idea, I added RING to @uniswap/default-token-list locally. It works 🚀

~ ./bin/cli quote --tokenIn 0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2 --tokenOut 0x9469d013805bffb7d3debe5e7839237e535ec483 --amount 1000 --exactIn --recipient 0xAb5801a7D398351b8bE11C439e05C5B3259aeC9B --protocols v2,v3
Best Route:
[V2] 85.00% = WETH --> RING, [V3] 15.00% = WETH -- 0.3% --> RING
        Raw Quote Exact In:
                16472715.82
        Gas Adjusted Quote In:
                16470330.17

Gas Used Quote Token: 2385.644467
Gas Used USD: 104.133692
Calldata: 0x5ae401dc0000000000000000000000000000000000000000000000000000000000000064000000000000000000000000000000000000000000000000000000000000004000000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000040000000000000000000000000000000000000000000000000000000000000016000000000000000000000000000000000000000000000000000000000000000e4472b43f300000000000000000000000000000000000000000000002e141ea081ca0800000000000000000000000000000000000000000000000b658b561723e7673ef0a70000000000000000000000000000000000000000000000000000000000000080000000000000000000000000ab5801a7d398351b8be11c439e05c5b3259aec9b0000000000000000000000000000000000000000000000000000000000000002000000000000000000000000c02aaa39b223fe8d0a0e5c4f27ead9083c756cc20000000000000000000000009469d013805bffb7d3debe5e7839237e535ec4830000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000e404e45aaf000000000000000000000000c02aaa39b223fe8d0a0e5c4f27ead9083c756cc20000000000000000000000009469d013805bffb7d3debe5e7839237e535ec4830000000000000000000000000000000000000000000000000000000000000bb8000000000000000000000000ab5801a7d398351b8be11c439e05c5b3259aec9b00000000000000000000000000000000000000000000000821ab0d44149800000000000000000000000000000000000000000000000238f21b6c7dcd9b6dd4cb000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
Value: 0x00

  blockNumber: "13900919"
  estimatedGasUsed: "228000"
  gasPriceWei: "121520560130"
@willpote
Copy link
Contributor

Thanks for digging into this and providing such a detailed report!

It would be best to support ERC20 with symbol() view returns (bytes32) but I can't think of an elegant way to do this given the current code (without adding retries).

We generally do not add tokens to the Uniswap default list and prefer users to use community owned lists, so that option is unlikely.

For now, I went with a third option which adds the token to the smart-order-router's hardcoded list of known tokens. We use this list, and the uniswap default token list, to seed our cache of token metadata.
5e60d46

We will release this to the API in the next few days so RING should be swappable soon

@hujw77
Copy link
Author

hujw77 commented Jan 13, 2022

Thanks for the commit 5e60d46.

It would be best to support ERC20 with symbol() view returns (bytes32) but I can't think of an elegant way to do this given the current code (without adding retries).

I agree with this approach. Because of there is a lot of token still have this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working p0
Projects
None yet
Development

No branches or pull requests

4 participants