Skip to content

[GEN-524] Channel balances disrupted by disabled nodes#500

Open
imclvr wants to merge 1 commit intomainfrom
fix/channels_view_broken_when_disabled
Open

[GEN-524] Channel balances disrupted by disabled nodes#500
imclvr wants to merge 1 commit intomainfrom
fix/channels_view_broken_when_disabled

Conversation

@imclvr
Copy link
Copy Markdown

@imclvr imclvr commented Apr 28, 2026

No description provided.

var result = new Dictionary<ulong, ChannelState>();
foreach (var node in nodes)
{
var listChannelsResponse = await _lightningClientService.ListChannels(node);
Copy link
Copy Markdown
Author

@imclvr imclvr Apr 28, 2026

Choose a reason for hiding this comment

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

This call is internally handling errors, catching an exception, logging it and then returning error.
So I believe we are safe from what the ticket says

Missing error handling in the UI: When lightning node returns an error or unexpected response, the lack of a try/catch block causes the entire balance rendering to break, preventing the user from seeing any channel balance information.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR addresses incorrect Lightning channel balance/state aggregation caused by including disabled nodes when computing channel states.

Changes:

  • Update LightningService.GetChannelsState() to fetch only non-disabled managed nodes from INodeRepository.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

public async Task<Dictionary<ulong, ChannelState>> GetChannelsState()
{
var nodes = await _nodeRepository.GetAllManagedByNodeGuard();
var nodes = await _nodeRepository.GetAllManagedByNodeGuard(withDisabled: false);
Copy link

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

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

GetChannelsState() now excludes disabled nodes via withDisabled: false, but the current unit tests for GetChannelsState don’t assert this behavior. Please add a test case where GetAllManagedByNodeGuard returns both enabled and IsNodeDisabled=true nodes and verify disabled nodes are not queried for channels and do not affect the resulting channel balances/state (or verify the repository call is made with withDisabled: false).

Suggested change
var nodes = await _nodeRepository.GetAllManagedByNodeGuard(withDisabled: false);
var nodes = (await _nodeRepository.GetAllManagedByNodeGuard(withDisabled: false))
.Where(n => !n.IsNodeDisabled)
.ToList();

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown

@manumonti manumonti left a comment

Choose a reason for hiding this comment

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

LGTM! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants