Skip to content
Open
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
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,14 @@
- [BREAKING] Removed unused public APIs and narrowed test-only helper visibility across VM and crypto crates ([#3424](https://github.com/0xMiden/miden-vm/pull/3424)).
- Enable simd128 Plonky3 backend for WASM builds and added related CI job ([#3433](https://github.com/0xMiden/miden-vm/pull/3433)).
- [BREAKING] Bumped Plonky3 related dependencies to integrate SVE2 and WASM-SIMD128 speed-ups and include a NEON bugfix. ([#3441](https://github.com/0xMiden/miden-vm/pull/3441)).
- [BREAKING] Changed `Package::read_from_trusted` and `Package::read_from_bytes_trusted` to skip embedded MAST and manifest cross-check validation, removed the package unchecked readers, and kept untrusted package reads on `Package::read_from` and `Package::read_from_bytes`, which validate MAST and drop debug sections ([#3418](https://github.com/0xMiden/miden-vm/pull/3418)).

#### Fixes

- Restored public `miden-lifted-stark` testing constants to preserve compatibility with version 0.28.1.
- Changed overspecified untrusted MAST forest input with wire hashes from an error log to a warning, and included the caller location in the warning ([#3418](https://github.com/0xMiden/miden-vm/pull/3418)).
- [BREAKING] Bound MMR peak commitments to the leaf count by hashing `[num_leaves, 0, 0, 0] || padded_peaks`, and updated the core library `mmr::pack`/`mmr::unpack` procedures to use the same preimage ([#3388](https://github.com/0xMiden/miden-vm/pull/3388)).
- Documented the program-entrypoint locals invariant on `Procedure::set_num_locals` and now assert it at that AST mutation site, so setting locals on an executable module's `begin`..`end` block panics at the producer boundary. The existing assembler assertion is retained as a backstop for entrypoints built directly via `Procedure::new` ([#3382](https://github.com/0xMiden/miden-vm/pull/3382)).
- Validated `SectionId` on deserialization: `Section::read_from()` now rejects invalid identifiers and the `serde` path delegates to `FromStr`, keeping both readers on the same invariant ([#3277](https://github.com/0xMiden/miden-vm/pull/3277)).
- Fixed `hash_bytes(&[])` returning `Word::default()`; the empty-bytes input now absorbs a padding marker and permutes, producing a nonzero digest consistent with the 10\* sponge padding rule ([#3366](https://github.com/0xMiden/miden-vm/pull/3366)).
- Fixed a latent `CryptoBox` (IES) key-derivation bug: HKDF-SHA256 output is now reduced into canonical Felts via `AeadScheme::key_from_uniform_bytes` instead of being fed into canonical decoding, which rejected noncanonical limbs at ~2^-30 per key ([#3366](https://github.com/0xMiden/miden-vm/pull/3366)).
Expand Down
25 changes: 16 additions & 9 deletions core/src/mast/serialization/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@
#[cfg(test)]
use alloc::string::ToString;
use alloc::{boxed::Box, format, vec::Vec};
use core::mem::size_of;
use core::{mem::size_of, panic::Location};

use miden_utils_sync::OnceLockCompat;

Expand Down Expand Up @@ -778,27 +778,32 @@ impl super::UntrustedMastForest {
}
}

pub(super) fn read_untrusted_with_flags<R: ByteReader>(
pub(super) fn read_untrusted_with_flags_and_caller<R: ByteReader>(
source: &mut R,
caller: &'static Location<'static>,
) -> Result<(super::UntrustedMastForest, u8), DeserializationError> {
let (flags, forest) = decode_from_reader(source, true)?;
log_untrusted_overspecification(flags);
log_untrusted_overspecification(flags, caller);
Ok((forest, flags.bits()))
}

pub(super) fn read_untrusted_with_flags_and_allocation_budget<R: ByteReader>(
pub(super) fn read_untrusted_with_flags_allocation_budget_and_caller<R: ByteReader>(
source: &mut R,
allocation_budget: usize,
caller: &'static Location<'static>,
) -> Result<(super::UntrustedMastForest, u8), DeserializationError> {
let (flags, forest) = decode_from_reader_inner(source, true, Some(allocation_budget))?;
log_untrusted_overspecification(flags);
log_untrusted_overspecification(flags, caller);
Ok((forest, flags.bits()))
}

fn log_untrusted_overspecification(flags: WireFlags) {
fn log_untrusted_overspecification(flags: WireFlags, caller: &'static Location<'static>) {
if !flags.is_hashless() {
log::error!(
"UntrustedMastForest expected HASHLESS input; supplied artifact includes wire node hashes, and validation will recompute them and require them to match"
log::warn!(
"UntrustedMastForest expected HASHLESS input at {}:{}:{}; supplied artifact includes wire node hashes, and validation will recompute them and require them to match",
caller.file(),
caller.line(),
caller.column(),
);
}
}
Expand Down Expand Up @@ -866,8 +871,9 @@ impl Deserializable for super::UntrustedMastForest {
/// After deserialization, callers should use [`super::UntrustedMastForest::validate()`]
/// to verify structural integrity and recompute all node hashes before using
/// the forest.
#[track_caller]
fn read_from<R: ByteReader>(source: &mut R) -> Result<Self, DeserializationError> {
read_untrusted_with_flags(source).map(|(forest, _flags)| forest)
super::UntrustedMastForest::read_from_reader(source)
}

/// Deserializes an [`super::UntrustedMastForest`] from bytes using budgeted deserialization.
Expand All @@ -878,6 +884,7 @@ impl Deserializable for super::UntrustedMastForest {
/// After deserialization, callers should use [`super::UntrustedMastForest::validate()`]
/// to verify structural integrity and recompute all node hashes before using
/// the forest.
#[track_caller]
fn read_from_bytes(bytes: &[u8]) -> Result<Self, DeserializationError> {
super::UntrustedMastForest::read_from_bytes(bytes)
}
Expand Down
65 changes: 58 additions & 7 deletions core/src/mast/serialization/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use crate::{
};

struct TestLogger {
messages: Mutex<Vec<String>>,
messages: Mutex<Vec<(log::Level, String)>>,
}

impl log::Log for TestLogger {
Expand All @@ -30,7 +30,7 @@ impl log::Log for TestLogger {

fn log(&self, record: &log::Record<'_>) {
if self.enabled(record.metadata()) {
self.messages.lock().unwrap().push(record.args().to_string());
self.messages.lock().unwrap().push((record.level(), record.args().to_string()));
}
}

Expand All @@ -41,11 +41,14 @@ static TEST_LOGGER: TestLogger = TestLogger { messages: Mutex::new(Vec::new()) }
static TEST_LOGGER_INIT: Once = Once::new();
static TEST_LOGGER_GUARD: Mutex<()> = Mutex::new(());

fn with_captured_error_logs<T>(f: impl FnOnce() -> T) -> (T, Vec<String>) {
with_captured_logs(log::LevelFilter::Error, f)
fn with_captured_warn_logs<T>(f: impl FnOnce() -> T) -> (T, Vec<(log::Level, String)>) {
with_captured_logs(log::LevelFilter::Warn, f)
}

fn with_captured_logs<T>(level: log::LevelFilter, f: impl FnOnce() -> T) -> (T, Vec<String>) {
fn with_captured_logs<T>(
level: log::LevelFilter,
f: impl FnOnce() -> T,
) -> (T, Vec<(log::Level, String)>) {
TEST_LOGGER_INIT.call_once(|| {
log::set_logger(&TEST_LOGGER).expect("test logger should be installed once");
});
Expand Down Expand Up @@ -1742,12 +1745,14 @@ fn assert_untrusted_overspec_logging(
expected_nodes: u32,
expected_log_fragments: &[&str],
) {
let (result, logs) = with_captured_error_logs(|| UntrustedMastForest::read_from_bytes(bytes));
let (result, logs) = with_captured_warn_logs(|| UntrustedMastForest::read_from_bytes(bytes));

let untrusted = result.unwrap();
assert_eq!(logs.len(), expected_log_fragments.len());
for expected in expected_log_fragments {
assert!(logs.iter().any(|msg| msg.contains(expected)));
assert!(logs.iter().any(|(level, msg)| {
*level == log::Level::Warn && msg.contains(expected) && msg.contains("input at ")
}));
}
assert_eq!(untrusted.validate().unwrap().num_nodes(), expected_nodes);

Expand Down Expand Up @@ -1779,6 +1784,52 @@ fn test_untrusted_overspecification_logging_matches_wire_mode() {
assert_untrusted_overspec_logging(&bytes, forest.num_nodes(), &["wire node hashes"]);
}

#[test]
fn test_untrusted_overspecification_logging_tracks_trait_call_site() {
let mut forest = MastForest::new();
let block_id = BasicBlockNodeBuilder::new(vec![Operation::Add])
.add_to_forest(&mut forest)
.unwrap();
forest.make_root(block_id);

let bytes = forest.to_bytes();
let mut reader = SliceReader::new(&bytes);
let expected_line = line!() + 2;
let (result, logs) =
with_captured_warn_logs(|| <UntrustedMastForest as Deserializable>::read_from(&mut reader));

result.unwrap();
let expected_location = format!("{}:{expected_line}:", file!());
assert!(logs.iter().any(|(level, msg)| {
*level == log::Level::Warn
&& msg.contains("wire node hashes")
&& msg.contains(&expected_location)
}));
}

#[test]
fn test_untrusted_overspecification_logging_tracks_trait_bytes_call_site() {
let mut forest = MastForest::new();
let block_id = BasicBlockNodeBuilder::new(vec![Operation::Add])
.add_to_forest(&mut forest)
.unwrap();
forest.make_root(block_id);

let bytes = forest.to_bytes();
let expected_line = line!() + 2;
let (result, logs) = with_captured_warn_logs(|| {
<UntrustedMastForest as Deserializable>::read_from_bytes(&bytes)
});

result.unwrap();
let expected_location = format!("{}:{expected_line}:", file!());
assert!(logs.iter().any(|(level, msg)| {
*level == log::Level::Warn
&& msg.contains("wire node hashes")
&& msg.contains(&expected_location)
}));
}

/// Test that untrusted validation in hashless mode recomputes non-external digests without any
/// general wire hash section.
#[test]
Expand Down
26 changes: 22 additions & 4 deletions core/src/mast/untrusted.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use alloc::vec::Vec;
use core::panic::Location;

use super::{AdviceMap, MastForest, MastForestError, serialization};
use crate::serde::{BudgetedReader, ByteReader, DeserializationError, SliceReader};
Expand Down Expand Up @@ -76,6 +77,18 @@ impl UntrustedMastForestReadOptions {
}

impl UntrustedMastForest {
/// Deserializes an [`UntrustedMastForest`] from a byte reader.
///
/// Note: This method does not apply budgeting. For untrusted bytes, prefer
/// [`read_from_bytes`](Self::read_from_bytes) or
/// [`read_from_bytes_with_options`](Self::read_from_bytes_with_options).
#[track_caller]
pub fn read_from_reader<R: ByteReader>(source: &mut R) -> Result<Self, DeserializationError> {
let caller = Location::caller();
Comment thread
bitwalker marked this conversation as resolved.
serialization::read_untrusted_with_flags_and_caller(source, caller)
.map(|(forest, _flags)| forest)
}

/// Validates the forest by checking structural invariants and recomputing all node hashes.
///
/// This method performs a complete validation of the deserialized forest:
Expand Down Expand Up @@ -142,6 +155,7 @@ impl UntrustedMastForest {
/// // Validate before use
/// let forest = untrusted.validate()?;
/// ```
#[track_caller]
pub fn read_from_bytes(bytes: &[u8]) -> Result<Self, DeserializationError> {
Self::read_from_bytes_with_options(bytes, UntrustedMastForestReadOptions::default())
}
Expand All @@ -151,16 +165,20 @@ impl UntrustedMastForest {
/// The wire byte budget limits wire-driven parsing and collection pre-sizing. The validation
/// helper-allocation budget is derived from that wire budget and caps tracked hashless helper
/// allocations such as digest slot tables and rebuilt digest tables.
#[track_caller]
pub fn read_from_bytes_with_options(
bytes: &[u8],
options: UntrustedMastForestReadOptions,
) -> Result<Self, DeserializationError> {
let caller = Location::caller();
let wire_byte_budget = options.wire_byte_budget(bytes.len());
let mut reader = BudgetedReader::new(SliceReader::new(bytes), wire_byte_budget);
let (forest, _flags) = serialization::read_untrusted_with_flags_and_allocation_budget(
&mut reader,
options.validation_allocation_budget(wire_byte_budget),
)?;
let (forest, _flags) =
serialization::read_untrusted_with_flags_allocation_budget_and_caller(
&mut reader,
options.validation_allocation_budget(wire_byte_budget),
caller,
)?;
if reader.has_more_bytes() {
return Err(DeserializationError::InvalidValue(
"extra bytes after MastForest payload".into(),
Expand Down
3 changes: 2 additions & 1 deletion crates/assembly/src/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use std::{
};

use miden_assembly_syntax::{ast::ModuleKind, diagnostics::Report};
use miden_core::serde::Deserializable;
use miden_mast_package::{Package as MastPackage, TargetType};
use miden_package_registry::{PackageCache, PackageId, Version as PackageVersion};
use miden_project::{
Expand Down Expand Up @@ -998,7 +999,7 @@ fn load_selected_preassembled_package(
fn load_package_from_path(path: &FsPath) -> Result<Arc<MastPackage>, Report> {
let bytes = fs::read(path)
.map_err(|error| Report::msg(format!("failed to read '{}': {error}", path.display())))?;
let package = MastPackage::read_from_bytes_trusted(&bytes).map_err(|error| {
let package = MastPackage::read_from_bytes(&bytes).map_err(|error| {
Report::msg(format!("failed to decode package '{}': {error}", path.display()))
})?;
Ok(Arc::new(package))
Expand Down
Loading
Loading