From 6622c4814ee29e4857698b80aacf4d39c7940934 Mon Sep 17 00:00:00 2001 From: Conner Fromknecht Date: Fri, 24 Jul 2020 13:13:56 -0700 Subject: [PATCH] multi: enforce routing.MinCLTVDelta=18 for invoices + chanupd This commit clamps all user-chosen CLTVs in LND to be at least 18, which is the new conservative value used in the sepc. This minimum is applied uniformly to forwarding CLTV deltas (via channel updates) as well as final CLTV deltas for new invoices. --- config.go | 2 +- lnrpc/invoicesrpc/addinvoice.go | 9 +++++++++ routing/pathfind_test.go | 26 +++++++++++++------------- routing/router.go | 21 +++++++++++++++++++++ routing/router_test.go | 7 +++---- 5 files changed, 47 insertions(+), 18 deletions(-) diff --git a/config.go b/config.go index 368661ba..3a52ca96 100644 --- a/config.go +++ b/config.go @@ -73,7 +73,7 @@ const ( // minTimeLockDelta is the minimum timelock we require for incoming // HTLCs on our channels. - minTimeLockDelta = 4 + minTimeLockDelta = routing.MinCLTVDelta // defaultAcceptorTimeout is the time after which an RPCAcceptor will time // out and return false if it hasn't yet received a response. diff --git a/lnrpc/invoicesrpc/addinvoice.go b/lnrpc/invoicesrpc/addinvoice.go index c1cde48a..573d2f38 100644 --- a/lnrpc/invoicesrpc/addinvoice.go +++ b/lnrpc/invoicesrpc/addinvoice.go @@ -17,6 +17,7 @@ import ( "github.com/lightningnetwork/lnd/lntypes" "github.com/lightningnetwork/lnd/lnwire" "github.com/lightningnetwork/lnd/netann" + "github.com/lightningnetwork/lnd/routing" "github.com/lightningnetwork/lnd/zpay32" ) @@ -228,6 +229,14 @@ func AddInvoice(ctx context.Context, cfg *AddInvoiceConfig, return nil, nil, fmt.Errorf("CLTV delta of %v is too large, max "+ "accepted is: %v", invoice.CltvExpiry, math.MaxUint16) case invoice.CltvExpiry != 0: + // Disallow user-chosen final CLTV deltas below the required + // minimum. + if invoice.CltvExpiry < routing.MinCLTVDelta { + return nil, nil, fmt.Errorf("CLTV delta of %v must be "+ + "greater than minimum of %v", + routing.MinCLTVDelta, invoice.CltvExpiry) + } + options = append(options, zpay32.CLTVExpiry(invoice.CltvExpiry)) default: diff --git a/routing/pathfind_test.go b/routing/pathfind_test.go index ba1f39eb..92696ca3 100644 --- a/routing/pathfind_test.go +++ b/routing/pathfind_test.go @@ -26,7 +26,6 @@ import ( "github.com/lightningnetwork/lnd/lnwire" "github.com/lightningnetwork/lnd/record" "github.com/lightningnetwork/lnd/routing/route" - "github.com/lightningnetwork/lnd/zpay32" ) const ( @@ -2076,7 +2075,7 @@ func TestPathFindSpecExample(t *testing.T) { const amt lnwire.MilliSatoshi = 4999999 route, err := ctx.router.FindRoute( bobNode.PubKeyBytes, carol, amt, noRestrictions, nil, nil, - zpay32.DefaultFinalCLTVDelta, + MinCLTVDelta, ) if err != nil { t.Fatalf("unable to find route: %v", err) @@ -2100,14 +2099,14 @@ func TestPathFindSpecExample(t *testing.T) { t.Fatalf("wrong hop fee: got %v, expected %v", fee, 0) } - // The CLTV expiry should be the current height plus 9 (the expiry for + // The CLTV expiry should be the current height plus 18 (the expiry for // the B -> C channel. if route.TotalTimeLock != - startingHeight+zpay32.DefaultFinalCLTVDelta { + startingHeight+MinCLTVDelta { t.Fatalf("wrong total time lock: got %v, expecting %v", route.TotalTimeLock, - startingHeight+zpay32.DefaultFinalCLTVDelta) + startingHeight+MinCLTVDelta) } // Next, we'll set A as the source node so we can assert that we create @@ -2132,7 +2131,7 @@ func TestPathFindSpecExample(t *testing.T) { // We'll now request a route from A -> B -> C. route, err = ctx.router.FindRoute( source.PubKeyBytes, carol, amt, noRestrictions, nil, nil, - zpay32.DefaultFinalCLTVDelta, + MinCLTVDelta, ) if err != nil { t.Fatalf("unable to find routes: %v", err) @@ -2145,15 +2144,16 @@ func TestPathFindSpecExample(t *testing.T) { } // The total amount should factor in a fee of 10199 and also use a CLTV - // delta total of 29 (20 + 9), + // delta total of 38 (20 + 18), expectedAmt := lnwire.MilliSatoshi(5010198) if route.TotalAmount != expectedAmt { t.Fatalf("wrong amount: got %v, expected %v", route.TotalAmount, expectedAmt) } - if route.TotalTimeLock != startingHeight+29 { + expectedDelta := uint32(20 + MinCLTVDelta) + if route.TotalTimeLock != startingHeight+expectedDelta { t.Fatalf("wrong total time lock: got %v, expecting %v", - route.TotalTimeLock, startingHeight+29) + route.TotalTimeLock, startingHeight+expectedDelta) } // Ensure that the hops of the route are properly crafted. @@ -2188,11 +2188,11 @@ func TestPathFindSpecExample(t *testing.T) { // The outgoing CLTV value itself should be the current height plus 30 // to meet Carol's requirements. if route.Hops[0].OutgoingTimeLock != - startingHeight+zpay32.DefaultFinalCLTVDelta { + startingHeight+MinCLTVDelta { t.Fatalf("wrong total time lock: got %v, expecting %v", route.Hops[0].OutgoingTimeLock, - startingHeight+zpay32.DefaultFinalCLTVDelta) + startingHeight+MinCLTVDelta) } // For B -> C, we assert that the final hop also has the proper @@ -2203,11 +2203,11 @@ func TestPathFindSpecExample(t *testing.T) { lastHop.AmtToForward, amt) } if lastHop.OutgoingTimeLock != - startingHeight+zpay32.DefaultFinalCLTVDelta { + startingHeight+MinCLTVDelta { t.Fatalf("wrong total time lock: got %v, expecting %v", lastHop.OutgoingTimeLock, - startingHeight+zpay32.DefaultFinalCLTVDelta) + startingHeight+MinCLTVDelta) } } diff --git a/routing/router.go b/routing/router.go index 9fa7bea5..84fb9d0b 100644 --- a/routing/router.go +++ b/routing/router.go @@ -46,6 +46,27 @@ const ( // stats related to processing new channels, updates, or node // announcements. defaultStatInterval = time.Minute + + // MinCLTVDelta is the minimum CLTV value accepted by LND for all + // timelock deltas. This includes both forwarding CLTV deltas set on + // channel updates, as well as final CLTV deltas used to create BOLT 11 + // payment requests. + // + // NOTE: For payment requests, BOLT 11 stipulates that a final CLTV + // delta of 9 should be used when no value is decoded. This however + // leads to inflexiblity in upgrading this default parameter, since it + // can create inconsistencies around the assumed value between sender + // and receiver. Specifically, if the receiver assumes a higher value + // than the sender, the receiver will always see the received HTLCs as + // invalid due to their timelock not meeting the required delta. + // + // We skirt this by always setting an explicit CLTV delta when creating + // invoices. This allows LND nodes to freely update the minimum without + // creating incompatibilities during the upgrade process. For some time + // LND has used an explicit default final CLTV delta of 40 blocks for + // bitcoin (160 for litecoin), though we now clamp the lower end of this + // range for user-chosen deltas to 18 blocks to be conservative. + MinCLTVDelta = 18 ) var ( diff --git a/routing/router_test.go b/routing/router_test.go index 3f31225b..9844b526 100644 --- a/routing/router_test.go +++ b/routing/router_test.go @@ -23,7 +23,6 @@ import ( "github.com/lightningnetwork/lnd/lnwire" "github.com/lightningnetwork/lnd/record" "github.com/lightningnetwork/lnd/routing/route" - "github.com/lightningnetwork/lnd/zpay32" ) var uniquePaymentID uint64 = 1 // to be used atomically @@ -223,7 +222,7 @@ func TestFindRoutesWithFeeLimit(t *testing.T) { route, err := ctx.router.FindRoute( ctx.router.selfNode.PubKeyBytes, target, paymentAmt, restrictions, nil, nil, - zpay32.DefaultFinalCLTVDelta, + MinCLTVDelta, ) if err != nil { t.Fatalf("unable to find any routes: %v", err) @@ -1284,7 +1283,7 @@ func TestAddEdgeUnknownVertexes(t *testing.T) { _, err = ctx.router.FindRoute( ctx.router.selfNode.PubKeyBytes, targetPubKeyBytes, paymentAmt, noRestrictions, nil, nil, - zpay32.DefaultFinalCLTVDelta, + MinCLTVDelta, ) if err != nil { t.Fatalf("unable to find any routes: %v", err) @@ -1327,7 +1326,7 @@ func TestAddEdgeUnknownVertexes(t *testing.T) { _, err = ctx.router.FindRoute( ctx.router.selfNode.PubKeyBytes, targetPubKeyBytes, paymentAmt, noRestrictions, nil, nil, - zpay32.DefaultFinalCLTVDelta, + MinCLTVDelta, ) if err != nil { t.Fatalf("unable to find any routes: %v", err)