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

Send "notfound" message when inventory is not found. #1132

Closed
lock9 opened this issue Sep 30, 2019 · 4 comments · Fixed by #1645
Closed

Send "notfound" message when inventory is not found. #1132

lock9 opened this issue Sep 30, 2019 · 4 comments · Fixed by #1645
Labels
discussion Initial issue state - proposed but not yet accepted enhancement Type - Changes that may affect performance, usability or add new features to existing modules. p2p Module - peer-to-peer message exchange and network optimisations, at TCP or UDP level (not HTTP).
Milestone

Comments

@lock9
Copy link
Contributor

lock9 commented Sep 30, 2019

(Originally from @decentralisedkev on #366)

Summary
When nodeA asks nodeB for a header, if nodeB does not have the header, he will not tell nodeA. nodeA will wait for nodeB until some set of time, which slows down nodeA.

Note: I know we have 'notfound' message but I can't affirm this is being used for that. If this is already implemented this way, we should close this issue.

Do you have any solution you want to propose?
Once nodeB cannot find the header, he should send back a notfound message with the hashes of the inv that he could not find, along with a separate message of the headers that he could find.

Where in the software does this update applies to?

  • P2P (TCP)

@shargon could you confirm if this already exists?

@lock9 lock9 added enhancement Type - Changes that may affect performance, usability or add new features to existing modules. discussion Initial issue state - proposed but not yet accepted p2p Module - peer-to-peer message exchange and network optimisations, at TCP or UDP level (not HTTP). labels Sep 30, 2019
@shargon
Copy link
Member

shargon commented Sep 30, 2019

I think that is not implemented yet

@ixje
Copy link
Contributor

ixje commented Oct 1, 2019

I think it should only send notfound if it cannot find any of the requested hashes. This way a client that does flight tracking doesn't have to wait for exceeding their set time threshold on the request. A client can determine what is missing from the response it receives; e.g.

scenario 1

  • -> request headers 1 - 100
  • <- not found
  • result; node doesn't have any of the requested headers

scenario 2

  • -> request headers 1 - 100
  • <- send headers 1 - 10
  • result; client can diff their requested vs received headers and learn that node does not have the other 90 headers

The original proposal would increase traffic quite a bit. For scenario 2 it would

  • send the full headers for 1 - 10
  • send notfound + 90 * 32 bytes (uint256) for all headers not found.

The client already has all these hashes so it is only generating traffic because the client is too lazy to track the request itselves. In the worst case scenario it would request the maximum 2000 headers, the node only has one and you'll get 1999 * 32 = 63968 bytes (excluding the Message bytes) of unnecessary traffic.

@shargon
Copy link
Member

shargon commented Oct 1, 2019

Maybe we can relay the message if we don't have this information

@ixje
Copy link
Contributor

ixje commented Oct 1, 2019

Maybe we can relay the message if we don't have this information

How would the information get back to the requesting party? I think we should minimize the responsibilities of the remote node in the syncing process and keep them as basic as possible. It either gives the requested data or not (or optionally informs it can't because it's missing). After that it is up to the requesting party to find an alternative way to get the data it wants (e.g. by trying a different node).

I still secretly hope we can get a new command as suggested in #522 to skip all this header requesting and sending overload. From the P2P handshake we can tell the current height of the remote node and we can sync by just sending

  • getblocks(start_height=0)
  • getblocks(start_height=500)
  • getblocks(start_height=1000)
  • etc

to get the full blocks, without needing any hash or figuring out if the remote node actually has the hash. Once local_node.height == remote_node.height you can use the very lightweight ping/pong messages to keep in sync. The current inv broadcasting that happens in neo-cli can be replaced by ping/pong broadcasting.

With the above you'll not even have to ask the client if they have certain hashes or blocks, you know before even asking based on their height!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Initial issue state - proposed but not yet accepted enhancement Type - Changes that may affect performance, usability or add new features to existing modules. p2p Module - peer-to-peer message exchange and network optimisations, at TCP or UDP level (not HTTP).
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants