Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

wallet: check we can produce valid signature for address before sending funds on-chain #7022

Closed
papssaj opened this issue Oct 11, 2022 · 7 comments · Fixed by #7130
Closed
Labels
safety General label for issues/PRs related to the safety of using the software wallet The wallet (lnwallet) which LND uses

Comments

@papssaj
Copy link

papssaj commented Oct 11, 2022

EDIT (guggero):

See comment below, this issue was turned from a bug report into a feature request.

Steps to implement:

  • Before sending any funds on chain, check each output pk script whether it belongs to the wallet.
  • For each output in a transaction that belongs to the wallet, produce a valid spend witness stack and validate it with a script VM to make sure the funds can be spent later on.

Original issue text:

I have a RaspiBlitz running with the following specifications:

bitcoin: v23.0.0
LND: 0.15.2-beta

A few days ago, I started using clightning in parallel to my LND node on my RaspiBlitz and wanted to now switch over the funds to that lightning node.

I closed all channels that the node has, which worked as expected. Now my LND node reports on-chain funds as expected, and also respective UTXOs. However, when I try to sweep the on-chain funds to my normal bitcoin wallet, I get the following error:

Unable To Broadcast Sweep Transaction: Unmatched Backend Error: -26: Non-mandatory-script-verify-flag (script Failed An Op_equalverify Operation)

I already did a rescan of the wallet, and it discovered the same UTXOs again. Can you please advise how I could regain access to my funds? Trying to open a channel to my new lightning instance (clightning) didn't work, it also fails to broadcast the funding transaction and creates channels that stay forever in 'pending' state.

Update: I managed to identify the UTXO which is causing a problem (by using the 'Lease' functionality in Ride the Lightning). Now there is only one UTXO left. However it has the largest balance so it would be critical to retrieve it.

@sputn1ck
Copy link
Collaborator

Could you maybe share some logs when trying to sweep the UTXO?

@papssaj
Copy link
Author

papssaj commented Oct 11, 2022

Yes. Please let me know if you need more info

2022-10-11 19:11:40.086 [INF] SWPR: Creating sweep transaction 15c846ef5de3922dbe5c814ed012575595d460ad0cdbb126a66d8                                             fe3320d9ad8 for 1 inputs (5f4bed427d989c2b8bf1b10a1d3621d753025aba2534358ec417a3cf801aa887:0 (WitnessKeyHash)) using                                              253 sat/kw, tx_weight=439, tx_fee=0.00000111 BTC, parents_count=0, parents_fee=0 BTC, parents_weight=0
2022-10-11 19:11:40.100 [DBG] RPCS: Sweeping all coins from wallet to addr=b.....


with tx=(*wire.MsgTx)(0x400a760980)({
 Version: (int32) 2,
 TxIn: ([]*wire.TxIn) (len=1 cap=15) {
  (*wire.TxIn)(0x4001c51860)({
   PreviousOutPoint: (wire.OutPoint) 5f4bed427d989c2b8bf1b10a1d3621d753025aba2534358ec417a3cf801aa887:0,
   SignatureScript: ([]uint8) <nil>,
   Witness: (wire.TxWitness) (len=2 cap=2) {
    ([]uint8) (len=71 cap=144) {
     00000000  30 44 02 20 1b a7 3f 41  61 be 56 52 ee 9c f3 ca  |0D. ..?Aa.VR....|
     00000010  0e 5e 3c 83 bc 1d a7 f0  67 a0 b1 9b d3 11 c2 95  |.^<.....g.......|
     00000020  9e ee f4 36 02 20 07 ed  c4 62 15 e8 dc 01 35 33  |...6. ...b....53|
     00000030  fa 08 fb 08 3b 33 db 96  73 ba b5 a9 c2 9c 29 5c  |....;3..s.....)\|
     00000040  4c e1 1f 71 25 90 01                              |L..q%..|
    },
    ([]uint8) (len=33 cap=33) {
     00000000  02 ad bc 73 8f 7c 5f 84  0d c8 0b 4c 0a 85 d1 e5  |...s.|_....L....|
     00000010  e6 b8 1b cd 14 8d 99 a6  87 14 12 b6 04 ae c4 80  |................|
     00000020  ce                                                |.|
    }
   },
   Sequence: (uint32) 0
  })
 },
 TxOut: ([]*wire.TxOut) (len=1 cap=15) {
  (*wire.TxOut)(0x4005e0c020)({
   Value: (int64) 2898880,
   PkScript: ([]uint8) (len=22 cap=500) {
    00000000  00 14 a5 99 ec e1 f5 07  fa ba ec a6 46 f6 a4 72  |............F..r|
    00000010  75 ff dd b5 0d b4                                 |u.....|
   }
  })
 },
 LockTime: (uint32) 758228
})

2022-10-11 19:11:40.108 [INF] LNWL: Inserting unconfirmed transaction 15c846ef5de3922dbe5c814ed012575595d460ad0cdbb1                                             26a66d8fe3320d9ad8
2022-10-11 19:11:40.153 [INF] LNWL: Removed invalid transaction: (*wire.MsgTx)(0x400a760980)({
 Version: (int32) 2,
 TxIn: ([]*wire.TxIn) (len=1 cap=15) {
  (*wire.TxIn)(0x4001c51860)({
   PreviousOutPoint: (wire.OutPoint) 5f4bed427d989c2b8bf1b10a1d3621d753025aba2534358ec417a3cf801aa887:0,
   SignatureScript: ([]uint8) <nil>,
   Witness: (wire.TxWitness) (len=2 cap=2) {
    ([]uint8) (len=71 cap=144) {
     00000000  30 44 02 20 1b a7 3f 41  61 be 56 52 ee 9c f3 ca  |0D. ..?Aa.VR....|
     00000010  0e 5e 3c 83 bc 1d a7 f0  67 a0 b1 9b d3 11 c2 95  |.^<.....g.......|
     00000020  9e ee f4 36 02 20 07 ed  c4 62 15 e8 dc 01 35 33  |...6. ...b....53|
     00000030  fa 08 fb 08 3b 33 db 96  73 ba b5 a9 c2 9c 29 5c  |....;3..s.....)\|
     00000040  4c e1 1f 71 25 90 01                              |L..q%..|
    },
    ([]uint8) (len=33 cap=33) {
     00000000  02 ad bc 73 8f 7c 5f 84  0d c8 0b 4c 0a 85 d1 e5  |...s.|_....L....|
     00000010  e6 b8 1b cd 14 8d 99 a6  87 14 12 b6 04 ae c4 80  |................|
     00000020  ce                                                |.|
    }
   },
   Sequence: (uint32) 0
  })
 },
 TxOut: ([]*wire.TxOut) (len=1 cap=15) {
  (*wire.TxOut)(0x4005e0c020)({
   Value: (int64) 2898880,
   PkScript: ([]uint8) (len=22 cap=500) {
    00000000  00 14 a5 99 ec e1 f5 07  fa ba ec a6 46 f6 a4 72  |............F..r|
    00000010  75 ff dd b5 0d b4                                 |u.....|
   }
  })
 },
 LockTime: (uint32) 758228
})


2022-10-11 19:11:40.154 [ERR] RPCS: [/lnrpc.Lightning/SendCoins]: unable to broadcast sweep transaction: unmatched b                                             ackend error: -26: non-mandatory-script-verify-flag (Script failed an OP_EQUALVERIFY operation)

@guggero
Copy link
Collaborator

guggero commented Oct 11, 2022

I took a quick look and it looks like the public key in the witness doesn't match the address on chain. This is really really weird and I can't think of any way this could happen. Is this an old node? Maybe affected by an old wallet related bug that we fixed in the meantime?

The funds should be recoverable though. Can you please run chantools genimportscript and check if the bc1qgahg...hdsk3 address is in the output? Then you can import the private key into Bitcoin Core or Electrum.

And once you were able to sweep the funds, would you consider sending us the wallet DB so we can dig into how this came about? Feel free to contact me on Slack if you have question about the fund recovery.

@guggero
Copy link
Collaborator

guggero commented Oct 14, 2022

I took a very close look at this. The user even provided the affected wallet file (since the funds in the address bc1qgahgvnsawaenptvl2lfngmwrmjf8ussewhdsk3 that can't be swept were the only ones remaining).
And after careful investigation I'm very sad to say that the only remaining plausible explanation for the problem is some kind of memory corruption at the time the change address was created (before it was stored in the wallet).
All available data in the wallet suggests that the address created (and funds sent to) does not match the private/public key that's stored for it in the wallet. So the funds are basically unspendable.
How this happened is very hard to say (a hardware issue with the RAM, a hardware issue with the storage, a bit flip in the memory/CPU during hashing of the public key).

It is quite hard to write software to protect against such unexpected bit flips. But we discussed this internally and think that we at least should try to create a signature (and verify it) before sending funds to a change address (or any other address the wallet identifies as "its own"). So I'm going to change the title and description of this issue to match that feature request.

@guggero guggero changed the title Unable to sweep on-chain funds wallet: check we can produce valid signature for address before sending funds on-chain Oct 14, 2022
@guggero guggero added wallet The wallet (lnwallet) which LND uses safety General label for issues/PRs related to the safety of using the software and removed utxo sweeping labels Oct 14, 2022
@guggero
Copy link
Collaborator

guggero commented Oct 31, 2022

@papssaj we're trying to gather more information on this. Can you please check Slack, I have a few questions about your hardware and setup. Thanks!

@ksedgwic
Copy link

Any chance this address is falling afoul of the Correct BIP-32 derivation issue fixed in btcsuite/btcutil#182 ?

The generation of the address and later recovery attempt would have to have been on different sides of that fix ... not sure if that is likely ...

@guggero
Copy link
Collaborator

guggero commented Nov 16, 2022

No, that's the first thing we checked. We actually never switched to the "correct" BIP-32 derivation yet in lnd, since a migration is quite tricky. So all keys derived should be consistent within lnd (even across recovery and rescans), just not necessarily with other key derivation tools.

But I still have another suspicion what could've mutated the keys and am working on a tool to verify or disprove that suspicion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
safety General label for issues/PRs related to the safety of using the software wallet The wallet (lnwallet) which LND uses
Projects
None yet
4 participants