[dBFT CONSENSUS]: dBFT 3.0 with double speakers model

[vncoelho]

close neo-project/neo#2029

First version of dBFT 3.0 enclosing part of NeoResearch's researching on consensus improvements.
@igormcoelho

Merry X-mas! 🗡️

[vncoelho]

Compiling but still need some checks.
The logic of the double speakers can be understood from the code and .pdf specification, as well as published papers.
Thanks to @igormcoelho

[igormcoelho]

Nice work @vncoelho , a very hot start for middle december 🔥

[vncoelho]

This will be changed to another formula that is more efficient.
It is described on the .pdf specification https://github.com/NeoResearch/dbft3-specification/blob/master/docs/dBFT3.0_Specification.pdf

[ZhangTao1596]

Done.

[vncoelho]

@igormcoelho, we still need to double check the new formula.

[Jim8y]

I dont think we are going to add this. As far as I can see, will take years as we still have many issues to fix in the core. But i will keep this.

[vncoelho]

I dont think we are going to add this. As far as I can see, will take years as we still have many issues to fix in the core. But i will keep this.

It will be implemented someday.
I am keeping this PR updated because we also already using it on neocompiler sometime.

This enables good possibilities for decentralization, and we have strong mathematical proofs on its functioning published in two papers.

[Jim8y]

I know you are working on it, so i will keep it. The reason it will not be added is not because its not good, just because we are slow to fix the core.

[vncoelho]

Better than keep is...review and test.

[vncoelho]

AI Reviewer error

 Error: RequestError [HttpError]: Unprocessable Entity: "Pull request review thread line must be part of the diff and Pull request review thread diff hunk can't be blank"
    at /home/runner/work/_actions/freeedcom/ai-codereviewer/a9a064dfa1db8c83f40ef63f6e247fa09c935ed6/webpack:/open-ai-reviewer/node_modules/@octokit/request/dist-node/index.js:78:1
    at processTicksAndRejections (node:internal/process/task_queues:96:5) {
  status: 422,
  response: {
    url: 'https://api.github.com/repos/neo-project/neo-modules/pulls/680/reviews',
    status: 422,
    headers: {
      'access-control-allow-origin': '*',
      'access-control-expose-headers': 'ETag, Link, Location, Retry-After, X-GitHub-OTP, X-RateLimit-Limit, X-RateLimit-Remaining, X-RateLimit-Used, X-RateLimit-Resource, X-RateLimit-Reset, X-OAuth-Scopes, X-Accepted-OAuth-Scopes, X-Poll-Interval, X-GitHub-Media-Type, X-GitHub-SSO, X-GitHub-Request-Id, Deprecation, Sunset',
      connection: 'close',
      'content-length': '260',
      'content-security-policy': "default-src 'none'",
      'content-type': 'application/json; charset=utf-8',
      date: 'Tue, 12 Mar 2024 14:53:51 GMT',
      'referrer-policy': 'origin-when-cross-origin, strict-origin-when-cross-origin',
      server: 'GitHub.com',
      'strict-transport-security': 'max-age=31536000; includeSubdomains; preload',
      vary: 'Accept-Encoding, Accept, X-Requested-With',
      'x-accepted-github-permissions': 'pull_requests=write',
      'x-content-type-options': 'nosniff',
      'x-frame-options': 'deny',
      'x-github-api-version-selected': '2022-11-28',
      'x-github-media-type': 'github.v3; format=json',
      'x-github-request-id': '0880:912F:2404726:450BAF9:65F06C7F',
      'x-ratelimit-limit': '5000',
      'x-ratelimit-remaining': '4996',
      'x-ratelimit-reset': '1710258793',
      'x-ratelimit-resource': 'core',
      'x-ratelimit-used': '4',
      'x-xss-protection': '0'
    },
    data: {
      message: 'Unprocessable Entity',
      errors: [Array],
      documentation_url: 'https://docs.github.com/rest/pulls/reviews#create-a-review-for-a-pull-request'
    }
  },
  request: {
    method: 'POST',
    url: 'https://api.github.com/repos/neo-project/neo-modules/pulls/680/reviews',
    headers: {
      accept: 'application/vnd.github.v3+json',
      'user-agent': 'octokit-rest.js/19.0.7 octokit-core.js/4.2.0 Node.js/16.20.2 (linux; x64)',
      authorization: 'token [REDACTED]',
      'content-type': 'application/json; charset=utf-8'
    },
    body: '{"comments":[{"body":"It\'s a good practice to keep `using` directives sorted alphabetically. Please move `using Neo.Network.P2P.Payloads;`, `using Neo.SmartContract;`, and `using Neo.Wallets;` after `using System.Runtime.CompilerServices;`.","path":"src/DBFTPlugin/Consensus/ConsensusContext.Get.cs","line":12},{"body":"It seems like you\'ve removed the `System`, `System.Collections.Generic`, `System.IO`, and `System.Linq` namespaces. If these namespaces are not used in the rest of the code, this is fine. However, if they are used, this will cause a compilation error. Please ensure that these namespaces are not required elsewhere in the code.","path":"src/DBFTPlugin/Consensus/ConsensusContext.cs","line":12},{"body":"It seems like you\'ve removed the `System`, `System.Collections.Generic`, and `System.Linq` namespaces. Please ensure that there are no classes or methods in the file that depend on these namespaces, otherwise, it may cause compilation errors.","path":"src/DBFTPlugin/Consensus/ConsensusService.OnMessage.cs","line":12},{"body":"Please maintain the original order of `using` directives to keep the code consistent and clean. The `using Neo.IO;` statement should be placed after `using System.IO;`.","path":"src/DBFTPlugin/Messages/Commit.cs","line":12},{"body":"Please remove the copyright comment block. It is not necessary to include it in every file, as it is already included in the LICENSE file in the main directory of the repository.","path":"src/DBFTPlugin/Messages/PreCommit.cs","line":1}],"event":"COMMENT"}',
    request: { hook: [Function: bound bound register] }
  }
}

[vncoelho]

body: '{"comments":[{"body":"Consider adding error handling for the `git submodule update --init --recursive` command to catch and handle any potential failures.","path":"scripts/load_submodule.sh","line":10},{"body":"It might be more informative to provide a success message that includes the names of the submodules that were updated.","path":"scripts/load_submodule.sh","line":12}],"event":"COMMENT"}',

[vncoelho]

Green again with last commit,
@neo-project/core need to review this.

@superboyiii already checked and NGD team, specially @ZhangTao1596, co-authored the implementation.
Since last verification it has been running without problems.
After 3.7.0 we will starting running this version again on neocompiler.io

@roman-khimov @AnnaShaleva , from your side, I also expect a review. But keep in mind you need also to update the neo-go client to be compatible with it.
If not, do not hesitate to run the C# node as your consensus node for sometime.
This PR is very important for the ecosystem and governance (after this PR is merged we can improve the decentralization of the consensus a lot). So, let's do not delay its review because of the need to update neo-go.

[roman-khimov]

I think we've discussed this in neo-project/neo#2029 already, my main points are mostly the same:

• on its own it's just an optimization for a very specific problem (please check block statistics on how often this does happen in practice)
• we know current dBFT code works, we can't say the same for this updated one (alternative implementation and TLA+ modelling can help somewhat), there is risk and this risk needs to be justified
• I agree that theoretically "we can improve the decentralization of the consensus a lot", but I have not seen any practical solution to that (remember Extensibles and associated problems), so it can't justify this change unless we have some realistic scheme for third-party node participation in consensus

For the overall project we have far more important problems to solve now I think (like state, for example), I'd try concentrating on them first and delivering optimizations like this one afterwards.

If not, do not hesitate to run the C# node as your consensus node for sometime.

We can't, we don't have resources for that.

What also worries me now is that this intersects with the Bane consensus work. It also changes the protocol and it has a priority now because there we have a problem that can't be fixed otherwise. So realistically I don't think we could get to this earlier than Bane is released. And then we'd like to have a single dBFT implementation, not two-three versions of it.

[vncoelho]

@roman-khimov, I do not agree with almost all these points.

First, we have two papers published with simulation of this environment discretized in an Mixed Integer Linear Programming Model that was tested for dBFT 2.0 as well.

The last time I reviewed the TLA+ you mentioned there were errors on its logic, do you remember that we reviewed it?
In this sense, I suggest you to run the code first and test yourself, do some benchmark (@superboyiii did that).
Furthermore, maybe study the MILP model and see the cases it cover (the paper is aforementioned).

Later, the benefits in governance have been made clear already.
A simple one is to allow any community member to be the fallback primary.

Finally, something that is missing in this PR is the UTs that were removed when dBFT was migrated to plugin.
I wonder @Jim8y sometime ago to help on that, but we stopped working when we were discussing move the neo-modules back to neo-core. This is important for this PR, but do not lock @neo-project/core to review and test it.
This can easily be a final step after agreement.

[shargon]

Repository moved to neo, please re-open there