Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@
- [BREAKING] Removed the stripped `MastForest` serialization mode. Normal forest bytes now describe execution data only ([#3268](https://github.com/0xMiden/miden-vm/pull/3268)).
- [BREAKING] Bump Plonky3 related dependencies to fix NEON arithmetic bug ([#3272](https://github.com/0xMiden/miden-vm/pull/3272)).
- [BREAKING] Bump Plonky3 and miden-crypto related dependencies ([#3275](https://github.com/0xMiden/miden-vm/pull/3275)).
- Added trusted binary serialization for `TraceProvingInputs` and sparse MAST replay data so pre-executed trace inputs can be sent to a trusted prover ([#3284](https://github.com/0xMiden/miden-vm/pull/3284)).

#### Fixes

Expand Down
7 changes: 6 additions & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion air/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ default = ["std"]
arbitrary = ["std", "dep:proptest"]
std = ["miden-core/std", "proptest?/std", "thiserror/std"]
concurrent = ["std"]
testing = []
testing = ["arbitrary"]

[dependencies]
# Miden dependencies
Expand Down
8 changes: 7 additions & 1 deletion core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,13 @@ serde = [
"miden-debug-types/serde",
"miden-utils-indexing/serde",
]
arbitrary = ["dep:proptest"]
arbitrary = [
"dep:proptest",
# TODO: switch to miden-crypto/arbitrary once it exists. For now, the crypto-owned
# Arbitrary impls for Felt and Word are exposed by testing:
# https://github.com/0xMiden/crypto/issues/1071
"miden-crypto/testing",
]
testing = ["arbitrary"]
fuzzing = []

Expand Down
49 changes: 49 additions & 0 deletions core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ pub mod field {
}

pub mod serde {
use alloc::collections::VecDeque;

pub use miden_crypto::utils::{
BudgetedReader, ByteReader, ByteWriter, Deserializable, DeserializationError, Serializable,
SliceReader,
Expand Down Expand Up @@ -73,6 +75,53 @@ pub mod serde {
err => err,
})
}

/// Serializable view over a [`VecDeque`].
///
/// This uses the same wire shape as `Vec<T>`: a length prefix followed by items in iteration
/// order.
pub struct SerializableVecDeque<'a, T>(pub &'a VecDeque<T>);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We are doing this because we don't have have serialization implemented for VecDeque in miden-crypto? If so, should we just implement it it there?

If we'd rather keep it here for now, I'd move it src/utils/mod.rs.

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.

#3314 moves the VecDeque helper to core::utils. I did not add it to miden-crypto for now because (besides release propagation delays) the only current caller is trace replay.


impl<T: Serializable> Serializable for SerializableVecDeque<'_, T> {
fn write_into<W: ByteWriter>(&self, target: &mut W) {
target.write_usize(self.0.len());
for item in self.0 {
item.write_into(target);
}
}
}

/// Reads a [`VecDeque`] encoded by [`SerializableVecDeque`].
pub fn read_vec_deque<T: Deserializable, R: ByteReader>(
source: &mut R,
) -> Result<VecDeque<T>, DeserializationError> {
let len = read_bounded_len(source, "VecDeque", T::min_serialized_size())?;
let mut values = VecDeque::with_capacity(len);
for _ in 0..len {
values.push_back(T::read_from(source)?);
}
Ok(values)
}

#[cfg(test)]
mod tests {
use alloc::{collections::VecDeque, vec::Vec};

use super::{Deserializable, Serializable, SerializableVecDeque, read_vec_deque};

#[test]
fn vec_deque_round_trip_uses_vec_shape() {
let values = VecDeque::from([1u32, 2, 3]);
let mut bytes = Vec::new();
SerializableVecDeque(&values).write_into(&mut bytes);

let restored = read_vec_deque(&mut super::SliceReader::new(&bytes)).unwrap();
assert_eq!(values, restored);

let vec = Vec::<u32>::read_from_bytes(&bytes).unwrap();
assert_eq!(vec, [1, 2, 3]);
}
}
}

pub mod crypto {
Expand Down
32 changes: 27 additions & 5 deletions core/src/mast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,9 @@ use proptest::prelude::*;
#[cfg(feature = "serde")]
use serde::{Deserialize, Serialize};

#[cfg(feature = "serde")]
use crate::serde::SliceReader;

mod node;
#[cfg(any(test, feature = "arbitrary"))]
pub use node::arbitrary;
Expand All @@ -61,19 +64,17 @@ pub use node::{
OpBatch, SplitNode, SplitNodeBuilder,
};

#[cfg(feature = "serde")]
use crate::serde::{Deserializable, Serializable, SliceReader};
use crate::{
Felt, Word,
advice::AdviceMap,
serde::{ByteWriter, DeserializationError},
serde::{ByteWriter, Deserializable, DeserializationError, Serializable},
utils::{Idx, IndexVec, hash_string_to_word},
};

mod serialization;
pub use serialization::{
AdviceMapView, AdviceValueView, MastForestReadMode, MastForestReadView, MastForestView,
MastForestWireView, MastNodeEntry, MastNodeInfo,
MastForestWireView, MastNodeEntry, MastNodeInfo, SparseMastForestReadOptions,
};

mod untrusted;
Expand Down Expand Up @@ -838,7 +839,10 @@ impl<T: ExecutableMastForest + ?Sized> ExecutableMastForest for Arc<T> {
#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)]
#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
#[cfg_attr(feature = "serde", serde(transparent))]
#[cfg_attr(all(feature = "arbitrary", test), miden_test_serde_macros::serde_test)]
#[cfg_attr(
all(feature = "arbitrary", test),
miden_test_serde_macros::serde_test(binary_serde(true))
)]
Comment thread
huitseeker marked this conversation as resolved.
pub struct MastNodeId(u32);

/// Operations that mutate a MAST often produce this mapping between old and new NodeIds.
Expand Down Expand Up @@ -907,6 +911,24 @@ impl From<MastNodeId> for u32 {
}
}

impl Serializable for MastNodeId {
fn write_into<W: ByteWriter>(&self, target: &mut W) {
Serializable::write_into(&self.0, target);
}
}

impl Deserializable for MastNodeId {
fn read_from<R: crate::serde::ByteReader>(
source: &mut R,
) -> Result<Self, DeserializationError> {
Ok(Self(<u32 as Deserializable>::read_from(source)?))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm not sure this is safe. I remember previously we've always avoided deserializing MAST node IDs directly and preferred using from_u32_with_node_count(). I believe this was because MAST node ID values were limited to $2^{30} - 1$ and so a full u32 value could result in an invalid MAST node ID.

If this is not a concern now, we should explicitly document this. But also, I'm a bit weary of adding this serialization option as it could be easily misused.

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.

Addressed in #3313/#3314:

}

fn min_serialized_size() -> usize {
<u32 as Deserializable>::min_serialized_size()
}
}

impl fmt::Display for MastNodeId {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "MastNodeId({})", self.0)
Expand Down
15 changes: 13 additions & 2 deletions core/src/mast/serialization/layout.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
use alloc::{format, string::ToString, vec::Vec};

use super::{FLAG_HASHLESS, FLAGS_RESERVED_MASK, MAGIC, MastForest, MastNodeEntry, VERSION};
use super::{
FLAG_HASHLESS, FLAG_SPARSE, FLAGS_RESERVED_MASK, MAGIC, MastForest, MastNodeEntry, VERSION,
};
use crate::{
mast::MastNodeId,
serde::{ByteReader, Deserializable, DeserializationError, SliceReader},
Expand Down Expand Up @@ -157,6 +159,10 @@ impl WireFlags {
pub(super) fn is_hashless(self) -> bool {
self.0 & FLAG_HASHLESS != 0
}

pub(super) fn is_sparse(self) -> bool {
self.0 & FLAG_SPARSE != 0
}
}

// LAYOUT SCANNING
Expand All @@ -171,6 +177,11 @@ pub(super) fn read_header_and_scan_layout<R: OffsetTrackingReader>(
// untrusted deserialization path.
let (raw_flags, _version) = read_and_validate_header(source)?;
let flags = WireFlags::new(raw_flags);
if flags.is_sparse() {
return Err(DeserializationError::InvalidValue(
"SPARSE flag is set; use SparseMastForest for sparse replay input".to_string(),
));
}
if flags.is_hashless() && !allow_hashless {
return Err(DeserializationError::InvalidValue(
"HASHLESS flag is set; use UntrustedMastForest for untrusted input".to_string(),
Expand Down Expand Up @@ -457,7 +468,7 @@ fn validate_budgeted_count<R: ByteReader>(
Ok(())
}

fn read_and_validate_header<R: ByteReader>(
pub(super) fn read_and_validate_header<R: ByteReader>(
source: &mut R,
) -> Result<(u8, [u8; 3]), DeserializationError> {
let magic: [u8; 4] = source.read_array()?;
Expand Down
26 changes: 21 additions & 5 deletions core/src/mast/serialization/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,14 @@
//! same contiguous array on the wire.
//!
//! Public entry points adopt these policies:
//! - [`MastForest::read_from_bytes`]: trusted execution payload, no hashless support.
//! - [`MastForest::read_from_bytes`]: trusted dense execution payload, no hashless or sparse
//! support.
//! - [`MastForestWireView::new`]: trusted wire-backed cache access; rejects hashless and legacy
//! debug-bearing payloads.
//! debug-bearing payloads, and rejects sparse payloads.
//! - [`crate::mast::SparseMastForest::read_from_bytes`] /
//! [`crate::mast::SparseMastForest::read_from_bytes_with_options`]: trusted sparse replay
//! payloads for serialized trace-generation inputs. Sparse payloads currently carry full-node
//! digests and do not recompute them on read.
//! - [`crate::mast::UntrustedMastForest::read_from_bytes`] /
//! [`crate::mast::UntrustedMastForest::read_from_bytes_with_options`]: untrusted parsing plus
//! later validation before use.
Expand Down Expand Up @@ -112,6 +117,9 @@ mod layout;
pub(super) use layout::ForestLayout;
use layout::{OffsetTrackingReader, TrackingReader, WireFlags, read_header_and_scan_layout};

mod sparse;
pub use sparse::SparseMastForestReadOptions;

mod resolved;
use resolved::{ResolvedSerializedForest, basic_block_offset_for_node_index};

Expand Down Expand Up @@ -171,10 +179,16 @@ const MAGIC: &[u8; 4] = b"MAST";
/// from local structure.
pub(super) const FLAG_HASHLESS: u8 = 0x02;

/// Flag indicating that the payload uses sparse MAST replay serialization.
///
/// Sparse payloads preserve the source forest's [`MastNodeId`] space and therefore cannot be read
/// through dense [`MastForest`] entry points.
pub(super) const FLAG_SPARSE: u8 = 0x04;
Comment on lines +182 to +186

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Question: what's the benefit of combining regular and sparse MAST forest serializations? Could we treat sparse MAST forest as a completely separate object with its own serialization? I think the use cases for these are fairly distinct and I don't foresee the need to serialize sparse MAST forest outside of the TraceProvingInputs.

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.

Addressed in #3313: Sparse payloads now use their own magic and version, and has its own writers/readers.


/// Mask for reserved flag bits that must be zero.
///
/// Bit 0 and bits 2-7 are reserved for future use. If any are set, deserialization fails.
const FLAGS_RESERVED_MASK: u8 = 0xfd;
/// Bit 0 and bits 3-7 are reserved for future use. If any are set, deserialization fails.
const FLAGS_RESERVED_MASK: u8 = 0xf9;

/// The format version.
///
Expand All @@ -201,13 +215,15 @@ const FLAGS_RESERVED_MASK: u8 = 0xfd;
/// records. MAST nodes are metadata-free identifiers. Before any public release on this branch,
/// the same unreleased wire version also reserved bit 0 and stopped using it as a forest-level
/// debug-presence flag.
/// - [0, 0, 5]: Added SPARSE flag (bit 2). Sparse payloads preserve sparse replay IDs and are
/// accepted only by SparseMastForest readers.
///
/// Legacy wire versions (pre-#3192 decorator terminology):
/// [0,0,1] stored metadata as serialized decorator variants in CSR per-node slots.
/// [0,0,2] removed AssemblyOp from the decorator enum and stored them separately in DebugInfo.
/// [0,0,3] removed the unused decorator-count wire field.
/// [0,0,4] eliminated the decorator wire slots entirely.
const VERSION: [u8; 3] = [0, 0, 4];
const VERSION: [u8; 3] = [0, 0, 5];

// MAST FOREST SERIALIZATION/DESERIALIZATION
// ================================================================================================
Expand Down
Loading
Loading