Skip to content

feat(common): add proof bound possession for Schnorr signing key - #3539

Open
damrobi wants to merge 21 commits into
mainfrom
damrobi/msnark/proof-bound-possession
Open

damrobi wants to merge 21 commits into
mainfrom
damrobi/msnark/proof-bound-possession

Conversation

@damrobi

@damrobi damrobi commented Sep 14, 2026

Copy link
Copy Markdown
Collaborator

Content

This PR includes the addition of a proof of bound possession for the Schnorr signing key. This proof is added in mithril-common under the future_snark feature. It is computed by the Initializers and verified during the registration of the signers. This PR also includes a deduplication mechanism for the Schnorr verification key at the STM level.

Summary

Adds Proof of Bound Possession (PoBP) for the Schnorr/SNARK signer verification key, entirely behind future_snark.

Changes

  • Crypto core (mithril-stm): Schnorr signing/verification keys gain create_pobp/verify_pobp; Initializer/StmInitializerWrapper create and store the proof. KeyRegistration::register gains a deduplication of entries based on the Schnorr verification key.
  • Challenge encoding (mithril-common): new ProofOfBoundPossessionPrefix (stake || epoch || pool_id) feeding the Schnorr challenge; pool_id uses the raw fixed-length Blake2b-224 hash rather than the bech32 string, removing the need for a length-prefixed encoding.
  • Verification wiring (mithril-common, mithril-aggregator): KeyRegWrapper::register now verifies the PoBP alongside the existing KES check; SignerRegistrationVerifier/leader/follower services thread the registration epoch through to that verification.
  • Entities & messages: Signer/SignerWithStake gain proof_of_bound_possession_for_snark; mirrored into RegisterSignerMessage, message adapters, and DB persistence (new nullable column via migration) so it survives aggregator restarts and follower sync.
  • Epoch threading: the registration epoch is now plumbed end-to-end — signer runtime → aggregator services → SignerBuilder/fixture builders — since PoBP verification must use the exact epoch the proof was bound to.
  • Test infrastructure: MithrilFixtureBuilder/fake_data gain epoch-aware builders (build_at_epoch, signers_with_stakes_at_epoch) to replace the implicit "always epoch 0" fixtures that broke once epoch-bound verification was enforced; extensive unit and integration test fixes across mithril-common/mithril-aggregator (including the aggregator's follower/leader integration suite) to build fixtures at the epoch they're actually registered/re-registered at, including cases where stake is updated mid-test.

Notes

  • All changes are #[cfg(feature = "future_snark")]-gated; default builds are unaffected.

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

Comments

Issue(s)

Closes #3537

@damrobi damrobi self-assigned this Sep 14, 2026
@github-actions

github-actions Bot commented Sep 14, 2026

Copy link
Copy Markdown

Test Results

     5 files  ± 0     221 suites  ±0   31m 42s ⏱️ - 1h 31m 44s
 3 652 tests  - 33   3 652 ✅  - 33  0 💤 ±0  0 ❌ ±0 
11 804 runs   - 28  11 804 ✅  - 28  0 💤 ±0  0 ❌ ±0 

Results for commit e7813b5. ± Comparison against base commit f7f05f6.

This pull request removes 58 and adds 25 tests. Note that renamed tests count towards both.
mithril-common ‑ crypto_helper::cardano::key_certification::test::test_initializer_wrapper_conversions
mithril-stm ‑ circuits::halo2::tests::golden::cases::negative::slow::index_out_of_bounds
mithril-stm ‑ circuits::halo2::tests::golden::cases::negative::slow::index_too_large_for_circuit_range
mithril-stm ‑ circuits::halo2::tests::golden::cases::negative::slow::indices_not_increasing
mithril-stm ‑ circuits::halo2::tests::golden::cases::negative::slow::leaf_merkle_path_mismatch
mithril-stm ‑ circuits::halo2::tests::golden::cases::negative::slow::leaf_swap_keep_merkle_path
mithril-stm ‑ circuits::halo2::tests::golden::cases::negative::slow::leaf_wrong_verification_key
mithril-stm ‑ circuits::halo2::tests::golden::cases::negative::slow::merkle_path_corrupt_sibling
mithril-stm ‑ circuits::halo2::tests::golden::cases::negative::slow::merkle_path_flip_position
mithril-stm ‑ circuits::halo2::tests::golden::cases::negative::slow::merkle_path_length_long
…
mithril-aggregator ‑ services::epoch_service::tests::precompute_epoch_data_excludes_the_higher_stake_signer_from_snark_registration_when_two_signers_share_a_snark_key
mithril-common ‑ crypto_helper::cardano::key_certification::test::register_fails_when_proof_of_bound_possession_for_snark_is_missing
mithril-common ‑ crypto_helper::cardano::key_certification::test::register_fails_when_proof_of_bound_possession_for_snark_was_signed_for_a_different_epoch
mithril-common ‑ crypto_helper::cardano::opcert::tests::compute_protocol_party_id_as_bytes_golden_value
mithril-common ‑ crypto_helper::cardano::opcert::tests::compute_protocol_party_id_as_hash_golden_value
mithril-common ‑ crypto_helper::cardano::opcert::tests::compute_protocol_party_id_golden_value
mithril-common ‑ crypto_helper::cardano::proof_of_bound_possession::tests::different_epochs_produce_different_bytes
mithril-common ‑ crypto_helper::cardano::proof_of_bound_possession::tests::different_pool_ids_produce_different_bytes
mithril-common ‑ crypto_helper::cardano::proof_of_bound_possession::tests::different_stakes_produce_different_bytes
mithril-common ‑ crypto_helper::cardano::proof_of_bound_possession::tests::to_prefix_bytes_handles_all_zero_pool_id
…

♻️ This comment has been updated with latest results.

@damrobi
damrobi force-pushed the damrobi/msnark/proof-bound-possession branch 3 times, most recently from f50c0ea to e3cd4c0 Compare September 14, 2026 13:55
@damrobi
damrobi deployed to testing-preview September 14, 2026 14:22 — with GitHub Actions Active
@damrobi
damrobi deployed to testing-2-preview September 14, 2026 14:22 — with GitHub Actions Active
@damrobi
damrobi force-pushed the damrobi/msnark/proof-bound-possession branch from e3cd4c0 to 2ce5e81 Compare September 14, 2026 15:06
@damrobi
damrobi deployed to testing-preview September 15, 2026 06:24 — with GitHub Actions Active
@damrobi
damrobi deployed to testing-2-preview September 15, 2026 06:24 — with GitHub Actions Active
@damrobi
damrobi force-pushed the damrobi/msnark/proof-bound-possession branch 2 times, most recently from 34044ba to a3d540b Compare September 15, 2026 11:58
@damrobi
damrobi deployed to testing-preview September 15, 2026 12:16 — with GitHub Actions Active
@damrobi
damrobi deployed to testing-2-preview September 15, 2026 12:16 — with GitHub Actions Active
@damrobi
damrobi force-pushed the damrobi/msnark/proof-bound-possession branch from a3d540b to 2f25caa Compare September 15, 2026 12:21
@damrobi
damrobi deployed to testing-2-preview September 15, 2026 12:47 — with GitHub Actions Active
@damrobi
damrobi deployed to testing-preview September 15, 2026 12:47 — with GitHub Actions Active
@damrobi
damrobi force-pushed the damrobi/msnark/proof-bound-possession branch 2 times, most recently from 26449d5 to 9328dfd Compare September 15, 2026 14:45
@damrobi
damrobi marked this pull request as ready for review September 15, 2026 15:06
@damrobi
damrobi deployed to testing-preview September 15, 2026 15:11 — with GitHub Actions Active
@damrobi
damrobi deployed to testing-2-preview September 15, 2026 15:11 — with GitHub Actions Active
@jpraynaud
jpraynaud requested a lite review from Copilot September 15, 2026 15:37
Comment thread mithril-stm/src/signature_scheme/schnorr_signature/signing_key.rs Outdated
Comment thread mithril-stm/src/signature_scheme/schnorr_signature/verification_key.rs Outdated
Comment thread mithril-common/src/crypto_helper/cardano/opcert.rs
Comment thread mithril-common/src/crypto_helper/cardano/proof_of_bound_possession.rs Outdated
Comment thread mithril-common/src/crypto_helper/types/alias.rs Outdated
.configuration_for_aggregation
.protocol_parameters,
#[cfg(feature = "future_snark")]
data.epoch.offset_to_signer_retrieval_epoch_saturating(),

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.

question(blocking): why did you choose to offset_to_signer_retrieval_epoch_saturating?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I chose those values because they match what is done in the inform_epoch() function to get the signer_retrieval_epoch and next_signer_retrieval_epoch that are used to get current_signers_with_stake and next_signers_with_stake. The same offset function is used in update_next_signers_with_stake to get next_signers_with_stake before calling precompute_epoch_data. It is possible that this is wrong but it looks consistent

.configuration_for_next_aggregation
.protocol_parameters,
#[cfg(feature = "future_snark")]
data.epoch.offset_to_next_signer_retrieval_epoch(),

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.

question(blocking): why did you choose to offset_to_next_signer_retrieval_epoch?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

See the answer above

Comment thread mithril-aggregator/tests/create_certificate_follower.rs Outdated
Comment thread mithril-stm/src/protocol/key_registration/register.rs Outdated
Comment on lines +72 to +78
if *entry < existing {
self.registered_keys_for_concatenation
.remove(&existing.get_verification_key_for_concatenation());
self.registration_entries.remove(&existing);
} else {
return Ok(());
}

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.

issue(blocking): if there is a collision with an existing Schnorr verification key, we keep the one with lower stake, but the BLS verification key is also removed. This could create a split in the effective computation of the AVK by the different versions of the signers and aggregators, and thus stop the certification. Only the Schnorr part should be impacted and not the BLS part with an update of the existing entry instead of a replace. Can you add a test that verifies the expected behavior first, make sure that it breaks and then make it green?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I update the registration to remove the snark key of entry when it is a duplicate with higher stake

@damrobi
damrobi deployed to testing-2-preview September 17, 2026 16:07 — with GitHub Actions Active
@damrobi
damrobi deployed to testing-preview September 17, 2026 16:07 — with GitHub Actions Active
the Schnorr signing key can create the proof by signing a DST, prefix and its verification key
the verification key can verify the proof given the prefix
It stores the stake, epoch and pool id and can convert them into a prefix used in the PoBP
…izer and KeyReg

- added creation of the pobp to the Initializer struct
- added pobp field to StmInitializerWrapper, updated setup, to/from_bytes and added an accessor to the new field
- StmInitializerWrapper strip_snark_keys now also removes pobp
- updated KeyRegWrapper implementation and added wrapper for StandardSchnorrSignature
- KeyRegWrapper now has a verify_proof_of_bound_possession_for_snark function that is called in register()
added proof_of_bound_possession_for_snark field and updated the implemented functions
added proof of bound possession field to diverse entities in common, signer and aggregator
added an epoch field to MithrilFixtureBuilder/MithrilFixture and a build_at_epoch/with_epoch entrypoint
threaded a real epoch through the test crypto_helper setup functions (previously stubbed to Epoch::default())
added fake_data::signers_with_stakes_at_epoch and the TEST-ONLY init_state_from_fixtures/init_state_from_fixtures_for_genesis container methods, needed when current/next signer sets require different PoBP epochs
…gator

threaded the pobp and epoch parameters through signer, common and aggregator
un-gated the verify epoch param so mockall automock derives correctly
fixed epoch problems in tests and adopted the new build_at_epoch/signers_with_stakes_at_epoch/init_state_from_fixtures helpers across the test suite
for the correctness of the PoBP
for the correctness of the deduplication
…sion and added an epoch field to KeyRegWrapper
@damrobi
damrobi force-pushed the damrobi/msnark/proof-bound-possession branch from ddab23d to e7813b5 Compare September 18, 2026 06:33
@damrobi
damrobi deployed to testing-preview September 18, 2026 07:00 — with GitHub Actions Active
@damrobi
damrobi deployed to testing-2-preview September 18, 2026 07:00 — with GitHub Actions Active
Comment on lines +98 to 133
/// Set the epoch for which the signers' Proof of Bound Possession will be created.
#[cfg(feature = "future_snark")]
pub fn with_epoch(mut self, epoch: Epoch) -> Self {
self.epoch = epoch;
self
}

/// Transform the specified parameters to a [MithrilFixture], creating each signer's Proof of
/// Bound Possession (when applicable) for the given epoch.
pub fn build_at_epoch(self, epoch: Epoch) -> MithrilFixture {
#[cfg(not(feature = "future_snark"))]
let _epoch = epoch;
#[cfg(feature = "future_snark")]
let builder = self.with_epoch(epoch);
#[cfg(not(feature = "future_snark"))]
let builder = self;
builder.build()
}

/// Transform the specified parameters to a [MithrilFixture].
pub fn build(self) -> MithrilFixture {
let protocol_stake_distribution = self.generate_stake_distribution();
let signers = crypto_helper::setup_signers_from_stake_distribution(
&protocol_stake_distribution,
&self.protocol_parameters.clone().into(),
#[cfg(feature = "future_snark")]
self.epoch,
);

MithrilFixture::new(
self.protocol_parameters,
signers,
protocol_stake_distribution,
#[cfg(feature = "future_snark")]
self.epoch,
)

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.

suggestion(blocking): I think you should reverse the method call order as it just simpler, and remove with_epoch and the associated epoch field since it will be mandatory in the future and is only used when building and prevent easy re-using (see next paragraph).

Another important change is that the build/build_at_epoch method should not consume self anymore, meaning you will be able to call it multiple times to produce signers for different epoch using the same options.

Suggested change
/// Set the epoch for which the signers' Proof of Bound Possession will be created.
#[cfg(feature = "future_snark")]
pub fn with_epoch(mut self, epoch: Epoch) -> Self {
self.epoch = epoch;
self
}
/// Transform the specified parameters to a [MithrilFixture], creating each signer's Proof of
/// Bound Possession (when applicable) for the given epoch.
pub fn build_at_epoch(self, epoch: Epoch) -> MithrilFixture {
#[cfg(not(feature = "future_snark"))]
let _epoch = epoch;
#[cfg(feature = "future_snark")]
let builder = self.with_epoch(epoch);
#[cfg(not(feature = "future_snark"))]
let builder = self;
builder.build()
}
/// Transform the specified parameters to a [MithrilFixture].
pub fn build(self) -> MithrilFixture {
let protocol_stake_distribution = self.generate_stake_distribution();
let signers = crypto_helper::setup_signers_from_stake_distribution(
&protocol_stake_distribution,
&self.protocol_parameters.clone().into(),
#[cfg(feature = "future_snark")]
self.epoch,
);
MithrilFixture::new(
self.protocol_parameters,
signers,
protocol_stake_distribution,
#[cfg(feature = "future_snark")]
self.epoch,
)
/// Transform the specified parameters to a [MithrilFixture], creating each signer's Proof of
/// Bound Possession (when applicable) for the given epoch.
pub fn build_at_epoch(&self, epoch: Epoch) -> MithrilFixture {
let protocol_stake_distribution = self.generate_stake_distribution();
let signers = crypto_helper::setup_signers_from_stake_distribution(
&protocol_stake_distribution,
&self.protocol_parameters.clone().into(),
#[cfg(feature = "future_snark")]
epoch,
);
MithrilFixture::new(
self.protocol_parameters.clone(),
signers,
protocol_stake_distribution,
#[cfg(feature = "future_snark")]
epoch,
)
}
/// Transform the specified parameters to a [MithrilFixture], creating each signer's Proof of
/// Bound Possession (when applicable) for the Epoch 1.
pub fn build(&self) -> MithrilFixture {
Self::build_at_epoch(Epoch(1))
}

Comment on lines +287 to +294
let current_fixture = MithrilFixtureBuilder::default()
.with_signers(signers_count)
.with_protocol_parameters(protocol_parameters.clone())
.build_at_epoch(current_retrieval_epoch);
let next_fixture = MithrilFixtureBuilder::default()
.with_signers(signers_count)
.with_protocol_parameters(protocol_parameters)
.build_at_epoch(time_point.epoch);

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.

issue(blocking): you should not rebuild a fixture builder like this, since you may be missing parameters that were used when the fixture was built (e.g. the StakeDistributionGenerationMethod). This caller to use any other builder parameters than the three used here.

suggestion: pass the MithrilFixtureBuilder by reference and refactor it so the build methods does not consume self (see comment in mithril-common/src/test/builder/fixture_builder.rs).

important: this comment apply for all other cases where a builder is rebuilt from a fixture like this in the code base.

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.

Add a SNARK verification key Proof of Bound Possession

6 participants