Merge pull request #7386 from guggero/remote-signer-p2tr-inputs

remote signer: fix channel funding with mixed funding input types

docs/release-notes/release-notes-0.16.0.md

@@ -151,6 +151,10 @@ current gossip sync query status.
 * [Add log message for edge
   case](https://github.com/lightningnetwork/lnd/pull/7115).
 
+* [A remote signer issue was fixed that caused opening channels to fail when
+  multiple mixed-type inputs were used for the funding
+  transaction](https://github.com/lightningnetwork/lnd/pull/7386).
+
 ## Build
 
 * [The project has updated to Go

funding/manager.go

@@ -1766,11 +1766,15 @@ func (f *Manager) handleFundingAccept(peer lnpeer.Peer,
 
 	pendingChanID := msg.PendingChannelID
 	peerKey := peer.IdentityKey()
+	var peerKeyBytes []byte
+	if peerKey != nil {
+		peerKeyBytes = peerKey.SerializeCompressed()
+	}
 
 	resCtx, err := f.getReservationCtx(peerKey, pendingChanID)
 	if err != nil {
-		log.Warnf("Can't find reservation (peerKey:%v, chan_id:%v)",
-			peerKey, pendingChanID)
+		log.Warnf("Can't find reservation (peerKey:%x, chan_id:%v)",
+			peerKeyBytes, pendingChanID)
 		return
 	}
 
@@ -1950,7 +1954,8 @@ func (f *Manager) handleFundingAccept(peer lnpeer.Peer,
 		addr, amt, packet, err := psbtErr.Intent.FundingParams()
 		if err != nil {
 			log.Errorf("Unable to process PSBT funding params "+
-				"for contribution from %v: %v", peerKey, err)
+				"for contribution from %x: %v", peerKeyBytes,
+				err)
 			f.failFundingFlow(peer, msg.PendingChannelID, err)
 			return
 		}
@@ -1958,7 +1963,7 @@ func (f *Manager) handleFundingAccept(peer lnpeer.Peer,
 		err = packet.Serialize(&buf)
 		if err != nil {
 			log.Errorf("Unable to serialize PSBT for "+
-				"contribution from %v: %v", peerKey, err)
+				"contribution from %x: %v", peerKeyBytes, err)
 			f.failFundingFlow(peer, msg.PendingChannelID, err)
 			return
 		}
@@ -1974,8 +1979,8 @@ func (f *Manager) handleFundingAccept(peer lnpeer.Peer,
 		}
 		psbtIntent = psbtErr.Intent
 	} else if err != nil {
-		log.Errorf("Unable to process contribution from %v: %v",
-			peerKey, err)
+		log.Errorf("Unable to process contribution from %x: %v",
+			peerKeyBytes, err)
 		f.failFundingFlow(peer, msg.PendingChannelID, err)
 		return
 	}

lntest/itest/lnd_funding_test.go

@@ -330,23 +330,43 @@ func testUnconfirmedChannelFunding(ht *lntemp.HarnessTest) {
 // testChannelFundingInputTypes tests that any type of supported input type can
 // be used to fund channels.
 func testChannelFundingInputTypes(ht *lntemp.HarnessTest) {
-	const (
-		chanAmt  = funding.MaxBtcFundingAmount
-		burnAddr = "bcrt1qxsnqpdc842lu8c0xlllgvejt6rhy49u6fmpgyz"
-	)
-
-	fundWithTypes := []func(amt btcutil.Amount, target *node.HarnessNode){
-		ht.FundCoins, ht.FundCoinsNP2WKH, ht.FundCoinsP2TR,
-	}
-
-	alice := ht.Alice
-
 	// We'll start off by creating a node for Carol.
 	carol := ht.NewNode("Carol", nil)
 
 	// Now, we'll connect her to Alice so that they can open a
 	// channel together.
-	ht.ConnectNodes(carol, alice)
+	ht.ConnectNodes(carol, ht.Alice)
+
+	runChannelFundingInputTypes(ht, ht.Alice, carol)
+}
+
+// runChannelFundingInputTypes tests that any type of supported input type can
+// be used to fund channels.
+func runChannelFundingInputTypes(ht *lntemp.HarnessTest, alice,
+	carol *node.HarnessNode) {
+
+	const (
+		chanAmt  = funding.MaxBtcFundingAmount
+		burnAddr = "bcrt1qxsnqpdc842lu8c0xlllgvejt6rhy49u6fmpgyz"
+	)
+
+	fundMixed := func(amt btcutil.Amount, target *node.HarnessNode) {
+		ht.FundCoins(amt/5, target)
+		ht.FundCoins(amt/5, target)
+		ht.FundCoinsP2TR(amt/5, target)
+		ht.FundCoinsP2TR(amt/5, target)
+		ht.FundCoinsP2TR(amt/5, target)
+	}
+	fundMultipleP2TR := func(amt btcutil.Amount, target *node.HarnessNode) {
+		ht.FundCoinsP2TR(amt/4, target)
+		ht.FundCoinsP2TR(amt/4, target)
+		ht.FundCoinsP2TR(amt/4, target)
+		ht.FundCoinsP2TR(amt/4, target)
+	}
+	fundWithTypes := []func(amt btcutil.Amount, target *node.HarnessNode){
+		ht.FundCoins, ht.FundCoinsNP2WKH, ht.FundCoinsP2TR, fundMixed,
+		fundMultipleP2TR,
+	}
 
 	// Creates a helper closure to be used below which asserts the
 	// proper response to a channel balance RPC.
@@ -386,8 +406,9 @@ func testChannelFundingInputTypes(ht *lntemp.HarnessTest) {
 	}
 
 	for _, funder := range fundWithTypes {
-		// We'll send her some confirmed funds.
-		funder(chanAmt*2, carol)
+		// We'll send her some confirmed funds. We send 10% more than
+		// we need to account for fees.
+		funder((chanAmt*11)/10, carol)
 
 		chanOpenUpdate := ht.OpenChannelAssertStream(
 			carol, alice, lntemp.OpenChannelParams{

lntest/itest/lnd_remote_signer_test.go

@@ -84,6 +84,12 @@ func testRemoteSigner(ht *lntemp.HarnessTest) {
 		fn: func(tt *lntemp.HarnessTest, wo, carol *node.HarnessNode) {
 			runBasicChannelCreationAndUpdates(tt, wo, carol)
 		},
+	}, {
+		name:      "channel funding input types",
+		sendCoins: false,
+		fn: func(tt *lntemp.HarnessTest, wo, carol *node.HarnessNode) {
+			runChannelFundingInputTypes(tt, carol, wo)
+		},
 	}, {
 		name:      "async payments",
 		sendCoins: true,
@@ -110,6 +116,11 @@ func testRemoteSigner(ht *lntemp.HarnessTest) {
 			runSignPsbtSegWitV1KeySpendBip86(tt, wo)
 			runSignPsbtSegWitV1KeySpendRootHash(tt, wo)
 			runSignPsbtSegWitV1ScriptSpend(tt, wo)
+
+			// The above tests all make sure we can sign for keys
+			// that aren't in the wallet. But we also want to make
+			// sure we can fund and then sign PSBTs from our wallet.
+			runFundAndSignPsbt(ht, wo)
 		},
 	}, {
 		name:      "sign output raw",

lnwallet/chanfunding/wallet_assembler.go

@@ -110,13 +110,12 @@ func (f *FullIntent) CompileFundingTx(extraInputs []*wire.TxIn,
 	prevOutFetcher := NewSegWitV0DualFundingPrevOutputFetcher(
 		f.coinSource, extraInputs,
 	)
+	sigHashes := txscript.NewTxSigHashes(fundingTx, prevOutFetcher)
+	for i, txIn := range fundingTx.TxIn {
 		signDesc := input.SignDescriptor{
-		SigHashes: txscript.NewTxSigHashes(
-			fundingTx, prevOutFetcher,
-		),
+			SigHashes:         sigHashes,
 			PrevOutputFetcher: prevOutFetcher,
 		}
-	for i, txIn := range fundingTx.TxIn {
 		// We can only sign this input if it's ours, so we'll ask the
 		// coin source if it can map this outpoint into a coin we own.
 		// If not, then we'll continue as it isn't our input.

lnwallet/rpcwallet/rpcwallet.go

@@ -159,6 +159,10 @@ func (r *RPCKeyRing) SendOutputs(outputs []*wire.TxOut,
 			return nil, fmt.Errorf("error looking up utxo: %v", err)
 		}
 
+		if txscript.IsPayToTaproot(info.PkScript) {
+			signDesc.HashType = txscript.SigHashDefault
+		}
+
 		// Now that we know the input is ours, we'll populate the
 		// signDesc with the per input unique information.
 		signDesc.Output = &wire.TxOut{
@@ -593,7 +597,7 @@ func (r *RPCKeyRing) ComputeInputScript(tx *wire.MsgTx,
 		signDesc.SignMethod = input.TaprootKeySpendBIP0086SignMethod
 		signDesc.WitnessScript = nil
 
-		sig, err := r.remoteSign(tx, signDesc, sigScript)
+		sig, err := r.remoteSign(tx, signDesc, nil)
 		if err != nil {
 			return nil, fmt.Errorf("error signing with remote"+
 				"instance: %v", err)
@@ -617,7 +621,7 @@ func (r *RPCKeyRing) ComputeInputScript(tx *wire.MsgTx,
 
 	// Let's give the TX to the remote instance now, so it can sign the
 	// input.
-	sig, err := r.remoteSign(tx, signDesc, sigScript)
+	sig, err := r.remoteSign(tx, signDesc, witnessProgram)
 	if err != nil {
 		return nil, fmt.Errorf("error signing with remote instance: %v",
 			err)