search

SoftwareVerde / bitcoin-cash-specification

Specification of the Bitcoin Cash protocols and consensus
Other
8 stars 5 forks source link

cleanup confusion #20

Closed zander closed 3 years ago

zander commented 3 years ago

bit 10 is supported by several nodes, others that are not used on mainnet anymore are removed and special cases that are not special are removed too.

thesquaregroot commented 3 years ago

It looks like NODE_WEAKBLOCKS is at least defined by Bitcoin Unlimited. Maybe it makes sense to move that and NODE_CF to their section?

Also, in removing the "node specific behavior" section it looks like that might also remove the only reference to the fact that nodes may selectively disconnect from peers based on the services they provide. Can you add a note to that effect somewhere further up the page?

zander commented 3 years ago

It looks like NODE_WEAKBLOCKS is at least defined by Bitcoin Unlimited.

They themselves wrote the point that its not used in production. I don't think it makes sense to put in a shared specification stuff things that happen in a private codebase but are not used on the network.

The point of the spec is to document the protocol.

Also, in removing the "node specific behavior" section it looks like that might also remove the only reference to the fact that nodes may selectively disconnect from peers based on the services they provide.

I'm not sure how that is part of the protocol, and eligable to be documented. Naturally any peer can decide to not engage in free communication with any other peer. There are literally hundreds of places where punishments are given to peers. I don't think it scales to try to describe them all. Think about it this way: you don't see TCP or HTTP describe that should you dislike the peer, you are free to disconnect. This freedome is implied.

thesquaregroot commented 3 years ago

While I appreciate that there may be differences of opinion about these sorts of things, there was an intention when starting the spec to include things like this. From the style guide:

In additional to documenting the "current" state of Bitcoin Cash, this specification also seeks to track partially implemented and experimental features, particularly where it may benefit node developers to avoid stepping on one another's toes.

While I agree that the primary goal should be documenting what actually happens currently on the network, being aware of what other developers have defined, even if only as placeholders, could help developers of BCH-related tools (not just node developers) be aware of where there may be clashes or inconsistencies between the nodes.

Similarly, the spec is intended to be read by people of many different backgrounds who may or may not be familiar with existing proposals for changes to the network, nor how things like TCP work. Regardless of that, I agree that will not be able to provide a description of every nuance of peer prioritization. But as it stands, I'm not certain that there is currently any mention of the fact that nodes are free to disconnect as they please (at a glance I wasn't able to find where it was mentioned if it is). I assume you would agree that we should at least make sure that potential oversight is addressed.

Beyond that, I believe it would also be worthwhile to point out that nodes may immediately disconnect upon discovering that an otherwise well-behaved peer has connected but does not provide a service it needs. That sequence of events can be hard to separate from other possibilities (e.g. a bug in the handshaking process). Having seen this happen, it seems worth calling out.

I'm open to it being rephrased or put elsewhere (maybe in /protocol/network/messages or /protocol/network/node-handshake), but I have trouble approving a pure removal of content that may be the only hint at something that I know can be a confusing interaction.