Skip to content

Add L-BTC funding option to WapuPay direct-fiat orders - #122

Open
andycreed0x wants to merge 5 commits into
developfrom
use-lbtc-on-wapupay
Open

Add L-BTC funding option to WapuPay direct-fiat orders#122
andycreed0x wants to merge 5 commits into
developfrom
use-lbtc-on-wapupay

Conversation

@andycreed0x

Copy link
Copy Markdown
Collaborator

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-order reload paths. The quote/preview endpoint (tentative-amount) returns a 500 for LBTC, so wapupay_quote and the CLI's pre-confirm preview intentionally stay USDT-only and never pass funding_method.

Main Changes

  • ✨ Add funding_method param ("USDT" default / "LBTC") to WapuPayManager.create_order, validated up front before any network call
  • ✨ Thread funding_method through the MCP tool (wapupay_create_order), the MCP input schema, and the CLI (aqua wapupay create-order --funding-method)
  • ✨ Make pay_instructions currency-aware: the L-BTC rail tells the caller to send the real funding_amount_sat sats, instead of USDT-derived base units
  • 🐛 Fix WapuPayOrder.from_dict silently dropping funding_amount_sat for 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 off funding_currency instead of funding_network alone
  • 🐛 Clear total_funding_amount_base_units on the L-BTC rail so a USDT-scale figure is never paired with the L-BTC asset_id in 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 of pay_instructions)
  • 📝 Correct the now-inaccurate "rail pinned to Liquid USDT" claims in AGENTS.md and module docstrings
  • ✅ Add tests: LBTC order creation/persistence round-trip, invalid funding_method rejection, from_dict sat-preservation regression, USDT default preserved, and a money-safety regression test for the base-units guard

Checklist

  • No hardcoded values (they should go in constants.py, .env, or our database)
  • Added/updated tests (if necessary)
  • Added/updated relevant documentation (if necessary)

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.
@andycreed0x
andycreed0x marked this pull request as ready for review August 14, 2026 19:17
reason="slow Esplora scan (50–120s); runs locally, skipped on CI",
)

# TEMPORARY — Boltz outage. Boltz is the swap provider behind Ankara, so

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call

@coelhogonzalo coelhogonzalo left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

#130

@TomasCast TomasCast left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Review my comments before merging

Comment thread src/aqua/wapupay.py
Comment on lines +435 to +446
# 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.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread src/aqua/wapupay.py
Comment on lines +884 to +895
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.")

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread src/aqua/cli/wapupay.py
Comment on lines 129 to 135
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,
)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When funding_method is LBTC, the confirmation flow still displays a USDT cost. The actual L-BTC amount is available only after order creation.

Comment thread src/aqua/wapupay.py
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:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@marinate305 marinate305 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed #130 — coelhogonzalo's follow-up addresses all of TomasCast's concerns including the fund_order rail safety gap and the satoshi integer validation. Happy to approve #122 once #130 is merged into this branch. Holding approval until then.

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.

5 participants