Updated to only disconnect bit 8 based on comments by the Bitcoin-ABC (bit 5 users) folks indicating they want to change their network magic to ensure good separation for their nodes. Based on the comments from the btc1 folks, I think we should consider merging this sooner rather than later.

It was pointed out that irrespecitve of whether Bitcoin-ABC changes their network magic to further split their nodes off from nodes wasting their resources, disconnecting nodes with bit 5 will only help their nodes and future Core nodes, so I went ahead and re-added that check.

@TheBlueMatt any reason you started referring to bit 5 instead of bit 6? Looks like the PR still refers to bit 6 (as does master branch of ABC).

@jheathco heh, sorry, inconsistent 0base vs 1-base naming.

@deadalnix awesome, sounds good. Note that its probably still a good idea for Core to do the disconnect-on-service-bit thing here as it just adds a second layer of ensuring the networks properly split and dont waste each others' resources.

I don't like the general idea since it burns a service bit unnecessarily, but since it's just for a year or so, and we don't seem to be in shortage of service bits, I don't mind it either.

utACK

Looks straightforward and correct to me - utACK cae246e.

Would be good to create an issue to track this to make sure this code gets removed after T>=1533096000.

NAK, this will cause a premature network split.

One remaining relaying node is enough to prevent a premature chain split. Which should be fine as not everyone will be upgrading to 0.15 at the same time. If you think there's a risk of that, run a node without this code and make sure to be connected to both nodes signaling and not signaling this flag.

Since the S2X hardfork blockheight is known, would it be better to only disconnect when the block prior to the hardfork block is found and relayed?

Deploying this change for NODE_SEGWIT2X - bit 7 - creates chain splits in the wild on an inconsistent basis -- the upgrade rate to (0.15?).

This creates chain splits even though Bitcoin Core and segwit2x nodes are validating 100% the same rules today; it creates chain splits because of a presumed future rule deviation.

The outcome is a bunch of non-deterministic islands. This is a very hostile and unsafe change prior to segwit2x fork deployment.

Consider, for example, what would happen if this code were hypothetically deployed on July 1 2017 for NODE_BIP148. Nodes would be split off prematurely, at an inconsistent rate (rate of upgrade).

It is safe and a convenient optimization for this codebase to make this change after a chain split, but not before.

@kek-coin we are absolutely not in the business of trying to determine what clients running incompatible consensus rules will do. Ignore @jgarzik's fudding, this obviously doesn't create a split until their incompatible rules kick in, it will only make upgraded nodes more cleanly separated, its not like the network wont still be well-connected.

Disconnecting service bits should only be enabled after peers have started banning each other on the network, thus proving the safety of service bit disconnect.

It is obviously unsafe to disconnect outside that set of conditions.

ETA: @TheBlueMatt Please reconsider that every disagreement of opinion is not "fudding", and we can have an honest debate.

Specific review:

1. The subject line says bits 6 and 8, yet the current code as of this writing masks bits 5 and 7.

2. bit 5 is NODE_BITCOIN_CASH = (1 << 5) in the BitcoinABC codebase, and bit 7 is NODE_SEGWIT2X = (1 << 7) in the btc1 codebase.

3. NODE_BITCOIN_CASH is provably divergent today, Bitcoin Core and BitcoinABC peers ban each other today, so it is obviously safe to add this disconnect logic as an optimization to what would automatically be arrived at already.

4. NODE_SEGWIT2X is not divergent today, Bitcoin Core and btc1 peers do not ban each other today, and including this bit in the disconnect logic would create new network disruptions that would not otherwise exist.

One bit is safe for the network as a whole, one bit is disruptive to the network as a whole - today.

This rashly merged commit exactly showed how corrupt the BCore Committee became and why the majority of Bitcoin community decided to go ahead with SegWit2X.

Can we properly define those bits in protocol.h?

Can we properly define those bits in protocol.h?

I also dislike the way how those bits where kidnapped (without the BIP process) but setting a proper constant is still the way to go (we have also done that for XTHIN).

Anyways.
Concept ACK.

@jonasschnelli

kidnapped

Yeah, another negative word to defame dissenters. Such dirty tricks should be avoided.

Anyways, glad to see that you BCore committee tries to leave Bitcoin and fork your 2nd altcoin. Will the token still be called BUASF? I can't wait to sell it to buy more Bitcoin.

I thought the #1 priority was to have as many nodes as possible?

I see some serious lack of imagination here, among the people pushing for this change.

Do you realize that when you start banning peers based on the content of their version messages, it will eventually make the content of these message obsolete?

With this change, you are breaking up the network protocol, by forcing certain nodes to stary lying about who they are and what they want.

@piotrnar If they do that they will just harm themselves for the most part, as they connect to the thousands of bitcoin core nodes out there that won't tell them about the blockchain they want to hear about. Where is your concern that Bitcoin also disconnects litecoin nodes?

There will always be malicious peers out there that will lie. That we can't help. Also, this PR disconnects, rather than banning. (In fact, it protects the peers addresses from banning, which will avoid compatible software on the same IPs getting banned everywhere in the network... which happened to some ABC users-- where their Bitcoin node had a hard time getting connections because ABC had gotten them banned all over the place).

@gmaxwell, this is how the protocol evolves: different people introduce different features, that later get approved (or not) by the mining majority.

There is nothing wrong with this process. I don't understand why you have a problem with it.

Are you saying that you are not going to ever allow the miners to vote on any consensus protocol changes, other then the ones that the bitcoin core team has accepted?

@gmaxwell

If they do that they will just harm themselves

Lie. The 'They' Piotrnar talked about is that the people who run your malicious code to ban others. gMAXWELL, your dishonest ego is quite laughable.

There will always be malicious peers out there that will lie.

So why are you always the malicious one that tells lies frequently?

Are you saying that you are not going to ever allow the miners to vote on any consensus protocol changes, other then the ones that the bitcoin core team has accepted?

That's exactly what Blockstream is about: they want to be the only ones to decide how and when Bitcoin must evolve. They ignore the community and reject miners because they think they are bad for Bitcoin.

@Pheromon
Exactly.
In the ego of gMAXWELL, people who refuse it is anti-decentralization and anti-users.
And people who say NO will be defamed with smear campaign.

They ignore the community and reject miners because they think they are bad for Bitcoin.

I don't know about "the community", but I must say that ignoring the miners can only end up very badly for this project. Doing so bitcoin core is digging its own hole.

@piotrnar
In the ego of Blockstream, those eight to ten guys are "the community". People who are against those eight guys are anti-decentralization, anti-Bitcoin, anti-users, and liars.

@piotrnar It sounds like you are expressing a misunderstanding of the Bitcoin system.

The consensus rules are what defines what is or isn't mining. Namecoin or BCash or whatever miners are not Bitcoin miners because they produce blocks which violate Bitcoin's rules. If you look at the Bitcoin whitepaper, section 8 paragraph 2 talks about how nodes are protected against being overpowered by a majority of hashpower mining blocks which are invalid because they enforce the rules themselves. It would have been possible to define Bitcoin a different way where nodes blindly trust miners, but that isn't how it works or has ever worked. A miner that breaks the rules is simply ignored, and this is an integral part of the system of incentives that keeps the system from becoming insecure.

This is all pretty far off-topic for this PR now... and it shows in that several of the last posts are nothing but random people (Pheromon, and CSmith7703) hurling abusive comments and insults at me. This is not appropriate here and not welcome. We don't come into your projects and insult you. Please take it elsewhere.

@gmaxwell

Half truth and actual lies again.
piotrnar knowns Bitcoin, Gmaxwell knowns BCore.
Since BCore is not Bitcoin, then it's fair for GMAXWELL and his blockstream buddies to ban other nodes.

I don't know about "the community", but I must say that ignoring the miners can only end up very badly for this project. Doing so bitcoin core is digging its own hole.

Exactly.
And maybe that's their real aim: destroy Bitcoin to save fiat money and banks (like their main investor, AXA).

Please lock this.

If you look at the Bitcoin whitepaper, section 8 paragraph 2 talks about

If you look at the Bitcoin WP, you will not find a block limit as a consensus rule, also it's clearly stated that big (enormous!) blocks would be handled by miners.

So, it seems like you point at the WP only when you want.

The consensus rules are what defines what is or isn't mining.

Sure - no question about it.

However, what we should be asking is: who, how and why defines (and guards) the consensus rules?

@Pheromon
Don't forget Gmaxwell looked upon Bitcoin WhitePaper and slandered Satoshi.

@piotrnar
The ego of Gmaxwell defines and guards the consensus rule. Blockstream has spent more than 40 Million in the past two years to form this definition.

Gmaxwell: the last posts are nothing but random people

Anyone who don't follow the ego of Gmaxwell are 'random people'.

Please stop political discussion immediately. Keep it technical.

@jonasschnelli This PR is purely political and out of evil will.

The review comments on pull requests are not meant for extended off-topic discussions. Please switch to another channel and keep this discussion for review comments on the code.

What is the rationale for this versus, say, building compatibility with nodes that use these service bits into the client?

Is there a plan for creating a version compatible with these nodes in the future? Is there any timeline for that?

If I'm running this client in production what is the recommended upgrade path in the case where only tiny shards of the network are not using these service bits?

@brynwaldwick Those nodes aren't and can't be made compatible. They are hard fork nodes.