Fix Timelock Recovery plugin for watch wallets #9862

oren-z0 · 2025-05-24T01:14:06Z

In watch-wallets (i.e. air-gapped solutions), the transactions can be signed by clicking the View button,
exporting the PSBT, loading and signing it on a different device, and then merging the signature.

Added assertions that all the transactions are completed before moving to the final window (for downloading PDFs and JSONs).

accumulator · 2025-06-11T09:46:35Z

Is there a reason for the extra check on parse_output? This looks to be already covered by the check right above it.

electrum/electrum/plugins/timelock_recovery/qt.py

Lines 408 to 412 in e30392c

    
           scriptpubkey, is_address = pi.parse_output(pi.text.strip()) 
        
           if not is_address: 
        
               payto_e.setStyleSheet(ColorScheme.RED.as_stylesheet(True)) 
        
               payto_e.setToolTip("Must be a valid address, not a script.") 
        
               return False

oren-z0 · 2025-06-11T21:02:46Z

@accumulator You're right, it's redundant.
I think I wrote it before I wrote the or pi.type != PaymentIdentifierType.SPK or not pi.spk_is_address part.
I'll replace it with a simple assert just in case.

accumulator · 2025-06-12T08:10:13Z

I cherry-picked a slightly changed version of your last commit to master, in 2c7afac

oren-z0 · 2025-06-12T14:15:44Z

@accumulator Great, I've removed my commit and rebased the branch.

f321x · 2025-07-03T11:18:33Z

electrum/plugins/timelock_recovery/qt.py

                recovery_tx_fee_label.setText(_("Fee: {}").format(self.config.format_amount_and_units(context.recovery_tx.get_fee())))
                if create_cancel_cb.isChecked():
                    context.cancellation_tx = context.make_unsigned_cancellation_tx(fee_policy)
                    assert all(tx_input.is_segwit() for tx_input in context.cancellation_tx.inputs())
+                    cancellation_tx_complete_label.setText(_("✓ Signed" if context.cancellation_tx.is_complete() else ""))


Can you separate the logic from the translation in these labels?
e.g. cancellation_tx_complete_label.setText(_("✓ Signed") if context.alert_tx.is_complete() else "")
Its more readable and prevents strings from not getting registered for translation.

✓ Done 😄

f321x · 2025-07-03T12:23:17Z

Tested, works fine for me with a regular singlesig and a watchonly wallet. One comment on the code (see above).

Two things i noticed could may be done to improve UX a bit (unrelated to this PR), adding a MIN/MAX button to the Cancellation time-window (days) field as the help text is a bit hidden, this would make it more intuitive to figure out the limits instead of brute forcing the input field until it turns red.
And wrapping the PDF saving call into a WaitingDialog as it blocks the GUI for a couple of seconds which would feel better if it gave some feedback (especially on slow computers).

Long Term recovery transactions should have a high fee policy, because we don't know when we will broadcast them. On the other hand, they won't need to be urgent when broadcasted either.

There could be flows where sign_transaction will return without actually signing the transaction. We also want to add the ability to sign the transactions externally, so here we check if they are already signed.

The Next button should be clicked only after the transactions have been signed

We don't want the txid to change because the new transaction has a new random locktime.

In the current logic, even if prompt_if_unsaved was False, the prompt will appear if the signing is complete (and unsaved).

It's ok to click the View button, then press Sign, and then close the window the signed transaction will be used by the on_closed callback

oren-z0 · 2025-07-03T12:54:29Z

@f321x Thanks. Fixed and rebased from master branch.

SomberNight · 2025-07-04T06:31:22Z

Thx.

oren-z0 force-pushed the fix-timelock-recovery-watch-wallets branch 9 times, most recently from b7a70e5 to 8de18de Compare May 30, 2025 14:07

oren-z0 force-pushed the fix-timelock-recovery-watch-wallets branch from 8de18de to 0f02f92 Compare June 10, 2025 20:27

oren-z0 force-pushed the fix-timelock-recovery-watch-wallets branch from 510d4d1 to 53bf51d Compare June 12, 2025 14:12

oren-z0 force-pushed the fix-timelock-recovery-watch-wallets branch from 53bf51d to ee257b3 Compare June 12, 2025 14:48

oren-z0 mentioned this pull request Jun 16, 2025

timelockrecovery plugin with watch-only wallet: generates unusable PDFs due to unsigned transactions #9961

Closed

oren-z0 force-pushed the fix-timelock-recovery-watch-wallets branch from ee257b3 to 20c6553 Compare June 28, 2025 19:10

f321x reviewed Jul 3, 2025

View reviewed changes

oren-z0 added 10 commits July 3, 2025 15:50

don't use config FEE_POLICY

ac38b4a

Long Term recovery transactions should have a high fee policy, because we don't know when we will broadcast them. On the other hand, they won't need to be urgent when broadcasted either.

throw exception if signing is not complete

d1a15ae

There could be flows where sign_transaction will return without actually signing the transaction. We also want to add the ability to sign the transactions externally, so here we check if they are already signed.

watch-only wallets should sign externally

7eb29f9

The Next button should be clicked only after the transactions have been signed

return tx in on_closed callback

78c0425

Rename labels to fee labels

fb53551

labels to show signed txes

2e96886

keep the same locktime

fb40bbe

We don't want the txid to change because the new transaction has a new random locktime.

allow signing transaction from View

951ca76

control prompt_if_unsaved

53b3c1d

In the current logic, even if prompt_if_unsaved was False, the prompt will appear if the signing is complete (and unsaved).

Ok to press Sign and not save as a file

afc62eb

It's ok to click the View button, then press Sign, and then close the window the signed transaction will be used by the on_closed callback

oren-z0 force-pushed the fix-timelock-recovery-watch-wallets branch from 20c6553 to afc62eb Compare July 3, 2025 12:50

SomberNight merged commit ce90d8f into spesmilo:master Jul 4, 2025
13 of 14 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix Timelock Recovery plugin for watch wallets #9862

Fix Timelock Recovery plugin for watch wallets #9862

Uh oh!

oren-z0 commented May 24, 2025

Uh oh!

accumulator commented Jun 11, 2025

Uh oh!

oren-z0 commented Jun 11, 2025

Uh oh!

accumulator commented Jun 12, 2025

Uh oh!

oren-z0 commented Jun 12, 2025

Uh oh!

f321x Jul 3, 2025 •

edited

Loading

Uh oh!

oren-z0 Jul 3, 2025

Uh oh!

f321x commented Jul 3, 2025 •

edited

Loading

Uh oh!

oren-z0 commented Jul 3, 2025

Uh oh!

Uh oh!

SomberNight commented Jul 4, 2025

Uh oh!

Uh oh!

Fix Timelock Recovery plugin for watch wallets #9862

Fix Timelock Recovery plugin for watch wallets #9862

Uh oh!

Conversation

oren-z0 commented May 24, 2025

Uh oh!

accumulator commented Jun 11, 2025

Uh oh!

oren-z0 commented Jun 11, 2025

Uh oh!

accumulator commented Jun 12, 2025

Uh oh!

oren-z0 commented Jun 12, 2025

Uh oh!

f321x Jul 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

oren-z0 Jul 3, 2025

Choose a reason for hiding this comment

Uh oh!

f321x commented Jul 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

oren-z0 commented Jul 3, 2025

Uh oh!

Uh oh!

SomberNight commented Jul 4, 2025

Uh oh!

Uh oh!

f321x Jul 3, 2025 •

edited

Loading

f321x commented Jul 3, 2025 •

edited

Loading