diff --git a/config.go b/config.go index 4125c21d3..3a0af8df2 100644 --- a/config.go +++ b/config.go @@ -317,6 +317,8 @@ type config struct { MaxOutgoingCltvExpiry uint32 `long:"max-cltv-expiry" description:"The maximum number of blocks funds could be locked up for when forwarding payments."` + MaxChannelFeeAllocation float64 `long:"max-channel-fee-allocation" description:"The maximum percentage of total funds that can be allocated to a channel's commitment fee. This only applies for the initiator of the channel. Valid values are within [0.1, 1]."` + net tor.Net Routing *routing.Conf `group:"routing" namespace:"routing"` @@ -432,7 +434,8 @@ func loadConfig() (*config, error) { Watchtower: &lncfg.Watchtower{ TowerDir: defaultTowerDir, }, - MaxOutgoingCltvExpiry: htlcswitch.DefaultMaxOutgoingCltvExpiry, + MaxOutgoingCltvExpiry: htlcswitch.DefaultMaxOutgoingCltvExpiry, + MaxChannelFeeAllocation: htlcswitch.DefaultMaxLinkFeeAllocation, } // Pre-parse the command line options to pick up an alternative config @@ -591,6 +594,13 @@ func loadConfig() (*config, error) { return nil, err } + // Ensure a valid max channel fee allocation was set. + if cfg.MaxChannelFeeAllocation <= 0 || cfg.MaxChannelFeeAllocation > 1 { + return nil, fmt.Errorf("invalid max channel fee allocation: "+ + "%v, must be within (0, 1]", + cfg.MaxChannelFeeAllocation) + } + // Validate the Tor config parameters. socks, err := lncfg.ParseAddressString( cfg.Tor.SOCKS, strconv.Itoa(defaultTorSOCKSPort), diff --git a/htlcswitch/link.go b/htlcswitch/link.go index 3b4d10920..661b8a929 100644 --- a/htlcswitch/link.go +++ b/htlcswitch/link.go @@ -4,6 +4,7 @@ import ( "bytes" "crypto/sha256" "fmt" + "math" prand "math/rand" "sync" "sync/atomic" @@ -50,6 +51,11 @@ const ( // DefaultMaxLinkFeeUpdateTimeout represents the maximum interval in // which a link should propose to update its commitment fee rate. DefaultMaxLinkFeeUpdateTimeout = 60 * time.Minute + + // DefaultMaxLinkFeeAllocation is the highest allocation we'll allow + // a channel's commitment fee to be of its balance. This only applies to + // the initiator of the channel. + DefaultMaxLinkFeeAllocation float64 = 0.5 ) // ForwardingPolicy describes the set of constraints that a given ChannelLink @@ -250,6 +256,11 @@ type ChannelLinkConfig struct { // accept for a forwarded HTLC. The value is relative to the current // block height. MaxOutgoingCltvExpiry uint32 + + // MaxFeeAllocation is the highest allocation we'll allow a channel's + // commitment fee to be of its balance. This only applies to the + // initiator of the channel. + MaxFeeAllocation float64 } // channelLink is the service which drives a channel's commitment update @@ -995,22 +1006,27 @@ out: // If we are the initiator, then we'll sample the // current fee rate to get into the chain within 3 // blocks. - feePerKw, err := l.sampleNetworkFee() + netFee, err := l.sampleNetworkFee() if err != nil { log.Errorf("unable to sample network fee: %v", err) continue } // We'll check to see if we should update the fee rate - // based on our current set fee rate. + // based on our current set fee rate. We'll cap the new + // fee rate to our max fee allocation. commitFee := l.channel.CommitFeeRate() - if !shouldAdjustCommitFee(feePerKw, commitFee) { + maxFee := l.channel.MaxFeeRate(l.cfg.MaxFeeAllocation) + newCommitFee := lnwallet.SatPerKWeight( + math.Min(float64(netFee), float64(maxFee)), + ) + if !shouldAdjustCommitFee(newCommitFee, commitFee) { continue } // If we do, then we'll send a new UpdateFee message to // the remote party, to be locked in with a new update. - if err := l.updateChannelFee(feePerKw); err != nil { + if err := l.updateChannelFee(newCommitFee); err != nil { log.Errorf("unable to update fee rate: %v", err) continue } diff --git a/htlcswitch/link_test.go b/htlcswitch/link_test.go index 8d17dbb66..3c337c169 100644 --- a/htlcswitch/link_test.go +++ b/htlcswitch/link_test.go @@ -1686,6 +1686,7 @@ func newSingleLinkTestHarness(chanAmt, chanReserve btcutil.Amount) ( MinFeeUpdateTimeout: 30 * time.Minute, MaxFeeUpdateTimeout: 40 * time.Minute, MaxOutgoingCltvExpiry: DefaultMaxOutgoingCltvExpiry, + MaxFeeAllocation: DefaultMaxLinkFeeAllocation, } const startingHeight = 100 @@ -3736,9 +3737,10 @@ func TestChannelLinkUpdateCommitFee(t *testing.T) { // First, we'll create our traditional three hop network. We'll only be // interacting with and asserting the state of two of the end points // for this test. + const aliceInitialBalance = btcutil.SatoshiPerBitcoin * 3 channels, cleanUp, _, err := createClusterChannels( - btcutil.SatoshiPerBitcoin*3, - btcutil.SatoshiPerBitcoin*5) + aliceInitialBalance, btcutil.SatoshiPerBitcoin*5, + ) if err != nil { t.Fatalf("unable to create channel: %v", err) } @@ -3757,8 +3759,15 @@ func TestChannelLinkUpdateCommitFee(t *testing.T) { {"alice", "bob", &lnwire.FundingLocked{}, false}, {"bob", "alice", &lnwire.FundingLocked{}, false}, + // First fee update. {"alice", "bob", &lnwire.UpdateFee{}, false}, + {"alice", "bob", &lnwire.CommitSig{}, false}, + {"bob", "alice", &lnwire.RevokeAndAck{}, false}, + {"bob", "alice", &lnwire.CommitSig{}, false}, + {"alice", "bob", &lnwire.RevokeAndAck{}, false}, + // Second fee update. + {"alice", "bob", &lnwire.UpdateFee{}, false}, {"alice", "bob", &lnwire.CommitSig{}, false}, {"bob", "alice", &lnwire.RevokeAndAck{}, false}, {"bob", "alice", &lnwire.CommitSig{}, false}, @@ -3775,62 +3784,73 @@ func TestChannelLinkUpdateCommitFee(t *testing.T) { defer n.stop() defer n.feeEstimator.Stop() - // For the sake of this test, we'll reset the timer to fire in a second - // so that Alice's link queries for a new network fee. - n.aliceChannelLink.updateFeeTimer.Reset(time.Millisecond) - startingFeeRate := channels.aliceToBob.CommitFeeRate() - // Next, we'll send the first fee rate response to Alice. - select { - case n.feeEstimator.byteFeeIn <- startingFeeRate: - case <-time.After(time.Second * 5): - t.Fatalf("alice didn't query for the new network fee") + // triggerFeeUpdate is a helper closure to determine whether a fee + // update was triggered and completed properly. + triggerFeeUpdate := func(feeEstimate, newFeeRate lnwallet.SatPerKWeight, + shouldUpdate bool) { + + t.Helper() + + // Record the fee rates before the links process the fee update + // to test the case where a fee update isn't triggered. + aliceBefore := channels.aliceToBob.CommitFeeRate() + bobBefore := channels.bobToAlice.CommitFeeRate() + + // For the sake of this test, we'll reset the timer so that + // Alice's link queries for a new network fee. + n.aliceChannelLink.updateFeeTimer.Reset(time.Millisecond) + + // Next, we'll send the first fee rate response to Alice. + select { + case n.feeEstimator.byteFeeIn <- feeEstimate: + case <-time.After(time.Second * 5): + t.Fatalf("alice didn't query for the new network fee") + } + + // Give the links some time to process the fee update. + time.Sleep(time.Second) + + // Record the fee rates after the links have processed the fee + // update and ensure they are correct based on whether a fee + // update should have been triggered. + aliceAfter := channels.aliceToBob.CommitFeeRate() + bobAfter := channels.bobToAlice.CommitFeeRate() + + switch { + case shouldUpdate && aliceAfter != newFeeRate: + t.Fatalf("alice's fee rate didn't change: expected %v, "+ + "got %v", newFeeRate, aliceAfter) + + case shouldUpdate && bobAfter != newFeeRate: + t.Fatalf("bob's fee rate didn't change: expected %v, "+ + "got %v", newFeeRate, bobAfter) + + case !shouldUpdate && aliceAfter != aliceBefore: + t.Fatalf("alice's fee rate shouldn't have changed: "+ + "expected %v, got %v", aliceAfter, aliceAfter) + + case !shouldUpdate && bobAfter != bobBefore: + t.Fatalf("bob's fee rate shouldn't have changed: "+ + "expected %v, got %v", bobBefore, bobAfter) + } } - time.Sleep(time.Second) + // Triggering the link to update the fee of the channel with the same + // fee rate should not send a fee update. + triggerFeeUpdate(startingFeeRate, startingFeeRate, false) - // The fee rate on the alice <-> bob channel should still be the same - // on both sides. - aliceFeeRate := channels.aliceToBob.CommitFeeRate() - bobFeeRate := channels.bobToAlice.CommitFeeRate() - if aliceFeeRate != startingFeeRate { - t.Fatalf("alice's fee rate shouldn't have changed: "+ - "expected %v, got %v", aliceFeeRate, startingFeeRate) - } - if bobFeeRate != startingFeeRate { - t.Fatalf("bob's fee rate shouldn't have changed: "+ - "expected %v, got %v", bobFeeRate, startingFeeRate) - } - - // We'll reset the timer once again to ensure Alice's link queries for a - // new network fee. - n.aliceChannelLink.updateFeeTimer.Reset(time.Millisecond) - - // Next, we'll set up a deliver a fee rate that's triple the current - // fee rate. This should cause the Alice (the initiator) to trigger a - // fee update. + // Triggering the link to update the fee of the channel with a much + // larger fee rate _should_ send a fee update. newFeeRate := startingFeeRate * 3 - select { - case n.feeEstimator.byteFeeIn <- newFeeRate: - case <-time.After(time.Second * 5): - t.Fatalf("alice didn't query for the new network fee") - } + triggerFeeUpdate(newFeeRate, newFeeRate, true) - time.Sleep(time.Second) - - // At this point, Alice should've triggered a new fee update that - // increased the fee rate to match the new rate. - aliceFeeRate = channels.aliceToBob.CommitFeeRate() - bobFeeRate = channels.bobToAlice.CommitFeeRate() - if aliceFeeRate != newFeeRate { - t.Fatalf("alice's fee rate didn't change: expected %v, got %v", - newFeeRate, aliceFeeRate) - } - if bobFeeRate != newFeeRate { - t.Fatalf("bob's fee rate didn't change: expected %v, got %v", - newFeeRate, aliceFeeRate) - } + // Triggering the link to update the fee of the channel with a fee rate + // that exceeds its maximum fee allocation should result in a fee rate + // corresponding to the maximum fee allocation. + const maxFeeRate lnwallet.SatPerKWeight = 207182320 + triggerFeeUpdate(maxFeeRate+1, maxFeeRate, true) } // TestChannelLinkAcceptDuplicatePayment tests that if a link receives an @@ -4232,6 +4252,7 @@ func (h *persistentLinkHarness) restartLink( // Set any hodl flags requested for the new link. HodlMask: hodl.MaskFromFlags(hodlFlags...), MaxOutgoingCltvExpiry: DefaultMaxOutgoingCltvExpiry, + MaxFeeAllocation: DefaultMaxLinkFeeAllocation, } const startingHeight = 100 diff --git a/htlcswitch/test_utils.go b/htlcswitch/test_utils.go index d0fe1abbb..50a73a234 100644 --- a/htlcswitch/test_utils.go +++ b/htlcswitch/test_utils.go @@ -1117,6 +1117,7 @@ func (h *hopNetwork) createChannelLink(server, peer *mockServer, OnChannelFailure: func(lnwire.ChannelID, lnwire.ShortChannelID, LinkFailureError) {}, OutgoingCltvRejectDelta: 3, MaxOutgoingCltvExpiry: DefaultMaxOutgoingCltvExpiry, + MaxFeeAllocation: DefaultMaxLinkFeeAllocation, }, channel, ) diff --git a/lnwallet/channel.go b/lnwallet/channel.go index 7cf16dd9d..dd7182716 100644 --- a/lnwallet/channel.go +++ b/lnwallet/channel.go @@ -6280,6 +6280,26 @@ func (lc *LightningChannel) CalcFee(feeRate SatPerKWeight) btcutil.Amount { return feeRate.FeeForWeight(input.CommitWeight) } +// MaxFeeRate returns the maximum fee rate given an allocation of the channel +// initiator's spendable balance. This can be useful to determine when we should +// stop proposing fee updates that exceed our maximum allocation. +// +// NOTE: This should only be used for channels in which the local commitment is +// the initiator. +func (lc *LightningChannel) MaxFeeRate(maxAllocation float64) SatPerKWeight { + lc.RLock() + defer lc.RUnlock() + + // The maximum fee depends of the available balance that can be + // committed towards fees. + balance, weight := lc.availableBalance() + feeBalance := float64( + balance.ToSatoshis() + lc.channelState.LocalCommitment.CommitFee, + ) + maxFee := feeBalance * maxAllocation + return SatPerKWeight(maxFee / (float64(weight) / 1000)) +} + // RemoteNextRevocation returns the channelState's RemoteNextRevocation. func (lc *LightningChannel) RemoteNextRevocation() *btcec.PublicKey { lc.RLock() diff --git a/peer.go b/peer.go index cad00dcb7..c57f23a4e 100644 --- a/peer.go +++ b/peer.go @@ -585,6 +585,7 @@ func (p *peer) addLink(chanPoint *wire.OutPoint, OutgoingCltvRejectDelta: p.outgoingCltvRejectDelta, TowerClient: p.server.towerClient, MaxOutgoingCltvExpiry: cfg.MaxOutgoingCltvExpiry, + MaxFeeAllocation: cfg.MaxChannelFeeAllocation, } link := htlcswitch.NewChannelLink(linkCfg, lnChan)