qml: fix paying multi-output invoices - #10875
Open
sashazykov wants to merge 1 commit into
Open
Conversation
The QML gui assumed onchain invoices have a single output. InvoiceDialog only displayed invoice.get_address() (the first output's address), and WalletMainView.payOnchain() passed only (invoice.address, invoice.amount) to the finalizer, so paying a multi-output invoice (e.g. saved with the desktop gui's "Pay to many" and opened on android) sent the entire invoice total to the first output's address. - QETxFinalizer: pass the QEInvoice into the finalizer and, if the invoice has more than one output, spend to the invoice outputs as-is, as the qt gui's send_tab does. Single-output invoices keep the (address, amount) path, as the paid amount can legitimately differ from the invoice output value (zero-amount invoices, lightning invoices paid via their onchain fallback). - InvoiceDialog: for multi-output invoices, list all outputs with their amounts instead of a single address, and hide the "Max" option, which only supports single-output invoices (also guard updateMaxAmount accordingly). The amount override does not apply to multi-output invoices: any leftover override is cleared on load, the amount editor is not offered, and an invoice whose amount is unspecified (all zero-value outputs, as the desktop multiline parser permits) cannot be paid, so an entered amount can never diverge from what the tx pays.
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
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.
Paying a multi-output ("pay to many") onchain invoice from the QML gui builds a transaction that sends the entire invoice total to the first output's address: the first recipient is overpaid, the others receive nothing — and the invoice then remains marked Unpaid, since onchain paid-detection requires every output to be satisfied.
The QML gui assumed onchain invoices have a single output, in two places:
WalletMainView.payOnchain()passes only(invoice.address, invoice.amount)to the finalizer, andQETxFinalizer.make_tx()rebuilds[PartialTxOutput.from_address_and_value(self.address, amount)]— whereaddressisinvoice.get_address()(the first output) andamountis the invoice total.InvoiceDialogonly displays the first output's address, so there is no way to notice before confirming.To reproduce: on desktop Qt, Send tab → "Pay to many" with several
address, amountlines → Save invoice; open the same wallet withelectrum -g qml(or on Android) and pay the saved invoice from the invoice list. The resulting tx has a single output paying the full total to the first address.The fix makes the finalizer spend to the invoice outputs as-is when the invoice has more than one output — matching the qt gui, which already pays
invoice.outputs, and following the direction of the existing TODO ingui/qt/send_tab.py("instead of passing outputs, use an invoice instead"). Single-output invoices keep the existing(address, amount)path, since the paid amount can legitimately differ from the invoice output value (zero-amount invoices, lightning invoices paid via their onchain fallback). It keepsmake_unsigned_transaction's defaultmerge_duplicate_outputs=False, as the QML gui has no UI forWALLET_MERGE_DUPLICATE_OUTPUTS; happy to pass the config through like qt's send tab if preferred.InvoiceDialognow lists all outputs with their amounts (a lightweight list rather thancontrols/TxOutput, which needs tx-level model roles and can't render'!'values), and the "Max" option is hidden for multi-output invoices, as amountOverride/updateMaxAmount only support a single address. The amount override does not apply to multi-output invoices at all: the pay path spends to the outputs as-is, so a leftover override is cleared on load, the amount editor is not offered, and an amountless multi-output invoice — all zero-value outputs, which the desktop multiline parser permits — cannot be paid. An entered amount can thus never silently diverge from what the tx pays.ConfirmTxDialogneeds no changes: it already renders the built tx's outputs, so the confirmation screen shows all recipients before signing.Before / after screenshots (same invoices, desktop QML, testnet)
3-output invoice, before / after:
Invoice with a
'!'line, before / after:Includes tests driving the same python objects the QML flow uses (
QEInvoice/QETxFinalizer), covering: multi-output payment, duplicate-address outputs, a'!'max line, weighted-max display values, the zero-amount override path, a lightning invoice paid via its onchain fallback address, and that the finalizer rebuilds its tx when the invoice property is set after the wallet binding has already triggered the first update(). They are async tests onElectrumTestCase(same astest_qml_qetransactionlistmodel.py) rather than the existingQETestCaseintests/qml/qt_util.py, as they need wallet fixtures and asyncio support that harness doesn't provide; happy to move them onto the refactored harness once #10699 lands.Tested on desktop QML (
-g qml) on testnet, and on an arm64 Android 16 device with a debug testnet apk carrying this fix:Device screenshots (Android 16)
Note: this overlaps textually with #10723 (
QETxFinalizer.__init__/make_txandWalletMainView.payOnchain) — happy to rebase on top of it or vice versa, whichever lands first.The QML gui still cannot create multi-output invoices; this only fixes displaying and paying ones that already exist in the wallet. Groundwork for #8624, but stands alone as a bug fix.