Skip to content

Commit 53a8d37

Browse files
Crypt-iQRoasbeef
authored andcommitted
discovery: implement ChannelAnnouncement banning
This commit hooks up the banman to the gossiper: - peers that are banned and don't have a channel with us will get disconnected until they are unbanned. - peers that are banned and have a channel with us won't get disconnected, but we will ignore their channel announcements until they are no longer banned. Note that this only disables gossip of announcements to us and still allows us to open channels to them.
1 parent ae33b76 commit 53a8d37

File tree

7 files changed

+480
-60
lines changed

7 files changed

+480
-60
lines changed

discovery/gossiper.go

Lines changed: 165 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -256,6 +256,11 @@ type Config struct {
256256
// here?
257257
AnnSigner lnwallet.MessageSigner
258258

259+
// ScidCloser is an instance of ClosedChannelTracker that helps the
260+
// gossiper cut down on spam channel announcements for already closed
261+
// channels.
262+
ScidCloser ClosedChannelTracker
263+
259264
// NumActiveSyncers is the number of peers for which we should have
260265
// active syncers with. After reaching NumActiveSyncers, any future
261266
// gossip syncers will be passive.
@@ -434,6 +439,9 @@ type AuthenticatedGossiper struct {
434439
// ChannelAnnouncement for the channel is received.
435440
prematureChannelUpdates *lru.Cache[uint64, *cachedNetworkMsg]
436441

442+
// banman tracks our peer's ban status.
443+
banman *banman
444+
437445
// networkMsgs is a channel that carries new network broadcasted
438446
// message from outside the gossiper service to be processed by the
439447
// networkHandler.
@@ -512,6 +520,7 @@ func New(cfg Config, selfKeyDesc *keychain.KeyDescriptor) *AuthenticatedGossiper
512520
maxRejectedUpdates,
513521
),
514522
chanUpdateRateLimiter: make(map[uint64][2]*rate.Limiter),
523+
banman: newBanman(),
515524
}
516525

517526
gossiper.syncMgr = newSyncManager(&SyncManagerCfg{
@@ -606,6 +615,8 @@ func (d *AuthenticatedGossiper) start() error {
606615

607616
d.syncMgr.Start()
608617

618+
d.banman.start()
619+
609620
// Start receiving blocks in its dedicated goroutine.
610621
d.wg.Add(2)
611622
go d.syncBlockHeight()
@@ -762,6 +773,8 @@ func (d *AuthenticatedGossiper) stop() {
762773

763774
d.syncMgr.Stop()
764775

776+
d.banman.stop()
777+
765778
close(d.quit)
766779
d.wg.Wait()
767780

@@ -2459,6 +2472,51 @@ func (d *AuthenticatedGossiper) handleChanAnnouncement(nMsg *networkMsg,
24592472
return nil, true
24602473
}
24612474

2475+
// Check if the channel is already closed in which case we can ignore
2476+
// it.
2477+
closed, err := d.cfg.ScidCloser.IsClosedScid(scid)
2478+
if err != nil {
2479+
log.Errorf("failed to check if scid %v is closed: %v", scid,
2480+
err)
2481+
nMsg.err <- err
2482+
2483+
return nil, false
2484+
}
2485+
2486+
if closed {
2487+
err = fmt.Errorf("ignoring closed channel %v", scid)
2488+
log.Error(err)
2489+
2490+
// If this is an announcement from us, we'll just ignore it.
2491+
if !nMsg.isRemote {
2492+
nMsg.err <- err
2493+
return nil, false
2494+
}
2495+
2496+
// Increment the peer's ban score if they are sending closed
2497+
// channel announcements.
2498+
d.banman.incrementBanScore(nMsg.peer.PubKey())
2499+
2500+
// If the peer is banned and not a channel peer, we'll
2501+
// disconnect them.
2502+
shouldDc, dcErr := d.ShouldDisconnect(nMsg.peer.IdentityKey())
2503+
if dcErr != nil {
2504+
log.Errorf("failed to check if we should disconnect "+
2505+
"peer: %v", dcErr)
2506+
nMsg.err <- dcErr
2507+
2508+
return nil, false
2509+
}
2510+
2511+
if shouldDc {
2512+
nMsg.peer.Disconnect(ErrPeerBanned)
2513+
}
2514+
2515+
nMsg.err <- err
2516+
2517+
return nil, false
2518+
}
2519+
24622520
// If this is a remote channel announcement, then we'll validate all
24632521
// the signatures within the proof as it should be well formed.
24642522
var proof *models.ChannelAuthProof
@@ -2533,7 +2591,7 @@ func (d *AuthenticatedGossiper) handleChanAnnouncement(nMsg *networkMsg,
25332591
// database and is now making decisions based on this DB state, before
25342592
// it writes to the DB.
25352593
d.channelMtx.Lock(scid.ToUint64())
2536-
err := d.cfg.Graph.AddEdge(edge, ops...)
2594+
err = d.cfg.Graph.AddEdge(edge, ops...)
25372595
if err != nil {
25382596
log.Debugf("Graph rejected edge for short_chan_id(%v): %v",
25392597
scid.ToUint64(), err)
@@ -2543,7 +2601,8 @@ func (d *AuthenticatedGossiper) handleChanAnnouncement(nMsg *networkMsg,
25432601
// If the edge was rejected due to already being known, then it
25442602
// may be the case that this new message has a fresh channel
25452603
// proof, so we'll check.
2546-
if graph.IsError(err, graph.ErrIgnored) {
2604+
switch {
2605+
case graph.IsError(err, graph.ErrIgnored):
25472606
// Attempt to process the rejected message to see if we
25482607
// get any new announcements.
25492608
anns, rErr := d.processRejectedEdge(ann, proof)
@@ -2571,7 +2630,55 @@ func (d *AuthenticatedGossiper) handleChanAnnouncement(nMsg *networkMsg,
25712630
nMsg.err <- nil
25722631

25732632
return anns, true
2574-
} else {
2633+
2634+
case graph.IsError(
2635+
err, graph.ErrNoFundingTransaction,
2636+
graph.ErrInvalidFundingOutput,
2637+
):
2638+
key := newRejectCacheKey(
2639+
scid.ToUint64(),
2640+
sourceToPub(nMsg.source),
2641+
)
2642+
_, _ = d.recentRejects.Put(key, &cachedReject{})
2643+
2644+
// Increment the peer's ban score. We check isRemote
2645+
// so we don't actually ban the peer in case of a local
2646+
// bug.
2647+
if nMsg.isRemote {
2648+
d.banman.incrementBanScore(nMsg.peer.PubKey())
2649+
}
2650+
2651+
case graph.IsError(err, graph.ErrChannelSpent):
2652+
key := newRejectCacheKey(
2653+
scid.ToUint64(),
2654+
sourceToPub(nMsg.source),
2655+
)
2656+
_, _ = d.recentRejects.Put(key, &cachedReject{})
2657+
2658+
// Since this channel has already been closed, we'll
2659+
// add it to the graph's closed channel index such that
2660+
// we won't attempt to do expensive validation checks
2661+
// on it again.
2662+
// TODO: Populate the ScidCloser by using closed
2663+
// channel notifications.
2664+
dbErr := d.cfg.ScidCloser.PutClosedScid(scid)
2665+
if dbErr != nil {
2666+
log.Errorf("failed to mark scid(%v) as "+
2667+
"closed: %v", scid, dbErr)
2668+
2669+
nMsg.err <- dbErr
2670+
2671+
return nil, false
2672+
}
2673+
2674+
// Increment the peer's ban score. We check isRemote
2675+
// so we don't accidentally ban ourselves in case of a
2676+
// bug.
2677+
if nMsg.isRemote {
2678+
d.banman.incrementBanScore(nMsg.peer.PubKey())
2679+
}
2680+
2681+
default:
25752682
// Otherwise, this is just a regular rejected edge.
25762683
key := newRejectCacheKey(
25772684
scid.ToUint64(),
@@ -2580,7 +2687,29 @@ func (d *AuthenticatedGossiper) handleChanAnnouncement(nMsg *networkMsg,
25802687
_, _ = d.recentRejects.Put(key, &cachedReject{})
25812688
}
25822689

2690+
if !nMsg.isRemote {
2691+
log.Errorf("failed to add edge for local channel: %v",
2692+
err)
2693+
nMsg.err <- err
2694+
2695+
return nil, false
2696+
}
2697+
2698+
shouldDc, dcErr := d.ShouldDisconnect(nMsg.peer.IdentityKey())
2699+
if dcErr != nil {
2700+
log.Errorf("failed to check if we should disconnect "+
2701+
"peer: %v", dcErr)
2702+
nMsg.err <- dcErr
2703+
2704+
return nil, false
2705+
}
2706+
2707+
if shouldDc {
2708+
nMsg.peer.Disconnect(ErrPeerBanned)
2709+
}
2710+
25832711
nMsg.err <- err
2712+
25842713
return nil, false
25852714
}
25862715

@@ -3385,3 +3514,36 @@ func (d *AuthenticatedGossiper) handleAnnSig(nMsg *networkMsg,
33853514
nMsg.err <- nil
33863515
return announcements, true
33873516
}
3517+
3518+
// isBanned returns true if the peer identified by pubkey is banned for sending
3519+
// invalid channel announcements.
3520+
func (d *AuthenticatedGossiper) isBanned(pubkey [33]byte) bool {
3521+
return d.banman.isBanned(pubkey)
3522+
}
3523+
3524+
// ShouldDisconnect returns true if we should disconnect the peer identified by
3525+
// pubkey.
3526+
func (d *AuthenticatedGossiper) ShouldDisconnect(pubkey *btcec.PublicKey) (
3527+
bool, error) {
3528+
3529+
pubkeySer := pubkey.SerializeCompressed()
3530+
3531+
var pubkeyBytes [33]byte
3532+
copy(pubkeyBytes[:], pubkeySer)
3533+
3534+
// If the public key is banned, check whether or not this is a channel
3535+
// peer.
3536+
if d.isBanned(pubkeyBytes) {
3537+
isChanPeer, err := d.cfg.ScidCloser.IsChannelPeer(pubkey)
3538+
if err != nil {
3539+
return false, err
3540+
}
3541+
3542+
// We should only disconnect non-channel peers.
3543+
if !isChanPeer {
3544+
return true, nil
3545+
}
3546+
}
3547+
3548+
return false, nil
3549+
}

0 commit comments

Comments
 (0)