-
Notifications
You must be signed in to change notification settings - Fork 925
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
WIP: Announce channels to the involved peers directly after funding locked #414
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is used to identify our own announcements. Signed-off-by: Christian Decker <[email protected]>
We are still generating only char* style aliases, but the field is defined to be unicode, which doesn't mix too well with char. Signed-off-by: Christian Decker <[email protected]>
First step towards `gossipd` managing the `node_announcement`. Signed-off-by: Christian Decker <[email protected]>
This will later be used to determine whether or not we should announce ourselves as a node. Signed-off-by: Christian Decker <[email protected]>
Signed-off-by: Christian Decker <[email protected]>
Signed-off-by: Christian Decker <[email protected]>
Signed-off-by: Christian Decker <[email protected]>
This is required to enable channels directly after funding locked. The update is marked as non-public by a 0 timestamp and a missing signature. Signed-off-by: Christian Decker <[email protected]>
We often wait for a channel to be broadcast to a peer, but the channel only becomes fully usable when a public announcement/update is sent. This adds a "local" or "public" specifier in the logs so that we can wait for the specific type that we need. Signed-off-by: Christian Decker <[email protected]>
Adds sending unsigned `channel_announcement`s and `channel_update`s when both sides reach `funding_locked`. This'll add the channels into the gossip's view, without adding them to the broadcast queue, in the next patch. Signed-off-by: Christian Decker <[email protected]>
Relatively simple: until we reach funding-depth the channels should be known locally, so we can already route through them, but they should not be announced to peers to which the connection is non-local. Signed-off-by: Christian Decker <[email protected]>
Using the implicit signal of having channel_announcement set doesn't always work. Signed-off-by: Christian Decker <[email protected]>
On top of the other gossip changes, this deserves its own new internal msg in gossip_wire.csv, rather than sending an announce stripped of signatures and detecting it that way? |
Probably true, the code-paths are different enough to warrant a special message. Will see how that works out 👍 |
Closing since #450 was merge |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This should fix #284 and depends on #413 (the first 8 commits are from there). It creates unsigned
channel_announcement
s andchannel_update
s for a channel that has just locked in. They are then sent to the other endpoint of the channel and togossipd
(other nodes would ignore them due to missing/invalid signatures, but better not send them anyway).gossipd
will process them adding the channel to the local view of the network, making them usable for routes, however it won't queue them for broadcast.The reason this is still WIP is that we have a flaky test
test_reconnect_sender_add1
which occasionally fails because thesendpay
command doesn't fail when running undervalgrind
. It appears that for some reason we appear to automatically re-attempt the payment after reconnect when this PR is merged, and the payment succeeds. For comparison see the log before (especially the lines surrounding the firstdev_disconnect
) and the log after (also around thefirst dev_disconnect
). I'm wondering how this can happen even though the test has nothing to do withgossipd
, since we construct the route manually.@rustyrussell any idea why this happens?