bips icon indicating copy to clipboard operation
bips copied to clipboard

Promote Draft->Final BIPs: 60 and 64

Open luke-jr opened this issue 9 years ago • 6 comments

Follow-up from #315

The following BIPs appear to meet the criteria for Final status, but have not been promoted to Accepted by their author yet, so theoretically need ACKs from the authors. Since @genjix is MIA, and @mikehearn ignores Bitcoin, we need to discuss how to handle such actions when BIP champions disappear - BIP 1 allows for me to assign an additional champion to take over, but I'm not sure anyone would necessarily want to do so in these cases.

BIP 60: BIP 37 was apparently not clear on whether the new "relay transactions" flag was mandatory or optional. BIP 60 sought to explicitly make it mandatory. In parallel, Jeff Garzik interpreted BIP 37's field as mandatory in bitcoin/bitcoin#2534 and Pieter Wuille implemented the changes required for that (and BIP 60) in bitcoin/bitcoin#2763 . Numerous forks of Bitcoin Core since then have also picked up this change.

BIP 64: The getutxo message was implemented for use between (pre-contentious hardforks) Bitcoin XT and Lighthouse. Both of these are unmaintained now, but this does not seem relevant.

luke-jr avatar Feb 24 '16 05:02 luke-jr

It's worth noting that the change to version.relay behavior in the satoshi client spanned release and protocol versions. And as you say, this difference was also picked up by forks. From libbitcoin comments:

    // The /Satoshi:0.8.x/ node (70001) reads (but does not send) relay byte.
    // The /Satoshi:0.9.x/ node (70002) sends (and reads) the relay byte.

In order to not fail a high number of connections it's necessary to allow peers reporting 70001 to not set the byte. Failing connections above 70001 results in a very small number of dropped connections, such as the following:

// "/Satoshi:1.1.1/" (70006) no relay
//     anarchistprime: bitcointalk.org/index.php?topic=1001407
//     This node is identifiable by a different genesis block.
// "/Cornell-Falcon-Network:0.1.0/" (70014) no relay
// "/therealbitcoin.org:0.9.99.99/" (99999) no relay

So it seems that the BIP60 treatment of relay as optional in 70001 and mandatory in 70002 is reasonably consistent on the network. Variations are likely forks of version 70001 or prior that failed to track protocol changes.

evoskuil avatar Mar 26 '17 08:03 evoskuil

@evoskuil is this ACK / NACK or did you want to see something changed?

Also, needs to be rebased.

jonathancross avatar Nov 07 '17 02:11 jonathancross

Isn't it a bit silly to require ACK from the authors if it has been implemented and is used? I guess you (@luke-jr) see the damage of "breaking protocol" as worse than PR's floating around and the "Status" field losing value? But you also do not seem to want to reject them as Expired. If you don't want to follow BIP-0002 (and expire according to protocol), we need a new BIP that explains the actual new process. The current process seems to be to let PR's linger about if they are controversial, that bloats the issue list.

ysangkok avatar Jul 25 '19 17:07 ysangkok

Implementation is clearly progress, so to say it hasn't made progress in 3 years is incorrect...

BIP 2 is what guides my decisions here.

luke-jr avatar Jul 26 '19 02:07 luke-jr

genjix, mikehearn, evoskuil, have all had plenty of time to comment

maybe just interpret and apply process

katesalazar avatar Apr 24 '24 18:04 katesalazar

My comments above on bip60 were just to provide additional information. I suppose however I can speak for @genjix in this case. I ACK bip60 and can offer the following comments in support.

The bip37 implementation broke the network protocol. Any peer that fully validated the version message would have simply dropped the connection with the new longer message. Otherwise the assumption had to be that any amount of trailing garbage at the end of that message was ok. I don’t believe this is/was allowed for any other message type.

Also, bip60 requires that no other messages may be sent between version and verack. The reason for this is also preservation of the ability to validate received messages, as opposed to being forced to accept any garbage. However I believe this has, more recently also been violated by certain proposals. There is at least one node that has implemented sendaddrv2 in this manner, as we sometimes see Libbitcoin dropping a peer for this, but I believe Core avoided this break by deviating from that proposal in compliance with bip60.

I wholeheartedly support the objective of this proposal, though I don’t believe the case was made as clearly as it could have been. Also, regarding the relay flag, it’s too late. All nodes have already had to adapt to the protocol break. Clarifying the specific intended behavior is worthwhile, but it is probably the case that many nodes just treat the byte as optional at independent of version level (since it precedes version negotiation).

It would however still be with documenting so that the version message is not extended again, beyond this break, as it will break protocol again. And I strongly recommend acceptance of the additional requirement for no other message traffic between version and verack, and also prior to version (the latter was previously proposed by an obsolete bip).

evoskuil avatar Apr 24 '24 21:04 evoskuil