Revert "Merge pull request #7 from gnosis/gas-estimate-offset" #14
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In #7 solutions that were flagged as market orders also got a surplus fee computed if they have a signed fee of 0 which should theoretically be correct for regular orders.
However, for quote requests we build fake auctions that contain a single
marketorder with a signed fee of0. With that change the solver tried to compute a surplus fee for those orders as well. But computing a surplus fee requires a reference price to know how expensive a quote would be to execute (to know what fee would be fair). This reference price does not exist for native price requests (i.e. sell token to buy exactly 0.1 WETH).So effectively the PR made dex solvers unusable as native price estimators. This was not caught because there is no test specifically for native price estimates.
Since this issues is blocking some other development reverting seems the most reasonable option for now. Making only the settlement overhead adjustable (without changing the surplus fee logic) can happen in a follow up PR.
This commit was created with
git revert 1740937 -m 1but had slight conflicts (mostly involving the removal of thescorefield).