Skip to content

fix listchannels rpc call race condition #9849

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions docs/release-notes/release-notes-0.20.0.md
Original file line number Diff line number Diff line change
@@ -20,6 +20,11 @@

# Bug Fixes

* [Fixed a bug](https://github.com/lightningnetwork/lnd/pull/9849) where due to
a disconnected peer the `peerAccessMan` would prevent the channel notfier to
send the OpenChannel notification to the Event Store which would cause the rpc
call `listchannels` to error in some edge cases.

# New Features

## Functional Enhancements
27 changes: 15 additions & 12 deletions rpcserver.go
Original file line number Diff line number Diff line change
@@ -5005,23 +5005,26 @@ func createRPCOpenChannel(r *rpcServer, dbChannel *channeldb.OpenChannel,
// being notified of it.
outpoint := dbChannel.FundingOutpoint
info, err := r.server.chanEventStore.GetChanInfo(outpoint, peer)
switch err {
// If the store does not know about the channel, we just log it.
case chanfitness.ErrChannelNotFound:
rpcsLog.Infof("channel: %v not found by channel event store",
outpoint)

// If we got our channel info, we further populate the channel.
case nil:
// If we got our channel info, populate the channel.
if err == nil {
channel.Uptime = int64(info.Uptime.Seconds())
channel.Lifetime = int64(info.Lifetime.Seconds())

// If we get an unexpected error, we return it.
default:
return nil, err
return channel, nil
}

return channel, nil
// In the case of expected errors, we just log it and return the
// channel.
if errors.Is(err, chanfitness.ErrChannelNotFound) ||
errors.Is(err, chanfitness.ErrPeerNotFound) {

rpcsLog.Infof("channel: %v not found by channel event store",
outpoint)
return channel, nil
}

// Return any unexpected errors.
return nil, err
}

// createRPCClosedChannel creates an *lnrpc.ClosedChannelSummary from a
6 changes: 4 additions & 2 deletions server.go
Original file line number Diff line number Diff line change
@@ -4306,7 +4306,8 @@ func (s *server) notifyOpenChannelPeerEvent(op wire.OutPoint,
remotePub *btcec.PublicKey) error {

// Call newOpenChan to update the access manager's maps for this peer.
if err := s.peerAccessMan.newOpenChan(remotePub); err != nil {
err := s.peerAccessMan.newOpenChan(remotePub)
if err != nil && !errors.Is(err, ErrNoPeerScore) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we should add exceptions here, otherwise this ErrNoErrSocre won't be used. If it's correct that this error is redundant, we should remove it in the accessman, otherwise we should make sure this error won't happen in the first place.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agree there is a deeper problem in the accessman, will address it.

return err
}

@@ -4323,7 +4324,8 @@ func (s *server) notifyPendingOpenChannelPeerEvent(op wire.OutPoint,

// Call newPendingOpenChan to update the access manager's maps for this
// peer.
if err := s.peerAccessMan.newPendingOpenChan(remotePub); err != nil {
err := s.peerAccessMan.newPendingOpenChan(remotePub)
if err != nil && !errors.Is(err, ErrNoPeerScore) {
return err
}