Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Using previous view proposal as the basis for new one during consensus process #2058

Open
roman-khimov opened this issue Nov 11, 2020 · 6 comments
Labels
Discussion Initial issue state - proposed but not yet accepted

Comments

@roman-khimov
Copy link
Contributor

Summary or problem description
We've noticed that on a busy network CNs tend to have a bit different mempool contents and they're not always able to get missing transactions proposed by another CN before view change happens. This is especially relevant for spammed network that runs close to mempool capacity on each node. I think the top priority for us in this situation should be in producing some block that will move the chain forward and drain at least some transactions from the mempool.

Do you have any solution you want to propose?
We've implemented a compatible modification to dBFT in neo-go (since release 0.72.0 February this year). To make block acceptance more probable on view change the new speaker first looks into the proposal from the previous view (if there was any, of course), then checks this set of transactions against its mempool evicting ones that it doesn't have and then checks the length of the resulting set. If the resulting set is more than half of the previous, it uses exactly that set for its proposal, if not than it proceeds as usual picking whatever it has in the mempool.

At this stage this modification is tested well enough to say that it doesn't break anything and the network runs correctly with it, so maybe it's time for C# node to do that too (somewhat similar idea is mentioned in #2029 also).

Neo Version

  • Neo 2
  • Neo 3

Where in the software does this update applies to?

  • Consensus
@roman-khimov roman-khimov added the Discussion Initial issue state - proposed but not yet accepted label Nov 11, 2020
@vncoelho
Copy link
Member

vncoelho commented Nov 11, 2020

Hi @roman-khimov, in this paper here "Challenges of PBFT-Inspired Consensus for Blockchain and Enhancements over Neo dBFT " https://www.mdpi.com/1999-5903/12/8/129 we name an improvement in this direction as dBFT 2.0+.

It is an improvement in the information sharing during change view for previous proposed blocks.
There you can find how we propose to handle conflicts in more details and contextualization.

On the original PBFT, information is also shared during change view, it is described at "Section 4.2" in our aforementioned study.

At Section 4.3 we describe the following "4.3. Aggregating Extra Information on dBFT 2.0: The dBFT 2.0+":

"We believe that some simple information can help a new primary to propose the creation of a block which is identical (or at least very similar to previous ones, which would help to speed-up consensus). The following strategy could be adopted:

  • 1.Replicas can inform which txs are present (or not) by providing a bit array (one bit per transaction index on block) connected to previous proposal;
  • 2.If f+1 supports the same transaction, it is guaranteed to exist (and be valid). If 2f+1 support that transaction, it is fundamental that the next primary should include it since at, least, f+1 honest nodes are going to realize the aforementioned guarantee;
  • 3.If a sufficient amount of transactions are present (within a threshold), next primary could propose the same block, resolving all possible issues with nodes committed in the past."

We believe that we should move on in this direction, similar as you are suggesting here.

I am glad to hear you implemented that on neo-go. Perhaps we can discuss and move forward this change for the C# too.

It is also aligned with dBFT 3.0. In that same paper, we also mentioned the dBFT 3.0+ with similar properties (this one here plus another one that would also help double speakers).

@vncoelho
Copy link
Member

@roman-khimov, if a valid proposal exists, it more probably that it will be accepted.
Maybe we can improve the extendByFactor to extend more in the cases we have mempool full and several txs are in the preparation.

@vncoelho
Copy link
Member

This might add more complexity and we have some cons. Maybe there is an attack in which the attacker can send a tx just few nodes know (or maybe just the node).

I suggest we solve the issue as I mentioned, which is a more natural way for the consensus to discuss (negotiate) when under stress.

Also, I suggest that you modify the neo-go to be follow a random choice of txs.

@roman-khimov
Copy link
Contributor Author

Maybe we can improve the extendByFactor to extend more in the cases we have mempool full and several txs are in the preparation.

I doubt we need to do anything to timers, we can have a bad CN too after all.

Maybe there is an attack in which the attacker can send a tx just few nodes know (or maybe just the node).

The change discussed here improves this situation, if the first Primary is to propose a set of transactions with some of them unknown to Backups and Backups are not able to get these transactions in time then the next Primary will evict exactly these transactions from the list and we'll get a set that every CN has (or at least something closer to that).

This change is well-tested by now, it's compatible and it works. We can of course do some additional tests with our improved neo-bench (especially 7+2 scenario where we get a badly inconsistent network).

@vncoelho
Copy link
Member

In this sense, @roman-khimov, it is an improvement.

I am reading my previous comment from 2 years ago.

It is an improvement in the information sharing during change view for previous proposed blocks.

In the original pBFT this is done when view is changed.

@vncoelho
Copy link
Member

Double checking my previous comment I see now:

I am glad to hear you implemented that on neo-go. Perhaps we can discuss and move forward this change for the C# too.
It is also aligned with dBFT 3.0. In that same paper, we also mentioned the dBFT 3.0+ with similar properties (this one here plus another one that would also help double speakers).

Maybe your team could push a draft of this mechanism for the C# nodes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Discussion Initial issue state - proposed but not yet accepted
Projects
None yet
Development

No branches or pull requests

2 participants