Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lnwallet+rpcserver: fix weight calculation for taproot channels #8037

Merged
merged 2 commits into from
Sep 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions itest/list_on_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
125 changes: 125 additions & 0 deletions itest/lnd_payment_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"crypto/sha256"
"encoding/hex"
"fmt"
"testing"
"time"

"github.com/btcsuite/btcd/btcutil"
Expand All @@ -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.
guggero marked this conversation as resolved.
Show resolved Hide resolved
//
// 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

Expand Down
10 changes: 8 additions & 2 deletions lnwallet/channel.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
guggero marked this conversation as resolved.
Show resolved Hide resolved
} 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) +
input.WitnessCommitmentTxWeight
weight := blockchain.GetTransactionWeight(uTx) + witnessWeight

effFeeRate := chainfee.SatPerKWeight(fee) * 1000 /
chainfee.SatPerKWeight(weight)
Expand Down
18 changes: 16 additions & 2 deletions rpcserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
guggero marked this conversation as resolved.
Show resolved Hide resolved
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.
Expand Down Expand Up @@ -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
Expand Down