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

feat: ensure snos program hash is checked #46

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

chudkowsky
Copy link
Contributor

To verify that we correctly settle the SNOS output, I added a snos_program_hash field, which can be set up similarly to the fact registry or program info. In the code, we assert this value against the third element of the SNOS output. This is because the SNOS proof is actually a proof of the bootloaded SNOS program, and in our case, the third value represents the bootloaded program hash—specifically, the Starknet OS program hash.

Copy link

@xJonathanLEI xJonathanLEI left a comment

Choose a reason for hiding this comment

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

Probably preferable to just extend program_info to include snos program hash as well?

Also the program_info field comment still says it's the StarknetOs program hash which is no longer true. This is probably an oversight from the previous PR introducing the layout bridge program tho.

Copy link
Collaborator

@glihm glihm left a comment

Choose a reason for hiding this comment

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

Thanks for adding this additional check @chudkowsky, let's switch to a structure and move forward with this change.

Comment on lines +31 to +32
/// Program info layout bridge program hash, config hash and StarknetOs program hash.
program_info: (felt252, felt252, felt252),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's change to a structure here and also inside the set_program_info.

Since we may have further changes in the future (integrity supporting dynamic layout) or some additional configuration item, using a a structure only change internal behavior.

Comment on lines +78 to +80
program_hash: felt252,
config_hash: felt252,
snos_program_hash: felt252
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same here using the ProgramInfo structure suggested in the previous comment.

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.

3 participants