search

sparrowwallet / sparrow

Desktop Bitcoin Wallet focused on security and privacy. Free and open source.
https://sparrowwallet.com/
Apache License 2.0
1.34k stars 190 forks source link

Label doesn't save if you don't broadcast TX with Sparrow #159

Open statusquont opened 3 years ago

statusquont commented 3 years ago

I'm not sure if this is similar to my previous issue #152 and should be fixed now (I'm still running v 1.4.2).

I created a PSBT transaction with a label, I signed it offline, and then broadcast the transaction through another node not using the Sparrow UI. Once Sparrow saw that TX in the mempool (and the toast notification showed up) the label I had entered when I constructed the transaction was no longer.

Maybe this is fixed in in the latest version? I thought I'd mention it anyways since it may be a unique edge case to consider.

craigraw commented 3 years ago

Since the label is not contained in the transaction hex, Sparrow must store it somewhere and fill it in when the broadcasted transaction has been seen the local mempool. Sparrow currently does this from the tab containing the transaction that was broadcasted. If this tab was closed before the transaction hex was broadcasted elsewhere, the label is unfortunately lost.

Does this match your experience?

statusquont commented 3 years ago

I am fairly certain that I left the transaction tab open as I was curious to see if the label would save once the broadcast transaction was received. I may have switched over to the wallet tab to look at the transactions section, but I do believe the TX tab was still open. I'll try to pay extra close attention to this detail next time.

statusquont commented 3 years ago

Just confirmed the bug with a new transaction. The transaction tab was still open and the label got wiped out once the transaction (broadcast from another node) was seen by sparrow.

I did have to switch over the my Addresses section in order to view/verify the change address, so the tx tab wasn't active at the time, but it was still open.

Also worth mentioning, the label never shows up anywhere in that transaction tab as far as I can tell, it's only visible in the wallet tab within the Send section. Pretty sure that's by design.

craigraw commented 3 years ago

I think I might have misunderstood your original post - when you said "the label I had entered when I constructed the transaction was no longer" - do you mean it wasn't attached to the new transaction in the Transactions tab, or disappeared from the Send tab?

I've just tested an external broadcast and the label does appear in the Transactions tab, so I want to check I'm understanding correctly.

statusquont commented 3 years ago

It wasn't attached to the new transaction in the Transactions tab.

statusquont commented 3 years ago

Just re-ran this test on a new transaction, just to confirm I wasn't hallucinating. ;) Here are the steps to reproduce:

From Wallet>Send tab. Create a new transaction, give it a label. Go through Create Transaction flow and sign the TX (I used a PSBT signed offline). [At this point Sparrow is no longer touched.] Copy the contents of the signed PSBT final.txn hex, paste it into the Blockstream Broadcast raw transaction form and broadcast it.

In Sparrow, the new transaction tab is still open and the tab title still has the correct label. In the Wallet>Send tab the fields have cleared out all of the transaction details (label, address, amount, fee). In the Wallet>Transactions tab there is no label for that new (Unconfirmed) transaction. In the Wallet>UTXO's tab there is no label for that new transaction.

So the only place the label that I originally give it, is in the title of the tab (the UI element). It's not saved in the wallet database anywhere as far as I can tell.

I'll leave it a bit longer to see if anything changes after the TX gets some confirmations.

Running Sparrow 1.4.3 (upgraded from 1.4.2 since issue first reported)

craigraw commented 3 years ago

I can't reproduce this (edit: at least, not with a Segwit transaction). I've followed these instructions as closely as possible, doing an airgapped signing with a Coldcard after pressing Save PSBT on the transaction tab (and not touching anything on Sparrow after that). The transaction was broadcast by going to Blockstream.info in the browser. Once it was picked up by the server, Sparrow applied the label to the transaction in the transaction table.

Is it possible that this test was done with a legacy (non-Segwit) transaction? If so, the transaction ID will change due to the signing. Since the signed transaction is not known to Sparrow in this workflow, it cannot determine the final txid, and cannot match on it to save the label from the transaction tab.

6102bitcoin commented 2 years ago

Is it worth adding a tip on hover about this next to the save psbt on transaction tab?

craigraw commented 2 years ago

A complicated thing to explain in a tooltip (and hopefully legacy txes should be on the way out). Would be good to have confirmation this was a legacy script type in any event.