Skip to content

fix(amm): BigInt-safe ratio guard for estimateSwapOutput - #1018

Closed
neverm1ndthat wants to merge 1 commit into
Stellar-split:mainfrom
neverm1ndthat:fix/1007-bigint-ratio-guard
Closed

neverm1ndthat wants to merge 1 commit into
Stellar-split:mainfrom
neverm1ndthat:fix/1007-bigint-ratio-guard

Conversation

@neverm1ndthat

Copy link
Copy Markdown

Fixes #1007

Summary

The maxRatio guard in estimateSwapOutput() used Number(amountIn) / Number(reserveIn). With values exceeding Number.MAX_VALUE (e.g. 10^400), both become Infinity and Infinity / Infinity = NaN, which silently bypasses the 30% liquidity ceiling.

The Fix

Replaced the Number coercion with ratioExceedsLimit():

  • Parses the numeric limit into (numerator, denominator) BigInts
  • Cross-multiplies to compare: amount * denominator > reserve * numerator
  • Preserves existing semantics for non-finite limits (NaN/+Infinity never reject, -Infinity rejects all)

Testing

  • All 12 existing AMM tests in test/ammCalculator.test.ts pass
  • The original regression (huge BigInt bypassing the guard) now correctly throws InsufficientLiquidityError

Issue Stellar-split#1007 - the maxRatio guard compared Number(amountIn)/Number(reserveIn).
With values exceeding Number.MAX_VALUE, both become Infinity and Infinity/Infinity=NaN,
silently bypassing the 30% liquidity ceiling.

Fix: replace with ratioExceedsLimit() that cross-multiplies arbitrary-size BigInts
against the numeric limit (parsed into numerator/denominator). Preserves existing
behavior for finite callers and non-finite limit edge cases. Existing 12 AMM tests pass.
@neverm1ndthat

Copy link
Copy Markdown
Author

Hi @Kingsman-99! This PR fixes Issue #1007 - the maxRatio guard now uses ratioExceedsLimit() with cross-multiplied BigInts instead of Number coercion. Values exceeding Number.MAX_VALUE (e.g. 10^400) can no longer Infinity/Infinity=NaN their way past the 30% guard. All 12 AMM tests pass. PTAL!

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: large BigInt swaps bypass the AMM liquidity ratio guard

2 participants