Skip to content

server: configurable feature negotiation + downgrade to optional compat #1838

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

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

cfromknecht
Copy link
Contributor

Alternative approach to #1801

@cfromknecht cfromknecht force-pushed the downgrade-required-features branch 21 times, most recently from f7704da to 9603a25 Compare September 5, 2018 00:21
@cfromknecht cfromknecht changed the title server: Stop requesting InitialRouting Sync + peer compatibility server: configurable feature negotiation + downgrade to optional compat Sep 5, 2018
@cfromknecht cfromknecht force-pushed the downgrade-required-features branch from 9603a25 to 26da8b8 Compare September 5, 2018 00:24
@cfromknecht cfromknecht force-pushed the downgrade-required-features branch from 26da8b8 to 895c969 Compare September 5, 2018 04:23
In this commit, we thread down an error chan from the
server's ConnectToPeer method to the point at which a
peer is started. This enables the caller to be aware
of issues in exchange init messages. This is additional
info is useful for subsystems like autopilot, which
will currently try to open a channel after knowing the
dial succeeded, but unaware if the peers are actually
compatible.
Since peer start errors are now bubbled up to the
caller, this error can now be received when
connecting non-permanent peers. If we encounter
this when trying EnsureConnceted, we will assume
the peers are trying to connect, and wait for the
peers to appear in each other's list of peers.
This commit adds 3 basic levels of feature bit
negotiation: off, optional, and required. Only
optional and required will be used initially, as
fully disabling a known feature would require a
fair amount of extra work. However, these values
are intended to correspond to command line flags
that can be passed to lnd on startup.
This commit adds a conf struct for setting
feature bits from the command line. The current
supported features are related gossip queries
and data loss protection.
This commit adds the required logic to downgrade
a required feature bit to an optional one if they
do not understand the required feature bit.

On the first connection, we will detect the case
in which we offered required and they gave optional.
If the peers are disconnected for any reason, we note
the peer's support for optional gq or dlp, and
initiate a downgraded attempt on the next connection
if we wish the feature to be required.

This downgrade will be applied on all subsequent
connections to remove uncertainty that not
understanding the required feature bits was the
reason for disconnecting the first time.

Finally, if the are using a downgraded connection
because the peer initially reported optional support,
but later advertises that they don't support the
feature at all, we fail the connection. This is
to maintain the fact that we can only enter a
downgraded connection if the user requested that
it be required. Accepting the connection would
break that contract with the user user's
expectations.
@cfromknecht cfromknecht force-pushed the downgrade-required-features branch from 895c969 to a5dbe37 Compare September 6, 2018 05:04
@Roasbeef Roasbeef added p2p Code related to the peer-to-peer behaviour P3 might get fixed, nice to have needs review PR needs review by regular contributors interop interop with other implementations labels Sep 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
interop interop with other implementations needs review PR needs review by regular contributors p2p Code related to the peer-to-peer behaviour P3 might get fixed, nice to have
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants