lightningnetwork / lnd

Lightning Network Daemon ⚡️
MIT License
7.65k stars 2.07k forks source link

Feature Request: Taproot wallet support for `ComputeInputScript` #6678

Closed benthecarman closed 2 years ago

benthecarman commented 2 years ago

Background

Signing a taproot input that belongs to the wallet with ComputeInputScript

Your environment

0.15.0

Steps to reproduce

You cannot sign a taproot input that belongs to the wallet with ComputeInputScript

Expected behaviour

It should work

Actual behaviour

Creates an invalid schnorr signature

benthecarman commented 2 years ago

Looking into this more, seems like it actually still produces an invalid schnorr signature. Not sure what I am doing wrong

alexbosworth commented 2 years ago

Did you try using SignOutputRaw

guggero commented 2 years ago

What's your exact use case? Are you trying to spend an output that belongs to the wallet? The prevOutputs needs to contain all previous outputs, otherwise things won't work.

guggero commented 2 years ago

Actually, ComputeInputScript ignores the prevOutputs completely and cannot be used to sign for p2tr inputs (at least not over the RPC). The correct method to use is SignOutputRaw or the FundPSBT/FinalizePSBT methods.

benthecarman commented 2 years ago

Did you try using SignOutputRaw

Yes I get the same issue. I've tried every different variation of hash type and sign method and always result in an invalid signature

Actually, ComputeInputScript ignores the prevOutputs completely and cannot be used to sign for p2tr inputs (at least not over the RPC).

That would be nice to document or fix because the current documentation suggests it can

The correct method to use is SignOutputRaw or the FundPSBT/FinalizePSBT methods.

Trying with SignOutputRaw but still get same errors. FundPSBT/FinalizePSBT are unusable for me here because the transaction is being constructed by a third party

For more context the end goal is to sign a coinjoin transaction that is all taproot inputs/outputs

alexbosworth commented 2 years ago

I can confirm that SignOutputRaw works but invalid signature can definitely be a tough one to debug from and there are various types of taproot signature types which can make it tricky too

benthecarman commented 2 years ago

I can confirm that SignOutputRaw work

Do you have an example somewhere? I am trying with a very basic 1 input 1 output transaction and still can't get it to work

guggero commented 2 years ago

That would be nice to document or fix because the current documentation suggests it can

What documentation do you mean? https://api.lightning.community/#computeinputscript only mentions p2wkh and nested p2wkh.

EDIT: Ah, you mean the text next to prev_outputs... Argh, that's because we re-use the same signrpc.SignReq input type but don't actually use all fields.

What parameters are you passing to SignOutputRaw? Are you trying to do BIP86 key spend? Does the wallet know the BIP86 address? Or only the raw private key?

alexbosworth commented 2 years ago

Sorry I'm actually not sure if it works for internal key space keys so I don't have an example if that's what you are referring to

However I don't know why you couldn't use PSBT in that case?

benthecarman commented 2 years ago

What parameters are you passing to SignOutputRaw? Are you trying to do BIP86 key spend? Does the wallet know the BIP86 address? Or only the raw private key?

Yes I am receiving to the LND wallet in a taproot address then trying to spend from it. I am using SIGN_METHOD_TAPROOT_KEY_SPEND_BIP0086, giving the prevOuts in order, and using the output to sign it (instead of key locator)

However I don't know why you couldn't use PSBT in that case?

This makes it super ugly where everytime I want to sign I need to create a dummy psbt, call fund psbt, and copy the input records to the real psbt so I can call signpsbt properly

alexbosworth commented 2 years ago

Ah yes it isn't the most beautiful method but that is the only way to get the derivation paths and pass them as far as I know

benthecarman commented 2 years ago

Here is what I am doing if you are curious: https://github.com/bitcoin-s/bitcoin-s/pull/4428/files

alexbosworth commented 2 years ago

Would it work for your use case if you receive to a P2TR address that isn't in the main wallet keys?

guggero commented 2 years ago

HashType.sigHashAll, definitely needs to be SigHashDefault

benthecarman commented 2 years ago

Would it work for your use case if you receive to a P2TR address that isn't in the main wallet keys?

Not really

HashType.sigHashAll, definitely needs to be SigHashDefault

Does not fix :(

guggero commented 2 years ago

You're not passing in any key descriptor, so it's going to use family 0 and index 0 which isn't the key you want to use. Can you control what address the external wallet sends the funds to? Because then I'd first derive a key (so you know the family and index), create an address for that to receive and then later specify that key family and index when signing.

guggero commented 2 years ago

BTW, this is probably the best documentation of the API right now: https://github.com/lightningnetwork/lnd/blob/master/lntest/itest/lnd_taproot_test.go#L275

benthecarman commented 2 years ago

I see, so is there no way to do it for outputs internal to the wallet at the moment?

alexbosworth commented 2 years ago

I would use FundPsbt/SignPsbt that should work for P2TR

guggero commented 2 years ago

I see, so is there no way to do it for outputs internal to the wallet at the moment?

No, not without using FundPSBT.

benthecarman commented 2 years ago

Okay, updated the issue as a feature request instead :)

guggero commented 2 years ago

SignOutputRaw isn't built to deal with wallet outputs, so the title should be changed to ComputeInputScript only.

But then you'd call ComputeInputScript with the full TX that has all inputs and outputs present?

benthecarman commented 2 years ago

Yeah that is how I was doing it before

https://github.com/ln-vortex/ln-vortex/blob/master/lnd/src/main/scala/com/lnvortex/lnd/LndVortexWallet.scala#L107

guggero commented 2 years ago

Can you please check if you get things working with #6680?

guggero commented 2 years ago

Correct invocation is:

    signResp, err := alice.SignerClient.ComputeInputScript(
        ctxt, &signrpc.SignReq{
            RawTxBytes: buf.Bytes(),
            SignDescs: []*signrpc.SignDescriptor{{
                Output:     utxoInfo[0],
                InputIndex: 0,
                Sighash:    uint32(txscript.SigHashDefault),
            }},
            PrevOutputs: utxoInfo,
        },
    )