Add a notfound message to getdata. #2192

pull mikehearn wants to merge 1 commits into bitcoin:master from mikehearn:notfoundmsg changing 1 files +17 −1

mikehearn commented at 5:50 pm on January 19, 2013: contributor

It is sent if any data that isn’t in the relayable set is requested.
Add a notfound message to getdata that is sent if any transactions that aren't in the relayable set are requested. 903d146030
BitcoinPullTester commented at 6:15 pm on January 19, 2013: none

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/903d146030e741441c288873ef3c682fb5019101 for binaries and test log.
jgarzik commented at 6:31 pm on January 19, 2013: contributor

This replaces a ping-after-getdata, correct?
mikehearn commented at 6:34 pm on January 19, 2013: contributor

Yeah. I have code that can do both. It’s a part of resolving the issues Peter Todd raised, from the bitcoinj side.
gavinandresen commented at 6:12 pm on January 21, 2013: contributor

Could an attacker fill up memory using vNotFound?
sipa commented at 6:13 pm on January 21, 2013: member

There’s no way for it to grow larger than the original getdata request, is there?
gavinandresen commented at 6:15 pm on January 21, 2013: contributor

ACK, no vulnerability because the response will always be smaller than the request.
gavinandresen commented at 6:17 pm on January 21, 2013: contributor

Quibble on the naming: “notfound” is pretty generic. Maybe “inv_unknown” ?
mikehearn commented at 6:21 pm on January 21, 2013: contributor

There’s a limit on how long message IDs can be :( inv_unknown is 11 so it just fits, but that naming scheme would be constraining in future.

Really, the whole protocol needs to get more explicit. This is just one example of a general problem.

How about we introduce a convention for cases where a command results in an error. e:getdata?
jgarzik commented at 6:53 pm on January 21, 2013: contributor

How about we introduce a convention where all situations that might result in a response do receive a response.
mikehearn commented at 7:47 pm on January 21, 2013: contributor

I’m all for that, but it has to be done piece-wise. This is one chunk of it.
sipa commented at 11:38 am on January 22, 2013: member

ACK on this change, but I disagree with requiring a response for everything.

In its core, the P2P protocol is a gossip system. Nodes make sure they tell eachother about stuff they learn, and request what they don’t know. This network layer caches and bundles requests, prevents duplicates, re-requests if necessary, …. Turning that into a request-response system would be wasting bandwith and performance. A fully fledged message-response system also needs things like request ID’s that get repeated in responses, as otherwise an announcement can be confused to be a response to something else (for example, invs can be both sent as response to a getblocks, or as an announcement for a new best block - addr can be a response to getaddr or an announcement). I think there’s just more to it than “making everything that might result a response do receive a response”.

On the other hand, for some things, this would make perfect sense. getdata obviously needs a response, and so does version (verack). It just needs to be dealt with on a case-by-case basis.
gavinandresen commented at 2:48 pm on January 23, 2013: contributor

Merging now. Can tweak later.
gavinandresen referenced this in commit a337505bd7 on Jan 23, 2013
gavinandresen merged this on Jan 23, 2013
gavinandresen closed this on Jan 23, 2013
mikehearn deleted the branch on Feb 24, 2013
jgarzik commented at 4:29 am on March 30, 2013: contributor

I still wonder if this wants a BIP.
rebroad commented at 1:22 am on April 2, 2013: contributor

This should certainly require a BIP. As a side note, I wonder if adding a timeout value to getdata is worthwhile also - i.e. most nodes wait 2 minutes before requesting a tx or block elsewhere. If a node responds more than 2 hours later it’s only likely to be sending something the node already has. a timeout would allow the node to decide whether the requesting node is still interested.
laudney referenced this in commit fed3880c6e on Mar 19, 2014
sidhujag referenced this in commit 30d691daa4 on Jul 31, 2018
DrahtBot locked this on Sep 8, 2021

Contributors
mikehearn BitcoinPullTester jgarzik gavinandresen sipa rebroad