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

Consensus cleanup #53

Open
5 of 8 tasks
benjaminbollen opened this issue Nov 27, 2019 · 2 comments
Open
5 of 8 tasks

Consensus cleanup #53

benjaminbollen opened this issue Nov 27, 2019 · 2 comments
Assignees

Comments

@benjaminbollen
Copy link
Contributor

benjaminbollen commented Nov 27, 2019

listing items in no considered order:

  • Axiom::newMetaChain needs to have a revision (difficulty ++), rlpBlockHeader is not desired so early: can it be declared after the validators have joinedDuringCreation
  • following from above: CoreStatus.Creation -> CoreStatus.Genesis to emphesis that the genesis file will be derived from the coordination on Core during this phase. (difficulty ++)
  • there is currently a duplication of the core status; a. CoreStatus is correctly tracked by Core, however, it is incorrectly "half-tracked" by Consensus. Proposal: Consensus should have a CoreLifetime {Undefined, Halted, Corrupted, Genesis, Activated}. Core can move from Genesis to Activated the first time it registerPrecommit (will be renamed precommitMetablock)
  • in an effort to make Consensus more readable, terminology can be improved
    • core::proposeMetablock
    • consensus::precommitMetablock onlyCore
    • consensus::commitMetablock onlyCommittee
    • for committee decisions that reject the precommitted metablock we will have a different code path
  • bytes20 chainId as a name is ambiguous (originally intended, but now reconsidered) with the layer-1 chainId used to sign transactions; we should move it to bytes32 metachainId, where we now have 12 bytes to define type and the origin chains chainId before appending the 20 bytes of the anchor address on said origin chain; first step, make simple proposal for type bytes (possible candidate follows EIP712 typehash)
    (difficulty +)
  • either make storage of anchor sensible or deprecate; make argued proposal
  • raised concern: "all validators are currently not iterable", which is valuable for member selection in committee. In reputation contract validators should be maked into a linked list. There is then still not a reversed list to know which core this validator is part of; one has to cross-reference the cores.
    • First we need to know the positive-reputation validators (regardless to which core they pertain);
    • what follows is that when a metablock gets committed the validators who correctly log out need to be registered as such; a reverse-lookup is not required if calls such as consensus::enterMember into a committee, also carry a variable to identify which core this validator is supposed to be active in; consensus can then assert whether the validator is active in the current metablock height of that core
  • when core calls consensus:precommitMetablock, the origin blockheight at which this call is makde must be stored in consensus: we need a global timeslice across all metablock heights in all cores, to know which validators were active in which metablockheight (as a function of the origin blockheight) (difficulty ++)

this list is incomplete as I just jotted down thoughts in between meetings; we can use it as a starting point to assign further tasks that lead towards fridays goal of tying up the main functionality of origin contracts

please use comments on this issue to add thoughts, more cleanup, questions or discuss picking up items

@0xsarvesh
Copy link
Collaborator

there is currently a duplication of the core status; a. CoreStatus is correctly tracked by Core, however, it is incorrectly "half-tracked" by Consensus. Proposal: Consensus should have a CoreLifetime {Undefined, Halted, Corrupted, Genesis, Activated}. Core can move from Genesis to Activated the first time it registerPrecommit (will be renamed precommitMetablock)

There is a mapping in consensus to track cores against chainID.

mapping(bytes20 /* chainId */ => address /* core */) public assignments;

Core already tracks its status.

CoreStatus public coreStatus;

So, it's possible to know the status of the core in consensus.
Why do we need to separately track status in consensus?

@0xsarvesh
Copy link
Collaborator

0xsarvesh commented Nov 29, 2019

From my understanding:
There is CoreStatus which is { undefined, halted, corrupted, creation, opened and precommitted}.
If the core is working as expected, it will keep changing status from opened to precommitted and vice versa.

Now from proposal: We have CoreLifetime which is { undefined, halted, corrupted, genesis, and activated}.

If below is correct
genesis == opened + height 0
activated == opened || precommitted.

Then we can derive the core lifecycle from the core status. Only source of the truth about the core is Core contract.

What do you think?

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

2 participants