feat: Waterfill - Slim Shady version - #433
Conversation
f166408 to
4655f3c
Compare
I treated the slow performance of this algorithm as if it was an offense to my mother, so I dig deeply in how to improve it. Compared to current main, it is like a 3x~5x speed. We will see once this is in dev for sure. The code was quite the AI spaghetti, with a lot of bad names, weird design and separation, questionable abstractions, etc. I had to work on these to even understand what the code was doing, before I could work on performance. What you see in this commit is a whole mix, I decided not to separate the AI clean-up from actual logic changes because it would take me another week to organize that in proper commits. This is far from ideal, so in an effort to make you focus on the important things to review I will describe the actual FEAT changes, and point at the code for you to look at. Here we go: swap_cache.rs - new file with a pool swap cache for repeated swap amounts can also interpolate. Interpolating was key here, for very small changes in input amount in relation to the cached value interpolation - caching can also interpolate amounts smartly. If amounts are close to the cached value and a boolean flag is used, a linear interpolation will be used to save from a pool simulation. errors and are also handed smartly. this also lives in swap_cache.rs. waterfill has solve "stages" and each of them decides whether interpolation can be used or not. the idea is to use interpolation when you dont need exact precision. we skip paths that use the same pool twice (at rank_at_full_amount). This is an edge case (very few >=3 token pools) and I dont think it ever makes sense to swap twice on the same pool, when you could just go to the final token directly. waterfill's single-path baseline is now decided based on this cache+interpolation ranking. results might change slightly but from running the benchmark there was no change at all in WF for 10k orders. Reuse some swap results at the chunking stages of waterfill. Here it is harder to use caching because amounts are quite different, we cannot interpolate, and state changes with each iteration. But still could take away some repetition from it. Timeout refactoring allows for finer resolution on solve tiems New swap-metrics feature, disabled by default, adds a MeteredProtocolSim which collects metrics in a thread-local cache for measuring specific protocol usage and solve time. Useful for local debugging. By default it is not activated.
The candidate ordering the split passes read was gross output, gas never subtracted, while the single-path baseline and the finished split were both judged net of gas. A three-hop route pays more gross than a one-hop route and costs three swaps; on net the short one wins, on gross it does not. Enough long routes above it and the short one falls out of the windows `select_disjoint` and `select_shared_candidates` slice off `by_output` — so the splits were built without the route that was about to beat them. `by_output` now holds `Option<BigInt>`: the net output of a path that filled, and nothing for a path that could not. Sorting descending keeps the filled paths in front, ordered by what they really pay, and leaves the unfilled ones last as before. A plain zero would not do that — net output goes negative once gas costs more than a path pays, and a path that filled for a loss still tells the split passes more than one that failed outright. What still separates the two orderings is membership: only paths that took the whole order can become the single-path baseline. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Interpolating and caching all swaps (not only the winners) will save us some pool simulations on ML as well.
9e86b1a to
d5e3a91
Compare
louise-poole
left a comment
There was a problem hiding this comment.
Veeeeeery nice! 🔥 Just commented a couple smaller things, have approved in the meantime.
| .map(|result| SwapResult { amount_out: result.amount, gas: result.gas }) | ||
| .map_err(|error| Refusal::of(&error)) | ||
| }, | ||
| true, |
There was a problem hiding this comment.
Interpolation is enabled here. This I rate is fine for an initial pass, but I see on water fill you properly re-simulate the top 8 ranked paths to get a true ranking of them. Maybe we should do the same here?
There was a problem hiding this comment.
yeah I think it is fine, ML is doing well without this extra simulation
| &committed[i], | ||
| chunk.clone(), | ||
| ) else { | ||
| if marginals[i].is_none() { |
There was a problem hiding this comment.
This could be None if the chunk failed to simulate no? Then we'd potentially retry that same path multiple times and fail each time.... Would be good if we could differentiate between 'failed' and 'haven't tried yet'.
There was a problem hiding this comment.
I tried this suggestion. for some reason it got a bit slower overall.. so not applying
Waterfill Slim Shady Version
I treated the slow performance of this algorithm as if it was an offense
to my mother, so I dig deeply in how to improve it. Compared to current
main, it is like a 3x~5x speed. We will see once this is in dev for sure.
The code was quite the AI spaghetti, with a lot of bad names, weird
design and separation, questionable abstractions, etc. I had to work
on these to even understand what the code was doing, before I could
work on performance.
What you see in this commit is a whole mix, I decided not to separate the
AI clean-up from actual logic changes because it would take me
another week to organize that in proper commits. This is far from ideal,
so in an effort to make you focus on the important things to review I will
describe the actual FEAT changes, and point at the code for you to look at.
Here we go:
swap_cache.rs- new file with a pool swap cache for repeated swap amounts can also interpolate. Interpolating was key here, for very small changes in input amount in relation to the cached valueswap_cache.rs. waterfill has solve "stages" and each of them decides whether interpolation can be used or not. the idea is to use interpolation when you dont need exact precision.rank_at_full_amount). This is an edge case (very few >=3 token pools) and I dont think it ever makes sense to swap twice on the same pool, when you could just go to the final token directly.swap-metricsfeature, disabled by default, adds aMeteredProtocolSimwhich collects metrics in a thread-local cache for measuring specific protocol usage and solve time. Useful for local debugging. By default it is not activated.