Merge pull request #8037 from yyforyongyu/fix-weight-calc

lnwallet+rpcserver: fix weight calculation for taproot channels

itest/list_on_test.go

@@ -177,6 +177,10 @@ var allTestCases = []*lntest.TestCase{
 		Name:     "list outgoing payments",
 		TestFunc: testListPayments,
 	},
+	{
+		Name:     "send direct payment",
+		TestFunc: testSendDirectPayment,
+	},
 	{
 		Name:     "immediate payment after channel opened",
 		TestFunc: testPaymentFollowingChannelOpen,

itest/lnd_payment_test.go

@@ -4,6 +4,7 @@ import (
 	"crypto/sha256"
 	"encoding/hex"
 	"fmt"
+	"testing"
 	"time"
 
 	"github.com/btcsuite/btcd/btcutil"
@@ -16,6 +17,130 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
+// testSendDirectPayment creates a topology Alice->Bob and then tests that
+// Alice can send a direct payment to Bob. This test modifies the fee estimator
+// to return floor fee rate(1 sat/vb).
+func testSendDirectPayment(ht *lntest.HarnessTest) {
+	// Grab Alice and Bob's nodes for convenience.
+	alice, bob := ht.Alice, ht.Bob
+
+	// Create a list of commitment types we want to test.
+	commitTyes := []lnrpc.CommitmentType{
+		lnrpc.CommitmentType_LEGACY,
+		lnrpc.CommitmentType_ANCHORS,
+		lnrpc.CommitmentType_SIMPLE_TAPROOT,
+	}
+
+	// testSendPayment opens a channel between Alice and Bob using the
+	// specified params. It then sends a payment from Alice to Bob and
+	// asserts it being successful.
+	testSendPayment := func(ht *lntest.HarnessTest,
+		params lntest.OpenChannelParams) {
+
+		// Check that there are no payments before test.
+		chanPoint := ht.OpenChannel(alice, bob, params)
+
+		// Now that the channel is open, create an invoice for Bob
+		// which expects a payment of 1000 satoshis from Alice paid via
+		// a particular preimage.
+		const paymentAmt = 1000
+		preimage := ht.Random32Bytes()
+		invoice := &lnrpc.Invoice{
+			RPreimage: preimage,
+			Value:     paymentAmt,
+		}
+		invoiceResp := bob.RPC.AddInvoice(invoice)
+
+		// With the invoice for Bob added, send a payment towards Alice
+		// paying to the above generated invoice.
+		payReqs := []string{invoiceResp.PaymentRequest}
+		ht.CompletePaymentRequests(alice, payReqs)
+
+		p := ht.AssertNumPayments(alice, 1)[0]
+		path := p.Htlcs[len(p.Htlcs)-1].Route.Hops
+
+		// Ensure that the stored path shows a direct payment to Bob
+		// with no other nodes in-between.
+		require.Len(ht, path, 1, "wrong number of routes in path")
+		require.Equal(ht, bob.PubKeyStr, path[0].PubKey, "wrong pubkey")
+
+		// The payment amount should also match our previous payment
+		// directly.
+		require.EqualValues(ht, paymentAmt, p.ValueSat,
+			"incorrect sat amount")
+		require.EqualValues(ht, paymentAmt*1000, p.ValueMsat,
+			"incorrect msat amount")
+
+		// The payment hash (or r-hash) should have been stored
+		// correctly.
+		correctRHash := hex.EncodeToString(invoiceResp.RHash)
+		require.Equal(ht, correctRHash, p.PaymentHash, "incorrect hash")
+
+		// As we made a single-hop direct payment, there should have
+		// been no fee applied.
+		require.Zero(ht, p.FeeSat, "fee should be 0")
+		require.Zero(ht, p.FeeMsat, "fee should be 0")
+
+		// Now verify that the payment request returned by the rpc
+		// matches the invoice that we paid.
+		require.Equal(ht, invoiceResp.PaymentRequest, p.PaymentRequest,
+			"incorrect payreq")
+
+		// Delete all payments from Alice. DB should have no payments.
+		alice.RPC.DeleteAllPayments()
+		ht.AssertNumPayments(alice, 0)
+
+		// TODO(yy): remove the sleep once the following bug is fixed.
+		// When the invoice is reported settled, the commitment dance
+		// is not yet finished, which can cause an error when closing
+		// the channel, saying there's active HTLCs. We need to
+		// investigate this issue and reverse the order to, first
+		// finish the commitment dance, then report the invoice as
+		// settled.
+		time.Sleep(2 * time.Second)
+
+		// Close the channel.
+		//
+		// NOTE: This implicitly tests that the channel link is active
+		// before closing this channel. The above payment will trigger
+		// a commitment dance in both of the nodes. If the node fails
+		// to update the commitment state, we will fail to close the
+		// channel as the link won't be active.
+		ht.CloseChannel(alice, chanPoint)
+	}
+
+	// Run the test cases.
+	for _, ct := range commitTyes {
+		ht.Run(ct.String(), func(t *testing.T) {
+			st := ht.Subtest(t)
+
+			// Set the fee estimate to 1sat/vbyte.
+			st.SetFeeEstimate(250)
+
+			// Restart the nodes with the specified commitment type.
+			args := lntest.NodeArgsForCommitType(ct)
+			st.RestartNodeWithExtraArgs(alice, args)
+			st.RestartNodeWithExtraArgs(bob, args)
+
+			// Make sure they are connected.
+			st.EnsureConnected(alice, bob)
+
+			// Open a channel with 100k satoshis between Alice and
+			// Bob with Alice being the sole funder of the channel.
+			params := lntest.OpenChannelParams{
+				Amt:            100_000,
+				CommitmentType: ct,
+			}
+
+			// Open private channel for taproot channels.
+			params.Private = ct ==
+				lnrpc.CommitmentType_SIMPLE_TAPROOT
+
+			testSendPayment(st, params)
+		})
+	}
+}
+
 func testListPayments(ht *lntest.HarnessTest) {
 	alice, bob := ht.Alice, ht.Bob
 

lnwallet/channel.go

@@ -3027,11 +3027,17 @@ func (lc *LightningChannel) fetchCommitmentView(remoteChain bool,
 	}
 	fee := lc.channelState.Capacity - totalOut
 
+	var witnessWeight int64
+	if lc.channelState.ChanType.IsTaproot() {
+		witnessWeight = input.TaprootKeyPathWitnessSize
+	} else {
+		witnessWeight = input.WitnessCommitmentTxWeight
+	}
+
 	// Since the transaction is not signed yet, we use the witness weight
 	// used for weight calculation.
 	uTx := btcutil.NewTx(commitTx.txn)
-	weight := blockchain.GetTransactionWeight(uTx) +
+	weight := blockchain.GetTransactionWeight(uTx) + witnessWeight
-		input.WitnessCommitmentTxWeight
 
 	effFeeRate := chainfee.SatPerKWeight(fee) * 1000 /
 		chainfee.SatPerKWeight(weight)

rpcserver.go

@@ -3476,10 +3476,17 @@ func (r *rpcServer) fetchPendingOpenChannels() (pendingOpenChannels, error) {
 		// broadcast.
 		// TODO(roasbeef): query for funding tx from wallet, display
 		// that also?
+		var witnessWeight int64
+		if pendingChan.ChanType.IsTaproot() {
+			witnessWeight = input.TaprootKeyPathWitnessSize
+		} else {
+			witnessWeight = input.WitnessCommitmentTxWeight
+		}
+
 		localCommitment := pendingChan.LocalCommitment
 		utx := btcutil.NewTx(localCommitment.CommitTx)
 		commitBaseWeight := blockchain.GetTransactionWeight(utx)
-		commitWeight := commitBaseWeight + input.WitnessCommitmentTxWeight
+		commitWeight := commitBaseWeight + witnessWeight
 
 		// FundingExpiryBlocks is the distance from the current block
 		// height to the broadcast height + MaxWaitNumBlocksFundingConf.
@@ -4227,10 +4234,17 @@ func createRPCOpenChannel(r *rpcServer, dbChannel *channeldb.OpenChannel,
 	// estimated weight of the witness to calculate the weight of
 	// the transaction if it were to be immediately unilaterally
 	// broadcast.
+	var witnessWeight int64
+	if dbChannel.ChanType.IsTaproot() {
+		witnessWeight = input.TaprootKeyPathWitnessSize
+	} else {
+		witnessWeight = input.WitnessCommitmentTxWeight
+	}
+
 	localCommit := dbChannel.LocalCommitment
 	utx := btcutil.NewTx(localCommit.CommitTx)
 	commitBaseWeight := blockchain.GetTransactionWeight(utx)
-	commitWeight := commitBaseWeight + input.WitnessCommitmentTxWeight
+	commitWeight := commitBaseWeight + witnessWeight
 
 	localBalance := localCommit.LocalBalance
 	remoteBalance := localCommit.RemoteBalance