fix(split routing): PathFrankWolfe bugs#249
Merged
Merged
Conversation
tamaralipows
force-pushed
the
split-routing/tnl/pfw-fixes
branch
from
59b2608 to
52dbbe7
Compare
compute_marginal_price_product used spot_price (decimal-normalized, human units) directly against raw token amounts, producing nonsensical price impact values (~-1e12) for cross-decimal pairs like WETH/USDC. This caused the PI gate to skip splitting on virtually every real trade. Apply 10^(decimals_out - decimals_in) correction per hop so that amount_in_raw * product ≈ amount_out_raw. Invisible in existing tests because all test tokens use 18 decimals; two new mixed-decimal tests added. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
For single-path routes, taking the last swap's amount_out is correct. For split routes, the last swap is only one leg of the split. The downstream gas refinement in WorkerPoolRouter subtracts net_gas (correctly totalled) from amount_out (partial), causing a BigUint underflow panic. Sum all swaps matching the output token, mirroring the logic already used in compute_split_net_amount_out. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
tamaralipows
force-pushed
the
split-routing/tnl/pfw-fixes
branch
2 times, most recently
from
84a8adc to
8471f7a
Compare
Keep compute_marginal_price_product as a clean float ratio in human units. Instead, normalize raw amounts to human units in compute_average_price_impact before comparing against the price product. Mathematically equivalent, but preserves the semantic that spot_price products are floating-point ratios, not decimal-scaled. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
tamaralipows
force-pushed
the
split-routing/tnl/pfw-fixes
branch
from
8471f7a to
d1ea2e1
Compare
dianacarvalho1
approved these changes
Jun 12, 2026
dianacarvalho1
left a comment
dianacarvalho1
left a comment
Collaborator
There was a problem hiding this comment.
Thank you @tamaralipows 🙏🏼 looks good!
Troshchk
approved these changes
Jun 12, 2026
Troshchk
left a comment
Troshchk
left a comment
Contributor
There was a problem hiding this comment.
Thank you @tamaralipows!
Just a small comment, otherwise looks good 💫
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
This PR is included in version 0.78.1 🎉 |
This file contains hidden or 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
4 participants
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.
I'll fix the base branch to be main before merging.
Bug 1: Price impact was not considering token decimals.
compute_marginal_price_productused spot_price (which returns decimal-normalized human units, e.g. 2000.0 for WETH/USDC) directly against raw token amounts. For cross-decimal pairs like WETH (18) / USDC (6), this produced price impact values of -10^12 instead of ~0.01, causing the PI gate to skip splitting on virtually every trade.The logs I was seeing:
Bug 2: Split route results panicked the server
When a split route was chosen,
amount_outwas computed from only the last swap (one leg of the split), butamount_out_net_gaswas correctly summed across all legs. Downstream gas refinement subtracted the larger total from the smaller partial, causing a BigUint underflow panic.