Conversation
samliok
left a comment
There was a problem hiding this comment.
I realllly like the FSA defined above the state variable. It's clear, and easy to follow. 🎸
I'm concerned that larger comments above the buildBlock... functions will be hard to maintain / get out of sync. I dont think the logic will change much so I think its fine, but they are bulkier function comments that are probably hard to edit manually in the future.
Although claude code can be prompted to do so and it'll do a good job of keeping the formatting.
can we seperate out the comments like the FSA into its own PR?
| // the hash of the sealing block of the previous epoch. | ||
| // This is used to be able to quickly fetch and verify the sealing blocks without having to retrieve the interleaving blocks, | ||
| // which allows to bootstrap the BLS keys of the validator set for each epoch before fully syncing the interleaving blocks. | ||
| PrevSealingBlockHash [32]byte `canoto:"fixed bytes,3"` |
There was a problem hiding this comment.
should this be a simplex.Digest?
There was a problem hiding this comment.
yes but it's unrelated to this PR. I can change it in the PR where I will flatten the protocol metadata
e450f56 to
92f3f18
Compare
Signed-off-by: Yacov Manevich <yacov.manevich@avalabs.org>
| // │ Zero │ ... │Sealing │ │Telock│ ... │Telock│ │ │first block │ ... | ||
| // │ block│ │ block │ │ │ │ │ │ │ of epoch s │ | ||
| // └──────┘ └────────┘ └──────┘ └──────┘ │ └────────────┘ | ||
| // EN = 1 EN = 1 EN = 1 EN = 1 │ EN = s |
There was a problem hiding this comment.
i think we changed the epoch to not always be 1 for the zero block
There was a problem hiding this comment.
That's true, but it can still be 1 if we're building on top of genesis.
Do you have an idea of what I should put here instead?
There was a problem hiding this comment.
i think we should note the first epoch is the height of the chain when starting. So if we are starting fresh from genesis it is 1, otherwise its the next sequence.
There was a problem hiding this comment.
so maybe just a comment saying the zero block does not always have epoch 1
There was a problem hiding this comment.
| // | ||
| // - EN : copied within an epoch; on the first block of a new epoch, EN | ||
| // equals the sequence number of the previous epoch's sealing block. | ||
| // - PSH : only set on a sealing block. In epoch 1 it points to the zero block; |
There was a problem hiding this comment.
this desc probably needs to be updated too?
There was a problem hiding this comment.
maybe instead of epoch 1, we just say in the first ever epoch
Signed-off-by: Yacov Manevich <yacov.manevich@avalabs.org>
No description provided.