From fe41baf21098f54759c58a3720fc89e1711e4428 Mon Sep 17 00:00:00 2001 From: Andrea Franz Date: Thu, 23 Jul 2026 13:51:26 +0200 Subject: [PATCH] =?UTF-8?q?fix(amm):=20address=20Copilot=20review=20?= =?UTF-8?q?=E2=80=94=20exact=20BigInt=20min=5Fout,=20fail=20on=20oversized?= =?UTF-8?q?=20pool=20fee,=20sync=20flake=20run=20docs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- apps/amm/flake.nix | 6 ++-- apps/amm/qml/components/swap/SwapCard.qml | 10 +++++- apps/amm/qml/state/DummySwapState.qml | 38 +++++++++++++++++++++++ programs/amm/client-ffi/src/pool.rs | 3 +- 4 files changed, 53 insertions(+), 4 deletions(-) diff --git a/apps/amm/flake.nix b/apps/amm/flake.nix index 43611a8..1b36c1a 100644 --- a/apps/amm/flake.nix +++ b/apps/amm/flake.nix @@ -18,8 +18,10 @@ # fails flake evaluation because the app directory is copied into the Nix # store as its own flake root, so `../..` can't escape it there. Instead, # the repo-root flake.nix builds this module directly (src = ./apps/amm) - # and resolves amm_client_ffi via `self`. Build/run the app from the repo - # root, e.g. `nix build .#packages..default` or `nix run .`. + # and resolves amm_client_ffi via `self`. The repo-root flake exposes the UI + # as a named attribute (there is no bare `default`): run it from the repo root + # with `nix run .#amm-ui`, and build just the FFI crate with + # `nix build .#amm_client_ffi`. outputs = inputs@{ logos-module-builder, ... }: logos-module-builder.lib.mkLogosQmlModule { src = ./.; diff --git a/apps/amm/qml/components/swap/SwapCard.qml b/apps/amm/qml/components/swap/SwapCard.qml index 497b5a8..17e140a 100644 --- a/apps/amm/qml/components/swap/SwapCard.qml +++ b/apps/amm/qml/components/swap/SwapCard.qml @@ -239,7 +239,15 @@ Rectangle { root.swapInProgress = true root.swapError = "" - var minOutStr = root.formatBaseUnits(root.minReceivedAmount) + // Compute the submitted slippage floor with exact integer (BigInt) math + // rather than the double-based preview: base-unit values for 18-decimal + // tokens exceed 2^53, where doubles would understate min_out and weaken + // price protection. Sell/buy reserves follow the pool's canonical order. + var minOutStr = swapState.minOutBaseUnits( + root.sellInput, + root.sellIsPoolA ? root.poolReserveA : root.poolReserveB, + root.sellIsPoolA ? root.poolReserveB : root.poolReserveA, + root.slippageTolerancePercent) // Max u64 sentinel: "ignore deadline", per AmmUiBackend.rep. var deadline = "18446744073709551615" diff --git a/apps/amm/qml/state/DummySwapState.qml b/apps/amm/qml/state/DummySwapState.qml index 3194620..e43f6f8 100644 --- a/apps/amm/qml/state/DummySwapState.qml +++ b/apps/amm/qml/state/DummySwapState.qml @@ -73,6 +73,44 @@ QtObject { return safeAmount * (1 - safeSlippage / 100); } + // Exact-integer minimum-received (base units) for a SwapExactInput, used as + // the on-chain slippage floor that is actually submitted. Computed in BigInt + // (arbitrary precision, mirroring the on-chain u256 math): base units for + // 18-decimal tokens exceed 2^53 and even overflow u128 intermediates, so JS + // doubles silently lose precision and would understate min_out — weakening + // the user's price protection. amountIn/reserveIn/reserveOut are base-unit + // integer strings; returns a decimal string. Falls back to the double + // estimate only if BigInt is unavailable in this Qt build. + function minOutBaseUnits(amountIn, reserveIn, reserveOut, slippagePercent) { + if (typeof BigInt !== "undefined") { + var toBig = function (x) { + var s = String(x).trim(); + return /^[0-9]+$/.test(s) ? BigInt(s) : BigInt(0); + }; + var zero = BigInt(0); + var denom = BigInt(10000); + var amtIn = toBig(amountIn); + var resIn = toBig(reserveIn); + var resOut = toBig(reserveOut); + if (amtIn <= zero || resIn <= zero || resOut <= zero) + return "0"; + + var feeBps = BigInt(Math.round(Math.min(10000, Math.max(0, Number(root.feeBps) || 0)))); + var amtInAfterFee = amtIn * (denom - feeBps) / denom; // floor + if (amtInAfterFee <= zero) + return "0"; + var out = resOut * amtInAfterFee / (resIn + amtInAfterFee); // floor + var slipBps = BigInt(Math.round(clampSlippagePercent(slippagePercent) * 100)); + if (slipBps < zero) slipBps = zero; + if (slipBps > denom) slipBps = denom; + var minOut = out * (denom - slipBps) / denom; // floor + return minOut.toString(); + } + // Legacy double fallback (no worse than before if BigInt is missing). + var estOut = amountOutFor(amountIn, reserveIn, reserveOut); + return String(Math.floor(Math.max(0, minReceived(estOut, slippagePercent)))); + } + function maxSent(amountIn, slippagePercent) { const safeAmount = parseAmount(amountIn); const safeSlippage = clampSlippagePercent(slippagePercent); diff --git a/programs/amm/client-ffi/src/pool.rs b/programs/amm/client-ffi/src/pool.rs index ad54c80..5a46556 100644 --- a/programs/amm/client-ffi/src/pool.rs +++ b/programs/amm/client-ffi/src/pool.rs @@ -27,7 +27,8 @@ pub fn decode_pool(bytes: &[u8]) -> Result { reserve_a: p.reserve_a, reserve_b: p.reserve_b, liquidity_supply: p.liquidity_pool_supply, - fees: u32::try_from(p.fees).unwrap_or(u32::MAX), + fees: u32::try_from(p.fees) + .map_err(|_| format!("pool fee {} exceeds u32::MAX", p.fees))?, }) }