Skip to content

fix(amm): normalize price scales before computing price impact - #1017

Open
neverm1ndthat wants to merge 1 commit into
Stellar-split:mainfrom
neverm1ndthat:fix/1011-price-impact-scale
Open

neverm1ndthat wants to merge 1 commit into
Stellar-split:mainfrom
neverm1ndthat:fix/1011-price-impact-scale

Conversation

@neverm1ndthat

Copy link
Copy Markdown

Fixes #1011

Summary

computePriceImpact() compared spot and effective prices at different decimal scales. An integer spot price like "1" (scale=1) and a fractional effective price like "0.91" (scale=10^12) produced wrong results because spotScaled - effectiveScaled treated them as same-scale values.

With an equal-reserve 1000/1000 pool and 100 input, the old code reported 0.00% impact instead of 9.00%.

The Fix

Bring both values to their common scale via cross-multiplication before computing percentage impact:

const commonScale = spot.scale > effective.scale ? spot.scale : effective.scale;
const spotCommon = spotScaled * (commonScale / spot.scale);
const effectiveCommon = effectiveScaled * (commonScale / effective.scale);

Testing

  • All 12 existing AMM tests in test/ammCalculator.test.ts pass

Issue Stellar-split#1011 - computePriceImpact() compared spot and effective prices at
different decimal scales. An integer spot price like 1 (scale=1) and a
fractional effective price like 0.91 (scale=10^12) produced spotScaled=1
vs effectiveScaled=910000... but the subtraction treated them as same-scale
values, so price impact always rounded to 0.00%.

Fix: bring both values to their common scale via cross-multiplication before
computing percentage impact. Existing 12 AMM tests continue to pass.
@neverm1ndthat

Copy link
Copy Markdown
Author

Hi @Kingsman-99! This PR fixes Issue #1011 - computePriceImpact() now normalizes spot and effective prices to their common decimal scale before comparing. An equal-reserve 1000/1000 pool with 100 input now correctly reports 9.00% impact instead of 0.00%. All 12 AMM tests pass. PTAL!

This branch has not been deployed

No deployments
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.

Bug: AMM price impact compares decimal values at different scales

1 participant