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

feat: Prune Paths + Fast ABI #183

Merged
merged 12 commits into from
Apr 28, 2021
Merged

feat: Prune Paths + Fast ABI #183

merged 12 commits into from
Apr 28, 2021

Conversation

dekz
Copy link
Member

@dekz dekz commented Mar 24, 2021

Description

TKR-24

Simbot sells

Prune paths

Prune paths where the rate (bestRate) when selling a fractional amount is less than the adjusted complete rate of selling the entire amount (adjustedCompleteRate) to the best source (sorted by adjustedCompleteRate).

Since selling anything to this path wouldn't result in any overall improvement. We can cut down on the paths to compare.

We should see no regressions, only improved response times (there is the small possibility of improved prices as we have more time to search over less paths)

Fast ABI

Use an ABI encoder/decoder which is more performant than our AbiEncoder (though lacks the transaction gas optimization).

Testing instructions

Types of changes

4000 USDC buy 
  before: 26,
  after: 21
400,000 USDC buy (drops Uni+Sushi, goes hard on Curves + MakerPSM)
  before: 21,
  after: 9
1 ETH sell (drops CryptoCom)
  before: 30,
  after: 26
100 ETH sell (drops Linkswap, Mooni, CryptoCom, Balancer)
  before: 25,
  after: 6
  MarketOperationUtils tests
    optimization
      100 routes
old: 1759ms 56.85 routes/s
new: 199ms 502.51 routes/s
        ✓ DEX QUOTES BNB/BUSD SMALL (1963ms)
old: 6907ms 14.48 routes/s
new: 370ms 270.27 routes/s
        ✓ DEX QUOTES BNB/BUSD LARGE (7283ms)
old: 1661ms 60.2 routes/s
new: 372ms 268.82 routes/s
        ✓ DEX QUOTES DAI/USDC LARGE (2041ms)
old: 22057ms 4.53 routes/s
new: 18470ms 5.41 routes/s
        ✓ DEX QUOTES ETH/DAI LARGE (40531ms)
old: 22361ms 4.47 routes/s
new: 19078ms 5.24 routes/s
NEW IS LARGER BY 0.005474% 45571513355722468522 units
        ✓ DEX QUOTES ETH/DAI LARGE (FREE GAS) (41442ms)
old: 2285ms 43.76 routes/s
new: 2218ms 45.09 routes/s
        ✓ DEX QUOTES ETH/DAI SMALL (4509ms)
old: 2141ms 46.71 routes/s
new: 2067ms 48.38 routes/s
        ✓ DEX QUOTES ETH/USDC SMALL (4213ms)


  7 passing (2m)

Checklist:

  • Prefix PR title with [WIP] if necessary.
  • Add tests to cover changes as needed.
  • Update documentation as needed.
  • Add new entries to the relevant CHANGELOG.jsons.

@dekz dekz force-pushed the bsc-marginal-rate-cull branch 4 times, most recently from 8fbeb3e to 049bec9 Compare March 24, 2021 05:49
@dekz dekz changed the title Filter paths by marginalRate < BestAdjustedRate Filter paths by minRate < BestCompleteAdjustedRate Mar 24, 2021
Base automatically changed from bsc-nerve-dodo to bsc March 25, 2021 04:20
@dekz dekz marked this pull request as draft March 31, 2021 02:26
@dekz dekz changed the title Filter paths by minRate < BestCompleteAdjustedRate Filter paths by minRate < BestCompleteAdjustedRate [TKR-24] Mar 31, 2021
@linear
Copy link

linear bot commented Mar 31, 2021

TKR-24 Prune paths

Prune paths where the smallest sell amount rate is less than the best adjusted rate.

@dekz dekz changed the base branch from bsc to development April 1, 2021 05:13
@dekz dekz force-pushed the bsc-marginal-rate-cull branch 2 times, most recently from 5bfb9f4 to e232e5e Compare April 1, 2021 05:29
@dekz dekz marked this pull request as ready for review April 1, 2021 05:38
Copy link
Contributor

@moodlezoup moodlezoup left a comment

Choose a reason for hiding this comment

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

looks good to me, just a couple of nits

@dekz dekz changed the title Filter paths by minRate < BestCompleteAdjustedRate [TKR-24] Filter paths by path.bestRate < bestCompletePath.completeAdjustedRate [TKR-24] Apr 20, 2021
@dekz dekz requested review from dorothy-zbornak and moodlezoup and removed request for hysz and abandeali1 April 20, 2021 03:46
const sortedPaths = paths.sort((a, b) => b.adjustedCompleteRate().comparedTo(a.adjustedCompleteRate()));
const sortedPaths = paths
.sort((a, b) => b.adjustedCompleteRate().comparedTo(a.adjustedCompleteRate()))
.filter(a => a.adjustedCompleteRate().isGreaterThanOrEqualTo(0));
Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn't it make more sense to have this filter() occur in reducePaths()?

Copy link
Member Author

Choose a reason for hiding this comment

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

image

@dekz dekz changed the title Filter paths by path.bestRate < bestCompletePath.completeAdjustedRate [TKR-24] feat: Prune Paths + Fast ABI Apr 27, 2021
@dekz dekz force-pushed the bsc-marginal-rate-cull branch 2 times, most recently from fa56749 to 248e5ab Compare April 27, 2021 11:15
@dekz dekz merged commit cd296b8 into development Apr 28, 2021
@dekz dekz deleted the bsc-marginal-rate-cull branch April 28, 2021 07:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants