Skip to content
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

chanbackup: even if they enable experimental-peer-storage, check peers #6072

Conversation

rustyrussell
Copy link
Contributor

Seems like LND is hanging up on receiving these messages, even though they're odd :(

So, when a peer connects, check if it supplies or wants peer backup (even if it doesn't support both, it shouldn't hang up, and I didn't want to separate the two paths).

And when we go to send our own, updated backup, check features before sending.

Fixes: #6065
Changelog-EXPERIMENTAL: experimental-peer-storage caused LND to hang up on us, so only send to peers which support it.

Seems like LND is hanging up on receiving these messages, even though
they're odd :(

So, when a peer connects, check if it supplies or wants peer backup
(even if it doesn't support both, it shouldn't hang up, and I didn't
want to separate the two paths).

And when we go to send our own, updated backup, check features before
sending.

Fixes: ElementsProject#6065
Signed-off-by: Rusty Russell <[email protected]>
Changelog-EXPERIMENTAL: `experimental-peer-storage` caused LND to hang up on us, so only send to peers which support it.
@rustyrussell rustyrussell added this to the v23.02.1 milestone Mar 6, 2023
@@ -436,17 +436,30 @@ static struct command_result *after_listpeers(struct command *cmd,
bool is_connected;
u8 *serialise_scb;

if (!peer_backup)
return notification_handled(cmd);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there a reason to short circuit here rather than in after_staticbackup?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was aiming for minimal changes. but I pulled this out of the loop. I think you're right, the check could be done earlier..

Copy link
Collaborator

@vincenzopalazzo vincenzopalazzo left a comment

Choose a reason for hiding this comment

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

ACK 7927b9b

@endothermicdev endothermicdev merged commit df10c62 into ElementsProject:master Mar 9, 2023
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.

Transient errors on v23.02
3 participants