Repository navigation
Fix: standardize on expires_at, drop duplicate funding_expires_at in WapuPay orders - #129
Merged
Merged
Conversation
andycreed0x
marked this pull request as ready for review
August 12, 2026 20:09
andycreed0x
added a commit
that referenced
this pull request
Sep 23, 2026
* ✨ feat: add L-BTC funding option to WapuPay direct-fiat orders 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. * expires at (#129) * Update test_cli_read_only.py * Update docs and test to use LBTC also * 👀 Use LBTC on wapupay PR review (#130) * 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. --------- Co-authored-by: Rocket <rocket@tanduwebs.com> Co-authored-by: Gonzalo Coelho <34550772+coelhogonzalo@users.noreply.github.com> Co-authored-by: rocket-morgan <259220738+rocket-morgan@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
WapuPay's order responses only ever return
expires_at; the codebase also carried a redundantfunding_expires_atfield that duplicated it. This drops the duplicate and standardizes onexpires_atend-to-end.Targets
use-lbtc-on-wapupay(#122) instead ofdevelopso it merges together with the L-BTC funding-rail work, since both touchWapuPayOrder/wapupay.py.Main Changes
funding_expires_atfield fromWapuPayOrderand_TENTATIVE_RESP_FIELDS—expires_atis now the single source of truthWapuPayManager's funding-window display now readsorder.expires_atserver.py,tools.py) to referenceexpires_attests/test_wapupay.pyand the manual test script (scripts/prompts/prompt_test_wapupay.md) to matchfunding_expires_atis removed from thewapupay_create_order/ order-status response shape. Any caller reading that field directly (instead ofexpires_at) needs to switch.Checklist