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

Get rid of Consensus #2894

Closed
damip opened this issue Aug 22, 2022 · 6 comments
Closed

Get rid of Consensus #2894

damip opened this issue Aug 22, 2022 · 6 comments
Assignees
Milestone

Comments

@damip
Copy link
Member

damip commented Aug 22, 2022

Rationale

Since the refactoring, block production, operation management etc... were all removed from Consensus.
Consensus now only manages the block graph, ticks for the block graph, and communication between graph and other modules.
It also uses async for no good reason.

TODO in this PR

  • remove massa-consensus-exports
  • remove massa-consensus-worker
  • split massa-graph into:
    • massa-consensus-worker: to run a separate worker thread with its own clock ticker and that directly talks to other modules (no async)
    • massa-consensus-exports for the consnensus controller trait and various exports (no async)
@gterzian gterzian assigned gterzian and unassigned gterzian Aug 23, 2022
@gterzian
Copy link
Contributor

I think we should first fix all consensus tests and do other clean-ups first, otherwise it's going to be very hard to resolve the conflicts with this proposed refactoring.

Also, the use of async and the fact that "consensus" doesn't do much besides managing graph, is not a big problem right now, so it could be done for the next testnet?

@damip
Copy link
Member Author

damip commented Aug 23, 2022

Yes let's keep it for testnet 15. Together with #2895

@damip damip added this to the TEST.15.X milestone Aug 23, 2022
@sebastien-forestier
Copy link

sebastien-forestier commented Sep 11, 2022

I still don't think "graph" is a good module name, there are graphs in other places, and it does not reflect what's going on inside... so why no going for "consensus" or "blockclique" or "Nakamoto" or "clique" or "bestclique" ?

@damip
Copy link
Member Author

damip commented Sep 13, 2022

because this module does all the block graph handling (dependency tree resolution AND graph storage AND compatibility graph computation AND clique computations AND nakamoto) but doesn't do all the consensus (the final state/ledger is also part of the consensus but is not handled by this module)

@damip damip modified the milestones: TEST.15.X, TEST.16.X Sep 23, 2022
@damip
Copy link
Member Author

damip commented Sep 23, 2022

Actually after thinking more about it I think you're right @sebastien-forestier , "consensus" sounds better :)

@AurelienFT
Copy link
Contributor

Done in #3162

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants