From dcac5a87f4b8c18832d59fc61e6e2f28a749be18 Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Wed, 3 Mar 2021 15:12:25 -0800 Subject: [PATCH 1/2] lncfg: expose channel update rate limiting options --- config.go | 5 ++++- lncfg/gossip.go | 6 ++++++ sample-lnd.conf | 5 +++++ server.go | 2 ++ 4 files changed, 17 insertions(+), 1 deletion(-) diff --git a/config.go b/config.go index 750d726d1..2a124ccc3 100644 --- a/config.go +++ b/config.go @@ -488,7 +488,10 @@ func DefaultConfig() Config { Backoff: defaultTLSBackoff, }, }, - Gossip: &lncfg.Gossip{}, + Gossip: &lncfg.Gossip{ + MaxChannelUpdateBurst: discovery.DefaultMaxChannelUpdateBurst, + ChannelUpdateInterval: discovery.DefaultChannelUpdateInterval, + }, MaxOutgoingCltvExpiry: htlcswitch.DefaultMaxOutgoingCltvExpiry, MaxChannelFeeAllocation: htlcswitch.DefaultMaxLinkFeeAllocation, MaxCommitFeeRateAnchors: lnwallet.DefaultAnchorsCommitMaxFeeRateSatPerVByte, diff --git a/lncfg/gossip.go b/lncfg/gossip.go index bf8473a70..de1fcae25 100644 --- a/lncfg/gossip.go +++ b/lncfg/gossip.go @@ -1,6 +1,8 @@ package lncfg import ( + "time" + "github.com/lightningnetwork/lnd/discovery" "github.com/lightningnetwork/lnd/routing/route" ) @@ -9,6 +11,10 @@ type Gossip struct { PinnedSyncersRaw []string `long:"pinned-syncers" description:"A set of peers that should always remain in an active sync state, which can be used to closely synchronize the routing tables of two nodes. The value should be comma separated list of hex-encoded pubkeys. Connected peers matching this pubkey will remain active for the duration of the connection and not count towards the NumActiveSyncer count."` PinnedSyncers discovery.PinnedSyncers + + MaxChannelUpdateBurst int `long:"max-channel-update-burst" description:"The maximum number of updates for a specific channel and direction that lnd will accept over the channel update interval."` + + ChannelUpdateInterval time.Duration `long:"channel-update-interval" description:"The interval used to determine how often lnd should allow a burst of new updates for a specific channel and direction."` } // Parse the pubkeys for the pinned syncers. diff --git a/sample-lnd.conf b/sample-lnd.conf index 5a94a3af0..baa7f7787 100644 --- a/sample-lnd.conf +++ b/sample-lnd.conf @@ -1043,3 +1043,8 @@ litecoin.node=ltcd ; peers can be specified by setting multiple flags/fields in the config. ; gossip.pinned-syncers=pubkey1 ; gossip.pinned-syncers=pubkey2 + +; The maximum number of updates for a specific channel and direction that lnd +; will accept over the channel update interval. +; gossip.max-channel-update-burst=10 +; gossip.channel-update-interval=1m diff --git a/server.go b/server.go index 5d7b21a9c..7c6681b6d 100644 --- a/server.go +++ b/server.go @@ -823,6 +823,8 @@ func newServer(cfg *Config, listenAddrs []net.Addr, SubBatchDelay: time.Second * 5, IgnoreHistoricalFilters: cfg.IgnoreHistoricalGossipFilters, PinnedSyncers: cfg.Gossip.PinnedSyncers, + MaxChannelUpdateBurst: cfg.Gossip.MaxChannelUpdateBurst, + ChannelUpdateInterval: cfg.Gossip.ChannelUpdateInterval, }, nodeKeyECDH.PubKey(), ) From 022d44f7767e9f779e5233d7fc00b986ee50a310 Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Wed, 3 Mar 2021 15:15:02 -0800 Subject: [PATCH 2/2] itest: test new channel update rate limiting options --- lntest/itest/lnd_test.go | 77 ++++++++++++++++++++++++++++++++++++---- 1 file changed, 70 insertions(+), 7 deletions(-) diff --git a/lntest/itest/lnd_test.go b/lntest/itest/lnd_test.go index 16c8abf18..1e79370ab 100644 --- a/lntest/itest/lnd_test.go +++ b/lntest/itest/lnd_test.go @@ -1762,6 +1762,13 @@ out: select { case graphUpdate := <-subscription.updateChan: for _, update := range graphUpdate.ChannelUpdates { + if len(expUpdates) == 0 { + t.Fatalf("received unexpected channel "+ + "update from %v for channel %v", + update.AdvertisingNode, + update.ChanId) + } + // For each expected update, check if it matches // the update we just received. for i, exp := range expUpdates { @@ -1811,6 +1818,9 @@ out: case err := <-subscription.errChan: t.Fatalf("unable to recv graph update: %v", err) case <-time.After(defaultTimeout): + if len(expUpdates) == 0 { + return + } t.Fatalf("did not receive channel update") } } @@ -2014,8 +2024,14 @@ func testUpdateChannelPolicy(net *lntest.NetworkHarness, t *harnessTest) { t.Fatalf("bob didn't report channel: %v", err) } - // Create Carol and a new channel Bob->Carol. - carol, err := net.NewNode("Carol", nil) + // Create Carol with options to rate limit channel updates up to 2 per + // day, and create a new channel Bob->Carol. + carol, err := net.NewNode( + "Carol", []string{ + "--gossip.max-channel-update-burst=2", + "--gossip.channel-update-interval=24h", + }, + ) if err != nil { t.Fatalf("unable to create new nodes: %v", err) } @@ -2062,7 +2078,6 @@ func testUpdateChannelPolicy(net *lntest.NetworkHarness, t *harnessTest) { MinHtlc: customMinHtlc, MaxHtlcMsat: defaultMaxHtlc, } - expectedPolicyCarol := &lnrpc.RoutingPolicy{ FeeBaseMsat: defaultFeeBase, FeeRateMilliMsat: defaultFeeRate, @@ -2386,6 +2401,57 @@ func testUpdateChannelPolicy(net *lntest.NetworkHarness, t *harnessTest) { ) } + // Now, to test that Carol is properly rate limiting incoming updates, + // we'll send two more update from Alice. Carol should accept the first, + // but not the second, as she only allows two updates per day and a day + // has yet to elapse from the previous update. + const numUpdatesTilRateLimit = 2 + for i := 0; i < numUpdatesTilRateLimit; i++ { + prevAlicePolicy := *expectedPolicy + baseFee *= 2 + expectedPolicy.FeeBaseMsat = baseFee + req.BaseFeeMsat = baseFee + + ctxt, cancel := context.WithTimeout(ctxb, defaultTimeout) + defer cancel() + _, err = net.Alice.UpdateChannelPolicy(ctxt, req) + if err != nil { + t.Fatalf("unable to update alice's channel policy: %v", err) + } + + // Wait for all nodes to have seen the policy updates for both + // of Alice's channels. Carol will not see the last update as + // the limit has been reached. + for idx, graphSub := range graphSubs { + expUpdates := []expectedChanUpdate{ + {net.Alice.PubKeyStr, expectedPolicy, chanPoint}, + {net.Alice.PubKeyStr, expectedPolicy, chanPoint3}, + } + // Carol was added last, which is why we check the last + // index. + if i == numUpdatesTilRateLimit-1 && idx == len(graphSubs)-1 { + expUpdates = nil + } + waitForChannelUpdate(t, graphSub, expUpdates) + } + + // And finally check that all nodes remembers the policy update + // they received. Since Carol didn't receive the last update, + // she still has Alice's old policy. + for idx, node := range nodes { + policy := expectedPolicy + // Carol was added last, which is why we check the last + // index. + if i == numUpdatesTilRateLimit-1 && idx == len(nodes)-1 { + policy = &prevAlicePolicy + } + assertChannelPolicy( + t, node, net.Alice.PubKeyStr, policy, chanPoint, + chanPoint3, + ) + } + } + // Close the channels. ctxt, _ = context.WithTimeout(ctxb, channelCloseTimeout) closeChannelAndAssert(ctxt, t, net, net.Alice, chanPoint, false) @@ -12831,10 +12897,7 @@ func testSwitchOfflineDeliveryOutgoingOffline( nodesMinusCarol := []*lntest.HarnessNode{net.Bob, net.Alice, dave} err = wait.Predicate(func() bool { predErr = assertNumActiveHtlcs(nodesMinusCarol, 0) - if predErr != nil { - return false - } - return true + return predErr == nil }, defaultTimeout) if err != nil { t.Fatalf("htlc mismatch: %v", predErr)