Skip to content

Conversation

@damrobi
Copy link
Collaborator

@damrobi damrobi commented Dec 10, 2025

Content

This PR includes changes to Mithril STM to simplify the code in the process of snarkifying it.

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
    • Update documentation website (if relevant)
    • No new TODOs introduced

Comments

I moved the errors into error.rs files and rename the MultiSignatureError to BlsSignatureError.

Issue(s)

Relates to #2794

@damrobi damrobi self-assigned this Dec 10, 2025
@damrobi damrobi added the refactoring 🛠️ Code refactoring and enhancements label Dec 10, 2025
@damrobi damrobi requested a review from curiecrypt December 10, 2025 09:16
@github-actions
Copy link

github-actions bot commented Dec 10, 2025

Test Results

    4 files  ±0    172 suites  ±0   23m 45s ⏱️ -14s
2 274 tests ±0  2 274 ✅ ±0  0 💤 ±0  0 ❌ ±0 
7 137 runs  ±0  7 137 ✅ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit c0cbd4b. ± Comparison against base commit 16c4ecf.

♻️ This comment has been updated with latest results.

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 refactors the Mithril STM codebase to simplify code organization in preparation for snarkification. The key changes include renaming MultiSignatureError to BlsSignatureError to better reflect its purpose, reorganizing error types into dedicated error.rs files within their respective modules, and standardizing import ordering across the codebase. The PR also completes the transition from the future_proof_system feature flag to future_snark.

  • Renamed MultiSignatureError to BlsSignatureError and moved it from protocol/error.rs to signature_scheme/bls_multi_signature/error.rs
  • Reorganized errors into module-specific error.rs files (AggregationError, AggregateSignatureError, SignatureError, MerkleTreeError)
  • Standardized import order to follow pattern: external crates, then use crate::, then use super::

Reviewed changes

Copilot reviewed 32 out of 32 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
mithril-stm/src/signature_scheme/bls_multi_signature/error.rs New file containing BlsSignatureError (renamed from MultiSignatureError) and blst_error_to_stm_error helper
mithril-stm/src/signature_scheme/bls_multi_signature/{verification_key,signing_key,signature,proof_of_possession}.rs Updated error references from MultiSignatureError to BlsSignatureError and reorganized imports
mithril-stm/src/signature_scheme/bls_multi_signature/helper.rs Updated import path for BlsSignatureError::SerializationError
mithril-stm/src/signature_scheme/bls_multi_signature/mod.rs Added error module and updated test imports to use BlsSignatureError
mithril-stm/src/signature_scheme/schnorr_signature/{verification_key,utils,signing_key,signature}.rs Reorganized imports following standard pattern and updated test import paths
mithril-stm/src/protocol/single_signature/error.rs New file containing SignatureError moved from protocol/error.rs
mithril-stm/src/protocol/single_signature/{signature,signature_registered_party,mod}.rs Updated to use local SignatureError and reorganized imports
mithril-stm/src/protocol/aggregate_signature/error.rs New file containing AggregationError and AggregateSignatureError moved from protocol/error.rs
mithril-stm/src/protocol/aggregate_signature/{signature,clerk,basic_verifier,mod}.rs Updated to use local error types and changed all future_proof_system feature flags to future_snark
mithril-stm/src/protocol/error.rs Reduced to only contain RegisterError, removing other error types that were moved to their respective modules
mithril-stm/src/protocol/mod.rs Updated exports to reflect new error locations
mithril-stm/src/protocol/participant/initializer.rs Reorganized imports
mithril-stm/src/membership_commitment/merkle_tree/error.rs New file containing MerkleTreeError moved from protocol/error.rs
mithril-stm/src/membership_commitment/merkle_tree/{tree,path,leaf,commitment,mod}.rs Updated to use local MerkleTreeError and reorganized imports
mithril-stm/src/lib.rs Added public export of BlsSignatureError
mithril-stm/Cargo.toml Removed future_proof_system feature and added both future_snark and benchmark-internals as required features for schnorr_sig benchmark
mithril-common/src/protocol/multi_signer.rs Updated test imports and error type references from MultiSignatureError to BlsSignatureError

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

@damrobi damrobi marked this pull request as ready for review December 12, 2025 08:58
@damrobi damrobi marked this pull request as draft December 12, 2025 08:59
@damrobi damrobi force-pushed the damrobi/msnark/simplify-code branch from 1b4e33f to 9e10e0b Compare December 12, 2025 09:15
@damrobi damrobi changed the title Damrobi/msnark/simplify code Moving mithril-stm errors into corresponding module Dec 12, 2025
@damrobi damrobi marked this pull request as ready for review December 12, 2025 09:19
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.

LGTM 👍

@damrobi damrobi force-pushed the damrobi/msnark/simplify-code branch from 2b51530 to c0cbd4b Compare December 12, 2025 09:30
Copy link
Collaborator

@curiecrypt curiecrypt left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@curiecrypt curiecrypt merged commit b3772ac into main Dec 15, 2025
60 checks passed
@curiecrypt curiecrypt deleted the damrobi/msnark/simplify-code branch December 15, 2025 13:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

refactoring 🛠️ Code refactoring and enhancements

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants