Skip to content

Commit 2eedbd2

Browse files
authored
Merge pull request #3429 from IntersectMBO/damrobi/msnark/refactor-stm-codec-and-keyreg
refactor(stm): harden legacy decoders and key registration
2 parents 936eadd + 7b4d7f8 commit 2eedbd2

7 files changed

Lines changed: 157 additions & 29 deletions

File tree

Cargo.lock

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

mithril-common/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ fixed = "1.31.0"
4646
hex = { workspace = true }
4747
kes-summed-ed25519 = { version = "0.2.1", features = ["serde_enabled", "sk_clone_enabled"] }
4848
mithril-merkle-tree = { path = "../internal/mithril-merkle-tree", version = "0.1.4" }
49-
mithril-stm = { path = "../mithril-stm", version = "0.11.2", default-features = false }
49+
mithril-stm = { path = "../mithril-stm", version = "0.11.3", default-features = false }
5050
nom = "8.0.0"
5151
rand_chacha = { workspace = true }
5252
rand_core = { workspace = true }

mithril-stm/CHANGELOG.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,13 @@ All notable changes to this project will be documented in this file.
55
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
66
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
77

8+
## 0.11.3 (07-22-2026)
9+
10+
### Changed
11+
12+
- Updated the `from_bytes_legacy` functions for `MerklePath`, `MerkleBatchPath` and `AggregateVerificationKeyForConcatenation`
13+
- Updated the `KeyRegistration` to track the registered key independently from the `registration_entries`
14+
815
## 0.11.2 (07-20-2026)
916

1017
### Changed

mithril-stm/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[package]
22
name = "mithril-stm"
3-
version = "0.11.2"
3+
version = "0.11.3"
44
edition = { workspace = true }
55
authors = { workspace = true }
66
homepage = { workspace = true }

mithril-stm/src/membership_commitment/merkle_tree/path.rs

Lines changed: 45 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -58,14 +58,20 @@ impl<D: Digest + FixedOutput> MerklePath<D> {
5858
u64_bytes.copy_from_slice(bytes.get(8..16).ok_or(MerkleTreeError::SerializationError)?);
5959
let len = usize::try_from(u64::from_be_bytes(u64_bytes))
6060
.map_err(|_| MerkleTreeError::SerializationError)?;
61-
let mut values = Vec::with_capacity(len);
61+
let mut values = Vec::new();
6262
for i in 0..len {
63+
let range_low = i
64+
.checked_mul(<D as Digest>::output_size())
65+
.and_then(|rl| rl.checked_add(16))
66+
.ok_or(MerkleTreeError::SerializationError)?;
67+
let range_high = i
68+
.checked_add(1)
69+
.and_then(|rh| rh.checked_mul(<D as Digest>::output_size()))
70+
.and_then(|rh| rh.checked_add(16))
71+
.ok_or(MerkleTreeError::SerializationError)?;
6372
values.push(
6473
bytes
65-
.get(
66-
16 + i * <D as Digest>::output_size()
67-
..16 + (i + 1) * <D as Digest>::output_size(),
68-
)
74+
.get(range_low..range_high)
6975
.ok_or(MerkleTreeError::SerializationError)?
7076
.to_vec(),
7177
);
@@ -118,27 +124,53 @@ impl<D: Digest + FixedOutput> MerkleBatchPath<D> {
118124
/// * Indices
119125
fn from_bytes_legacy(bytes: &[u8]) -> StmResult<Self> {
120126
let mut u64_bytes = [0u8; 8];
121-
u64_bytes.copy_from_slice(&bytes[..8]);
127+
u64_bytes.copy_from_slice(bytes.get(..8).ok_or(MerkleTreeError::SerializationError)?);
122128
let len_v = usize::try_from(u64::from_be_bytes(u64_bytes))
123129
.map_err(|_| MerkleTreeError::SerializationError)?;
124130

125-
u64_bytes.copy_from_slice(&bytes[8..16]);
131+
u64_bytes.copy_from_slice(bytes.get(8..16).ok_or(MerkleTreeError::SerializationError)?);
126132
let len_i = usize::try_from(u64::from_be_bytes(u64_bytes))
127133
.map_err(|_| MerkleTreeError::SerializationError)?;
128134

129-
let mut values = Vec::with_capacity(len_v);
135+
let mut values = Vec::new();
130136
for i in 0..len_v {
137+
let range_low = i
138+
.checked_mul(<D as Digest>::output_size())
139+
.and_then(|rl| rl.checked_add(16))
140+
.ok_or(MerkleTreeError::SerializationError)?;
141+
let range_high = i
142+
.checked_add(1)
143+
.and_then(|rh| rh.checked_mul(<D as Digest>::output_size()))
144+
.and_then(|rh| rh.checked_add(16))
145+
.ok_or(MerkleTreeError::SerializationError)?;
131146
values.push(
132-
bytes[16 + i * <D as Digest>::output_size()
133-
..16 + (i + 1) * <D as Digest>::output_size()]
147+
bytes
148+
.get(range_low..range_high)
149+
.ok_or(MerkleTreeError::SerializationError)?
134150
.to_vec(),
135151
);
136152
}
137-
let offset = 16 + len_v * <D as Digest>::output_size();
153+
let offset = len_v
154+
.checked_mul(<D as Digest>::output_size())
155+
.and_then(|off| off.checked_add(16))
156+
.ok_or(MerkleTreeError::SerializationError)?;
138157

139-
let mut indices = Vec::with_capacity(len_v);
158+
let mut indices = Vec::new();
140159
for i in 0..len_i {
141-
u64_bytes.copy_from_slice(&bytes[offset + i * 8..offset + (i + 1) * 8]);
160+
let range_low = i
161+
.checked_mul(8)
162+
.and_then(|rl| rl.checked_add(offset))
163+
.ok_or(MerkleTreeError::SerializationError)?;
164+
let range_high = i
165+
.checked_add(1)
166+
.and_then(|rh| rh.checked_mul(8))
167+
.and_then(|rh| rh.checked_add(offset))
168+
.ok_or(MerkleTreeError::SerializationError)?;
169+
u64_bytes.copy_from_slice(
170+
bytes
171+
.get(range_low..range_high)
172+
.ok_or(MerkleTreeError::SerializationError)?,
173+
);
142174
indices.push(
143175
usize::try_from(u64::from_be_bytes(u64_bytes))
144176
.map_err(|_| MerkleTreeError::SerializationError)?,

mithril-stm/src/proof_system/concatenation/aggregate_key.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,10 +48,11 @@ impl<D: MembershipDigest> AggregateVerificationKeyForConcatenation<D> {
4848
let mut u64_bytes = [0u8; 8];
4949
let size = bytes.len();
5050

51-
u64_bytes.copy_from_slice(&bytes[size - 8..]);
51+
let split = size.checked_sub(8).ok_or(MerkleTreeError::SerializationError)?;
52+
u64_bytes.copy_from_slice(bytes.get(split..).ok_or(MerkleTreeError::SerializationError)?);
5253
let stake = u64::from_be_bytes(u64_bytes);
5354
let mt_commitment = MerkleTreeBatchCommitment::from_bytes(
54-
bytes.get(..size - 8).ok_or(MerkleTreeError::SerializationError)?,
55+
bytes.get(..split).ok_or(MerkleTreeError::SerializationError)?,
5556
)?;
5657
Ok(Self {
5758
mt_commitment,

mithril-stm/src/protocol/key_registration/register.rs

Lines changed: 99 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
1+
use std::collections::{BTreeSet, HashSet};
2+
13
use digest::{Digest, FixedOutput};
2-
use std::collections::BTreeSet;
34

45
use crate::{
5-
Parameters, RegisterError, SignerIndex, Stake, StmResult,
6+
Parameters, RegisterError, SignerIndex, Stake, StmResult, VerificationKeyForConcatenation,
67
VerificationKeyProofOfPossessionForConcatenation,
78
membership_commitment::{MerkleTree, MerkleTreeLeaf},
89
protocol::key_registration::ClosedRegistrationEntry,
@@ -14,16 +15,22 @@ use crate::VerificationKeyForSnark;
1415
use super::RegistrationEntry;
1516

1617
/// Key Registration
17-
#[derive(Clone, Default, PartialEq, Eq, PartialOrd, Ord, Debug)]
18+
#[derive(Clone, Default, PartialEq, Eq, Debug)]
1819
pub struct KeyRegistration {
1920
registration_entries: BTreeSet<RegistrationEntry>,
21+
registered_keys_for_concatenation: HashSet<VerificationKeyForConcatenation>,
22+
#[cfg(feature = "future_snark")]
23+
registered_keys_for_snark: HashSet<VerificationKeyForSnark>,
2024
}
2125

2226
impl KeyRegistration {
2327
/// Initialize an empty registration
2428
pub fn initialize() -> Self {
2529
Self {
2630
registration_entries: Default::default(),
31+
registered_keys_for_concatenation: Default::default(),
32+
#[cfg(feature = "future_snark")]
33+
registered_keys_for_snark: Default::default(),
2734
}
2835
}
2936

@@ -32,11 +39,28 @@ impl KeyRegistration {
3239
/// # Error
3340
/// The function fails when the entry is already registered.
3441
pub fn register_by_entry(&mut self, entry: &RegistrationEntry) -> StmResult<()> {
35-
if !self.registration_entries.contains(entry) {
36-
self.registration_entries.insert(*entry);
37-
return Ok(());
42+
let vk_concatenation = entry.get_verification_key_for_concatenation();
43+
let is_already_registered =
44+
self.registered_keys_for_concatenation.contains(&vk_concatenation);
45+
46+
#[cfg(feature = "future_snark")]
47+
let is_already_registered = is_already_registered
48+
|| entry
49+
.get_verification_key_for_snark()
50+
.is_some_and(|vk_snark| self.registered_keys_for_snark.contains(&vk_snark));
51+
52+
if is_already_registered {
53+
return Err(RegisterError::EntryAlreadyRegistered(Box::new(*entry)).into());
3854
}
39-
Err(RegisterError::EntryAlreadyRegistered(Box::new(*entry)).into())
55+
56+
self.registered_keys_for_concatenation.insert(vk_concatenation);
57+
#[cfg(feature = "future_snark")]
58+
if let Some(vk_snark) = entry.get_verification_key_for_snark() {
59+
self.registered_keys_for_snark.insert(vk_snark);
60+
}
61+
self.registration_entries.insert(*entry);
62+
63+
Ok(())
4064
}
4165

4266
/// Registers a new signer with the given verification key proof of possession and stake.
@@ -308,6 +332,66 @@ mod tests {
308332
}
309333
}
310334

335+
#[test]
336+
fn register_by_entry_rejects_same_verification_key_with_different_stake() {
337+
let mut rng = ChaCha20Rng::from_seed([0u8; 32]);
338+
let mut kr = KeyRegistration::initialize();
339+
let vk_pop = VerificationKeyProofOfPossessionForConcatenation::from(
340+
&BlsSigningKey::generate(&mut rng),
341+
);
342+
343+
let first_entry = RegistrationEntry::new(
344+
vk_pop,
345+
100,
346+
#[cfg(feature = "future_snark")]
347+
None,
348+
)
349+
.unwrap();
350+
kr.register_by_entry(&first_entry)
351+
.expect("registering a new verification key should succeed");
352+
353+
let second_entry = RegistrationEntry::new(
354+
vk_pop,
355+
200,
356+
#[cfg(feature = "future_snark")]
357+
None,
358+
)
359+
.unwrap();
360+
let result = kr.register_by_entry(&second_entry);
361+
362+
assert!(matches!(
363+
result.unwrap_err().downcast_ref::<RegisterError>(),
364+
Some(RegisterError::EntryAlreadyRegistered(_))
365+
));
366+
}
367+
368+
#[cfg(feature = "future_snark")]
369+
#[test]
370+
fn register_by_entry_rejects_same_snark_key_with_different_concatenation_key() {
371+
let mut rng = ChaCha20Rng::from_seed([0u8; 32]);
372+
let mut kr = KeyRegistration::initialize();
373+
let schnorr_vk =
374+
SchnorrVerificationKey::new_from_signing_key(SchnorrSigningKey::generate(&mut rng));
375+
376+
let first_vk_pop = VerificationKeyProofOfPossessionForConcatenation::from(
377+
&BlsSigningKey::generate(&mut rng),
378+
);
379+
let first_entry = RegistrationEntry::new(first_vk_pop, 100, Some(schnorr_vk)).unwrap();
380+
kr.register_by_entry(&first_entry)
381+
.expect("registering a new verification key pair should succeed");
382+
383+
let second_vk_pop = VerificationKeyProofOfPossessionForConcatenation::from(
384+
&BlsSigningKey::generate(&mut rng),
385+
);
386+
let second_entry = RegistrationEntry::new(second_vk_pop, 200, Some(schnorr_vk)).unwrap();
387+
let result = kr.register_by_entry(&second_entry);
388+
389+
assert!(matches!(
390+
result.unwrap_err().downcast_ref::<RegisterError>(),
391+
Some(RegisterError::EntryAlreadyRegistered(_))
392+
));
393+
}
394+
311395
proptest! {
312396
#[test]
313397
fn test_keyreg(stake in vec(1..1u64 << 60, 2..=10),
@@ -333,8 +417,10 @@ mod tests {
333417
VerificationKeyProofOfPossessionForConcatenation::from(&sk)
334418
};
335419

336-
// Record successful registrations
420+
// Record successful registrations, keyed by verification key since that's
421+
// the uniqueness criterion enforced by register_by_entry
337422
let mut keys = BTreeSet::new();
423+
let mut registered_entries = BTreeSet::new();
338424

339425
for (i, &stake) in stake.iter().enumerate() {
340426
let mut pk = gen_keys[i % gen_keys.len()];
@@ -350,15 +436,17 @@ mod tests {
350436

351437
match entry_result {
352438
Ok(entry) => {
439+
let vk = entry.get_verification_key_for_concatenation();
353440
let reg = kr.register_by_entry(&entry);
354441
match reg {
355442
Ok(_) => {
356-
assert!(keys.insert(entry));
443+
assert!(keys.insert(vk));
444+
assert!(registered_entries.insert(entry));
357445
},
358446
Err(error) => match error.downcast_ref::<RegisterError>(){
359447
Some(RegisterError::EntryAlreadyRegistered(e1)) => {
360448
assert!(e1.as_ref() == &entry);
361-
assert!(keys.contains(&entry));
449+
assert!(keys.contains(&vk));
362450
},
363451
_ => {panic!("Unexpected error: {error}")}
364452
}
@@ -380,7 +468,7 @@ mod tests {
380468
let retrieved_keys = closed.closed_registration_entries.iter()
381469
.map(|entry| (*entry).clone().into())
382470
.collect::<BTreeSet<RegistrationEntry>>();
383-
assert!(retrieved_keys == keys);
471+
assert!(retrieved_keys == registered_entries);
384472
}
385473
}
386474
}

0 commit comments

Comments
 (0)