Add L-BTC funding option to WapuPay direct-fiat orders - #122
Conversation
Direct-fiat orders can now be funded with L-BTC on Liquid, not just USDT. Add a `funding_method` param (default "USDT") threaded through create_order → the MCP tool → the MCP schema → the CLI (`--funding-method`), and make pay_instructions currency-aware: the L-BTC rail sends real satoshis (funding_amount_sat) instead of USDT-derived base units. Also fixes a persistence bug this feature would otherwise trigger: WapuPayOrder.from_dict dropped funding_amount_sat for any Liquid-network order, assuming Liquid always meant USDT. That wiped the real L-BTC sat amount on every reload (order-status, orders, fund-order). Now keyed off funding_currency instead, and total_funding_amount_base_units is cleared on the L-BTC rail so a USDT-scale figure is never paired with the L-BTC asset_id. The quote/preview endpoint stays USDT-only (WapuPay 500s on LBTC there), so wapupay_quote and the CLI's pre-confirm preview are untouched. Verified live against WapuPay's stage sandbox for both rails, including the order-status/fund-order reload paths.
| reason="slow Esplora scan (50–120s); runs locally, skipped on CI", | ||
| ) | ||
|
|
||
| # TEMPORARY — Boltz outage. Boltz is the swap provider behind Ankara, so |
coelhogonzalo
left a comment
coelhogonzalo
left a comment
There was a problem hiding this comment.
LGTM. I added a review PR that could be merged into this one addressing 6 issues in separate commits.
The first issues is the most important. The rest, not so much so you can decide whether to drop them from the PR or edit them. Feel free to modify the use-lbtc-on-wapupay-pr-review branch freely
TomasCast
left a comment
TomasCast
left a comment
There was a problem hiding this comment.
Done. Review my comments before merging
| # 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(): | ||
| raise ValueError( | ||
| f"WapuPay returned a fractional total_amount_sats: " | ||
| f"{self.total_amount_sats!r} (satoshis must be whole)" | ||
| ) | ||
| self.total_amount_sats = int(self.total_amount_sats) | ||
| # funding_amount_sat is record-only; keep it an int for a clean round-trip. |
There was a problem hiding this comment.
On the satoshi check, what isn’t covered is a non-float value (like a JSON string "1.5") which skips the isinstance(..., float) branch and gets stored as-is, then can show up in L-BTC pay instructions.
I know we don't expect such types on that value, but still worth a small whole-int validator for total_amount_sats and funding_amount_sat at the wire seam (reject str / bool / non-finite / fractional) so we don’t rely on upstream staying well-behaved.
| 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() | ||
| self.storage.save_wapupay_order(order) | ||
| raise ValueError(f"{detail}; funding was issued but is NOT safe to pay.") |
There was a problem hiding this comment.
On the funded=True path, we overwrite funding_currency back to the requested rail, but leave the funding response’s asset_id / amounts alone. So the saved order can say USDT while still holding an L-BTC asset_id (or the reverse). The comment says we leave it “self-consistent,” but we don’t.
That’s risky because fund_order never re-runs _assert_rail and clears last_error, then _funded_result picks the amount unit from funding_currency against whatever asset_id is still on the record. Worth either keeping the echoed rail + matching asset/amounts and blocking recovery while mismatched, or clearing the conflicting funding fields when we restore the requested rail — not rewriting currency alone.
| click.echo( | ||
| f"Pay: {amount_ars} ARS to {alias}\n" | ||
| f"Cost: {preview.get('usdt_amount')} USDT + {preview.get('fee')} fee " | ||
| f"= {preview.get('total_amount')} USDT\n" | ||
| f"Rate: {preview.get('exchange_rate')} ARS/USDT", | ||
| err=True, | ||
| ) |
There was a problem hiding this comment.
When funding_method is LBTC, the confirmation flow still displays a USDT cost. The actual L-BTC amount is available only after order creation.
| 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 fund_order(self, tentative_id: str) -> dict: |
There was a problem hiding this comment.
Cursor's founding:
fund_order is missing the rail safety that create_order has.
On create we pin funding_currency from the request and call _assert_rail before returning pay_instructions. fund_order only merges the funding response and returns _funded_result — no pin, no assert.
That’s a problem for thin/cross-device records and for the funded=False → wapupay_fund_order recovery path: if funding_currency is omitted (or wrong) while asset_id is L-BTC and total_amount_usdt is present, we can emit USDT base-unit instructions against an L-BTC asset (~10⁸× overpay risk).
Please mirror create’s rail handling in fund_order (assert against the stored/requested rail; for thin records, require a clear funding_currency or refuse to invent a send amount), and add LBTC tests for that path.
I think it makes sense, please take a look on it
* 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. * fix: validate asset_id against the funding rail in _assert_rail _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. * fix: enforce the stored rail on fund_order and order_status 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. * fix: annotate the stored order when a funding response is rejected 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. * 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. * fix: USDT thin-record fallback names the base-units field 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.
5 participants
Purpose
Direct-fiat orders created via WapuPay can now be funded with L-BTC on Liquid, in addition to USDT. Sending
funding_method: "LBTC"on order creation gets a Liquid address funded with real satoshis instead of USDT.Description
Verified live against WapuPay's stage sandbox for both rails, including the
order-status/fund-orderreload paths. The quote/preview endpoint (tentative-amount) returns a 500 for LBTC, sowapupay_quoteand the CLI's pre-confirm preview intentionally stay USDT-only and never passfunding_method.Main Changes
funding_methodparam ("USDT"default /"LBTC") toWapuPayManager.create_order, validated up front before any network callfunding_methodthrough the MCP tool (wapupay_create_order), the MCP input schema, and the CLI (aqua wapupay create-order --funding-method)pay_instructionscurrency-aware: the L-BTC rail tells the caller to send the realfunding_amount_satsats, instead of USDT-derived base unitsWapuPayOrder.from_dictsilently droppingfunding_amount_satfor any Liquid-network order — it assumed Liquid always meant USDT, which would have wiped the real L-BTC sat amount on every reload (order-status,orders,fund-order). Now keyed offfunding_currencyinstead offunding_networkalonetotal_funding_amount_base_unitson the L-BTC rail so a USDT-scale figure is never paired with the L-BTCasset_idin the response (found and fixed during adversarial review — a defensive guard against a would-be ~10⁸x overpayment if a consumer read that field directly instead ofpay_instructions)AGENTS.mdand module docstringsfunding_methodrejection,from_dictsat-preservation regression, USDT default preserved, and a money-safety regression test for the base-units guardChecklist