Skip to content

Add inlined documentation about state transitions - #388

Merged
yacovm merged 3 commits into
mainfrom
MSM-ASCII
Jun 3, 2026
Merged

Add inlined documentation about state transitions#388
yacovm merged 3 commits into
mainfrom
MSM-ASCII

Conversation

@yacovm

@yacovm yacovm commented May 20, 2026

Copy link
Copy Markdown
Collaborator

No description provided.

@yacovm yacovm changed the title Msm ascii Add inlined documentation about state transitions May 20, 2026
samliok
samliok previously approved these changes May 22, 2026

@samliok samliok left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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.

@samliok
samliok dismissed their stale review May 22, 2026 19:46

can we seperate out the comments like the FSA into its own PR?

Comment thread msm/encoding.go
// 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"`

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

should this be a simplex.Digest?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

yes but it's unrelated to this PR. I can change it in the PR where I will flatten the protocol metadata

@yacovm
yacovm force-pushed the MSM-ASCII branch 2 times, most recently from e450f56 to 92f3f18 Compare June 2, 2026 19:58
Signed-off-by: Yacov Manevich <yacov.manevich@avalabs.org>
Comment thread msm/msm.go Outdated
// │ Zero │ ... │Sealing │ │Telock│ ... │Telock│ │ │first block │ ...
// │ block│ │ block │ │ │ │ │ │ │ of epoch s │
// └──────┘ └────────┘ └──────┘ └──────┘ │ └────────────┘
// EN = 1 EN = 1 EN = 1 EN = 1 │ EN = s

@samliok samliok Jun 3, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

i think we changed the epoch to not always be 1 for the zero block

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

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?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

so maybe just a comment saying the zero block does not always have epoch 1

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Comment thread msm/msm.go Outdated
//
// - 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;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

this desc probably needs to be updated too?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

maybe instead of epoch 1, we just say in the first ever epoch

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

yacovm added 2 commits June 3, 2026 18:45
Signed-off-by: Yacov Manevich <yacov.manevich@avalabs.org>
Signed-off-by: Yacov Manevich <yacov.manevich@avalabs.org>
samliok
samliok approved these changes Jun 3, 2026
@yacovm
yacovm merged commit b518ab1 into main Jun 3, 2026
6 checks passed
@yacovm
yacovm deleted the MSM-ASCII branch June 3, 2026 20:50
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.

2 participants