fix: psbt_nostr: don't allow to save tx without txid
What changed, and why it matters
This commit fixes a bug in Electrum's PSBT-over-Nostr plugin. Previously, the plugin could try to save a Bitcoin transaction to the wallet history even when the transaction didn't yet have a transaction ID (txid). Such transactions are incomplete (for example, unsigned legacy transactions). Saving them could cause errors, confusion, or corrupt wallet state. The fix adds a check that prevents saving unless a txid exists, and hides or disables the 'Save to Wallet' button in the user interface when saving isn't allowed.
Reviewers should verify that all code paths which call add_transaction_to_wallet() in the psbt_nostr plugin now guarantee a txid, and that the UI no longer offers save actions for txid-less transactions. Consider whether the new assertion could raise unexpectedly in legitimate edge cases and whether downstream wallet recovery handles skipped saves gracefully.
Security signals we found
Prevents saving incomplete transactions without txid to wallet history
Adds assertion enforcing txid presence before wallet persistence
UI now conditionally hides 'Save to Wallet' when txid is absent
Improves error logging by passing error message to on_add_fail
Avoids potential wallet state corruption or crashes from invalid transaction records
Evidence from the diff
The patch hardens the psbt_nostr plugin by enforcing that a transaction must have a computed txid before it can be persisted to the wallet history. In psbt_nostr.py, an assertion is added to add_transaction_to_wallet() requiring tx.txid(). The QML and Qt UI layers now compute a can_be_saved flag from tx.txid() is not None and conditionally expose the ‘Save to Wallet’ action. Qt’s send_to_cosigners() now only calls add_transaction_to_wallet() when txid is present, and on_add_fail now receives an error message for better logging. The change prevents incomplete PSBTs/legacy unsigned transactions from being added to the wallet history.
Changed components
electrum/plugins/psbt_nostr/psbt_nostr.pyelectrum/plugins/psbt_nostr/qml.pyelectrum/plugins/psbt_nostr/qml/PsbtReceiveDialog.qmlelectrum/plugins/psbt_nostr/qml/main.qmlelectrum/plugins/psbt_nostr/qt.pyInspect captured patch +27 / −15
diff --git a/electrum/plugins/psbt_nostr/psbt_nostr.py b/electrum/plugins/psbt_nostr/psbt_nostr.py
index b084c0e..de085bf 100644
--- a/electrum/plugins/psbt_nostr/psbt_nostr.py
+++ b/electrum/plugins/psbt_nostr/psbt_nostr.py
@@ -278,6 +278,7 @@ class CosignerWallet(Logger):
on_failure: Callable = None,
on_success: Callable = None
) -> None:
+ assert tx.txid(), "Shouldn't allow to save tx without txid"
try:
# TODO: adding tx should be handled more gracefully here:
# 1) don't replace tx with same tx with less signatures
diff --git a/electrum/plugins/psbt_nostr/qml.py b/electrum/plugins/psbt_nostr/qml.py
index 7275fc3..3ad7ec1 100644
--- a/electrum/plugins/psbt_nostr/qml.py
+++ b/electrum/plugins/psbt_nostr/qml.py
@@ -48,7 +48,7 @@ class QReceiveSignalObject(QObject):
QObject.__init__(self)
self._plugin = plugin
- cosignerReceivedPsbt = pyqtSignal(str, str, str, str)
+ cosignerReceivedPsbt = pyqtSignal(str, str, str, str, bool)
sendPsbtFailed = pyqtSignal(str, arguments=['reason'])
sendPsbtSuccess = pyqtSignal()
@@ -138,7 +138,8 @@ class QmlCosignerWallet(EventListener, CosignerWallet):
def on_event_psbt_nostr_received(self, wallet, pubkey, event_id, tx: 'PartialTransaction', label: str):
if self.wallet == wallet:
self.tx = tx
- self.plugin.so.cosignerReceivedPsbt.emit(pubkey, event_id, tx.serialize(), label)
+ can_be_saved = tx.txid() is not None
+ self.plugin.so.cosignerReceivedPsbt.emit(pubkey, event_id, tx.serialize(), label, can_be_saved)
def close(self):
super().close()
@@ -173,5 +174,5 @@ class QmlCosignerWallet(EventListener, CosignerWallet):
def reject_psbt(self, event_id):
self.mark_pending_event_rcvd(event_id)
- def on_add_fail(self):
- self.logger.error('failed to add tx to wallet')
+ def on_add_fail(self, error_msg: str):
+ self.logger.error(f'failed to add tx to wallet: {error_msg}')
diff --git a/electrum/plugins/psbt_nostr/qml/PsbtReceiveDialog.qml b/electrum/plugins/psbt_nostr/qml/PsbtReceiveDialog.qml
index 19d1352..286324f 100644
--- a/electrum/plugins/psbt_nostr/qml/PsbtReceiveDialog.qml
+++ b/electrum/plugins/psbt_nostr/qml/PsbtReceiveDialog.qml
@@ -17,6 +17,7 @@ ElDialog {
}
property string tx_label
+ property bool can_be_saved
property int choice: PsbtReceiveDialog.Choice.None
// TODO: it might be better to defer popup until no dialogs are shown
@@ -81,6 +82,7 @@ ElDialog {
Layout.preferredWidth: 1
text: qsTr('Save to Wallet')
icon.source: Qt.resolvedUrl('../../../gui/icons/wallet.png')
+ visible: dialog.can_be_saved
onClicked: {
choice = PsbtReceiveDialog.Choice.Save
doAccept()
diff --git a/electrum/plugins/psbt_nostr/qml/main.qml b/electrum/plugins/psbt_nostr/qml/main.qml
index 2c2d5f9..3990738 100644
--- a/electrum/plugins/psbt_nostr/qml/main.qml
+++ b/electrum/plugins/psbt_nostr/qml/main.qml
@@ -7,9 +7,10 @@ import "../../../gui/qml/components/controls"
Item {
Connections {
target: AppController ? AppController.plugin('psbt_nostr') : null
- function onCosignerReceivedPsbt(pubkey, event, tx, label) {
+ function onCosignerReceivedPsbt(pubkey, event, tx, label, can_be_saved) {
var dialog = psbtReceiveDialog.createObject(app, {
- tx_label: label
+ tx_label: label,
+ can_be_saved: can_be_saved
})
dialog.accepted.connect(function () {
if (dialog.choice == PsbtReceiveDialog.Choice.Open) {
diff --git a/electrum/plugins/psbt_nostr/qt.py b/electrum/plugins/psbt_nostr/qt.py
index dc9f86f..eee3285 100644
--- a/electrum/plugins/psbt_nostr/qt.py
+++ b/electrum/plugins/psbt_nostr/qt.py
@@ -102,7 +102,8 @@ class QtCosignerWallet(EventListener, CosignerWallet):
self.obj.cosignerReceivedPsbt.emit(*args) # put on UI thread via signal
def send_to_cosigners(self, tx: Union['Transaction', 'PartialTransaction'], label: str):
- self.add_transaction_to_wallet(tx, label=label, on_failure=self.on_add_fail)
+ if tx.txid():
+ self.add_transaction_to_wallet(tx, label=label, on_failure=self.on_add_fail)
self.send_psbt(tx, label)
def do_send(self, messages: List[Tuple[str, dict]], txid: Optional[str] = None):
@@ -120,8 +121,10 @@ class QtCosignerWallet(EventListener, CosignerWallet):
except Exception as e:
self.window.show_error(str(e))
return
- self.window.show_message(
- _("Your transaction was sent to your cosigners via Nostr.") + '\n\n' + txid)
+ message = _("Your transaction was sent to your cosigners via Nostr.")
+ if txid:
+ message += '\n\n' + txid
+ self.window.show_message(message)
def on_receive(self, pubkey, event_id, tx, label):
msg = '<br/>'.join([
@@ -129,13 +132,17 @@ class QtCosignerWallet(EventListener, CosignerWallet):
_("A transaction was received from your cosigner with label: <br/><big>{}</big><br/>").format(label),
_("Do you want to open it now?")
])
- result = self.window.show_message(msg, rich_text=True, icon=QMessageBox.Icon.Question, buttons=[
- QMessageBox.StandardButton.Open,
- (QPushButton('Discard'), QMessageBox.ButtonRole.DestructiveRole, 100),
- (QPushButton('Save to wallet'), QMessageBox.ButtonRole.AcceptRole, 101)]
- )
+ buttons = [
+ QMessageBox.StandardButton.Open,
+ (QPushButton('Discard'), QMessageBox.ButtonRole.DestructiveRole, 100),
+ ]
+ if tx.txid(): # cannot add tx without txid to wallet history (e.g. unsigned legacy tx)
+ buttons.append(
+ (QPushButton('Save to wallet'), QMessageBox.ButtonRole.AcceptRole, 101) # type: ignore
+ )
+ result = self.window.show_message(msg, rich_text=True, icon=QMessageBox.Icon.Question, buttons=buttons)
if result == QMessageBox.StandardButton.Open:
- if label:
+ if label and tx.txid():
self.wallet.set_label(tx.txid(), label)
show_transaction(tx, parent=self.window, prompt_if_unsaved=True, on_closed=partial(self.on_tx_dialog_closed, event_id))
else:
Why this scored 34/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.