From 5d908710c46ba945fa3d3ceffaf588aaef1b0d3c Mon Sep 17 00:00:00 2001 From: Gonzalo Coelho Date: Sat, 15 Aug 2026 21:00:16 -0300 Subject: [PATCH 1/6] fix: infer funding rail from asset_id when WapuPay omits funding_currency A thin cross-device record (fund_order/order_status with no local order) has funding_currency=None, so every denomination branch silently fell back to USDT semantics. If the funding response carried total_amount_usdt next to the L-BTC asset_id, pay_instructions paired a USDT-scale amount (~10^8x) with the L-BTC asset. On reload, from_dict then scrubbed the real total_amount_sats as legacy-USDT residue and kept the wrong base units. - Back-fill funding_currency from a known Liquid policy asset_id (LBTC_ASSET_ID -> LBTC, USDT_LIQUID_ASSET_ID -> USDT) in apply_tentative and in from_dict (before the legacy scrub, so real sats survive reload). - Gate the USDT pay_instructions branch on an explicit USDT rail. - When the rail stays unknown (unknown asset), refuse to name any send amount and point at order_status instead. Finding 1 (HIGH) of the PR #122 review report. --- src/aqua/wapupay.py | 44 +++++++++++++++++++++++++++++-- tests/test_wapupay.py | 61 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 103 insertions(+), 2 deletions(-) diff --git a/src/aqua/wapupay.py b/src/aqua/wapupay.py index 5616cbb..508ff04 100644 --- a/src/aqua/wapupay.py +++ b/src/aqua/wapupay.py @@ -54,6 +54,7 @@ _mask, _redact, ) +from .assets import LBTC_ASSET_ID, USDT_LIQUID_ASSET_ID logger = logging.getLogger(__name__) @@ -82,6 +83,16 @@ FUNDING_METHODS = (FUNDING_METHOD_USDT, FUNDING_METHOD_LBTC) FUNDING_NETWORK_LIQUID = "LIQUID" +# A known Liquid policy asset pins the rail. Used ONLY when WapuPay omits +# funding_currency (thin cross-device records / legacy files): asset_id is the +# field lw_send_asset actually spends by, so it is the one unambiguous rail +# signal in a funding response. An unknown asset stays un-inferred — the +# denomination branches must then refuse to name a send amount. +_RAIL_BY_ASSET_ID = { + LBTC_ASSET_ID: FUNDING_METHOD_LBTC, + USDT_LIQUID_ASSET_ID: FUNDING_METHOD_USDT, +} + # Fiat side is always Argentine pesos CURRENCY_PAYMENT_ARS = "ARS" CURRENCY_TAKEN_USDT = "USDT" @@ -394,6 +405,13 @@ def to_dict(self) -> dict: @classmethod def from_dict(cls, data: dict) -> "WapuPayOrder": data = dict(data) + # Back-fill a missing rail from a known asset_id BEFORE the legacy + # scrub below: a currency-less record with the L-BTC asset would + # otherwise be treated as legacy USDT and lose its real sat amount. + if not (data.get("funding_currency") or "").strip(): + inferred = _RAIL_BY_ASSET_ID.get(data.get("asset_id") or "") + if inferred: + data["funding_currency"] = inferred # Drop stale sat amounts from legacy records: the USDT-on-Liquid rail # never has real sats. The L-BTC-on-Liquid rail DOES (total_amount_sats # is the real amount to send), so it must survive a reload — key off @@ -428,6 +446,13 @@ def apply_tentative(self, resp: dict) -> None: if field in _MONEY_FIELDS: value = _to_decimal(value) setattr(self, field, value) + # A response that omits funding_currency (thin cross-device records) + # must not default to USDT semantics: infer the rail from a known + # asset_id before deriving any denomination-dependent amount. + if not self.funding_currency: + inferred = _RAIL_BY_ASSET_ID.get(self.asset_id or "") + if inferred: + self.funding_currency = inferred # Always recalculate integer USDT base units (precision-8) for Liquid from # total_amount_usdt to avoid stale values; distinct from funding_amount_sat (BTC). @@ -1018,7 +1043,10 @@ def _funded_result(order: "WapuPayOrder") -> dict: f"WapuPay's fee — send the full amount or WapuPay won't " f"settle.{payout_note}{expires_note}" ) - elif not order.is_lbtc and order.total_funding_amount_base_units is not None: + elif ( + (order.funding_currency or "").upper() == FUNDING_METHOD_USDT + and order.total_funding_amount_base_units is not None + ): fee_display = order.fee_amount_usdt if order.fee_amount_usdt is not None else 0 result["pay_instructions"] = ( f"Send exactly {order.total_amount_usdt} USDT " @@ -1028,7 +1056,7 @@ def _funded_result(order: "WapuPayOrder") -> dict: f"WapuPay's {fee_display} USDT fee — send the full " f"amount or WapuPay won't settle.{payout_note}{expires_note}" ) - else: + elif (order.funding_currency or "").upper() in FUNDING_METHODS: # Thin record (e.g. order created on another device): the funding # response carries no total, so the exact amount isn't known locally. # Don't fabricate a "None" amount (No-lies rule) — point the user at @@ -1044,4 +1072,16 @@ def _funded_result(order: "WapuPayOrder") -> dict: f"tentative_id={order.tentative_id} to fetch {missing}, " f"then pay that exact amount with lw_send_asset." ) + else: + # Rail unknown: WapuPay omitted funding_currency and the asset_id is + # not a known policy asset, so even the DENOMINATION of the amount + # is unknown. Naming any figure here risks the ~10^8x sat/base-unit + # mixup — refuse to instruct a send until a refresh supplies the rail. + result["pay_instructions"] = ( + f"Funding address ready ({order.address_destination}), but the " + f"funding rail (USDT vs L-BTC) and the exact amount to send are " + f"not known locally. Call wapupay_order_status with " + f"tentative_id={order.tentative_id} to fetch funding_currency " + f"and the amount before paying anything." + ) return result diff --git a/tests/test_wapupay.py b/tests/test_wapupay.py index 858690e..3f1b56f 100644 --- a/tests/test_wapupay.py +++ b/tests/test_wapupay.py @@ -849,6 +849,66 @@ def test_fund_order_thin_record_with_total_but_no_fee_uses_placeholder(storage): assert "0 USDT fee" in out["pay_instructions"] +def test_fund_order_thin_record_infers_lbtc_rail_from_asset_id(storage): # Sig:5 + """Cross-device fund_order for an L-BTC order whose funding response omits + funding_currency: the L-BTC asset_id pins the rail. Without the inference + the thin record fell into the USDT branch and paired the USDT-scale total + (~10^8x too much) with the L-BTC asset id.""" + funding = {k: v for k, v in FUNDING_RESP_LBTC.items() if k != "funding_currency"} + fake = FakeClient({"issue_funding": funding}) + m = make_manager(storage, fake) + out = m.fund_order(TENTATIVE_ID) + assert out["funding_currency"] == FUNDING_METHOD_LBTC + assert out["total_funding_amount_base_units"] is None + instr = out["pay_instructions"] + assert "25127 sats" in instr + assert "base units" not in instr + # The inferred rail is persisted, so a reload keeps the real sat amount + # instead of scrubbing it as legacy-USDT residue. + saved = storage.load_wapupay_order(TENTATIVE_ID) + assert saved.funding_currency == FUNDING_METHOD_LBTC + assert saved.total_amount_sats == 25127 + assert saved.total_funding_amount_base_units is None + + +def test_fund_order_unknown_rail_refuses_to_name_a_send_amount(storage): # Sig:5 + """funding_currency absent AND asset_id not a known policy asset: even the + DENOMINATION of any figure is unknown, so pay_instructions must never say + "Send exactly" — it points at order-status to fetch the rail first.""" + funding = { + "tentative_id": TENTATIVE_ID, "status": "FUNDING_ISSUED", + "address_destination": "lq1qqfunding0address", + "asset_id": "ab" * 32, # not a known Liquid policy asset + "total_amount_usdt": 15.78, "total_amount_sats": 25127, + } + fake = FakeClient({"issue_funding": funding}) + m = make_manager(storage, fake) + out = m.fund_order(TENTATIVE_ID) + instr = out["pay_instructions"] + assert "Send exactly" not in instr + assert "wapupay_order_status" in instr + assert "None" not in instr + + +def test_from_dict_infers_lbtc_rail_before_the_legacy_scrub(): # Sig:5 + """A record persisted without funding_currency but with the L-BTC asset must + reload as L-BTC: inference runs before the legacy-USDT scrub, so the real + sat amount survives and the stale USDT-scale base units are cleared.""" + poisoned = { + "tentative_id": TENTATIVE_ID, "status": "FUNDING_ISSUED", + "type": "", "amount_ars": "", "alias": "", "created_at": "t0", + "funding_network": FUNDING_NETWORK_LIQUID, + "asset_id": LBTC_ASSET_ID, + "total_amount_usdt": "15.78", + "total_funding_amount_base_units": 1578000000, + "total_amount_sats": 25127, + } + o = WapuPayOrder.from_dict(poisoned) + assert o.funding_currency == FUNDING_METHOD_LBTC + assert o.total_amount_sats == 25127 + assert o.total_funding_amount_base_units is None + + def test_from_dict_migration_drops_stale_sat_on_none_network(storage): # Sig:5 """A legacy thin record (funding_network missing) with a stale USDT-derived funding_amount_sat must NOT load as a real BTC sat. It's dropped, and the @@ -1254,6 +1314,7 @@ def test_funded_result_keeps_payout_clause_when_known(storage): # Sig:5 order = WapuPayOrder( tentative_id=TENTATIVE_ID, status="FUNDING_ISSUED", type="fiat_transfer", amount_ars="10000", alias="al.cbu", created_at="t0", + funding_currency=FUNDING_METHOD_USDT, address_destination="lq1x", asset_id="ce091", total_amount_usdt=Decimal("7.13"), ) order._derive_base_units() From 76254e03f937a9c7c8977d53e8e169d952f9b3d6 Mon Sep 17 00:00:00 2001 From: Gonzalo Coelho Date: Sat, 15 Aug 2026 21:02:03 -0300 Subject: [PATCH 2/6] fix: validate asset_id against the funding rail in _assert_rail MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit _assert_rail guarded only the funding_currency echo; asset_id — the field lw_send_asset actually spends by — was passed verbatim from WapuPay's response into pay_instructions. An L-BTC order whose funding response carried the USDT asset id told the caller to send the sat figure as USDT base units: the order never settles and funds leave in an unquoted asset. Both rails settle in a Liquid policy asset whose id is a global constant (LBTC_ASSET_ID / USDT_LIQUID_ASSET_ID), so the check is free: any other asset_id for a known rail is an upstream contract violation and raises, annotating the persisted record (funded=True) like the currency flip. Finding 2 (MEDIUM) of the PR #122 review report. --- src/aqua/wapupay.py | 53 ++++++++++++++++++++++++++++++------------- tests/test_wapupay.py | 34 ++++++++++++++++++++++++++- 2 files changed, 70 insertions(+), 17 deletions(-) diff --git a/src/aqua/wapupay.py b/src/aqua/wapupay.py index 508ff04..e96ef70 100644 --- a/src/aqua/wapupay.py +++ b/src/aqua/wapupay.py @@ -92,6 +92,7 @@ LBTC_ASSET_ID: FUNDING_METHOD_LBTC, USDT_LIQUID_ASSET_ID: FUNDING_METHOD_USDT, } +_ASSET_ID_BY_RAIL = {rail: asset for asset, rail in _RAIL_BY_ASSET_ID.items()} # Fiat side is always Argentine pesos CURRENCY_PAYMENT_ARS = "ARS" @@ -891,31 +892,51 @@ def create_order( return self._funded_result(order) def _assert_rail(self, order: "WapuPayOrder", funding_method: str, *, funded: bool) -> None: - """Refuse to continue if WapuPay's echoed rail contradicts the request. + """Refuse to continue if WapuPay's echo contradicts the expected rail. The rail selects the denomination of the amount the user is told to send - (sats vs USDT base units) while ``asset_id`` selects the asset. If the two - disagree the caller can overpay by ~10^8x, so this raises rather than - re-deriving (CLAUDE.md invariant 5 — no silent fallback). + (sats vs USDT base units) while ``asset_id`` selects the asset + ``lw_send_asset`` actually spends. Both are checked: a flipped + ``funding_currency`` re-denominates the amount (~10^8x overpay), and a + flipped ``asset_id`` sends the right figure in the wrong asset. Either + way this raises rather than re-deriving (CLAUDE.md invariant 5 — no + silent fallback). """ + detail = None + rail_flipped = False echoed = (order.funding_currency or "").upper() - if not echoed or echoed == funding_method: + if echoed and echoed != funding_method: + rail_flipped = True + detail = ( + f"WapuPay echoed funding_currency={order.funding_currency!r} for a " + f"funding_method={funding_method!r} order; refusing to continue. " + f"The tentative exists upstream as {order.tentative_id}" + ) + else: + # Both rails settle in a Liquid policy asset whose id is a global + # constant, so any other asset_id is an upstream contract violation. + expected_asset = _ASSET_ID_BY_RAIL.get(funding_method) + if expected_asset and order.asset_id and order.asset_id != expected_asset: + detail = ( + f"WapuPay returned asset_id={order.asset_id!r} for a " + f"funding_method={funding_method!r} order (expected " + f"{expected_asset}); refusing to continue. " + f"The tentative exists upstream as {order.tentative_id}" + ) + if detail is None: return - detail = ( - f"WapuPay echoed funding_currency={order.funding_currency!r} for a " - f"funding_method={funding_method!r} order; refusing to continue. " - f"The tentative exists upstream as {order.tentative_id}" - ) if funded: # Already persisted: record why it stalled so the local record isn't # a silent orphan, then refuse to hand back pay_instructions. order.last_error = detail - # Restore the REQUESTED rail before saving. Clearing the derived - # amount here would not stick — from_dict re-derives it on every - # load — so the record must be left self-consistent (requested rail - # + matching asset_id) instead of carrying a contradictory mix. - order.funding_currency = funding_method - order._derive_base_units() + if rail_flipped: + # Restore the REQUESTED rail before saving. Clearing the derived + # amount here would not stick — from_dict re-derives it on every + # load — so the record must keep the requested denomination + # instead of the flipped one. (asset_id keeps the echoed value; + # last_error marks the record as not safe to pay.) + order.funding_currency = funding_method + order._derive_base_units() self.storage.save_wapupay_order(order) raise ValueError(f"{detail}; funding was issued but is NOT safe to pay.") raise ValueError(f"{detail} and will expire on its own; it was NOT funded.") diff --git a/tests/test_wapupay.py b/tests/test_wapupay.py index 3f1b56f..e7de0a2 100644 --- a/tests/test_wapupay.py +++ b/tests/test_wapupay.py @@ -31,7 +31,7 @@ _mask, _redact, ) -from aqua.assets import LBTC_ASSET_ID +from aqua.assets import LBTC_ASSET_ID, USDT_LIQUID_ASSET_ID from aqua.jan3_accounts import Jan3AccountsManager, Jan3Session from aqua.storage import Storage from aqua.wapupay import ( @@ -1583,6 +1583,38 @@ def test_create_order_rejects_rail_flip_on_the_funding_response(storage): # Sig assert saved.last_error +def test_create_order_rejects_wrong_asset_for_lbtc_rail(storage): # Sig:5 + """funding_currency selects the denomination, but asset_id selects the asset + lw_send_asset actually spends. A funding response pairing the L-BTC rail + with a non-L-BTC asset would make the caller send the sat figure in USDT + base units — the order never settles and the funds leave in an asset + WapuPay did not quote. Contract violation on a money path: raise.""" + funding = dict(FUNDING_RESP_LBTC, asset_id=USDT_LIQUID_ASSET_ID) + fake = FakeClient({"create_tentative": dict(CREATE_RESP_LBTC), "issue_funding": funding}) + m = make_manager(storage, fake) + with pytest.raises(ValueError, match="asset_id"): + m.create_order( + amount_ars="24000", alias="al.cbu", transfer_type="fast_fiat_transfer", + funding_method=FUNDING_METHOD_LBTC, + ) + # Persisted record is annotated (not a silent orphan) and keeps the rail. + saved = storage.load_wapupay_order(TENTATIVE_ID) + assert saved.last_error and "asset_id" in saved.last_error + assert saved.funding_currency == FUNDING_METHOD_LBTC + + +def test_create_order_rejects_wrong_asset_for_usdt_rail(storage): # Sig:5 + """Symmetric guard: a USDT-rail funding response carrying the L-BTC asset id + must raise instead of instructing an L-BTC send for a USDT-quoted order.""" + funding = dict(FUNDING_RESP, asset_id=LBTC_ASSET_ID) + fake = FakeClient({"create_tentative": dict(CREATE_RESP), "issue_funding": funding}) + m = make_manager(storage, fake) + with pytest.raises(ValueError, match="asset_id"): + m.create_order(amount_ars="10000", alias="al.cbu", transfer_type="fiat_transfer") + saved = storage.load_wapupay_order(TENTATIVE_ID) + assert saved.last_error and "asset_id" in saved.last_error + + def test_total_amount_sats_must_be_a_whole_number(): # Sig:5 """Sats are integers end-to-end (invariant 1). A fractional wire value is a contract violation — truncating it downward would underpay.""" From 0178bb161683dc11b9b460418d27066a4f094771 Mon Sep 17 00:00:00 2001 From: Gonzalo Coelho Date: Sat, 15 Aug 2026 21:05:56 -0300 Subject: [PATCH 3/6] fix: enforce the stored rail on fund_order and order_status MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit create_order raised via _assert_rail when WapuPay's echo contradicted the requested rail, but the re-issue path (fund_order) and the poll path (order_status) merged the response unchecked: a flipped echo silently re-denominated the stored record and fund_order then emitted instructions for a rail the user never chose. Both paths now run the same _assert_rail check against the rail stored before the merge (via a shared _assert_known_rail helper). Thin records with no stored rail still get the asset-consistency half of the check against the echoed rail. order_status is restructured so only the NETWORK failure degrades to the last-known-local warning — a money-contract violation in the response now raises instead of displaying. Finding 3 (LOW) of the PR #122 review report. --- src/aqua/wapupay.py | 37 +++++++++++++++++++++++++++---- tests/test_wapupay.py | 51 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 84 insertions(+), 4 deletions(-) diff --git a/src/aqua/wapupay.py b/src/aqua/wapupay.py index e96ef70..573db0d 100644 --- a/src/aqua/wapupay.py +++ b/src/aqua/wapupay.py @@ -941,6 +941,22 @@ def _assert_rail(self, order: "WapuPayOrder", funding_method: str, *, funded: bo raise ValueError(f"{detail}; funding was issued but is NOT safe to pay.") raise ValueError(f"{detail} and will expire on its own; it was NOT funded.") + def _assert_known_rail(self, order: "WapuPayOrder", expected_rail: str) -> None: + """Run ``_assert_rail`` against the best-known rail after a re-merge. + + ``expected_rail`` is the rail stored BEFORE the merge (empty for thin + records) — comparing against it catches a flip on the re-issue / poll + paths. Without a stored rail, the merged/inferred one is used so the + asset-consistency half of the check still runs. No rail at all (thin + record, unknown asset): nothing to assert — ``_funded_result`` already + refuses to name a send amount for an unknown rail. + """ + rail = expected_rail if expected_rail in FUNDING_METHODS else ( + order.funding_currency or "" + ).upper() + if rail in FUNDING_METHODS: + self._assert_rail(order, rail, funded=True) + def fund_order(self, tentative_id: str) -> dict: """Issue (or re-issue) funding instructions for an existing order.""" # Validate the id BEFORE it reaches URL construction / the network. @@ -958,7 +974,13 @@ def fund_order(self, tentative_id: str) -> dict: alias="", created_at=datetime.now(UTC).isoformat(), ) + # The stored rail is the one the user chose at create time; enforce it + # against the re-issued echo the same way create_order does. Thin + # records have no stored rail — the merged/inferred one still gets the + # asset-consistency half of the check. + expected_rail = (order.funding_currency or "").upper() order.apply_tentative(funding) + self._assert_known_rail(order, expected_rail) order.last_error = None self.storage.save_wapupay_order(order) return self._funded_result(order) @@ -972,8 +994,17 @@ def order_status(self, tentative_id: str) -> dict: order = self.storage.load_wapupay_order(tentative_id) warning = None + latest = None + # Only the NETWORK failure degrades to a warning (the last-known local + # record is still useful). A response that violates the money contract + # (rail flip, wrong asset, malformed amounts) must raise, not display. try: latest = self.client.get_tentative(tentative_id, api_key=key) + except Exception as e: + if order is None: + raise + warning = f"Could not refresh status: {e}" + if latest is not None: if order is None: order = WapuPayOrder( tentative_id=tentative_id, @@ -983,13 +1014,11 @@ def order_status(self, tentative_id: str) -> dict: alias="", created_at=datetime.now(UTC).isoformat(), ) + expected_rail = (order.funding_currency or "").upper() order.apply_tentative(latest) + self._assert_known_rail(order, expected_rail) order.last_checked_at = datetime.now(UTC).isoformat() self.storage.save_wapupay_order(order) - except Exception as e: - if order is None: - raise - warning = f"Could not refresh status: {e}" result = order.to_dict() result["is_final"] = order_is_final(order.status) diff --git a/tests/test_wapupay.py b/tests/test_wapupay.py index e7de0a2..33dac6b 100644 --- a/tests/test_wapupay.py +++ b/tests/test_wapupay.py @@ -972,6 +972,57 @@ def test_order_status_unknown_order_raises_when_remote_fails(storage): # Sig:3 m.order_status(unknown) +def test_fund_order_rejects_rail_flip_on_existing_record(storage): # Sig:5 + """create_order enforces the requested rail; the re-issue path must enforce + the STORED rail the same way. A stored L-BTC order whose re-issued funding + echoes USDT would otherwise silently re-denominate and emit instructions + for a rail the user never chose.""" + storage.save_wapupay_order(WapuPayOrder( + tentative_id=TENTATIVE_ID, status="CREATED", type="fast_fiat_transfer", + amount_ars="24000", alias="al.cbu", created_at="t0", + funding_currency=FUNDING_METHOD_LBTC, funding_network=FUNDING_NETWORK_LIQUID, + )) + funding = dict(FUNDING_RESP_LBTC, funding_currency=FUNDING_METHOD_USDT) + m = make_manager(storage, FakeClient({"issue_funding": funding})) + with pytest.raises(ValueError, match="funding_method"): + m.fund_order(TENTATIVE_ID) + saved = storage.load_wapupay_order(TENTATIVE_ID) + assert saved.funding_currency == FUNDING_METHOD_LBTC # stored rail kept + assert saved.total_funding_amount_base_units is None + assert saved.last_error + + +def test_order_status_rejects_rail_flip_on_existing_record(storage): # Sig:5 + """The poll path must not silently re-denominate a stored order either: + merging a flipped echo and persisting it would poison the record that + fund_order later advertises. A rail flip raises; only a NETWORK failure + degrades to the warning fallback.""" + storage.save_wapupay_order(WapuPayOrder( + tentative_id=TENTATIVE_ID, status="FUNDING_ISSUED", type="fast_fiat_transfer", + amount_ars="24000", alias="al.cbu", created_at="t0", + funding_currency=FUNDING_METHOD_LBTC, funding_network=FUNDING_NETWORK_LIQUID, + total_amount_sats=25127, + )) + latest = dict(FUNDING_RESP_LBTC, funding_currency=FUNDING_METHOD_USDT) + m = make_manager(storage, FakeClient({"get_tentative": latest})) + with pytest.raises(ValueError, match="funding_method"): + m.order_status(TENTATIVE_ID) + saved = storage.load_wapupay_order(TENTATIVE_ID) + assert saved.funding_currency == FUNDING_METHOD_LBTC + assert saved.total_amount_sats == 25127 + assert saved.last_error + + +def test_fund_order_thin_record_rejects_echoed_rail_with_wrong_asset(storage): # Sig:5 + """Thin record: no stored rail to compare, but the echoed rail still gets + the asset-consistency check — an L-BTC echo with a non-L-BTC asset raises + instead of instructing a send in an asset WapuPay did not quote.""" + funding = dict(FUNDING_RESP_LBTC, asset_id=USDT_LIQUID_ASSET_ID) + m = make_manager(storage, FakeClient({"issue_funding": funding})) + with pytest.raises(ValueError, match="asset_id"): + m.fund_order(TENTATIVE_ID) + + def test_fund_order_rejects_malformed_id_without_network(storage): # Sig:5 fake = FakeClient({"issue_funding": ValueError("must not be called")}) m = make_manager(storage, fake) From 57eb6b7cbbecda1e75802286dbafd5d7fc1aa49a Mon Sep 17 00:00:00 2001 From: Gonzalo Coelho Date: Sat, 15 Aug 2026 21:07:32 -0300 Subject: [PATCH 4/6] fix: annotate the stored order when a funding response is rejected MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In create_order, the post-funding apply_tentative(funding) ran outside the try that records last_error. Its contract-violation raise (fractional total_amount_sats) left the persisted record CREATED with no last_error while funding existed upstream, and recovery via fund_order hit the same un-annotated raise forever. Both create_order and fund_order now annotate the record via a shared _annotate_rejected_response helper, mirroring _assert_rail(funded=True). The clean STORED record is annotated — the half-merged in-memory order is not saved, so a rejected response never leaves its contract-violating values on disk. Finding 4 (LOW) of the PR #122 review report. --- src/aqua/wapupay.py | 27 +++++++++++++++++++++++++-- tests/test_wapupay.py | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 2 deletions(-) diff --git a/src/aqua/wapupay.py b/src/aqua/wapupay.py index 573db0d..8dfe250 100644 --- a/src/aqua/wapupay.py +++ b/src/aqua/wapupay.py @@ -882,7 +882,11 @@ def create_order( ) return result - order.apply_tentative(funding) + try: + order.apply_tentative(funding) + except ValueError as e: + self._annotate_rejected_response(tentative_id, e) + raise # Re-check after the SECOND merge: the funding response overwrites # funding_currency, so a rail that flips here would re-derive the other # rail's amounts while asset_id still points at the first one. @@ -957,6 +961,21 @@ def _assert_known_rail(self, order: "WapuPayOrder", expected_rail: str) -> None: if rail in FUNDING_METHODS: self._assert_rail(order, rail, funded=True) + def _annotate_rejected_response(self, tentative_id: str, error: Exception) -> None: + """Mark the persisted record with why a WapuPay response was rejected. + + Mirrors ``_assert_rail(funded=True)``: a raise after funding was issued + must not leave the local record a silent orphan. The half-merged + in-memory order is NOT saved — a rejected response must not leave its + contract-violating values on disk — the clean stored record is + annotated instead. No stored record (thin path): nothing to annotate. + """ + stored = self.storage.load_wapupay_order(tentative_id) + if stored is None: + return + stored.last_error = f"Funding response rejected: {error}" + self.storage.save_wapupay_order(stored) + def fund_order(self, tentative_id: str) -> dict: """Issue (or re-issue) funding instructions for an existing order.""" # Validate the id BEFORE it reaches URL construction / the network. @@ -979,7 +998,11 @@ def fund_order(self, tentative_id: str) -> dict: # records have no stored rail — the merged/inferred one still gets the # asset-consistency half of the check. expected_rail = (order.funding_currency or "").upper() - order.apply_tentative(funding) + try: + order.apply_tentative(funding) + except ValueError as e: + self._annotate_rejected_response(tentative_id, e) + raise self._assert_known_rail(order, expected_rail) order.last_error = None self.storage.save_wapupay_order(order) diff --git a/tests/test_wapupay.py b/tests/test_wapupay.py index 33dac6b..d4a4382 100644 --- a/tests/test_wapupay.py +++ b/tests/test_wapupay.py @@ -1013,6 +1013,42 @@ def test_order_status_rejects_rail_flip_on_existing_record(storage): # Sig:5 assert saved.last_error +def test_create_order_annotates_record_when_funding_response_is_rejected(storage): # Sig:5 + """A funding response that violates the money contract (fractional sats) + raises AFTER the order was persisted. The stored record must be annotated + with last_error — not left a silent CREATED orphan while upstream funding + exists — and must not carry the rejected fractional value.""" + funding = dict(FUNDING_RESP_LBTC, total_amount_sats=25127.5) + fake = FakeClient({"create_tentative": dict(CREATE_RESP_LBTC), "issue_funding": funding}) + m = make_manager(storage, fake) + with pytest.raises(ValueError, match="total_amount_sats"): + m.create_order( + amount_ars="24000", alias="al.cbu", transfer_type="fast_fiat_transfer", + funding_method=FUNDING_METHOD_LBTC, + ) + saved = storage.load_wapupay_order(TENTATIVE_ID) + assert saved.last_error and "total_amount_sats" in saved.last_error + assert saved.total_amount_sats == 25127 # the create response's clean value + + +def test_fund_order_annotates_stored_record_when_response_is_rejected(storage): # Sig:5 + """Recovery via fund_order hits the same contract-violation raise; the + stored record gets the same annotation instead of staying un-fundable + with no recorded reason.""" + storage.save_wapupay_order(WapuPayOrder( + tentative_id=TENTATIVE_ID, status="CREATED", type="fast_fiat_transfer", + amount_ars="24000", alias="al.cbu", created_at="t0", + funding_currency=FUNDING_METHOD_LBTC, funding_network=FUNDING_NETWORK_LIQUID, + )) + funding = dict(FUNDING_RESP_LBTC, total_amount_sats=25127.5) + m = make_manager(storage, FakeClient({"issue_funding": funding})) + with pytest.raises(ValueError, match="total_amount_sats"): + m.fund_order(TENTATIVE_ID) + saved = storage.load_wapupay_order(TENTATIVE_ID) + assert saved.last_error and "total_amount_sats" in saved.last_error + assert saved.total_amount_sats is None # rejected value never persisted + + def test_fund_order_thin_record_rejects_echoed_rail_with_wrong_asset(storage): # Sig:5 """Thin record: no stored rail to compare, but the echoed rail still gets the asset-consistency check — an L-BTC echo with a non-L-BTC asset raises From b2d1abf6a465eaf2f1288cc060c346a27253446f Mon Sep 17 00:00:00 2001 From: Gonzalo Coelho Date: Sat, 15 Aug 2026 21:08:25 -0300 Subject: [PATCH 5/6] fix: reject non-positive and non-integer total_amount_sats The new strictness check on the L-BTC boundary handled only fractional floats: a zero or negative integer sailed through into pay_instructions ('Send exactly -25127 sats of L-BTC ...'), and a string-typed value would round-trip into storage uncoerced. The USDT rail already rejects non-positive totals inside usdt_to_base_units, so the L-BTC seam was asymmetrically weaker. One shared validation now requires a positive int (whole floats coerced, bool excluded) and raises the same contract-violation ValueError otherwise. This also covers the string-typed hardening noted in the review's finding 8. Finding 5 (LOW) of the PR #122 review report. --- src/aqua/wapupay.py | 22 ++++++++++++++-------- tests/test_wapupay.py | 10 ++++++++++ 2 files changed, 24 insertions(+), 8 deletions(-) diff --git a/src/aqua/wapupay.py b/src/aqua/wapupay.py index 8dfe250..8775586 100644 --- a/src/aqua/wapupay.py +++ b/src/aqua/wapupay.py @@ -459,16 +459,22 @@ def apply_tentative(self, resp: dict) -> None: self._derive_base_units() # Sats are integers end-to-end (see CLAUDE.md invariant 1). total_amount_sats - # is the L-BTC send amount, so a fractional wire value is a contract - # violation, not something to round: truncating it would underpay and - # WapuPay would not settle. - if isinstance(self.total_amount_sats, float): - if not self.total_amount_sats.is_integer(): + # is the L-BTC send amount, so anything but a positive whole number is a + # contract violation, not something to coerce: rounding a fraction would + # underpay, and a zero/negative/string value has no payable meaning. The + # USDT rail already rejects non-positive totals (usdt_to_base_units) — + # this keeps the L-BTC boundary equally strict. + if self.total_amount_sats is not None: + value = self.total_amount_sats + if isinstance(value, float) and value.is_integer(): + value = int(value) + if isinstance(value, bool) or not isinstance(value, int) or value <= 0: raise ValueError( - f"WapuPay returned a fractional total_amount_sats: " - f"{self.total_amount_sats!r} (satoshis must be whole)" + f"WapuPay returned an invalid total_amount_sats: " + f"{self.total_amount_sats!r} (satoshis must be a positive " + f"whole number)" ) - self.total_amount_sats = int(self.total_amount_sats) + self.total_amount_sats = value # funding_amount_sat is record-only; keep it an int for a clean round-trip. if isinstance(self.funding_amount_sat, float): self.funding_amount_sat = int(self.funding_amount_sat) diff --git a/tests/test_wapupay.py b/tests/test_wapupay.py index d4a4382..d2bce17 100644 --- a/tests/test_wapupay.py +++ b/tests/test_wapupay.py @@ -1712,6 +1712,16 @@ def test_total_amount_sats_must_be_a_whole_number(): # Sig:5 _lbtc_order(total_amount_sats=10498.5) +def test_total_amount_sats_must_be_a_positive_integer(): # Sig:5 + """The strict boundary rejects the whole class of non-payable values — zero, + negative, and string-typed sats — with the same contract-violation + ValueError as fractional ones. "Send exactly -25127 sats" must never reach + pay_instructions, and a str would round-trip into storage uncoerced.""" + for bad in (0, -25127, -25127.0, "25127", True): + with pytest.raises(ValueError, match="total_amount_sats"): + _lbtc_order(total_amount_sats=bad) + + def test_from_dict_drops_stale_total_amount_sats_on_usdt_record(): # Sig:5 """A USDT-on-Liquid record has no real sats; a stale sat total must not survive a reload and become an L-BTC send amount.""" From 35607625c77383176a7101a1aeb69af144e55286 Mon Sep 17 00:00:00 2001 From: Gonzalo Coelho Date: Sat, 15 Aug 2026 21:09:26 -0300 Subject: [PATCH 6/6] fix: USDT thin-record fallback names the base-units field MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The fallback told a USDT payer to fetch total_amount_usdt — a decimal — and 'pay that exact amount with lw_send_asset', whose amount parameter is integer base units: following it literally underpays by ~10^8x (or errors on the non-integer). Name total_funding_amount_base_units instead — the field that is directly payable and that order_status re-derives on load — mirroring how the L-BTC half already names total_amount_sats. Finding 6 (LOW) of the PR #122 review report. --- src/aqua/wapupay.py | 15 ++++++++++----- tests/test_wapupay.py | 18 ++++++++++++++++++ 2 files changed, 28 insertions(+), 5 deletions(-) diff --git a/src/aqua/wapupay.py b/src/aqua/wapupay.py index 8775586..831907c 100644 --- a/src/aqua/wapupay.py +++ b/src/aqua/wapupay.py @@ -1139,11 +1139,16 @@ def _funded_result(order: "WapuPayOrder") -> dict: # Thin record (e.g. order created on another device): the funding # response carries no total, so the exact amount isn't known locally. # Don't fabricate a "None" amount (No-lies rule) — point the user at - # order-status to fetch the real total first. The missing field and - # the unit differ per rail, so name the right one: telling an L-BTC - # payer to fetch a USDT figure invites a ~10^8x overpayment. - missing = "total_amount_sats" if order.is_lbtc else "total_amount_usdt" - unit = "L-BTC satoshi" if order.is_lbtc else "USDT" + # order-status to fetch the real total first. Name the field that is + # DIRECTLY payable via lw_send_asset (integer sats / base units) per + # rail: pointing an L-BTC payer at a USDT figure invites a ~10^8x + # overpay, and pointing a USDT payer at the decimal total_amount_usdt + # invites a ~10^8x underpay (lw_send_asset takes integer base units). + missing = ( + "total_amount_sats" if order.is_lbtc + else "total_funding_amount_base_units" + ) + unit = "L-BTC satoshi" if order.is_lbtc else "integer USDT base-unit" result["pay_instructions"] = ( f"Funding address ready ({order.address_destination}, " f"asset_id={order.asset_id}), but the exact {unit} amount to send " diff --git a/tests/test_wapupay.py b/tests/test_wapupay.py index d2bce17..6600000 100644 --- a/tests/test_wapupay.py +++ b/tests/test_wapupay.py @@ -1617,6 +1617,24 @@ def test_lbtc_without_total_sats_states_no_amount_and_never_says_usdt(): # Sig: assert "None" not in instr +def test_usdt_thin_fallback_names_the_base_units_field(): # Sig:5 + """The USDT fallback must point at total_funding_amount_base_units — the + integer amount lw_send_asset actually takes — not the decimal + total_amount_usdt: an agent literally paying the decimal with + lw_send_asset underpays by ~10^8x (or errors on a non-integer).""" + order = WapuPayOrder( + tentative_id=TENTATIVE_ID, status="FUNDING_ISSUED", type="fiat_transfer", + amount_ars="10000", alias="al.cbu", created_at="t0", + funding_currency=FUNDING_METHOD_USDT, + address_destination="lq1qqfunding0address", asset_id=USDT_LIQUID_ASSET_ID, + ) + instr = WapuPayManager._funded_result(order)["pay_instructions"] + assert "total_funding_amount_base_units" in instr + assert "total_amount_usdt" not in instr + assert "wapupay_order_status" in instr + assert "None" not in instr + + def test_create_order_persists_requested_rail_when_wapupay_omits_it(storage): # Sig:5 """The caller's requested rail is authoritative. If WapuPay's responses omit funding_currency, the order must still be L-BTC — otherwise every