Skip to content

query_messages: fail the connection if the peer is not on the same network#3123

Merged
niftynei merged 3 commits into
ElementsProject:masterfrom
darosior:complete_to_0
Oct 9, 2019
Merged

query_messages: fail the connection if the peer is not on the same network#3123
niftynei merged 3 commits into
ElementsProject:masterfrom
darosior:complete_to_0

Conversation

@darosior
Copy link
Copy Markdown
Contributor

@darosior darosior commented Oct 1, 2019

This PR :

… have up-to-date infos

It is most likely not on the same network, and in any case not a good peer to gossip with.
@darosior darosior changed the title Complete to 0 query_messages: fail the connection if the peer is not on the same network Oct 1, 2019
Copy link
Copy Markdown
Member

@cdecker cdecker left a comment

Choose a reason for hiding this comment

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

Minor question about the error returned, otherwise looks good 👍

Comment thread gossipd/queries.c
Comment on lines +682 to +684
return towire_errorfmt(peer, NULL,
"No up to date infos about this network: %s",
tal_hex(tmpctx, msg));
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should we maybe add the chain hash instead of the entire message they sent us?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We send the whole message for all errors, I can make it chain_hash instead but that's anyway not very useful to the peer (he kind of errored first)

@niftynei
Copy link
Copy Markdown
Collaborator

niftynei commented Oct 9, 2019

ACK d78d888

@niftynei niftynei merged commit 7fd2f6d into ElementsProject:master Oct 9, 2019
@darosior darosior deleted the complete_to_0 branch October 10, 2019 06:31
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.

3 participants