From e3536dd770a11b6be6e9c060ae752f4fbc0736f2 Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Thu, 10 Nov 2022 13:10:51 +0100 Subject: [PATCH 1/6] mod+keychain+lntest: bump btcd to version with tweak fix This commit bumps to the latest version of btcd that fixes a key mutation issue when signing for Taproot outputs. --- go.mod | 4 ++-- go.sum | 8 ++++---- keychain/btcwallet.go | 2 +- lntest/mock/secretkeyring.go | 2 +- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/go.mod b/go.mod index 5042db478..fb3b779d8 100644 --- a/go.mod +++ b/go.mod @@ -3,8 +3,8 @@ module github.com/lightningnetwork/lnd require ( github.com/NebulousLabs/go-upnp v0.0.0-20180202185039-29b680b06c82 github.com/Yawning/aez v0.0.0-20211027044916-e49e68abd344 - github.com/btcsuite/btcd v0.23.3 - github.com/btcsuite/btcd/btcec/v2 v2.2.1 + github.com/btcsuite/btcd v0.23.4 + github.com/btcsuite/btcd/btcec/v2 v2.2.2 github.com/btcsuite/btcd/btcutil v1.1.2 github.com/btcsuite/btcd/btcutil/psbt v1.1.5 github.com/btcsuite/btcd/chaincfg/chainhash v1.0.1 diff --git a/go.sum b/go.sum index eb3a1283a..90bd4b610 100644 --- a/go.sum +++ b/go.sum @@ -77,13 +77,13 @@ github.com/btcsuite/btcd v0.22.0-beta.0.20220207191057-4dc4ff7963b4/go.mod h1:7a github.com/btcsuite/btcd v0.22.0-beta.0.20220316175102-8d5c75c28923/go.mod h1:taIcYprAW2g6Z9S0gGUxyR+zDwimyDMK5ePOX+iJ2ds= github.com/btcsuite/btcd v0.23.0/go.mod h1:0QJIIN1wwIXF/3G/m87gIwGniDMDQqjVn4SZgnFpsYY= github.com/btcsuite/btcd v0.23.1/go.mod h1:0QJIIN1wwIXF/3G/m87gIwGniDMDQqjVn4SZgnFpsYY= -github.com/btcsuite/btcd v0.23.3 h1:4KH/JKy9WiCd+iUS9Mu0Zp7Dnj17TGdKrg9xc/FGj24= -github.com/btcsuite/btcd v0.23.3/go.mod h1:0QJIIN1wwIXF/3G/m87gIwGniDMDQqjVn4SZgnFpsYY= +github.com/btcsuite/btcd v0.23.4 h1:IzV6qqkfwbItOS/sg/aDfPDsjPP8twrCOE2R93hxMlQ= +github.com/btcsuite/btcd v0.23.4/go.mod h1:0QJIIN1wwIXF/3G/m87gIwGniDMDQqjVn4SZgnFpsYY= github.com/btcsuite/btcd/btcec/v2 v2.1.0/go.mod h1:2VzYrv4Gm4apmbVVsSq5bqf1Ec8v56E48Vt0Y/umPgA= github.com/btcsuite/btcd/btcec/v2 v2.1.1/go.mod h1:ctjw4H1kknNJmRN4iP1R7bTQ+v3GJkZBd6mui8ZsAZE= github.com/btcsuite/btcd/btcec/v2 v2.1.3/go.mod h1:ctjw4H1kknNJmRN4iP1R7bTQ+v3GJkZBd6mui8ZsAZE= -github.com/btcsuite/btcd/btcec/v2 v2.2.1 h1:xP60mv8fvp+0khmrN0zTdPC3cNm24rfeE6lh2R/Yv3E= -github.com/btcsuite/btcd/btcec/v2 v2.2.1/go.mod h1:9/CSmJxmuvqzX9Wh2fXMWToLOHhPd11lSPuIupwTkI8= +github.com/btcsuite/btcd/btcec/v2 v2.2.2 h1:5uxe5YjoCq+JeOpg0gZSNHuFgeogrocBYxvg6w9sAgc= +github.com/btcsuite/btcd/btcec/v2 v2.2.2/go.mod h1:9/CSmJxmuvqzX9Wh2fXMWToLOHhPd11lSPuIupwTkI8= github.com/btcsuite/btcd/btcutil v1.0.0/go.mod h1:Uoxwv0pqYWhD//tfTiipkxNfdhG9UrLwaeswfjfdF0A= github.com/btcsuite/btcd/btcutil v1.1.0/go.mod h1:5OapHB7A2hBBWLm48mmw4MOHNJCcUBTwmWH/0Jn8VHE= github.com/btcsuite/btcd/btcutil v1.1.1/go.mod h1:nbKlBMNm9FGsdvKvu0essceubPiAcI57pYBNnsLAa34= diff --git a/keychain/btcwallet.go b/keychain/btcwallet.go index 754efaef2..90378a3ee 100644 --- a/keychain/btcwallet.go +++ b/keychain/btcwallet.go @@ -473,7 +473,7 @@ func (b *BtcWalletKeyRing) SignMessageSchnorr(keyLoc KeyLocator, } if len(taprootTweak) > 0 { - privKey = txscript.TweakTaprootPrivKey(privKey, taprootTweak) + privKey = txscript.TweakTaprootPrivKey(*privKey, taprootTweak) } var digest []byte diff --git a/lntest/mock/secretkeyring.go b/lntest/mock/secretkeyring.go index f7e590edd..770b0e4d3 100644 --- a/lntest/mock/secretkeyring.go +++ b/lntest/mock/secretkeyring.go @@ -86,7 +86,7 @@ func (s *SecretKeyRing) SignMessageSchnorr(_ keychain.KeyLocator, privKey := s.RootKey if len(taprootTweak) > 0 { - privKey = txscript.TweakTaprootPrivKey(privKey, taprootTweak) + privKey = txscript.TweakTaprootPrivKey(*privKey, taprootTweak) } return schnorr.Sign(privKey, digest) From 082d68d0cafe27db0ab9a7ff445fad4e10b5f0ec Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Thu, 10 Nov 2022 13:10:52 +0100 Subject: [PATCH 2/6] mod: bump btcwallet to version with addr validation This commit bumps the btcwallet dependency to the version that includes the address validation that asserts we can sign for an address before we use it. --- go.mod | 8 ++++---- go.sum | 13 ++++++++----- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/go.mod b/go.mod index fb3b779d8..e26cfec7d 100644 --- a/go.mod +++ b/go.mod @@ -9,7 +9,7 @@ require ( github.com/btcsuite/btcd/btcutil/psbt v1.1.5 github.com/btcsuite/btcd/chaincfg/chainhash v1.0.1 github.com/btcsuite/btclog v0.0.0-20170628155309-84c8d2346e9f - github.com/btcsuite/btcwallet v0.16.1 + github.com/btcsuite/btcwallet v0.16.2-0.20221109224534-84bf4e34c816 github.com/btcsuite/btcwallet/wallet/txauthor v1.3.2 github.com/btcsuite/btcwallet/wallet/txrules v1.2.0 github.com/btcsuite/btcwallet/walletdb v1.4.0 @@ -44,7 +44,7 @@ require ( github.com/ltcsuite/ltcd v0.0.0-20190101042124-f37f8bf35796 github.com/miekg/dns v1.1.43 github.com/prometheus/client_golang v1.11.0 - github.com/stretchr/testify v1.7.1 + github.com/stretchr/testify v1.8.0 github.com/tv42/zbase32 v0.0.0-20160707012821-501572607d02 github.com/urfave/cli v1.22.9 go.etcd.io/etcd/client/pkg/v3 v3.5.0 @@ -119,7 +119,7 @@ require ( github.com/sirupsen/logrus v1.7.0 // indirect github.com/soheilhy/cmux v0.1.5 // indirect github.com/spf13/pflag v1.0.5 // indirect - github.com/stretchr/objx v0.2.0 // indirect + github.com/stretchr/objx v0.4.0 // indirect github.com/syndtr/goleveldb v1.0.1-0.20210819022825-2ae1ddf74ef7 // indirect github.com/tmc/grpc-websocket-proxy v0.0.0-20201229170055-e5319fda7802 // indirect github.com/ulikunitz/xz v0.5.10 // indirect @@ -153,7 +153,7 @@ require ( gopkg.in/errgo.v1 v1.0.1 // indirect gopkg.in/natefinch/lumberjack.v2 v2.0.0 // indirect gopkg.in/yaml.v2 v2.4.0 // indirect - gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b // indirect + gopkg.in/yaml.v3 v3.0.1 // indirect sigs.k8s.io/yaml v1.2.0 // indirect ) diff --git a/go.sum b/go.sum index 90bd4b610..f31eac054 100644 --- a/go.sum +++ b/go.sum @@ -98,8 +98,8 @@ github.com/btcsuite/btcd/chaincfg/chainhash v1.0.1/go.mod h1:7SFka0XMvUgj3hfZtyd github.com/btcsuite/btclog v0.0.0-20170628155309-84c8d2346e9f h1:bAs4lUbRJpnnkd9VhRV3jjAVU7DJVjMaK+IsvSeZvFo= github.com/btcsuite/btclog v0.0.0-20170628155309-84c8d2346e9f/go.mod h1:TdznJufoqS23FtqVCzL0ZqgP5MqXbb4fg/WgDys70nA= github.com/btcsuite/btcutil v0.0.0-20190425235716-9e5f4b9a998d/go.mod h1:+5NJ2+qvTyV9exUAL/rxXi3DcLg2Ts+ymUAY5y4NvMg= -github.com/btcsuite/btcwallet v0.16.1 h1:nD8qXJeAU8c7a0Jlx5jwI2ufbf/9ouy29XGapRLTmos= -github.com/btcsuite/btcwallet v0.16.1/go.mod h1:NCO8+5rIcbUm5CtVNSQM0xrtK4iYprlyuvpGzhkejaM= +github.com/btcsuite/btcwallet v0.16.2-0.20221109224534-84bf4e34c816 h1:t4wbkXekvTc1eOGDv2h8l6mkLPnqP93hnRHvNtgpiHQ= +github.com/btcsuite/btcwallet v0.16.2-0.20221109224534-84bf4e34c816/go.mod h1:d8AETQyIIWTtC9CnoCMBmDARp9P65oX4IoBdEP3fDK4= github.com/btcsuite/btcwallet/wallet/txauthor v1.2.3/go.mod h1:T2xSiKGpUkSLCh68aF+FMXmKK9mFqNdHl9VaqOr+JjU= github.com/btcsuite/btcwallet/wallet/txauthor v1.3.2 h1:etuLgGEojecsDOYTII8rYiGHjGyV5xTqsXi+ZQ715UU= github.com/btcsuite/btcwallet/wallet/txauthor v1.3.2/go.mod h1:Zpk/LOb2sKqwP2lmHjaZT9AdaKsHPSbNLm2Uql5IQ/0= @@ -600,16 +600,18 @@ github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An github.com/spf13/viper v1.7.0/go.mod h1:8WkrPz2fc9jxqZNCJI/76HCieCp4Q8HaLFoCha5qpdg= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/objx v0.1.1/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= -github.com/stretchr/objx v0.2.0 h1:Hbg2NidpLE8veEBkEZTL3CvlkUIVzuU9jDplZO54c48= github.com/stretchr/objx v0.2.0/go.mod h1:qt09Ya8vawLte6SNmTgCsAVtYtaKzEcn8ATUoHMkEqE= +github.com/stretchr/objx v0.4.0 h1:M2gUjqZET1qApGOWNSnZ49BAIMX4F/1plDv3+l31EJ4= +github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw= github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs= github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4= github.com/stretchr/testify v1.5.1/go.mod h1:5W2xD1RspED5o8YsWQXVCued0rvSQ+mT+I5cxcmMvtA= github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= -github.com/stretchr/testify v1.7.1 h1:5TQK59W5E3v0r2duFAb7P95B6hEeOyEnHRa8MjYSMTY= github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= +github.com/stretchr/testify v1.8.0 h1:pSgiaMZlXftHpm5L7V1+rVB+AZJydKsMxsQBIJw4PKk= +github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= github.com/subosito/gotenv v1.2.0/go.mod h1:N0PQaV/YGNqwC0u51sEeR/aUtSLEXKX9iv69rRypqCw= github.com/syndtr/goleveldb v1.0.1-0.20210819022825-2ae1ddf74ef7 h1:epCh84lMvA70Z7CTTCmYQn2CKbY8j86K7/FAIr141uY= github.com/syndtr/goleveldb v1.0.1-0.20210819022825-2ae1ddf74ef7/go.mod h1:q4W45IWZaF22tdD+VEXcAWRA037jwmWEB5VWYORlTpc= @@ -1077,8 +1079,9 @@ gopkg.in/yaml.v2 v2.3.0/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.4.0 h1:D8xgwECY7CYvx+Y2n4sBz93Jn9JRvxdiyyo8CTfuKaY= gopkg.in/yaml.v2 v2.4.0/go.mod h1:RDklbk79AGWmwhnvt/jBztapEOGDOx6ZbXqjP6csGnQ= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= -gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b h1:h8qDotaEPuJATrMmW04NCwg7v22aHH28wwpauUhK9Oo= gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= +gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= +gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= honnef.co/go/tools v0.0.0-20180728063816-88497007e858/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= honnef.co/go/tools v0.0.0-20190106161140-3f1c8253044a/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= From b3f6fe315e11087f02eb3808987bd12e50164697 Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Thu, 10 Nov 2022 13:10:53 +0100 Subject: [PATCH 3/6] itest: add test coverage for Taproot tweak key mutation --- lntest/itest/lnd_psbt_test.go | 15 +++++++++++++++ lntest/itest/lnd_remote_signer_test.go | 8 +++++--- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/lntest/itest/lnd_psbt_test.go b/lntest/itest/lnd_psbt_test.go index a2495ce75..4644d54bc 100644 --- a/lntest/itest/lnd_psbt_test.go +++ b/lntest/itest/lnd_psbt_test.go @@ -1151,6 +1151,21 @@ func assertPsbtSpend(ctx context.Context, t *harnessTest, finalTx, err := psbt.Extract(signedPacket) require.NoError(t.t, err) + // Make sure we can also sign a second time. This makes sure any key + // tweaking that happened for the signing didn't affect any keys in the + // cache. + signResp2, err := alice.WalletKitClient.SignPsbt( + ctx, &walletrpc.SignPsbtRequest{ + FundedPsbt: buf.Bytes(), + }, + ) + require.NoError(t.t, err) + signedPacket2, err := psbt.NewFromRawBytes( + bytes.NewReader(signResp2.SignedPsbt), false, + ) + require.NoError(t.t, err) + verifySigned(signedPacket2) + buf.Reset() err = finalTx.Serialize(&buf) require.NoError(t.t, err) diff --git a/lntest/itest/lnd_remote_signer_test.go b/lntest/itest/lnd_remote_signer_test.go index 2e66f9c31..717cbdaa7 100644 --- a/lntest/itest/lnd_remote_signer_test.go +++ b/lntest/itest/lnd_remote_signer_test.go @@ -103,7 +103,8 @@ func testRemoteSigner(net *lntest.NetworkHarness, t *harnessTest) { runCPFP(net, tt, wo, carol) }, }, { - name: "psbt", + name: "psbt", + randomSeed: true, fn: func(tt *harnessTest, wo, carol *lntest.HarnessNode) { runPsbtChanFunding(net, tt, carol, wo) runSignPsbtSegWitV0P2WKH(tt, net, wo) @@ -124,8 +125,9 @@ func testRemoteSigner(net *lntest.NetworkHarness, t *harnessTest) { runSignVerifyMessage(tt, net, wo) }, }, { - name: "taproot", - sendCoins: true, + name: "taproot", + sendCoins: true, + randomSeed: true, fn: func(tt *harnessTest, wo, carol *lntest.HarnessNode) { ctxt, cancel := context.WithTimeout( ctxb, 3*defaultTimeout, From ed2e5425151f02664caa33b126c6aa8e462d70c0 Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Thu, 10 Nov 2022 13:10:54 +0100 Subject: [PATCH 4/6] rpcwallet: don't re-use sign desc in loop This commit fixes an issue with signing for mixed inputs in a remote signing setup where the re-use of the sign descriptor would lead to the sign method not being reset correctly if a p2wkh input is following a p2tr input. --- lnwallet/rpcwallet/rpcwallet.go | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/lnwallet/rpcwallet/rpcwallet.go b/lnwallet/rpcwallet/rpcwallet.go index 6ae9af166..71d74e7e8 100644 --- a/lnwallet/rpcwallet/rpcwallet.go +++ b/lnwallet/rpcwallet/rpcwallet.go @@ -139,12 +139,15 @@ func (r *RPCKeyRing) SendOutputs(outputs []*wire.TxOut, // We know at this point that we only have inputs from our own wallet. // So we can just compute the input script using the remote signer. outputFetcher := lnwallet.NewWalletPrevOutputFetcher(r.WalletController) - signDesc := input.SignDescriptor{ - HashType: txscript.SigHashAll, - SigHashes: txscript.NewTxSigHashes(tx, outputFetcher), - PrevOutputFetcher: outputFetcher, - } for i, txIn := range tx.TxIn { + signDesc := input.SignDescriptor{ + HashType: txscript.SigHashAll, + SigHashes: txscript.NewTxSigHashes( + tx, outputFetcher, + ), + PrevOutputFetcher: outputFetcher, + } + // We can only sign this input if it's ours, so we'll ask the // watch-only wallet if it can map this outpoint into a coin we // own. If not, then we can't continue because our wallet state From 5bd3e917507e91e17444061c8b5728256e969cf1 Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Thu, 10 Nov 2022 13:10:55 +0100 Subject: [PATCH 5/6] rpcwallet: pass prev output fetcher into ComputeInputScript This commit fixes signing of Taproot inputs when some of the other inputs of the transaction are not known to the wallet (such as a Pool account for example). If we want to sign for a Taproot (change) input when depositing into a Pool account the wallet won't know the Pool account input. So we need to make sure we pass it along inside the PrevOutputFetcher (which contains the UTXO information extracted from the PSBT). --- lnwallet/rpcwallet/rpcwallet.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lnwallet/rpcwallet/rpcwallet.go b/lnwallet/rpcwallet/rpcwallet.go index 71d74e7e8..9172c2525 100644 --- a/lnwallet/rpcwallet/rpcwallet.go +++ b/lnwallet/rpcwallet/rpcwallet.go @@ -293,9 +293,10 @@ func (r *RPCKeyRing) FinalizePsbt(packet *psbt.Packet, _ string) error { Value: int64(utxo.Value), PkScript: utxo.PkScript, }, - HashType: in.SighashType, - SigHashes: sigHashes, - InputIndex: idx, + HashType: in.SighashType, + SigHashes: sigHashes, + InputIndex: idx, + PrevOutputFetcher: prevOutFetcher, } // Find out what UTXO we are signing. Wallets _should_ always From 8b522aa49ac64e2e03ba88f783a35c74b1ac7d8c Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Thu, 10 Nov 2022 13:10:56 +0100 Subject: [PATCH 6/6] docs: add release notes for v0.15.5-beta --- docs/release-notes/release-notes-0.15.5.md | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 docs/release-notes/release-notes-0.15.5.md diff --git a/docs/release-notes/release-notes-0.15.5.md b/docs/release-notes/release-notes-0.15.5.md new file mode 100644 index 000000000..9eba14e93 --- /dev/null +++ b/docs/release-notes/release-notes-0.15.5.md @@ -0,0 +1,10 @@ +# Release Notes + +## Bug Fixes + +* [A Taproot related key tweak issue was fixed in `btcd` that affected remote + signing setups](https://github.com/lightningnetwork/lnd/pull/7130). + +# Contributors (Alphabetical Order) + +* Oliver Gugger