Adding request/reply id in messages
10 messages, 11 to 13 Apr 2012. Started by Christian Bodt.
Christian Bodt (3), Amir Taaki (2), Gavin Andresen, Pieter Wuille, Jeff Garzik, Peter Vessenes, Wladimir J. van der Laan
Hi,
I would like to discuss the following bitcoin protocol improvement proposal:
Adding request/reply id in all messages (in the message header, based on what was done for the "checksum" field)
The original reason is that I found it very hard to implement robust blockchain downloading as we are missing context information: The download protocol relies on the other peer sending one (or more) "inv" reponse(s) to "getblocks" and sending the "hashContinue". But if the other peer doesn't send a response to getblock, send a partial response to getblocks, mixes it with some unrelated blocks inventories or doesn't send the "hashContinue" it is very hard to detect and handle (e.g. ban the peer and switch to another). This could cause some DoS attacks by misbehaving peers. The problems are that 1/ we don't know how many "inv" messages to wait for after "getblocks" and 2/ we don't know how to distinguish between result of "getblocks" and spontaneous "inv" notifications. The same is true for "addr" messages (it is both a notification and reply) but this is less of a problem as a lack of response to getaddr doesn't constitute a DoS.
The idea would be to add a new "requestid" field in messages and define the
following:
- verack sends back the requestid given in version.
- inv sends back the requestid given in getblocks or a special value
in case of a notification.
- addr sends back the requestid given in getaddr or a special value
in case of a notification.
- tx sends back the requestid given in getdata.
- block sends back the requestid given in getdata
- headers sends back the requestid given in getheaders.
- reply sends back the requestid given in checkorder/submitorder.
- pong sends back the requestid given in ping.
With the following rules:
- getblocks always results in a single"inv" reply (0-500 elements
allowed).
- getdata still result in multiple 'tx' or 'block' 's (as currently).
This is inspired by the common request/reply pattern (the id is sometimes called "correlation id"). This would permit better asynchronous messaging (multiple request of the same type in parallel. This could be useful for getblocks/getdata/getheaders and future messages).
Historic note: It seems similar to the existing mechanism of "CRequestTracker, hashReply & PushRequest" in the current bitcoin source code except requestId would only be a 32bits instead of the 256bits of hashReply .
I have implemented a prototype here: https://github.com/sirk390/bitcoin/ This could allow to remove "hashContinue" in a second step like this: https://github.com/sirk390/bitcoin/tree/better_block_download
I'm discussing it now as I read the PONG BIP and saw some similar nonce field added. I was not sure if this nonce field would still be required if we had request/reply ids .
What do you think?
Best Regards, Christian Bodt, France Note: I am working on a python implementation of bitcoin. See https://github.com/sirk390/coinpy
On Wed, Apr 11, 2012 at 2:39 PM, Christian Bodt <sirk390@gmail•com> wrote:
Quoted text (4 lines)
> I would like to discuss the following bitcoin protocol improvement proposal: > > Adding request/reply id in all messages (in the message header, > based on what was done for the "checksum" field)
That seems like a perfectly reasonable protocol improvement to me. Anybody else have an opinion?
On Thu, Apr 12, 2012 at 11:41:05AM -0400, Gavin Andresen wrote:
Quoted text (8 lines)
> On Wed, Apr 11, 2012 at 2:39 PM, Christian Bodt <sirk390@gmail•com> wrote: > > I would like to discuss the following bitcoin protocol improvement proposal: > > > > Adding request/reply id in all messages (in the message header, > > based on what was done for the "checksum" field) > > That seems like a perfectly reasonable protocol improvement to me. > Anybody else have an opinion?
If there is a reasonable use for it, I have no objections.
However: the bitcoin P2P protocol is not fully request-reply based, and trying to use it that may be be less intuitive than how it looks. For example, doing a second identical "getblocks" request will not result in more "inv" replies, as the client prevents retransmits. This is not a large problem, but maybe such an extension should also include an extra "denied" message, which is sent if the client is unwilling to answer (and may also be used to report transactions that are not accepted into the memory pool, for example).
This is a bad idea. The bitcoin protocol is (mostly) stateless. Stateless protocols are more secure.
________________________________ From: Pieter Wuille <pieter.wuille@gmail•com> To: Gavin Andresen <gavinandresen@gmail•com> Cc: bitcoin-development@lists•sourceforge•net Sent: Thursday, April 12, 2012 5:01 PM Subject: Re: [Bitcoin-development] Adding request/reply id in messages
On Thu, Apr 12, 2012 at 11:41:05AM -0400, Gavin Andresen wrote:
Quoted text (8 lines)
> On Wed, Apr 11, 2012 at 2:39 PM, Christian Bodt <sirk390@gmail•com> wrote: > > I would like to discuss the following bitcoin protocol improvement proposal: > > > > Adding request/reply id in all messages (in the message header, > > based on what was done for the "checksum" field) > > That seems like a perfectly reasonable protocol improvement to me. > Anybody else have an opinion?
If there is a reasonable use for it, I have no objections.
However: the bitcoin P2P protocol is not fully request-reply based, and trying to use it that may be be less intuitive than how it looks. For example, doing a second identical "getblocks" request will not result in more "inv" replies, as the client prevents retransmits. This is not a large problem, but maybe such an extension should also include an extra "denied" message, which is sent if the client is unwilling to answer (and may also be used to report transactions that are not accepted into the memory pool, for example).
Quoted text (4 lines)
> For example, doing a second > identical "getblocks" request will not result in more "inv" replies, as > the client > prevents retransmits.
Yes, the current prototype implementation modifies that. "getblocks" always results in one "inv" with [0-500] elements. It also removes the filtering on previously transmitted block invs. see patch: https://github.com/sirk390/bitcoin/commit/17a27e3d56efa107697b53c193364a33951f1dd1
On Wed, Apr 11, 2012 at 2:39 PM, Christian Bodt <sirk390@gmail•com> wrote:
Quoted text (16 lines)
> Hi, > > I would like to discuss the following bitcoin protocol improvement proposal: > > Adding request/reply id in all messages (in the message header, > based on what was done for the "checksum" field) > > The original reason is that I found it very hard to implement robust > blockchain downloading as we are missing context information: > The download protocol relies on the other peer sending one (or more) "inv" > reponse(s) to "getblocks" and sending the "hashContinue". > But if the other peer doesn't send a response to getblock, send a partial > response to getblocks, mixes it with some unrelated blocks inventories > or doesn't send the "hashContinue" it is very hard to detect and handle > (e.g. ban the peer and switch to another). This could cause some DoS > attacks by misbehaving peers.
If the peer is misbehaving, then disconnect. Your protocol change does not offer any clear benefits in this area, as these sorts of attacks/misbehaviors/bugs are still just as possible, and just as damaging (or not).
Just disconnect the strange peer!
Quoted text (9 lines)
> The problems are that 1/ we don't know how many "inv" messages to wait for > after "getblocks" and 2/ we don't know how to distinguish between result of > "getblocks" and spontaneous "inv" notifications. > The same is true for "addr" messages (it is both a notification and reply) > but this is less of a problem as a lack of response to getaddr > doesn't constitute a DoS. > > The idea would be to add a new "requestid" field in messages and define the > following:
Stateless protocols have a lot of value. They are easiest to implement, and easier to prove correct. Existing clients like ArtForz' half-a-node, variants of which are deployed all over the place in bitcoin-land, rely on the stateless-ness to one degree or another.
Stateful protocols, too, have their problems as well. One must add code to help remain "synchronized" between local and remote states, which your suggested change only hints at. NFSv4 and RPC have a long history of dealing with stateful-ness issues. Obviously bitcoin P2P is nowhere near as complex, but the history of NFS development offers several lessons applicable to your proposed change.
Overall, IMO your listed reasons for needing this major change (stateless->stateful) do not really justify the change. Handling initial block download can be accomplished in a number of ways, and peer(s) may crash or return odd results. You must handle these cases properly, regardless of the presence of req/reply id's.
Jeff elaborated the problems very well, but I just want to add this:
Your change is essentially relying (trusting) the server to track a piece of information (your state). Anytime you start depending on another node in some way, it is opening yourself up to be exploited. Nodes should be doing their owning state maintainance, not relying on external parties.
I am very much against the change. As someone who has implemented the complete bitcoin protocol, I had no problems implementing the blockchain download. In fact, I dislike that nodes have to store the last inventory they sent as part of a getblocks in order to trigger the next round. It's be better if there was no state whatsoever.
________________________________ From: Jeff Garzik <jgarzik@exmulti•com> To: sirk390@gmail•com Cc: bitcoin-development@lists•sourceforge•net Sent: Thursday, April 12, 2012 6:12 PM Subject: Re: [Bitcoin-development] Adding request/reply id in messages
On Wed, Apr 11, 2012 at 2:39 PM, Christian Bodt <sirk390@gmail•com> wrote:
Quoted text (16 lines)
> Hi, > > I would like to discuss the following bitcoin protocol improvement proposal: > > Adding request/reply id in all messages (in the message header, > based on what was done for the "checksum" field) > > The original reason is that I found it very hard to implement robust > blockchain downloading as we are missing context information: > The download protocol relies on the other peer sending one (or more) "inv" > reponse(s) to "getblocks" and sending the "hashContinue". > But if the other peer doesn't send a response to getblock, send a partial > response to getblocks, mixes it with some unrelated blocks inventories > or doesn't send the "hashContinue" it is very hard to detect and handle > (e.g. ban the peer and switch to another). This could cause some DoS > attacks by misbehaving peers.
If the peer is misbehaving, then disconnect. Your protocol change does not offer any clear benefits in this area, as these sorts of attacks/misbehaviors/bugs are still just as possible, and just as damaging (or not).
Just disconnect the strange peer!
Quoted text (9 lines)
> The problems are that 1/ we don't know how many "inv" messages to wait for > after "getblocks" and 2/ we don't know how to distinguish between result of > "getblocks" and spontaneous "inv" notifications. > The same is true for "addr" messages (it is both a notification and reply) > but this is less of a problem as a lack of response to getaddr > doesn't constitute a DoS. > > The idea would be to add a new "requestid" field in messages and define the > following:
Stateless protocols have a lot of value. They are easiest to implement, and easier to prove correct. Existing clients like ArtForz' half-a-node, variants of which are deployed all over the place in bitcoin-land, rely on the stateless-ness to one degree or another.
Stateful protocols, too, have their problems as well. One must add code to help remain "synchronized" between local and remote states, which your suggested change only hints at. NFSv4 and RPC have a long history of dealing with stateful-ness issues. Obviously bitcoin P2P is nowhere near as complex, but the history of NFS development offers several lessons applicable to your proposed change.
Overall, IMO your listed reasons for needing this major change (stateless->stateful) do not really justify the change. Handling initial block download can be accomplished in a number of ways, and peer(s) may crash or return odd results. You must handle these cases properly, regardless of the presence of req/reply id's.
I agree that it would be nice if the protocol stayed stateless.
I also think we should try and keep in our heads the aggregate bitcoin-universe cost of implementing any protocol change; even a very small change, something that truly only takes one hour of time from each bitcoin node client developer to implement, test and bugfix (hah!) Has a cost in the (tens?) of thousands of USD added up across those who need to understand, implement, discuss, etc.
Peter
On Thu, Apr 12, 2012 at 10:24 AM, Amir Taaki <zgenjix@yahoo•com> wrote:
Quoted text (109 lines)
> Jeff elaborated the problems very well, but I just want to add this: > > Your change is essentially relying (trusting) the server to track a piece > of information (your state). Anytime you start depending on another node in > some way, it is opening yourself up to be exploited. Nodes should be doing > their owning state maintainance, not relying on external parties. > > > I am very much against the change. As someone who has implemented the > complete bitcoin protocol, I had no problems implementing the blockchain > download. In fact, I dislike that nodes have to store the last inventory > they sent as part of a getblocks in order to trigger the next round. It's > be better if there was no state whatsoever. > > ________________________________ > From: Jeff Garzik <jgarzik@exmulti•com> > To: sirk390@gmail•com > Cc: bitcoin-development@lists•sourceforge•net > Sent: Thursday, April 12, 2012 6:12 PM > Subject: Re: [Bitcoin-development] Adding request/reply id in messages > > On Wed, Apr 11, 2012 at 2:39 PM, Christian Bodt <sirk390@gmail•com> wrote: > > Hi, > > > > I would like to discuss the following bitcoin protocol improvement > proposal: > > > > Adding request/reply id in all messages (in the message header, > > based on what was done for the "checksum" field) > > > > The original reason is that I found it very hard to implement robust > > blockchain downloading as we are missing context information: > > The download protocol relies on the other peer sending one (or more) > "inv" > > reponse(s) to "getblocks" and sending the "hashContinue". > > But if the other peer doesn't send a response to getblock, send a partial > > response to getblocks, mixes it with some unrelated blocks inventories > > or doesn't send the "hashContinue" it is very hard to detect and handle > > (e.g. ban the peer and switch to another). This could cause some DoS > > attacks by misbehaving peers. > > If the peer is misbehaving, then disconnect. Your protocol change > does not offer any clear benefits in this area, as these sorts of > attacks/misbehaviors/bugs are still just as possible, and just as > damaging (or not). > > Just disconnect the strange peer! > > > The problems are that 1/ we don't know how many "inv" messages to wait > for > > after "getblocks" and 2/ we don't know how to distinguish between result > of > > "getblocks" and spontaneous "inv" notifications. > > The same is true for "addr" messages (it is both a notification and > reply) > > but this is less of a problem as a lack of response to getaddr > > doesn't constitute a DoS. > > > > The idea would be to add a new "requestid" field in messages and define > the > > following: > > > Stateless protocols have a lot of value. They are easiest to > implement, and easier to prove correct. Existing clients like > ArtForz' half-a-node, variants of which are deployed all over the > place in bitcoin-land, rely on the stateless-ness to one degree or > another. > > Stateful protocols, too, have their problems as well. One must add > code to help remain "synchronized" between local and remote states, > which your suggested change only hints at. NFSv4 and RPC have a long > history of dealing with stateful-ness issues. Obviously bitcoin P2P > is nowhere near as complex, but the history of NFS development offers > several lessons applicable to your proposed change. > > Overall, IMO your listed reasons for needing this major change > (stateless->stateful) do not really justify the change. Handling > initial block download can be accomplished in a number of ways, and > peer(s) may crash or return odd results. You must handle these cases > properly, regardless of the presence of req/reply id's. > > -- > Jeff Garzik > exMULTI, Inc. > jgarzik@exmulti•com > > > ------------------------------------------------------------------------------ > For Developers, A Lot Can Happen In A Second. > Boundary is the first to Know...and Tell You. > Monitor Your Applications in Ultra-Fine Resolution. Try it FREE! > http://p.sf.net/sfu/Boundary-d2dvs2 > _______________________________________________ > Bitcoin-development mailing list > Bitcoin-development@lists•sourceforge•net > https://lists.sourceforge.net/lists/listinfo/bitcoin-development > > > ------------------------------------------------------------------------------ > For Developers, A Lot Can Happen In A Second. > Boundary is the first to Know...and Tell You. > Monitor Your Applications in Ultra-Fine Resolution. Try it FREE! > http://p.sf.net/sfu/Boundary-d2dvs2 > _______________________________________________ > Bitcoin-development mailing list > Bitcoin-development@lists•sourceforge•net > https://lists.sourceforge.net/lists/listinfo/bitcoin-development >
On Thu, Apr 12, 2012 at 7:24 PM, Amir Taaki <zgenjix@yahoo•com> wrote:
Quoted text (4 lines)
> Jeff elaborated the problems very well, but I just want to add this: > > Your change is essentially relying (trusting) the server to track a piece > of information (your state).
No, it is more about distinguishing between replies (multiple asynchroneous request) and spontaneous notifications of the other peer. Every state would still be tracked locally on the client side.
I don't understand why you say my proposal would make the protocol more stateful. I think it doesn't. Each reply is only the result of the current request only, and there is no new session information. As you see in my implementation, there is not even a new variable. Request/reply id is a very robust pattern that is compatible with stateless protocols.
Indead, this change doesn't directly improve on peer that don't answer requests: it only enables to do so easily in a secondary step. This step can only be done when all peers on the network are running the modified code.
Quoted text (7 lines)
> > > I don't understand why you say my proposal would make the protocol more > stateful. I think it doesn't. > Each reply is only the result of the current request only, and there is > no new session information. >
I also wondered this. My first thought was that it's basically the same as the PING message, a nonce that is repeated immediately on reply. This makes it easier to multiplex operations over a single channel. I'm not against this basic idea, and it is easy to ignore for clients that don't want to use it.
I think the state comes in here:
- inv sends back the requestid given in getblocks or a special value
in case of a notification.
- addr sends back the requestid given in getaddr or a special value
in case of a notification.
"*command1* sends back the requestid given in *command2*".
This requires keeping state on the connection between command1 and command2. Arguably, this state already exists in the current protocol, but I'd rather see it reduced than extended.
Also... Many of the described commands don't need this as they already have a natural "nonce". For example, the id of the requested block header. If this is passed in the reply, and the caller can correlate the request and reply without a special nonce administration.
Wladimir