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

Follower maintains ledger state as it follows the blockchain #62

Merged
merged 6 commits into from
Feb 2, 2024

Conversation

davidrusu
Copy link
Contributor

Leader coins are now spent when they become slot leader.

Next step is to have the leaderproof produce a new coin that the validator can then use lead another slot.

Comment on lines 174 to 180
self.local_chain = Chain([])
self.epoch = EpochState(
stake_distribution_snapshot=genesis_state,
nonce_snapshot=genesis_state,
)
self.ledger_state_snapshot = genesis_state
self.ledger_state = genesis_state
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note, with this change there is no genesis-block, instead we have a genesis ledger-state, and the chain is empty.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's generally useful to have a genesis block that generates a ledger state, so that you can pass it around and blocks can always assume to have a parent

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not convinced of that, it seems like the useful thing is knowing that every block has a "context" or "environment" (or state) that it is being evaluate against, therefore an original "context" makes sense to me.

Do you want to go with this approach until we see a case where a genesis block is useful?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's ok for me

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not convinced of that, it seems like the useful thing is knowing that every block has a "context" or "environment" (or state) that it is being evaluate against, therefore an original "context" makes sense to me.

This is interesting, but, could you specify what would this state actually is compound of? I cannot get the full idea in my head 😅

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for now the genesis state holds

  • initial coin distribution
  • initial nonce for epoch randomness

Hopefully not much more than that in the final design.

cryptarchia/cryptarchia.py Outdated Show resolved Hide resolved
cryptarchia/cryptarchia.py Show resolved Hide resolved
def apply(self, block: BlockHeader):
assert block.parent == self.block
self.block = block.id()
self.nullifiers.add(block.leader_proof.nullifier)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the new commitment will be added in the next PR right?

Copy link
Contributor Author

@davidrusu davidrusu Feb 1, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, that's correct, leader proofs will generate a new commitment (That's next PR)

Comment on lines 174 to 180
self.local_chain = Chain([])
self.epoch = EpochState(
stake_distribution_snapshot=genesis_state,
nonce_snapshot=genesis_state,
)
self.ledger_state_snapshot = genesis_state
self.ledger_state = genesis_state
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's generally useful to have a genesis block that generates a ledger state, so that you can pass it around and blocks can always assume to have a parent

else:
# we have re-org'd, therefore we must roll back out ledger state and
# re-apply blocks from the new chain
ledger_state = self.ledger_state_snapshot.copy()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: it's not clear that ledger_state_snapshot corresponds to the genesis state

Also, we might keep a ledger state for each chain to avoid this kind of re-org stuff. A linear history without rollbacks is easier to understand, when changing fork you simply switch to the ledger state of that fork

Copy link
Contributor Author

@davidrusu davidrusu Feb 1, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ledger_state_snapshot does not necessarily correspond with genesis state. It is the most recently finalized ledger_state.

after 3k/f slots, it's safe to mark a ledger state as finalized because we are very confident that there won't be a fork that lasts longer than 3k/f slots long.

So really the full spec will have something like:

  • ledger_state_finalized
  • ledger_state_finalizing
  • ledger_state_current

where when ledger_state_finalizing is older than 3k/f slots, it transitions to ledger_state_finalized and ledger_state_currentis copied to ledger_state_finalizing

something like that, but the logic is slightly more complicated in that we want to keep track of a few different snapshots for the different fields in EpochState

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not convinced it makes sense to keep all this finalization logic here. In a sense it's external to the protocol what you consider finalized (different applications may make different tradeoffs).

At this level I would rather keep a ledger for each fork and just switch between them. The finality 'gadget' can be added on top of this

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah that makes sense, I'll try it with a ledger state per fork and see how that feels

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(discused offline, we'll drop the "snapshot" here and instead replay from genesis in the case of re-orgs)

HEADER-UNSIGNED = %x00 HEADER-COMMON
HEADER-COMMON = CONTENT-SIZE CONTENT-ID BLOCK-DATE PARENT-ID
CONTENT-SIZE = U32
BLOCK-DATE = BLOCK-SLOT
BLOCK-SLOT = U64
PARENT-ID = HEADER-ID
LEADER-PROOF = <NOT SPECIFIED YET>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mmm, isn't the point of this to specify things?
If we don't plan on having this ready for v1 I would remove this, as otherwise it's not a spec.

Apart from this, HEADER-UNSIGNED was meant to be one possible variant without leader proof. If we want to add a variant with it, I would suggest we create another rule and add it as a choice, e.g.:

HEADER = VERSION (HEADER-UNSIGNED / HEADER-SIGNEDv1) ; either HEADER-UNSIGNED or HEADER-SIGNEDv1 can follow 
HEADER-SIGNEDv1 = %x01 HEADER-COMMON LEADER-PROOF

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought the intention was to have HEADER-UNSIGNED be the data that is signed over and the the "sig" goes outside of that and signs over the "unsigned" data.

mmm, isn't the point of this to specify things?

The leader proof details depend on the proof system we will use, I think there's still value in setting up the structure that will be filled in by the CL details, no? right now the leader proof is just a mock structure that skips over the cryptography bits.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the purpose of having an unsigned variant?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The leader proof details depend on the proof system we will use, I think there's still value in setting up the structure that will be filled in by the CL details, no? right now the leader proof is just a mock structure that skips over the cryptography bits.

I would leave this out of the specs for v1 if the spec are meant to be a reference to implement the protocol in a compatible way. What would you do in the implementation with a spec that says "I don't know"? IMO, if we don't know how the transaction bits will look like, we better leave this off for now, but do that in a way that 1) signals it will be introduced in the future 2) sets a path for said future improvement.

The header unsigned variant was meant exactly for those two purposes, and does not need to stay in all future revisions (even though a genesis block, if we will have one, won't contain a leadership proof)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So for the purposes of the v1 spec, we do want mock leader proofs in the headers since these leader proofs are used in the state update logic and this logic goes beyond merely proof verification. (i.e. there is leader proof importing for orphan blocks that needs to be specified)

I could specify a MOCK-LEADER-PROOF here that is implementable, in fact I'll do that now, and similarly to the unsigned variant, we can replace it with the real deal once we understand CL more.

Copy link
Contributor

@zeegomo zeegomo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀

@davidrusu davidrusu merged commit d7b5e0b into master Feb 2, 2024
1 check passed
@davidrusu davidrusu deleted the epoch-state-spec branch February 2, 2024 09:32
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

Successfully merging this pull request may close these issues.

None yet

3 participants