Skip to content

refactor(stm): Remove possibility of panic during conversion from KeyRegistration to MerkleTree #2959

Merged
damrobi merged 4 commits into
mainfrom
damrobi/msnark/replace-into-impl-avkforconcat
Jan 30, 2026
Merged

refactor(stm): Remove possibility of panic during conversion from KeyRegistration to MerkleTree #2959
damrobi merged 4 commits into
mainfrom
damrobi/msnark/replace-into-impl-avkforconcat

Conversation

@damrobi

@damrobi damrobi commented Jan 26, 2026

Copy link
Copy Markdown
Collaborator

Content

This PR includes a refactoring to remove the possibility of panicking during the conversion from KeyRegistration to MerkleTree. This includes the current conversion for concatenation and the future conversion for SNARK.

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)

Relates to #2954

Comment thread mithril-common/src/protocol/multi_signer.rs Fixed
Comment thread mithril-common/src/protocol/multi_signer.rs Fixed
Comment thread mithril-common/src/protocol/signer_builder.rs Fixed
Comment thread mithril-common/src/protocol/signer_builder.rs Fixed
Comment thread mithril-stm/src/proof_system/concatenation/clerk.rs Fixed
Comment thread mithril-stm/src/proof_system/concatenation/clerk.rs Fixed
@github-actions

github-actions Bot commented Jan 26, 2026

Copy link
Copy Markdown

Test Results

    5 files  ±0    172 suites  ±0   32m 23s ⏱️ -52s
2 426 tests ±0  2 426 ✅ ±0  0 💤 ±0  0 ❌ ±0 
7 550 runs  ±0  7 550 ✅ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit 2fceaf2. ± Comparison against base commit 40266e6.

♻️ This comment has been updated with latest results.

@hjeljeli32 hjeljeli32 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGMT.
I just left 2 non-blocking comments.

Comment thread mithril-aggregator/src/services/epoch_service.rs Outdated
Comment thread demo/protocol-demo/src/demonstrator.rs Outdated
@damrobi

damrobi commented Jan 27, 2026

Copy link
Copy Markdown
Collaborator Author

As @hjeljeli32 noted in his comment, I added a few unwraps in the code outside of the STM library where I thought we were in test environment. I will check again that it is indeed the case but please have a look at that during your review

@jpraynaud jpraynaud changed the title refactor(stm): replace From trait with TryFrom for AggregateVerificationKeyForConcatenation conversion refactor(stm): implement TryFrom for AggregateVerificationKeyForConcatenation conversion Jan 27, 2026
@damrobi
damrobi force-pushed the damrobi/msnark/replace-into-impl-avkforconcat branch from d66d22a to 41d1dfe Compare January 28, 2026 11:52
@damrobi damrobi changed the title refactor(stm): implement TryFrom for AggregateVerificationKeyForConcatenation conversion refactor(stm): Remove possibility of panic during conversion from KeyRegistration to MerkleTree Jan 28, 2026
@damrobi
damrobi temporarily deployed to testing-preview January 28, 2026 12:03 — with GitHub Actions Inactive

@jpraynaud jpraynaud left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM 👍

@damrobi
damrobi marked this pull request as ready for review January 29, 2026 16:26
@damrobi
damrobi force-pushed the damrobi/msnark/replace-into-impl-avkforconcat branch from 89a8d1d to 6090068 Compare January 29, 2026 16:28
@damrobi
damrobi temporarily deployed to testing-preview January 29, 2026 16:40 — with GitHub Actions Inactive
@damrobi
damrobi force-pushed the damrobi/msnark/replace-into-impl-avkforconcat branch from 6090068 to 2fceaf2 Compare January 30, 2026 08:30
@damrobi
damrobi temporarily deployed to testing-preview January 30, 2026 08:45 — with GitHub Actions Inactive
@damrobi
damrobi merged commit 2f5a13f into main Jan 30, 2026
98 of 99 checks passed
@damrobi
damrobi deleted the damrobi/msnark/replace-into-impl-avkforconcat branch January 30, 2026 09:05
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.

4 participants