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: multicall return gas used in case of failure #534

Merged

jsy1218 merged 4 commits into main from jsy1218/multicall-return-gas-used-in-case-of-failure

Apr 17, 2024

Member

jsy1218 commented Apr 17, 2024

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
feature
What is the current behavior? (You can also link to an open issue here)
Meanwhile we are comparing the current quoter quotes vs new view-only quoter quotes in prod in shadow sampling for mainnet at 1%, we encounter a case, where we have some quotes that are failing due to the out-of-gas from multicall (e.g. this simulation), but from routing-api side, we don't have a good way to programmatically determine this is the case, within compareQuotes.

This is the case where some quotes from multicalls fail, but overall success rate is still above the threshold. This is different from the entire multicall failures due to out of gas. In that case, on-chain quote provider will throw ProviderOutOfGasError, and routing-api can generalize to where currentQuoterSucceeded = false

What is the new behavior (if this is a feature change)?
Within SOR, we will return both gasUsed and gasLimit per call from the multicalls, so that routing-api can calculate that the difference between gasUsed and gasLimit may be small enough to be due to out of gas.
Other information:
This works in tendom with routing-api changes.

jsy1218 self-assigned this

jsy1218 requested a review from a team as a code owner

April 17, 2024 05:41

jsy1218 mentioned this pull request

fix: current quoter out-of-gas quote comparison Uniswap/routing-api#593

Merged

cgkol approved these changes

View reviewed changes

Contributor

cgkol left a comment

When are we planning on doing this?

 so that routing-api can calculate that the difference between gasUsed and gasLimit may be small enough to be due to out of gas

Member Author

jsy1218 commented Apr 17, 2024

When are we planning on doing this?

 so that routing-api can calculate that the difference between gasUsed and gasLimit may be small enough to be due to out of gas

Sometime today. I have the routing-api draft PR, just need to add some test

jsy1218 added 4 commits

April 17, 2024 08:40


return gas used from multicall in case of a failed quote

d5e867a


fix compiling error

f4fe049


add target contract address to the failed multicall debug log

189130c


also return gas limit

83f4374

jsy1218 force-pushed the jsy1218/multicall-return-gas-used-in-case-of-failure branch from d0677c4 to 83f4374 Compare

April 17, 2024 15:41

jsy1218 merged commit 9a3065a into main

23 of 24 checks passed

jsy1218 deleted the jsy1218/multicall-return-gas-used-in-case-of-failure branch

April 17, 2024 15:55

jsy1218 mentioned this pull request

chore: bump to v3.28.1 #535

Merged

jsy1218 added a commit that referenced this pull request


chore: bump to v3.28.1 (#535)

452c55a

- **What kind of change does this PR introduce?** (Bug fix, feature, docs update, ...)
Release #534

jsy1218 added a commit to Uniswap/routing-api that referenced this pull request


fix: current quoter out-of-gas quote comparison (#593)

267ec26

Once SOR returns per-quote-call gasUsed and gasLimit (Uniswap/smart-order-router#534), then routing-api can calculate per-quote call failure is due to likely out-of-gas error. 

I was deciding between using gasUsed and gasLimit vs the encoded error from the return data, but the [simulation](https://www.tdly.co/shared/simulation/39de04bb-d794-43b0-aed2-8ea31957ca77) shows me that the encoded error from the return data might not be accurate. For this simulation, the error indicates [SLP](https://docs.uniswap.org/contracts/v3/reference/error-codes), which is not true:

 ```
cast 4byte-decode 0x08c379a00000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000000353504c0000000000000000000000000000000000000000000000000000000000
1) "Error(string)"
"SPL"
```

Now testing coverage added.

jsy1218 added a commit that referenced this pull request


fix: multicall return gas used in case of failure (#534)

721bf14

* return gas used from multicall in case of a failed quote

* fix compiling error

* add target contract address to the failed multicall debug log

* also return gas limit

jsy1218 added a commit that referenced this pull request


chore: bump to v3.28.1 (#535)

f404877

- **What kind of change does this PR introduce?** (Bug fix, feature, docs update, ...)
Release #534

jsy1218 added a commit that referenced this pull request


fix: multicall return gas used in case of failure (#534)

483607a

* return gas used from multicall in case of a failed quote

* fix compiling error

* add target contract address to the failed multicall debug log

* also return gas limit

jsy1218 added a commit that referenced this pull request


fix: multicall return gas used in case of failure (#534)

42dd561

* return gas used from multicall in case of a failed quote

* fix compiling error

* add target contract address to the failed multicall debug log

* also return gas limit

jsy1218 added a commit that referenced this pull request


chore: log quote sucecss rate metric (#563)

ad23481

* fix: multicall return gas used in case of failure (#534)

* return gas used from multicall in case of a failed quote

* fix compiling error

* add target contract address to the failed multicall debug log

* also return gas limit

* fix: multicall return gas used in case of failure (#534)

* return gas used from multicall in case of a failed quote

* fix compiling error

* add target contract address to the failed multicall debug log

* also return gas limit

* log quote success rate metric

jsy1218 added a commit that referenced this pull request


fix: multicall return gas used in case of failure (#534)

9f2ba06

* return gas used from multicall in case of a failed quote

* fix compiling error

* add target contract address to the failed multicall debug log

* also return gas limit

jsy1218 added a commit that referenced this pull request


chore: bump to v3.28.11 (#565)

9df621c

* fix: multicall return gas used in case of failure (#534)

* return gas used from multicall in case of a failed quote

* fix compiling error

* add target contract address to the failed multicall debug log

* also return gas limit

* 3.28.11

jsy1218 added a commit that referenced this pull request


fix: multicall return gas used in case of failure (#534)

87f15b6

* return gas used from multicall in case of a failed quote

* fix compiling error

* add target contract address to the failed multicall debug log

* also return gas limit

jsy1218 added a commit that referenced this pull request


chore: bump to v3.28.1 (#535)

a640459

- **What kind of change does this PR introduce?** (Bug fix, feature, docs update, ...)
Release #534

jsy1218 added a commit that referenced this pull request


chore: log quote sucecss rate metric (#563)

55ac717

* fix: multicall return gas used in case of failure (#534)

* return gas used from multicall in case of a failed quote

* fix compiling error

* add target contract address to the failed multicall debug log

* also return gas limit

* fix: multicall return gas used in case of failure (#534)

* return gas used from multicall in case of a failed quote

* fix compiling error

* add target contract address to the failed multicall debug log

* also return gas limit

* log quote success rate metric

jsy1218 added a commit that referenced this pull request


chore: bump to v3.28.11 (#565)

6a2b2d2

* fix: multicall return gas used in case of failure (#534)

* return gas used from multicall in case of a failed quote

* fix compiling error

* add target contract address to the failed multicall debug log

* also return gas limit

* 3.28.11

jsy1218 added a commit that referenced this pull request


fix: multicall return gas used in case of failure (#534)

0481ef8

* return gas used from multicall in case of a failed quote

* fix compiling error

* add target contract address to the failed multicall debug log

* also return gas limit

jsy1218 added a commit that referenced this pull request


chore: log optimistic quote metrics prefix in onchain quoting (#568)

1e86780

* fix: multicall return gas used in case of failure (#534)

* return gas used from multicall in case of a failed quote

* fix compiling error

* add target contract address to the failed multicall debug log

* also return gas limit

* log optimistic quote metrics prefix in onchain quoting

* fix prettier

* add underscore metrics prefix

* use provider config

jsy1218 added a commit that referenced this pull request


fix: multicall return gas used in case of failure (#534)

* return gas used from multicall in case of a failed quote

* fix compiling error

* add target contract address to the failed multicall debug log

* also return gas limit

jsy1218 added a commit that referenced this pull request


chore: bump to v3.28.1 (#535)

8c73a42

- **What kind of change does this PR introduce?** (Bug fix, feature, docs update, ...)
Release #534

jsy1218 added a commit that referenced this pull request


chore: log quote sucecss rate metric (#563)

34c9967

* fix: multicall return gas used in case of failure (#534)

* return gas used from multicall in case of a failed quote

* fix compiling error

* add target contract address to the failed multicall debug log

* also return gas limit

* fix: multicall return gas used in case of failure (#534)

* return gas used from multicall in case of a failed quote

* fix compiling error

* add target contract address to the failed multicall debug log

* also return gas limit

* log quote success rate metric

jsy1218 added a commit that referenced this pull request


chore: bump to v3.28.11 (#565)

952ce75

* fix: multicall return gas used in case of failure (#534)

* return gas used from multicall in case of a failed quote

* fix compiling error

* add target contract address to the failed multicall debug log

* also return gas limit

* 3.28.11

jsy1218 added a commit that referenced this pull request


chore: log optimistic quote metrics prefix in onchain quoting (#568)

5b8791c

* fix: multicall return gas used in case of failure (#534)

* return gas used from multicall in case of a failed quote

* fix compiling error

* add target contract address to the failed multicall debug log

* also return gas limit

* log optimistic quote metrics prefix in onchain quoting

* fix prettier

* add underscore metrics prefix

* use provider config

jsy1218 added a commit that referenced this pull request


fix: multicall return gas used in case of failure (#534)

ca9977f

* return gas used from multicall in case of a failed quote

* fix compiling error

* add target contract address to the failed multicall debug log

* also return gas limit

jsy1218 added a commit that referenced this pull request


chore: onchain quoter batch param differed by optimistic cached routes (

e339c03

#570)

* fix: multicall return gas used in case of failure (#534)

* return gas used from multicall in case of a failed quote

* fix compiling error

* add target contract address to the failed multicall debug log

* also return gas limit

* onchain quoter batch param differed by optimistic cached routes

* fix prettier onchain quoter

* fix build

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