Skip to content

htlcswitch: stricter HasActiveLink#2452

Merged
Roasbeef merged 2 commits into
lightningnetwork:masterfrom
cfromknecht:stricter-has-active-link
Jan 11, 2019
Merged

htlcswitch: stricter HasActiveLink#2452
Roasbeef merged 2 commits into
lightningnetwork:masterfrom
cfromknecht:stricter-has-active-link

Conversation

@cfromknecht
Copy link
Copy Markdown
Contributor

This PR modifies the behavior of the htlcswitch's HasActiveLink, such that it will only return true if 1) the channel is no longer pending (short channel id has confirmed) and 2) the link's EligibleToForward method returns true. This deviates from the prior behavior by requiring the second condition.

HasActiveLink is currently used by the server to determine whether or not its status has changed, and if it should passively send out a ChannelUpdate to enable to disable the channel. This same method will be used by the new ChanStatusManager in #2411. Adding the stricter behavior will ensure that we don't produce any false positives, since a link that can't send updates (isn't eligible to forward) has no business being enabled on the network.

A test has been added (which fails without the first commit) to ensure the new behavior meets these expectations.

Broken off from #2411

This commit modifies the behavior of the
HasActiveLink method within the switch to
only return true if the link is in the
link index and is eligible to forward
HTLCs.

The prior version returns true whenever
the link is found in the link index,
which may return true for pending
channels that are not actually active.
Copy link
Copy Markdown
Contributor

@wpaulino wpaulino left a comment

Choose a reason for hiding this comment

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

LGTM 🚦

Copy link
Copy Markdown
Member

@Roasbeef Roasbeef left a comment

Choose a reason for hiding this comment

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

LGTM 🦎

@Roasbeef Roasbeef merged commit 5fde362 into lightningnetwork:master Jan 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants