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

Organize STM code - Merkle Tree Module #2374

Draft
wants to merge 16 commits into
base: main
Choose a base branch
from

Conversation

curiecrypt
Copy link
Collaborator

@curiecrypt curiecrypt commented Mar 17, 2025

Content

This PR includes the version of mithril-stm where the merkle_tree is a module containing:

  • leaf
  • tree
  • path: Path, BatchPath,
  • commitment: MerkleTreeCommitment, MerkleTreeCommitmentBatchCompat.
  • Heap helpers are moved to merkle_tree/mod.rs from merkle_tree/tree.rs.

Pre-submit checklist

  • Branch
    • Tests are provided (if possible)
    • Crates versions are updated (if relevant)
    • CHANGELOG file is updated (if relevant)
    • Commit sequence broadly makes sense
    • Key commits have useful messages
  • PR
    • All check jobs of the CI have succeeded
    • Self-reviewed the diff
    • Useful pull request description
    • Reviewer requested
  • Documentation
    • Update README file (if relevant)
    • Update documentation website (if relevant)
    • Add dev blog post (if relevant)
    • Add ADR blog post or Dev ADR entry (if relevant)
    • No new TODOs introduced

Comments

Issue(s)

Relates to #2369

Copy link

github-actions bot commented Mar 17, 2025

Test Results

    3 files  ±0     57 suites  ±0   11m 32s ⏱️ +12s
1 772 tests ±0  1 772 ✅ ±0  0 💤 ±0  0 ❌ ±0 
2 170 runs  ±0  2 170 ✅ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit d266e8d. ± Comparison against base commit ba14422.

This pull request removes 9 and adds 9 tests. Note that renamed tests count towards both.
mithril-stm ‑ merkle_tree::tests::test_bytes_batch_path
mithril-stm ‑ merkle_tree::tests::test_bytes_path
mithril-stm ‑ merkle_tree::tests::test_bytes_tree
mithril-stm ‑ merkle_tree::tests::test_bytes_tree_commitment
mithril-stm ‑ merkle_tree::tests::test_bytes_tree_commitment_batch_compat
mithril-stm ‑ merkle_tree::tests::test_create_batch_proof
mithril-stm ‑ merkle_tree::tests::test_create_invalid_batch_proof
mithril-stm ‑ merkle_tree::tests::test_create_invalid_proof
mithril-stm ‑ merkle_tree::tests::test_create_proof
mithril-stm ‑ merkle_tree::tree::tests::test_bytes_batch_path
mithril-stm ‑ merkle_tree::tree::tests::test_bytes_path
mithril-stm ‑ merkle_tree::tree::tests::test_bytes_tree
mithril-stm ‑ merkle_tree::tree::tests::test_bytes_tree_commitment
mithril-stm ‑ merkle_tree::tree::tests::test_bytes_tree_commitment_batch_compat
mithril-stm ‑ merkle_tree::tree::tests::test_create_batch_proof
mithril-stm ‑ merkle_tree::tree::tests::test_create_invalid_batch_proof
mithril-stm ‑ merkle_tree::tree::tests::test_create_invalid_proof
mithril-stm ‑ merkle_tree::tree::tests::test_create_proof

♻️ This comment has been updated with latest results.

@jpraynaud jpraynaud changed the base branch from main to curiecrypt/organize-stm March 17, 2025 15:17
Copy link
Member

@jpraynaud jpraynaud left a comment

Choose a reason for hiding this comment

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

Maybe a more natural structure would be:

  • a path sub module with Path and BatchPath
  • a commitment sub module with MerkleTreeCommitment and MerkleTreeCommitmentBatchCompat

WDYT?

@curiecrypt curiecrypt force-pushed the curiecrypt/organize-stm branch 3 times, most recently from c66fe98 to b15c070 Compare March 18, 2025 14:49
Base automatically changed from curiecrypt/organize-stm to main March 18, 2025 14:59
Copy link
Member

@jpraynaud jpraynaud left a comment

Choose a reason for hiding this comment

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

Few comments seen together 👍

Co-authored-by: Jean-Philippe Raynaud <[email protected]>
@curiecrypt curiecrypt deployed to testing-preview March 20, 2025 15:00 — with GitHub Actions Active
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