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

Introduce Pickles drawio diagram #1693

Merged
merged 30 commits into from
Feb 23, 2024
Merged

Conversation

volhovm
Copy link
Member

@volhovm volhovm commented Jan 16, 2024

This introduces the pickles structure diagram, intended to be a part of the mina book.

@volhovm volhovm mentioned this pull request Jan 16, 2024
@volhovm volhovm force-pushed the volhovm/add-pickles-drawio-diagram branch from 5e5d17d to 6bed969 Compare January 17, 2024 15:58
@volhovm volhovm marked this pull request as ready for review January 22, 2024 21:11
@volhovm volhovm self-assigned this Feb 14, 2024
Copy link
Member

@mrmr1993 mrmr1993 left a comment

Choose a reason for hiding this comment

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

Comments about diagrams:

  • MFNS (digest?) -- ignored/bound by SE
    • `(digest?) is false
  • in general, the MFNS names seem unhelpful, I would prefer that they were expanded
  • run application logic of previous statements should be run application logic of statement
  • Assert equal ????? should be computed from unpacked preimage when verifying

book/src/pickles/diagrams.md Outdated Show resolved Hide resolved
book/src/pickles/overview.md Outdated Show resolved Hide resolved
book/src/pickles/overview.md Outdated Show resolved Hide resolved
book/src/pickles/overview.md Outdated Show resolved Hide resolved
book/src/pickles/overview.md Outdated Show resolved Hide resolved
@volhovm
Copy link
Member Author

volhovm commented Feb 20, 2024

@mrmr1993 Ready for another review pass, all addressed.

@mrmr1993 mrmr1993 merged commit 401aceb into master Feb 23, 2024
4 checks passed
@mrmr1993 mrmr1993 deleted the volhovm/add-pickles-drawio-diagram branch February 23, 2024 17:22
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