Skip to content

Conversation

@curiecrypt
Copy link
Collaborator

@curiecrypt curiecrypt commented Nov 20, 2025

Content

This PR includes re-organization of the modules in mithril-stm to make the library ready for SNARK integration.

Pre-submit checklist

  • Branch
    • 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
    • No new TODOs introduced

Comments

Issue(s)

Closes #2793

@github-actions
Copy link

github-actions bot commented Nov 20, 2025

Test Results

    4 files  ±0    168 suites  ±0   22m 25s ⏱️ - 1m 51s
2 214 tests ±0  2 214 ✅ ±0  0 💤 ±0  0 ❌ ±0 
6 905 runs  ±0  6 905 ✅ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit fe11a48. ± Comparison against base commit 0a23618.

This pull request removes 46 and adds 46 tests. Note that renamed tests count towards both.
mithril-stm ‑ aggregate_signature::signature::tests::aggregate_signature_golden_concatenation::golden_conversions
mithril-stm ‑ aggregate_signature::signature::tests::aggregate_signature_type_golden::golden_bytes_encoding_prefix
mithril-stm ‑ aggregate_signature::tests::batch_verify
mithril-stm ‑ aggregate_signature::tests::test_adversary_quorum
mithril-stm ‑ aggregate_signature::tests::test_aggregate_sig
mithril-stm ‑ aggregate_signature::tests::test_core_verifier
mithril-stm ‑ aggregate_signature::tests::test_dedup
mithril-stm ‑ aggregate_signature::tests::test_initializer_serialize_deserialize
mithril-stm ‑ aggregate_signature::tests::test_invalid_proof_index_bound
mithril-stm ‑ aggregate_signature::tests::test_invalid_proof_index_unique
…
mithril-stm ‑ membership_commitment::merkle_tree::tree::tests::test_bytes_batch_path
mithril-stm ‑ membership_commitment::merkle_tree::tree::tests::test_bytes_path
mithril-stm ‑ membership_commitment::merkle_tree::tree::tests::test_bytes_tree
mithril-stm ‑ membership_commitment::merkle_tree::tree::tests::test_bytes_tree_commitment
mithril-stm ‑ membership_commitment::merkle_tree::tree::tests::test_bytes_tree_commitment_batch_compat
mithril-stm ‑ membership_commitment::merkle_tree::tree::tests::test_create_batch_proof
mithril-stm ‑ membership_commitment::merkle_tree::tree::tests::test_create_invalid_batch_proof
mithril-stm ‑ membership_commitment::merkle_tree::tree::tests::test_create_invalid_proof
mithril-stm ‑ membership_commitment::merkle_tree::tree::tests::test_create_proof
mithril-stm ‑ protocol::aggregate_signature::signature::tests::aggregate_signature_golden_concatenation::golden_conversions
…

♻️ This comment has been updated with latest results.

@curiecrypt curiecrypt force-pushed the curiecrypt/msnark/re-organize-modules branch from 1ec9ee5 to 1673005 Compare November 26, 2025 12:07
@curiecrypt curiecrypt requested a review from jpraynaud November 26, 2025 12:09
@curiecrypt curiecrypt marked this pull request as ready for review November 26, 2025 12:11
@jpraynaud jpraynaud requested a review from Copilot November 26, 2025 16:55
Copilot finished reviewing on behalf of jpraynaud November 26, 2025 16:57
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR reorganizes the mithril-stm library's module structure in preparation for SNARK integration. The refactoring consolidates code into four main modules: signature_scheme (for BLS and Schnorr signatures), protocol (for STM protocol logic), membership_commitment (for Merkle trees), and proof_system (for concatenation proofs). This reorganization improves separation of concerns and prepares the codebase for future SNARK-friendly implementations.

Key changes:

  • Introduces new top-level module structure grouping related functionality
  • Updates import paths throughout the codebase to reflect new organization
  • Moves deprecated aliases from lib.rs to protocol/mod.rs
  • Changes blst_error_to_stm_error visibility from pub(crate) to pub

Reviewed changes

Copilot reviewed 30 out of 36 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
mithril-stm/src/lib.rs Simplified to export from new top-level modules; moved deprecated aliases to protocol module
mithril-stm/src/signature_scheme/mod.rs New module aggregating BLS and Schnorr signature schemes
mithril-stm/src/signature_scheme/schnorr_signature/mod.rs Empty placeholder module for future Schnorr implementation
mithril-stm/src/protocol/mod.rs New module aggregating protocol components with deprecated aliases
mithril-stm/src/protocol/error.rs Changed blst_error_to_stm_error visibility to public
mithril-stm/src/protocol/eligibility_check.rs Moved from root to protocol module
mithril-stm/src/membership_commitment/mod.rs New module for Merkle tree implementations
mithril-stm/src/proof_system/mod.rs New module for proof system implementations
mithril-stm/src/signature_scheme/bls_multi_signature/*.rs Updated import paths to reference new module structure
mithril-stm/src/protocol/aggregate_signature/*.rs Updated import paths for new module locations
mithril-stm/src/protocol/participant/*.rs Updated import paths for signature scheme and protocol references
mithril-stm/Cargo.toml Version bumped from 0.6.0 to 0.6.1
mithril-stm/CHANGELOG.md Added entry for version 0.6.1 documenting reorganization

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@curiecrypt curiecrypt force-pushed the curiecrypt/msnark/re-organize-modules branch from 14f8b4b to bfb326e Compare November 27, 2025 16:43
@curiecrypt curiecrypt requested a review from jpraynaud November 27, 2025 16:47
@curiecrypt curiecrypt force-pushed the curiecrypt/msnark/re-organize-modules branch from 0ed080a to fe11a48 Compare November 27, 2025 18:52
@curiecrypt curiecrypt requested a review from jpraynaud November 28, 2025 13:41
Comment on lines +1 to 2
use serde::{Deserialize, Serialize};
use std::cmp::Ordering;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
use serde::{Deserialize, Serialize};
use std::cmp::Ordering;
use std::cmp::Ordering;
use serde::{Deserialize, Serialize};

Comment on lines 1 to +4
use anyhow::anyhow;
use blake2::digest::{Digest, FixedOutput};
use serde::{Deserialize, Serialize};
use std::{collections::HashMap, fmt::Display, hash::Hash};
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
use anyhow::anyhow;
use blake2::digest::{Digest, FixedOutput};
use serde::{Deserialize, Serialize};
use std::{collections::HashMap, fmt::Display, hash::Hash};
use std::{collections::HashMap, fmt::Display, hash::Hash};
use anyhow::anyhow;
use blake2::digest::{Digest, FixedOutput};
use serde::{Deserialize, Serialize};

Comment on lines +2 to 7
use anyhow::anyhow;
use blake2::digest::{Digest, FixedOutput};
use std::{
collections::{HashMap, hash_map::Entry},
sync::Arc,
};
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
use anyhow::anyhow;
use blake2::digest::{Digest, FixedOutput};
use std::{
collections::{HashMap, hash_map::Entry},
sync::Arc,
};
use std::{
collections::{HashMap, hash_map::Entry},
sync::Arc,
};
use anyhow::anyhow;
use blake2::digest::{Digest, FixedOutput};

Comment on lines +1 to 7
use anyhow::{Context, anyhow};
use blake2::digest::{Digest, FixedOutput};
use serde::{Deserialize, Serialize};
use std::{
cmp::Ordering,
hash::{Hash, Hasher},
};
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
use anyhow::{Context, anyhow};
use blake2::digest::{Digest, FixedOutput};
use serde::{Deserialize, Serialize};
use std::{
cmp::Ordering,
hash::{Hash, Hasher},
};
use std::{
cmp::Ordering,
hash::{Hash, Hasher},
};
use anyhow::{Context, anyhow};
use blake2::digest::{Digest, FixedOutput};
use serde::{Deserialize, Serialize};

BlsSigningKey, POP,
helper::unsafe_helpers::{compress_p1, scalar_to_pk_in_g1, uncompress_p1},
};
use crate::{MultiSignatureError, StmResult, blst_error_to_stm_error};
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need the blst_error_to_stm_error to be public?

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.

Re-organize modules of STM library

3 participants