Bugs on MoreThanFNodesCommittedOrLost - Logic probably not used for changeview

[vncoelho]

Also related to issue #780,
there is something strange in the logic for avoiding nodes to accept payloads.

As I remember, this logic was also created for avoiding a node to change view if more than F nodes are committed or lost.

This needs a double check.

neo-modules/src/DBFTPlugin/Consensus/ConsensusContext.cs

Lines 96 to 102 in c187c26

	public bool NotAcceptingPayloadsDueToViewChanging => ViewChanging && !MoreThanFNodesCommittedOrLost;
	// A possible attack can happen if the last node to commit is malicious and either sends change view after his
	// commit to stall nodes in a higher view, or if he refuses to send recovery messages. In addition, if a node
	// asking change views loses network or crashes and comes back when nodes are committed in more than one higher
	// numbered view, it is possible for the node accepting recovery to commit in any of the higher views, thus
	// potentially splitting nodes among views and stalling the network.
	public bool MoreThanFNodesCommittedOrLost => (CountCommitted + CountFailed) > F;

[vncoelho]

I double checked again and it looks like the problem in a missing use of MoreThanFNodesCommittedOrLost.

Currently, the logic for NotAcceptingPayloadsDueToViewChanging => ViewChanging && !MoreThanFNodesCommittedOrLost; is correct. However, we are still changing view if MoreThanFNodesCommittedOrLost. This should be avoided because nodes will be isolated.

[ZhangTao1596]

Consensus nodes can't change view if MoreThanFNodesCommittedOrLost . Not enough nodes.

[vncoelho]

Exactly, @ZhangTao1596 .
Currently, it is changing.

[vncoelho]

But In the beginning of dbft 2.0 we designed for not changing. I am not sure what happened.