From 87ff463abdbadc3705d3fda22579e20b0a53b154 Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Fri, 26 Jul 2019 18:05:58 -0700 Subject: [PATCH 1/2] htlcswitch+lnd: make max cltv expiry configurable --- config.go | 4 ++++ htlcswitch/link.go | 23 ++++++++++++++--------- htlcswitch/link_test.go | 13 ++++++++----- htlcswitch/test_utils.go | 1 + peer.go | 1 + 5 files changed, 28 insertions(+), 14 deletions(-) diff --git a/config.go b/config.go index 6dea524dc8a2..ab38668633ca 100644 --- a/config.go +++ b/config.go @@ -26,6 +26,7 @@ import ( "github.com/lightningnetwork/lnd/chanbackup" "github.com/lightningnetwork/lnd/channeldb" "github.com/lightningnetwork/lnd/discovery" + "github.com/lightningnetwork/lnd/htlcswitch" "github.com/lightningnetwork/lnd/htlcswitch/hodl" "github.com/lightningnetwork/lnd/lncfg" "github.com/lightningnetwork/lnd/lnrpc/routerrpc" @@ -312,6 +313,8 @@ type config struct { StaggerInitialReconnect bool `long:"stagger-initial-reconnect" description:"If true, will apply a randomized staggering between 0s and 30s when reconnecting to persistent peers on startup. The first 10 reconnections will be attempted instantly, regardless of the flag's value"` + MaxOutgoingCltvExpiry uint32 `long:"max-cltv-expiry" description:"The maximum number of blocks funds could be locked up for when forwarding payments."` + net tor.Net Routing *routing.Conf `group:"routing" namespace:"routing"` @@ -425,6 +428,7 @@ func loadConfig() (*config, error) { Watchtower: &lncfg.Watchtower{ TowerDir: defaultTowerDir, }, + MaxOutgoingCltvExpiry: htlcswitch.DefaultMaxOutgoingCltvExpiry, } // Pre-parse the command line options to pick up an alternative config diff --git a/htlcswitch/link.go b/htlcswitch/link.go index cd2410dd8cd2..251ad2bd026d 100644 --- a/htlcswitch/link.go +++ b/htlcswitch/link.go @@ -30,16 +30,16 @@ func init() { } const ( - // maxCltvExpiry is the maximum outgoing time lock that the node accepts - // for forwarded payments. The value is relative to the current block - // height. The reason to have a maximum is to prevent funds getting - // locked up unreasonably long. Otherwise, an attacker willing to lock - // its own funds too, could force the funds of this node to be locked up - // for an indefinite (max int32) number of blocks. + // DefaultMaxOutgoingCltvExpiry is the maximum outgoing time lock that + // the node accepts for forwarded payments. The value is relative to the + // current block height. The reason to have a maximum is to prevent + // funds getting locked up unreasonably long. Otherwise, an attacker + // willing to lock its own funds too, could force the funds of this node + // to be locked up for an indefinite (max int32) number of blocks. // // The value 5000 is based on the maximum number of hops (20), the // default cltv delta (144) and some extra margin. - maxCltvExpiry = 5000 + DefaultMaxOutgoingCltvExpiry = 5000 // DefaultMinLinkFeeUpdateTimeout represents the minimum interval in // which a link should propose to update its commitment fee rate. @@ -249,6 +249,11 @@ type ChannelLinkConfig struct { // encrypting, and uploading of justice transactions to the daemon's // configured set of watchtowers. TowerClient TowerClient + + // MaxCltvExpiry is the maximum outgoing timelock that the link should + // accept for a forwarded HTLC. The value is relative to the current + // block height. + MaxOutgoingCltvExpiry uint32 } // channelLink is the service which drives a channel's commitment update @@ -2337,10 +2342,10 @@ func (l *channelLink) htlcSatifiesPolicyOutgoing(policy ForwardingPolicy, } // Check absolute max delta. - if timeout > maxCltvExpiry+heightNow { + if timeout > l.cfg.MaxOutgoingCltvExpiry+heightNow { l.errorf("outgoing htlc(%x) has a time lock too far in the "+ "future: got %v, but maximum is %v", payHash[:], - timeout-heightNow, maxCltvExpiry) + timeout-heightNow, l.cfg.MaxOutgoingCltvExpiry) return &lnwire.FailExpiryTooFar{} } diff --git a/htlcswitch/link_test.go b/htlcswitch/link_test.go index 7e4a9302636d..fc34422e7a81 100644 --- a/htlcswitch/link_test.go +++ b/htlcswitch/link_test.go @@ -1689,9 +1689,10 @@ func newSingleLinkTestHarness(chanAmt, chanReserve btcutil.Amount) ( FwdPkgGCTicker: ticker.NewForce(15 * time.Second), // Make the BatchSize and Min/MaxFeeUpdateTimeout large enough // to not trigger commit updates automatically during tests. - BatchSize: 10000, - MinFeeUpdateTimeout: 30 * time.Minute, - MaxFeeUpdateTimeout: 40 * time.Minute, + BatchSize: 10000, + MinFeeUpdateTimeout: 30 * time.Minute, + MaxFeeUpdateTimeout: 40 * time.Minute, + MaxOutgoingCltvExpiry: DefaultMaxOutgoingCltvExpiry, } const startingHeight = 100 @@ -4238,8 +4239,9 @@ func (h *persistentLinkHarness) restartLink( MinFeeUpdateTimeout: 30 * time.Minute, MaxFeeUpdateTimeout: 40 * time.Minute, // Set any hodl flags requested for the new link. - HodlMask: hodl.MaskFromFlags(hodlFlags...), - DebugHTLC: len(hodlFlags) > 0, + HodlMask: hodl.MaskFromFlags(hodlFlags...), + DebugHTLC: len(hodlFlags) > 0, + MaxOutgoingCltvExpiry: DefaultMaxOutgoingCltvExpiry, } const startingHeight = 100 @@ -5559,6 +5561,7 @@ func TestHtlcSatisfyPolicy(t *testing.T) { BaseFee: 10, }, FetchLastChannelUpdate: fetchLastChannelUpdate, + MaxOutgoingCltvExpiry: DefaultMaxOutgoingCltvExpiry, }, } diff --git a/htlcswitch/test_utils.go b/htlcswitch/test_utils.go index 5ae959bc329e..212e20e3a3ae 100644 --- a/htlcswitch/test_utils.go +++ b/htlcswitch/test_utils.go @@ -1115,6 +1115,7 @@ func (h *hopNetwork) createChannelLink(server, peer *mockServer, MaxFeeUpdateTimeout: maxFeeUpdateTimeout, OnChannelFailure: func(lnwire.ChannelID, lnwire.ShortChannelID, LinkFailureError) {}, OutgoingCltvRejectDelta: 3, + MaxOutgoingCltvExpiry: DefaultMaxOutgoingCltvExpiry, }, channel, ) diff --git a/peer.go b/peer.go index 36a6fee72e2a..c2f22e5b802d 100644 --- a/peer.go +++ b/peer.go @@ -585,6 +585,7 @@ func (p *peer) addLink(chanPoint *wire.OutPoint, MaxFeeUpdateTimeout: htlcswitch.DefaultMaxLinkFeeUpdateTimeout, OutgoingCltvRejectDelta: p.outgoingCltvRejectDelta, TowerClient: p.server.towerClient, + MaxOutgoingCltvExpiry: cfg.MaxOutgoingCltvExpiry, } link := htlcswitch.NewChannelLink(linkCfg, lnChan) From 24ca962f75a96cec2a72799e85680d73840c0c76 Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Fri, 26 Jul 2019 18:06:40 -0700 Subject: [PATCH 2/2] htlcswitch: lower max outgoing cltv expiry to one week worth of blocks The current value was based on the previous default CLTV delta of 144 blocks. This has been lowered to 40 since lnd v0.6.0-beta, making the current value of 5000 blocks a bit high. Lowering it to one week should be more than enough to account for the other major lightning implementations. Eclair currently has a default CLTV delta of 144, while c-lightning's is 14. --- htlcswitch/link.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/htlcswitch/link.go b/htlcswitch/link.go index 251ad2bd026d..98e717f38b67 100644 --- a/htlcswitch/link.go +++ b/htlcswitch/link.go @@ -37,9 +37,11 @@ const ( // willing to lock its own funds too, could force the funds of this node // to be locked up for an indefinite (max int32) number of blocks. // - // The value 5000 is based on the maximum number of hops (20), the - // default cltv delta (144) and some extra margin. - DefaultMaxOutgoingCltvExpiry = 5000 + // The value 1008 corresponds to on average one week worth of blocks and + // is based on the maximum number of hops (20), the default cltv delta + // (40) and some extra margin to account for the other lightning + // implementations. + DefaultMaxOutgoingCltvExpiry = 1008 // DefaultMinLinkFeeUpdateTimeout represents the minimum interval in // which a link should propose to update its commitment fee rate.