From ec24767b9bab5bb2d005d9a5671c19b8d91297e1 Mon Sep 17 00:00:00 2001 From: Martin Habovstiak Date: Sat, 17 Jul 2021 20:26:29 +0200 Subject: [PATCH 1/3] lnwallet: don't enforce new reserved value in PSBT midstep This change avoids enforcing new reserved value when PSBT funding is not finished yet as new inputs and outputs may still be added that could change the outcome of the check. This originally failed in the scenario when funding a channel from external wallet *and depositing to on-chain wallet* was done simultaneously in a single transaction. If such transaction confirms then reserved UTXO is guaranteed to be available but the check didn't take it into account. The enforcement still occurs in the final step of PSBT funding flow, so it is safe. It also occurs in case of non-PSBT funding. --- lnwallet/wallet.go | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/lnwallet/wallet.go b/lnwallet/wallet.go index feb8b0cd9..412487cb9 100644 --- a/lnwallet/wallet.go +++ b/lnwallet/wallet.go @@ -706,6 +706,12 @@ func (l *LightningWallet) handleFundingReserveRequest(req *InitFundingReserveMsg return } + // We need to avoid enforcing reserved value in the middle of PSBT + // funding because some of the following steps may add UTXOs funding + // the on-chain wallet. + // The enforcement still happens at the last step - in PsbtFundingVerify + enforceNewReservedValue := true + // If no chanFunder was provided, then we'll assume the default // assembler, which is backed by the wallet's internal coin selection. if req.ChanFunder == nil { @@ -720,6 +726,9 @@ func (l *LightningWallet) handleFundingReserveRequest(req *InitFundingReserveMsg DustLimit: DustLimitForSize(input.P2WSHSize), } req.ChanFunder = chanfunding.NewWalletAssembler(cfg) + } else { + _, isPsbtFunder := req.ChanFunder.(*chanfunding.PsbtAssembler) + enforceNewReservedValue = !isPsbtFunder } localFundingAmt := req.LocalFundingAmt @@ -819,13 +828,15 @@ func (l *LightningWallet) handleFundingReserveRequest(req *InitFundingReserveMsg // when the PSBT has been verified. isPublic := req.Flags&lnwire.FFAnnounceChannel != 0 hasAnchors := req.CommitType.HasAnchors() - err = l.enforceNewReservedValue(fundingIntent, isPublic, hasAnchors) - if err != nil { - fundingIntent.Cancel() + if enforceNewReservedValue { + err = l.enforceNewReservedValue(fundingIntent, isPublic, hasAnchors) + if err != nil { + fundingIntent.Cancel() - req.err <- err - req.resp <- nil - return + req.err <- err + req.resp <- nil + return + } } // The total channel capacity will be the size of the funding output we From 2b3b70d40bae9eaad1b98bbd564e781f7806787f Mon Sep 17 00:00:00 2001 From: Martin Habovstiak Date: Wed, 11 Aug 2021 17:36:45 +0200 Subject: [PATCH 2/3] test: don't enforce reserved value in PSBT midstep This adds an integration test that makes sure channel can be funded from empty wallet using PSBT if the funding transaction contains an output belonging to the wallet, satisfying the reserve. --- lntest/itest/lnd_psbt_test.go | 185 ++++++++++++++++++++++++++ lntest/itest/lnd_test_list_on_test.go | 4 + 2 files changed, 189 insertions(+) diff --git a/lntest/itest/lnd_psbt_test.go b/lntest/itest/lnd_psbt_test.go index 22ab54a5d..5dfdac776 100644 --- a/lntest/itest/lnd_psbt_test.go +++ b/lntest/itest/lnd_psbt_test.go @@ -6,8 +6,10 @@ import ( "crypto/rand" "fmt" + "github.com/btcsuite/btcd/txscript" "github.com/btcsuite/btcd/wire" "github.com/btcsuite/btcutil" + "github.com/btcsuite/btcutil/psbt" "github.com/lightningnetwork/lnd/funding" "github.com/lightningnetwork/lnd/lnrpc" "github.com/lightningnetwork/lnd/lnrpc/walletrpc" @@ -466,6 +468,189 @@ func testPsbtChanFundingExternal(net *lntest.NetworkHarness, t *harnessTest) { closeChannelAndAssert(t, net, carol, chanPoint2, false) } +// testPsbtChanFundingSingleStep checks whether PSBT funding works also when the +// wallet of both nodes are empty and one of them uses PSBT and an external +// wallet to fund the channel while creating reserve output in the same +// transaction. +func testPsbtChanFundingSingleStep(net *lntest.NetworkHarness, t *harnessTest) { + ctxb := context.Background() + const chanSize = funding.MaxBtcFundingAmount + + args := nodeArgsForCommitType(lnrpc.CommitmentType_ANCHORS) + + // First, we'll create two new nodes that we'll use to open channels + // between for this test. But in this case both nodes have an empty + // wallet. + carol := net.NewNode(t.t, "carol", args) + defer shutdownAndAssert(net, t, carol) + + dave := net.NewNode(t.t, "dave", args) + defer shutdownAndAssert(net, t, dave) + + net.SendCoins(t.t, btcutil.SatoshiPerBitcoin, net.Alice) + + // Get new address for anchor reserve. + reserveAddrReq := &lnrpc.NewAddressRequest{ + Type: lnrpc.AddressType_WITNESS_PUBKEY_HASH, + } + addrResp, err := carol.NewAddress(ctxb, reserveAddrReq) + require.NoError(t.t, err) + reserveAddr, err := btcutil.DecodeAddress(addrResp.Address, harnessNetParams) + require.NoError(t.t, err) + reserveAddrScript, err := txscript.PayToAddrScript(reserveAddr) + require.NoError(t.t, err) + + // Before we start the test, we'll ensure both sides are connected so + // the funding flow can be properly executed. + net.EnsureConnected(t.t, carol, dave) + + // At this point, we can begin our PSBT channel funding workflow. We'll + // start by generating a pending channel ID externally that will be used + // to track this new funding type. + var pendingChanID [32]byte + _, err = rand.Read(pendingChanID[:]) + require.NoError(t.t, err) + + // Now that we have the pending channel ID, Carol will open the channel + // by specifying a PSBT shim. + ctxt, cancel := context.WithTimeout(ctxb, defaultTimeout) + defer cancel() + chanUpdates, tempPsbt, err := openChannelPsbt( + ctxt, carol, dave, lntest.OpenChannelParams{ + Amt: chanSize, + FundingShim: &lnrpc.FundingShim{ + Shim: &lnrpc.FundingShim_PsbtShim{ + PsbtShim: &lnrpc.PsbtShim{ + PendingChanId: pendingChanID[:], + NoPublish: false, + }, + }, + }, + }, + ) + require.NoError(t.t, err) + + decodedPsbt, err := psbt.NewFromRawBytes(bytes.NewReader(tempPsbt), false) + require.NoError(t.t, err) + + reserveTxOut := wire.TxOut{ + Value: 10000, + PkScript: reserveAddrScript, + } + + decodedPsbt.UnsignedTx.TxOut = append( + decodedPsbt.UnsignedTx.TxOut, &reserveTxOut, + ) + decodedPsbt.Outputs = append(decodedPsbt.Outputs, psbt.POutput{}) + + var psbtBytes bytes.Buffer + err = decodedPsbt.Serialize(&psbtBytes) + require.NoError(t.t, err) + + ctxt, cancel = context.WithTimeout(ctxb, defaultTimeout) + defer cancel() + fundReq := &walletrpc.FundPsbtRequest{ + Template: &walletrpc.FundPsbtRequest_Psbt{ + Psbt: psbtBytes.Bytes(), + }, + Fees: &walletrpc.FundPsbtRequest_SatPerVbyte{ + SatPerVbyte: 2, + }, + } + fundResp, err := net.Alice.WalletKitClient.FundPsbt(ctxt, fundReq) + require.NoError(t.t, err) + + // Make sure the wallets are actually empty + unspentCarol, err := carol.ListUnspent(ctxb, &lnrpc.ListUnspentRequest{}) + require.NoError(t.t, err) + require.Len(t.t, unspentCarol.Utxos, 0) + + unspentDave, err := dave.ListUnspent(ctxb, &lnrpc.ListUnspentRequest{}) + require.NoError(t.t, err) + require.Len(t.t, unspentDave.Utxos, 0) + + // We have a PSBT that has no witness data yet, which is exactly what we + // need for the next step: Verify the PSBT with the funding intents. + _, err = carol.FundingStateStep(ctxb, &lnrpc.FundingTransitionMsg{ + Trigger: &lnrpc.FundingTransitionMsg_PsbtVerify{ + PsbtVerify: &lnrpc.FundingPsbtVerify{ + PendingChanId: pendingChanID[:], + FundedPsbt: fundResp.FundedPsbt, + }, + }, + }) + require.NoError(t.t, err) + + // Now we'll ask Alice's wallet to sign the PSBT so we can finish the + // funding flow. + ctxt, cancel = context.WithTimeout(ctxb, defaultTimeout) + defer cancel() + finalizeReq := &walletrpc.FinalizePsbtRequest{ + FundedPsbt: fundResp.FundedPsbt, + } + finalizeRes, err := net.Alice.WalletKitClient.FinalizePsbt(ctxt, finalizeReq) + require.NoError(t.t, err) + + // We've signed our PSBT now, let's pass it to the intent again. + _, err = carol.FundingStateStep(ctxb, &lnrpc.FundingTransitionMsg{ + Trigger: &lnrpc.FundingTransitionMsg_PsbtFinalize{ + PsbtFinalize: &lnrpc.FundingPsbtFinalize{ + PendingChanId: pendingChanID[:], + SignedPsbt: finalizeRes.SignedPsbt, + }, + }, + }) + require.NoError(t.t, err) + + // Consume the "channel pending" update. This waits until the funding + // transaction was fully compiled. + ctxt, cancel = context.WithTimeout(ctxb, defaultTimeout) + defer cancel() + updateResp, err := receiveChanUpdate(ctxt, chanUpdates) + require.NoError(t.t, err) + upd, ok := updateResp.Update.(*lnrpc.OpenStatusUpdate_ChanPending) + require.True(t.t, ok) + chanPoint := &lnrpc.ChannelPoint{ + FundingTxid: &lnrpc.ChannelPoint_FundingTxidBytes{ + FundingTxidBytes: upd.ChanPending.Txid, + }, + OutputIndex: upd.ChanPending.OutputIndex, + } + + var finalTx wire.MsgTx + err = finalTx.Deserialize(bytes.NewReader(finalizeRes.RawFinalTx)) + require.NoError(t.t, err) + + txHash := finalTx.TxHash() + block := mineBlocks(t, net, 6, 1)[0] + assertTxInBlock(t, block, &txHash) + err = carol.WaitForNetworkChannelOpen(chanPoint) + require.NoError(t.t, err) + + // Next, to make sure the channel functions as normal, we'll make some + // payments within the channel. + payAmt := btcutil.Amount(100000) + invoice := &lnrpc.Invoice{ + Memo: "new chans", + Value: int64(payAmt), + } + ctxt, cancel = context.WithTimeout(ctxb, defaultTimeout) + defer cancel() + resp, err := dave.AddInvoice(ctxt, invoice) + require.NoError(t.t, err) + err = completePaymentRequests( + carol, carol.RouterClient, []string{resp.PaymentRequest}, + true, + ) + require.NoError(t.t, err) + + // To conclude, we'll close the newly created channel between Carol and + // Dave. This function will also block until the channel is closed and + // will additionally assert the relevant channel closing post + // conditions. + closeChannelAndAssert(t, net, carol, chanPoint, false) +} + // openChannelPsbt attempts to open a channel between srcNode and destNode with // the passed channel funding parameters. If the passed context has a timeout, // then if the timeout is reached before the channel pending notification is diff --git a/lntest/itest/lnd_test_list_on_test.go b/lntest/itest/lnd_test_list_on_test.go index 5fa6b9592..21a80fdc5 100644 --- a/lntest/itest/lnd_test_list_on_test.go +++ b/lntest/itest/lnd_test_list_on_test.go @@ -287,6 +287,10 @@ var allTestCases = []*testCase{ name: "batch channel funding", test: testBatchChanFunding, }, + { + name: "psbt channel funding single step", + test: testPsbtChanFundingSingleStep, + }, { name: "sendtoroute multi path payment", test: testSendToRouteMultiPath, From 333fe39a518862f841e10f786f23a4d2d1964062 Mon Sep 17 00:00:00 2001 From: Martin Habovstiak Date: Mon, 22 Nov 2021 09:49:19 +0100 Subject: [PATCH 3/3] docs: added release notes regarding #5539 --- docs/release-notes/release-notes-0.14.2.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/docs/release-notes/release-notes-0.14.2.md b/docs/release-notes/release-notes-0.14.2.md index 1425b8967..7e980ace3 100644 --- a/docs/release-notes/release-notes-0.14.2.md +++ b/docs/release-notes/release-notes-0.14.2.md @@ -5,6 +5,15 @@ * [Return the nearest known fee rate when a given conf target cannot be found from Web API fee estimator.](https://github.com/lightningnetwork/lnd/pull/6062) +## Wallet + +* A bug that prevented opening anchor-based channels from external wallets when + the internal wallet was empty even though the transaction contained a + sufficiently large output belonging to the internal wallet + [was fixed](https://github.com/lightningnetwork/lnd/pull/5539) + In other words, freshly-installed LND can now be initailized with multiple + channels from an external (e.g. hardware) wallet *in a single transaction*. + ## Build System * [Clean up Makefile by using go @@ -35,6 +44,7 @@ * Andras Banki-Horvath * Harsha Goli +* Martin Habovštiak * Naveen Srinivasan * Oliver Gugger * Yong Yu