From 3ab5899853ec0a891f103ea08f90de63f4953032 Mon Sep 17 00:00:00 2001 From: Conner Fromknecht Date: Mon, 20 Apr 2020 22:12:49 -0700 Subject: [PATCH 1/2] lnwallet/channel: fix log typo --- lnwallet/channel.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lnwallet/channel.go b/lnwallet/channel.go index 5f6eef92..4cf75c0a 100644 --- a/lnwallet/channel.go +++ b/lnwallet/channel.go @@ -2414,7 +2414,7 @@ func (lc *LightningChannel) fetchCommitmentView(remoteChain bool, chainfee.SatPerKWeight(weight) if effFeeRate < chainfee.FeePerKwFloor { return nil, fmt.Errorf("height=%v, for ChannelPoint(%v) "+ - "attempts to create commitment wigh feerate %v: %v", + "attempts to create commitment with feerate %v: %v", nextHeight, lc.channelState.FundingOutpoint, effFeeRate, spew.Sdump(commitTx)) } From 89bd58786ecb7fe6fcd5bd9ac5767a797cc09ed7 Mon Sep 17 00:00:00 2001 From: Conner Fromknecht Date: Mon, 20 Apr 2020 23:37:34 -0700 Subject: [PATCH 2/2] lnwallet/channel: enforce absolute fee floor of 250 sat/kw This enforces the _actualized_ fee rate of the commitment transaction, rather than the fee floor used for estimation. The new value of 250 sat/kw corresponds to 1 sat/byte, rather than 253 which is only rounded up during estimation to account for the fact that BOLT 3 rounds down to the nearest satoshi and that the vbyte fee estimation is lossy. Previously we would incorrectly fail to sign the next commitment even though the fee was technically high enough. Restarting with this commit should solve the issue as long as the channel hasn't already gone to chain. --- lnwallet/chainfee/rates.go | 7 ++++++- lnwallet/channel.go | 2 +- lnwallet/channel_test.go | 38 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 45 insertions(+), 2 deletions(-) diff --git a/lnwallet/chainfee/rates.go b/lnwallet/chainfee/rates.go index 48c3d7df..69c458a4 100644 --- a/lnwallet/chainfee/rates.go +++ b/lnwallet/chainfee/rates.go @@ -9,8 +9,13 @@ import ( const ( // FeePerKwFloor is the lowest fee rate in sat/kw that we should use for - // determining transaction fees. + // estimating transaction fees before signing. FeePerKwFloor SatPerKWeight = 253 + + // AbsoluteFeePerKwFloor is the lowest fee rate in sat/kw of a + // transaction that we should ever _create_. This is the the equivalent + // of 1 sat/byte in sat/kw. + AbsoluteFeePerKwFloor SatPerKWeight = 250 ) // SatPerKVByte represents a fee rate in sat/kb. diff --git a/lnwallet/channel.go b/lnwallet/channel.go index 4cf75c0a..600820f2 100644 --- a/lnwallet/channel.go +++ b/lnwallet/channel.go @@ -2412,7 +2412,7 @@ func (lc *LightningChannel) fetchCommitmentView(remoteChain bool, effFeeRate := chainfee.SatPerKWeight(fee) * 1000 / chainfee.SatPerKWeight(weight) - if effFeeRate < chainfee.FeePerKwFloor { + if effFeeRate < chainfee.AbsoluteFeePerKwFloor { return nil, fmt.Errorf("height=%v, for ChannelPoint(%v) "+ "attempts to create commitment with feerate %v: %v", nextHeight, lc.channelState.FundingOutpoint, diff --git a/lnwallet/channel_test.go b/lnwallet/channel_test.go index 318c0b96..9f10be16 100644 --- a/lnwallet/channel_test.go +++ b/lnwallet/channel_test.go @@ -7638,3 +7638,41 @@ func TestChannelMaxFeeRate(t *testing.T) { assertMaxFeeRate(0.000001, 690) assertMaxFeeRate(0.0000001, chainfee.FeePerKwFloor) } + +// TestChannelFeeRateFloor asserts that valid commitments can be proposed and +// received using chainfee.FeePerKwFloor as the initiator's fee rate. +func TestChannelFeeRateFloor(t *testing.T) { + t.Parallel() + + alice, bob, cleanUp, err := CreateTestChannels( + channeldb.SingleFunderTweaklessBit, + ) + if err != nil { + t.Fatalf("unable to create test channels: %v", err) + } + defer cleanUp() + + // Set the fee rate to the proposing fee rate floor. + minFee := chainfee.FeePerKwFloor + + // Alice is the initiator, so only she can propose fee updates. + if err := alice.UpdateFee(minFee); err != nil { + t.Fatalf("unable to send fee update") + } + if err := bob.ReceiveUpdateFee(minFee); err != nil { + t.Fatalf("unable to receive fee update") + } + + // Check that alice can still sign commitments. + sig, htlcSigs, _, err := alice.SignNextCommitment() + if err != nil { + t.Fatalf("alice unable to sign commitment: %v", err) + } + + // Check that bob can still receive commitments. + err = bob.ReceiveNewCommitment(sig, htlcSigs) + if err != nil { + t.Fatalf("bob unable to process alice's new commitment: %v", + err) + } +}