New BIP for "sendheaders" p2p message #221

This is in substantially the same form as proposed on the mailing list and discussed on IRC. I'll update this pull after a BIP number is assigned. @theuni -- perhaps we can discuss any outstanding concerns with this approach in this PR?

@sdaftuar Right away, I'll mention that I don't consider my issues with the BIP as-is to be blockers. I don't wish to put the brakes on this.

The approach here is fine by me (though I would prefer to see something a bit more abstract, to help with new capabilities/preferences in the future).

I would very much prefer, though, for tightened requirements on when the message can be sent. It's true that the current protocol is quite stateful already, but I don't see that as a good reason to keep piling on.

How about requiring that it's sent before "verack"? That would allow us to add new similar commands in the future, as it would effectively make "verack" mean "i'm done setting up the connection now".

@theuni We moved off this conversation during the meeting before I really understood why there was all this concern around statefulness. First, I mostly view this message as advisory, you certainly don't have to send direct headers if someone sends you this message and in some cases you won't. Second, I don't know why we want to preclude more intelligent logic in the future on when this is turned on and when it isn't. It's entirely possible you'd want to turn this on only for a few of your peers who tend to be the first to announce you things and save a little bit of bandwidth from other peers who will stay in inv mode. And maybe you dynamically adjust that as you go. I'm not arguing we should or need to do that now, or that its definitely a good idea, but I just don't know why we would want to preclude it.

@morcos by that logic you'd need a way to disable it too, no? The logic couldn't be all that intelligent if it couldn't be changed back.

I'm arguing that since it already can't be changed, it may-as-well just be a session property.

@theuni sure and maybe we'd add a way to disable in the future. anyway, i don't really feel strongly.. i just wanted to understand the reasoning.

@morcos my aim is to cut down on the amount of communication necessary between layers. If a node's property can considered immutable for the life of the connection, it doesn't have to poll for changes and be prepared to cope with them. That eases work for implementations significantly.

As mentioned, I don't wish for this to hold up the BIP in any significant way. But since there seems to be a consensus at the moment about not allowing this to be disabled once set, I think it makes sense to discuss whether we should take that a step further and handle it as part of the initial handshake so that it may be considered immutable. If there's a compelling reason not to, I'll drop the request.

Few points from our IRC conversation last week:

Agreed that it would be nice in theory for capabilities like this to be exchanged at the time a connection is initiated (eg by sending in between version and verack). However from discussing with others on IRC, such an approach would be problematic for existing software which doesn't expect anything before a verack, so this seems like a non-starter.

It would also be nice in theory to have a generic "capabilities" message which would include all flags such as this one. However, coordinating a change to an existing message in the future when we wanted to add a new flag would presumably run into similar difficulties as the suggestion to merely add this flag to the version message. And I am not presently aware of any future use case at this time anyway... So my inclination is to continue with the proposal as drafted.

After discussion with @theuni, I added some language to the BIP ("Additional constraints") to make it clear that how implementations choose to support this message can be subject to whatever choices those implementors make. That is, this is completely optional. @gmaxwell Can you assign a BIP number?

Thanks @gmaxwell, updated. Should I also update the README page to include a reference to the BIP as part of this PR?

Also I'm not sure I understand what the criteria is for getting this PR merged. Anything else I should do here (more mailing list notice etc)?

@sdaftuar updating the README.md is preferred, yes.

Rebased and added a commit that updates the README.

I'd like it in 0.12.

Anything else that needs to be done here?