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: contributorIt 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: noneAutomatic 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: contributorThis replaces a ping-after-getdata, correct?
-
mikehearn commented at 6:34 pm on January 19, 2013: contributorYeah. 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: contributorCould an attacker fill up memory using vNotFound?
-
sipa commented at 6:13 pm on January 21, 2013: memberThere’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: contributorACK, no vulnerability because the response will always be smaller than the request.
-
gavinandresen commented at 6:17 pm on January 21, 2013: contributorQuibble 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: contributorHow 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: contributorI’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: contributorMerging 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: contributorI still wonder if this wants a BIP.
-
rebroad commented at 1:22 am on April 2, 2013: contributorThis 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
This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-10-30 00:12 UTC
More mirrored repositories can be found on mirror.b10c.me