diff --git a/.cargo/config.toml b/.cargo/config.toml index 8af5541ffd..18284bf64d 100644 --- a/.cargo/config.toml +++ b/.cargo/config.toml @@ -6,22 +6,28 @@ # # NOTE: Only lints that the codebase currently passes are enabled. # Additional lints can be enabled incrementally as the codebase is cleaned up. -# NOTE: for the `make lint` command to work well, this must contain the same list of lints as the xclippy-fix command below +# NOTE: the `xclippy` alias below is the single source of truth for the project +# clippy lints; the fix path (`make xclippy-fix`) derives them from here so the +# check and fix commands can never drift apart. xclippy = [ "clippy", "--workspace", "--all-targets", "--all-features", "--", "-Wclippy::all", # Lints that currently pass in the codebase: "-Wclippy::await_holding_lock", "-Wclippy::char_lit_as_u8", + "-Wclippy::checked_conversions", "-Wclippy::dbg_macro", + "-Wclippy::debug_assert_with_mut_call", "-Wclippy::derive_partial_eq_without_eq", "-Wclippy::disallowed_methods", "-Wclippy::empty_enums", "-Wclippy::exit", + "-Wclippy::expl_impl_clone_on_copy", "-Wclippy::explicit_deref_methods", "-Wclippy::filter_map_next", "-Wclippy::flat_map_option", "-Wclippy::float_cmp_const", + "-Wclippy::fn_params_excessive_bools", "-Wclippy::if_let_mutex", "-Wclippy::imprecise_flops", "-Wclippy::inefficient_to_string", @@ -42,6 +48,7 @@ xclippy = [ "-Wclippy::needless_borrow", "-Wclippy::needless_collect", "-Wclippy::needless_continue", + "-Wclippy::option_option", "-Wclippy::path_buf_push_overwrite", "-Wclippy::rc_mutex", "-Wclippy::redundant_clone", @@ -53,6 +60,7 @@ xclippy = [ "-Wclippy::string_add_assign", "-Wclippy::todo", "-Wclippy::trait_duplication_in_bounds", + "-Wclippy::unimplemented", "-Wclippy::trivially_copy_pass_by_ref", "-Wclippy::uninlined_format_args", "-Wclippy::unnested_or_patterns", @@ -66,14 +74,10 @@ xclippy = [ "-Wunused_lifetimes", "-Wunused_qualifications", # The following lints are disabled because they trigger warnings: - # -Wclippy::checked_conversions (12 warnings) - # -Wclippy::debug_assert_with_mut_call (3 warnings) # -Wclippy::doc_markdown (1293 warnings) # -Wclippy::enum_glob_use (17 warnings) - # -Wclippy::expl_impl_clone_on_copy (1 warning) # -Wclippy::explicit_into_iter_loop (7 warnings) # -Wclippy::fallible_impl_from (3 warnings) - # -Wclippy::fn_params_excessive_bools (1 warning) # -Wclippy::from_iter_instead_of_collect (4 warnings) # -Wclippy::implicit_clone (5 warnings) # -Wclippy::map_err_ignore (78 warnings) @@ -82,12 +86,10 @@ xclippy = [ # -Wclippy::match_wild_err_arm (6 warnings) # -Wclippy::match_wildcard_for_single_variants (8 warnings) # -Wclippy::needless_for_each (7 warnings) - # -Wclippy::option_option (6 warnings) # -Wclippy::ptr_as_ptr (15 warnings) # -Wclippy::semicolon_if_nothing_returned (97 warnings) # -Wclippy::single_match_else (54 warnings) # -Wclippy::string_lit_as_bytes (15 warnings) - # -Wclippy::unimplemented (1 warning) # -Wclippy::unused_self (11 warnings) # -Wclippy::cast_lossless (603 warnings) # -Wclippy::manual_assert (17 warnings) @@ -97,67 +99,5 @@ xclippy = [ # -Wunreachable_pub (545 warnings) ] -# Clippy fix with the same lints as xclippy -# NOTE: for the `make lint` command to work well, this must contain the same list of lints as the xclippy setting above -xclippy-fix = [ - "clippy", "--fix", "--allow-staged", "--allow-dirty", "--workspace", "--all-targets", "--all-features", "--", - "-Wclippy::all", - "-Wclippy::await_holding_lock", - "-Wclippy::char_lit_as_u8", - "-Wclippy::dbg_macro", - "-Wclippy::derive_partial_eq_without_eq", - "-Wclippy::disallowed_methods", - "-Wclippy::empty_enums", - "-Wclippy::exit", - "-Wclippy::explicit_deref_methods", - "-Wclippy::filter_map_next", - "-Wclippy::flat_map_option", - "-Wclippy::float_cmp_const", - "-Wclippy::if_let_mutex", - "-Wclippy::imprecise_flops", - "-Wclippy::inefficient_to_string", - "-Wclippy::invalid_upcast_comparisons", - "-Wclippy::large_digit_groups", - "-Wclippy::large_stack_arrays", - "-Wclippy::large_types_passed_by_value", - "-Wclippy::let_unit_value", - "-Wclippy::linkedlist", - "-Wclippy::lossy_float_literal", - "-Wclippy::macro_use_imports", - "-Wclippy::manual_ok_or", - "-Wclippy::map_flatten", - "-Wclippy::mem_forget", - "-Wclippy::missing_enforced_import_renames", - "-Wclippy::mut_mut", - "-Wclippy::mutex_integer", - "-Wclippy::needless_borrow", - "-Wclippy::needless_collect", - "-Wclippy::needless_continue", - "-Wclippy::path_buf_push_overwrite", - "-Wclippy::rc_mutex", - "-Wclippy::redundant_clone", - "-Wclippy::redundant_closure_for_method_calls", - "-Wclippy::ref_option_ref", - "-Wclippy::rest_pat_in_fully_bound_structs", - "-Wclippy::same_functions_in_if_condition", - "-Wclippy::string_add", - "-Wclippy::string_add_assign", - "-Wclippy::todo", - "-Wclippy::trait_duplication_in_bounds", - "-Wclippy::trivially_copy_pass_by_ref", - "-Wclippy::uninlined_format_args", - "-Wclippy::unnested_or_patterns", - "-Wclippy::useless_transmute", - "-Wclippy::verbose_file_reads", - "-Wclippy::zero_sized_map_values", - "-Wfuture_incompatible", - "-Wnonstandard_style", - "-Wtrivial_numeric_casts", - "-Wunexpected_cfgs", - "-Wunused_lifetimes", - "-Wunused_qualifications", - "-Wclippy::unnecessary_wraps", -] - [target.wasm32-unknown-unknown] rustflags = ['--cfg', 'getrandom_backend="unsupported"', '-C', 'target-feature=+simd128'] diff --git a/CHANGELOG.md b/CHANGELOG.md index dc70e8a7af..b612bf0c2f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ #### Changes - [BREAKING] Recursive MASM verification now accepts a claim commitment and authenticates the advice-supplied claim and kernel witness. Rust callers construct request-addressed inputs with `RecursiveVerifierInputs::for_request` ([#3447](https://github.com/0xMiden/miden-vm/pull/3447)). +- Adopted cargo-fixit, aligned clippy lints with crypto, cleared cargo-shear warnings ([#3479](https://github.com/0xMiden/miden-vm/pull/3479)) #### Fixes diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index dadad6bf33..cf18f92ee2 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -97,7 +97,7 @@ For example, a new change to the AIR crate might have the following message: `fe - [Rustfmt](https://github.com/rust-lang/rustfmt) and [Clippy](https://github.com/rust-lang/rust-clippy) linting is included in CI pipeline. Anyways it's preferable to run linting locally before push: ``` - cargo fix --allow-staged --allow-dirty --all-targets --all-features; cargo fmt; cargo clippy --workspace --all-targets --all-features -- -D warnings + cargo fixit --allow-staged --allow-dirty --all-targets --all-features; cargo fmt; cargo clippy --workspace --all-targets --all-features -- -D warnings ``` ### Versioning diff --git a/Cargo.lock b/Cargo.lock index 32d7a5ac07..b7223cc2e2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2130,7 +2130,6 @@ dependencies = [ "miden-rowan", "miden-utils-diagnostics", "pretty_assertions", - "thiserror", ] [[package]] diff --git a/Makefile b/Makefile index 87617eaa40..5a2e25d18a 100644 --- a/Makefile +++ b/Makefile @@ -72,6 +72,13 @@ FEATURES_verifier := # which runs `make clippy` with RUSTFLAGS=-D warnings (see .github/workflows/lint.yml). DENY_WARNINGS := RUSTFLAGS="$(RUSTFLAGS) -D warnings" +# The clippy lint set is defined once, in the `xclippy` cargo alias in +# .cargo/config.toml, and extracted here so the check and fix paths cannot +# drift apart. cargo-fixit (a faster drop-in for `cargo clippy --fix`) cannot +# take lint flags as trailing arguments, so the fix path passes them via +# RUSTFLAGS instead. +CLIPPY_LINT_FLAGS := $(shell sed -n '/^xclippy = \[/,/^]/p' .cargo/config.toml | grep -oE '"-[WD][^"]+"' | tr -d '"' | tr '\n' ' ') + .PHONY: clippy clippy: ## Runs Clippy with configs (alias for xclippy) $(DENY_WARNINGS) cargo +stable xclippy @@ -86,8 +93,16 @@ xclippy: ## Runs Clippy with custom lint config from .cargo/config.toml fix: xclippy-fix format ## Applies automatic lint and format fixes .PHONY: xclippy-fix -xclippy-fix: ## Runs Clippy with --fix using the same lints as xclippy - cargo +stable xclippy-fix +xclippy-fix: ## Applies clippy lint fixes via cargo-fixit (a faster `cargo clippy --fix`) + @if ! command -v cargo-fixit >/dev/null 2>&1; then \ + echo "cargo-fixit is not installed; skipping clippy lint fixes." >&2; \ + echo "It is a faster drop-in replacement for 'cargo clippy --fix'." >&2; \ + echo "Install it with: cargo install cargo-fixit --locked" >&2; \ + else \ + RUSTFLAGS="$(CLIPPY_LINT_FLAGS)" cargo +stable fixit --clippy \ + --allow-dirty --allow-staged \ + --workspace --all-targets --all-features; \ + fi .PHONY: format diff --git a/air/src/constraints/decoder/tests.rs b/air/src/constraints/decoder/tests.rs deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/air/src/proof_order.rs b/air/src/proof_order.rs index 39646f305e..9a9189c309 100644 --- a/air/src/proof_order.rs +++ b/air/src/proof_order.rs @@ -105,7 +105,7 @@ impl ProofOrder { /// Decode a Lehmer rank into its AIR permutation. fn from_rank(rank: usize) -> Self { debug_assert!(rank < PROOF_ORDER_COUNT); - debug_assert!(rank <= u32::MAX as usize); + debug_assert!(u32::try_from(rank).is_ok()); let tag = rank as u32; let mut rank = rank; diff --git a/core/Cargo.toml b/core/Cargo.toml index 8db1161d9c..d40192262c 100644 --- a/core/Cargo.toml +++ b/core/Cargo.toml @@ -73,3 +73,8 @@ rstest = { workspace = true } serde_json = { workspace = true } miden-utils-testing.workspace = true trybuild.workspace = true + +[package.metadata.cargo-shear] +# `tests/ui/` holds trybuild `compile_fail` fixtures loaded by path at runtime +# (see core/tests/public_api.rs), so static analysis cannot link them. +ignored-paths = ["tests/ui/**"] diff --git a/core/src/mast/serialization/tests.rs b/core/src/mast/serialization/tests.rs index 4a7b6974b7..644284c4c8 100644 --- a/core/src/mast/serialization/tests.rs +++ b/core/src/mast/serialization/tests.rs @@ -2096,7 +2096,7 @@ fn locate_single_block_indptr_and_digest_offsets(bytes: &[u8]) -> (usize, usize) assert_eq!(discriminant, 3, "expected a Block node"); let payload = node_type_u64 & 0x0f_ff_ff_ff_ff_ff_ff_ff; - assert!(payload <= u32::MAX as u64, "Block ops_offset payload must fit in u32"); + assert!(u32::try_from(payload).is_ok(), "Block ops_offset payload must fit in u32"); let ops_offset = payload as usize; let digest_offset = view.node_hash_offset().unwrap(); diff --git a/crates/ace-codegen/src/tests/basic.rs b/crates/ace-codegen/src/tests/basic.rs index 213637c38e..fca25c1c7a 100644 --- a/crates/ace-codegen/src/tests/basic.rs +++ b/crates/ace-codegen/src/tests/basic.rs @@ -94,7 +94,7 @@ impl LiftedAir for TestAir { _aux_inputs: &[F], _challenges: &[EF], ) -> (RowMajorMatrix, Vec) { - unimplemented!("ACE codegen tests do not build concrete traces") + unreachable!("ACE codegen tests do not build concrete traces") } fn eval>(&self, builder: &mut AB) { diff --git a/crates/assembly-syntax-cst/Cargo.toml b/crates/assembly-syntax-cst/Cargo.toml index f93081fc64..bb39f76f10 100644 --- a/crates/assembly-syntax-cst/Cargo.toml +++ b/crates/assembly-syntax-cst/Cargo.toml @@ -19,12 +19,11 @@ doctest = false [features] default = ["std"] -std = ["miden-debug-types/std", "miden-utils-diagnostics/std", "rowan/std", "thiserror/std"] +std = ["miden-debug-types/std", "miden-utils-diagnostics/std", "rowan/std"] [dependencies] miden-debug-types.workspace = true miden-utils-diagnostics.workspace = true -thiserror.workspace = true rowan = { package = "miden-rowan", version = "0.16", default-features = false } [dev-dependencies] diff --git a/crates/assembly-syntax/src/parser/value.rs b/crates/assembly-syntax/src/parser/value.rs index dc677a4cc7..fac28e0368 100644 --- a/crates/assembly-syntax/src/parser/value.rs +++ b/crates/assembly-syntax/src/parser/value.rs @@ -417,11 +417,11 @@ impl proptest::arbitrary::Arbitrary for IntValue { #[inline] pub(crate) fn shrink_u64_hex(n: u64) -> IntValue { - if n <= (u8::MAX as u64) { + if u8::try_from(n).is_ok() { IntValue::U8(n as u8) - } else if n <= (u16::MAX as u64) { + } else if u16::try_from(n).is_ok() { IntValue::U16(n as u16) - } else if n <= (u32::MAX as u64) { + } else if u32::try_from(n).is_ok() { IntValue::U32(n as u32) } else { IntValue::Felt(Felt::new_unchecked(n)) diff --git a/crates/debug-types/src/span.rs b/crates/debug-types/src/span.rs index 287a540448..7f9a2be3f7 100644 --- a/crates/debug-types/src/span.rs +++ b/crates/debug-types/src/span.rs @@ -48,6 +48,7 @@ impl Spanned for alloc::sync::Arc { /// This type is used to wrap any `T` with a [SourceSpan], and is typically used when it is not /// convenient to add a [SourceSpan] to the type - most commonly because we don't control the type. +#[derive(Clone, Copy)] pub struct Span { span: SourceSpan, spanned: T, @@ -94,17 +95,6 @@ impl Spanned for Span { } } -impl Copy for Span {} - -impl Clone for Span { - fn clone(&self) -> Self { - Self { - span: self.span, - spanned: self.spanned.clone(), - } - } -} - impl Default for Span { fn default() -> Self { Self { diff --git a/crates/lib/core/Cargo.toml b/crates/lib/core/Cargo.toml index bd78fcb122..fe497e1765 100644 --- a/crates/lib/core/Cargo.toml +++ b/crates/lib/core/Cargo.toml @@ -35,7 +35,7 @@ std = [ "miden-utils-sync/std", ] testing = ["arbitrary"] -constraints-tools = ["std", "dep:miden-air", "dep:miden-ace-codegen", "dep:miden-constraint-compiler"] +constraints-tools = ["std", "dep:miden-air", "dep:miden-constraint-compiler"] [dependencies] # Miden dependencies @@ -46,7 +46,6 @@ miden-precompiles.workspace = true miden-processor.workspace = true miden-utils-sync.workspace = true miden-air = { workspace = true, optional = true } -miden-ace-codegen = { workspace = true, optional = true } miden-constraint-compiler = { workspace = true, optional = true } # External dependencies diff --git a/crates/lib/core/build.rs b/crates/lib/core/build.rs index 2baf58987c..8c9500f814 100644 --- a/crates/lib/core/build.rs +++ b/crates/lib/core/build.rs @@ -172,6 +172,14 @@ fn local_procedure_docs(module: &Module) -> Vec<(String, Option)> { procedures } +/// Resolve the documentation for a re-exported item. +/// +/// The outer `Option` distinguishes "target procedure not found" (skip the +/// re-export entirely) from "found", and the inner `Option` carries the docs +/// (`None` when the target has no doc comment). Encoding these three states +/// without nesting would require a bespoke enum, so we allow `option_option` +/// here deliberately. +#[allow(clippy::option_option)] fn reexport_target_docs( import: &ast::ItemImport, current_module_path: &miden_assembly_syntax::Path, diff --git a/crates/lib/core/tests/pcs/fri/channel.rs b/crates/lib/core/tests/pcs/fri/channel.rs deleted file mode 100644 index d680d69b1d..0000000000 --- a/crates/lib/core/tests/pcs/fri/channel.rs +++ /dev/null @@ -1,81 +0,0 @@ -use miden_core::Word; -use miden_utils_testing::{ - Felt, FieldElement, MerkleTreeVC, - crypto::{BatchMerkleProof, ElementHasher, Hasher as HasherTrait, PartialMerkleTree}, - serde::DeserializationError, -}; -use winter_fri::{FriProof, VerifierError}; - -pub trait UnBatch { - fn unbatch( - &mut self, - positions: &[usize], - domain_size: usize, - layer_commitments: Vec<::Digest>, - ) -> (Vec, Vec<(Word, Vec)>); -} - -pub struct MidenFriVerifierChannel< - E: FieldElement, - H: ElementHasher + ElementHasher, -> { - layer_commitments: Vec, - layer_proofs: Vec>, - layer_queries: Vec>, - remainder: Vec, -} - -impl MidenFriVerifierChannel -where - E: FieldElement, - H: ElementHasher + ElementHasher, -{ - /// Builds a new verifier channel from the specified [FriProof]. - /// - /// # Errors - /// Returns an error if the specified `proof` could not be parsed correctly. - pub fn new( - proof: FriProof, - layer_commitments: Vec, - domain_size: usize, - folding_factor: usize, - ) -> Result { - let remainder = proof.parse_remainder()?; - let (layer_queries, layer_proofs) = - proof.parse_layers::>(domain_size, folding_factor)?; - - Ok(MidenFriVerifierChannel { - layer_commitments, - layer_proofs, - layer_queries, - remainder, - }) - } - - pub fn take_fri_remainder(&mut self) -> Vec { - self.remainder.clone() - } - - pub fn layer_proofs(&mut self) -> Vec> { - self.layer_proofs.drain(..).collect() - } - - pub fn layer_queries(&mut self) -> Vec> { - self.layer_queries.clone() - } - - pub fn read_fri_layer_commitments(&mut self) -> Vec { - self.layer_commitments.drain(..).collect() - } - - pub fn read_remainder( - &mut self, - expected_commitment: &::Digest, - ) -> Result, VerifierError> { - let poly = self.take_fri_remainder(); - let commitment = H::hash_elements(&poly); - assert_eq!(&commitment, expected_commitment); - - Ok(poly) - } -} diff --git a/crates/lib/core/tests/pcs/fri/mod.rs b/crates/lib/core/tests/pcs/fri/mod.rs deleted file mode 100644 index 27d4b8fc04..0000000000 --- a/crates/lib/core/tests/pcs/fri/mod.rs +++ /dev/null @@ -1,281 +0,0 @@ -use alloc::collections::BTreeMap; - -use miden_utils_testing::{Felt, PrimeField64, crypto::MerkleStore}; - -mod channel; - -pub(crate) mod verifier_fri_e2f4; -use miden_core::Word; -pub use verifier_fri_e2f4::*; - -const FRI_PREPROCESS_SOURCE: &str = " - use miden::core::stark::constants - - const MAX_FRI_QUERIES = 150 - const MAX_FRI_LAYERS = 32 - const MAX_FRI_REMAINDER_WORDS = 64 - - proc preprocess - adv_push - # => [num_queries, g, ...] - dup u32gt.0 assert.err=\"number of FRI queries must be nonzero\" - dup u32lte.MAX_FRI_QUERIES assert.err=\"number of FRI queries exceeds FRI workspace\" - - exec.constants::fri_com_ptr - # => [layer_ptr, num_queries, g, ...] - dup.1 mul.4 sub - # => [query_ptr, num_queries, g, ...] - dup exec.constants::set_fri_queries_address - swap - sub.1 - padw - push.1 - while.true - adv_loadw - dup.5 - u32wrapping_add.4 - swap.6 - mem_storew_le - dup.4 - sub.1 - swap.5 - neq.0 - end - #=> [X, x, layer_ptr, g] - - drop - #=> [X, layer_ptr, g] - - dup.4 - movdn.5 - #=> [X, layer_ptr, layer_ptr, g] - - adv_push - dup u32lte.MAX_FRI_LAYERS assert.err=\"number of FRI layers exceeds FRI workspace\" - - dup push.0 neq - if.true - mul.2 - sub.1 - movdn.4 - #=> [X, num_layers, layer_ptr, layer_ptr, g] - - push.1 - while.true - adv_loadw - dup.5 - u32wrapping_add.4 - swap.6 - mem_storew_le - dup.4 - sub.1 - swap.5 - neq.0 - end - #=> [X, x, remainder_poly_ptr, layer_ptr, g] - - drop - else - drop - end - #=> [X, remainder_poly_ptr, layer_ptr, g] - - dup.4 - movdn.5 - #=> [X, remainder_poly_ptr, remainder_poly_ptr, layer_ptr, g] - - adv_push - dup u32gt.0 assert.err=\"FRI remainder polynomial must be nonzero\" - dup u32lte.MAX_FRI_REMAINDER_WORDS assert.err=\"FRI remainder polynomial exceeds FRI workspace\" - - dup mul.2 exec.constants::set_remainder_poly_size - - sub.1 - movdn.4 - #=> [X, len_remainder/2, remainder_poly_ptr, remainder_poly_ptr, layer_ptr, g] - - push.1 - while.true - adv_loadw - dup.5 - u32wrapping_add.4 - swap.6 - mem_storew_le - dup.4 - sub.1 - swap.5 - neq.0 - end - #=> [X, x, x, remainder_poly_ptr, layer_ptr, g] - dropw drop drop - #=> [remainder_poly_ptr, layer_ptr, g] - - exec.constants::set_remainder_poly_address - drop drop - end -"; - -#[test] -fn fri_verify_rejects_empty_query_region() { - let source = " - use miden::core::pcs::fri::frie2f4 - use miden::core::stark::constants - - begin - push.1 exec.constants::set_lde_domain_generator - push.64 exec.constants::set_remainder_poly_size - push.4294912800 exec.constants::set_remainder_poly_address - push.4294912800 exec.constants::set_fri_queries_address - exec.frie2f4::verify - end - "; - - let test = build_test!(source, &[]); - expect_assert_error_message!(test, contains "fri query region must be non-empty"); -} - -#[test] -fn fri_fold4_ext2_remainder64() { - let source = format!( - "{FRI_PREPROCESS_SOURCE} - use miden::core::pcs::fri::frie2f4 - - begin - exec.preprocess - exec.frie2f4::verify - end - " - ); - - let trace_len_e = 14; - let blowup_exp = 3; - let depth = trace_len_e + blowup_exp; - let domain_size = 1 << depth; - - let FriResult { - partial_trees, - advice_maps, - positions, - alphas, - commitments, - remainder, - num_queries, - } = fri_prove_verify_fold4_ext2(trace_len_e).unwrap(); - - let advice_stack = prepare_advice_stack( - depth, - domain_size, - num_queries, - positions, - alphas, - commitments, - remainder, - ); - - let advice_map: BTreeMap> = BTreeMap::from_iter(advice_maps); - let domain_generator = Felt::get_root_of_unity(domain_size.ilog2()).as_canonical_u64(); - - let mut store = MerkleStore::new(); - for partial_tree in &partial_trees { - store.extend(partial_tree.inner_nodes()); - } - let test = build_test!(&source, &[domain_generator], &advice_stack, store, advice_map.clone()); - - test.expect_stack(&[]); -} - -#[test] -fn fri_fold4_ext2_remainder128() { - let source = format!( - "{FRI_PREPROCESS_SOURCE} - use miden::core::pcs::fri::frie2f4 - - begin - exec.preprocess - exec.frie2f4::verify - end - " - ); - - let trace_len_e = 13; - let blowup_exp = 3; - let depth = trace_len_e + blowup_exp; - let domain_size = 1 << depth; - - let FriResult { - partial_trees, - advice_maps, - positions, - alphas, - commitments, - remainder, - num_queries, - } = fri_prove_verify_fold4_ext2(trace_len_e).unwrap(); - - let advice_stack = prepare_advice_stack( - depth, - domain_size, - num_queries, - positions, - alphas, - commitments, - remainder, - ); - - let advice_map: BTreeMap> = BTreeMap::from_iter(advice_maps); - let domain_generator = Felt::get_root_of_unity(domain_size.ilog2()).as_canonical_u64(); - - let mut store = MerkleStore::new(); - for partial_tree in &partial_trees { - store.extend(partial_tree.inner_nodes()); - } - let test = build_test!(&source, &[domain_generator], &advice_stack, store, advice_map.clone()); - - test.expect_stack(&[]); -} - -fn prepare_advice_stack( - depth: usize, - domain_size: u32, - num_queries: usize, - position_eval: Vec, - alphas: Vec, - com: Vec, - remainder: Vec, -) -> Vec { - let mut stack = vec![]; - let remainder_length = remainder.len() / 2; - let num_layers = (com.len() / 4) - 1; - - stack.push(num_queries as u64); - - stack.extend_from_slice(&position_eval[..]); - - stack.push(num_layers as u64); - - let mut current_domain_size = domain_size as u64; - let mut current_depth = depth as u64; - - for i in 0..num_layers { - current_domain_size /= 4; - - stack.extend_from_slice(&com[(4 * i)..(4 * i + 4)]); - stack.extend_from_slice(&alphas[(4 * i)..(4 * i + 2)]); - // - 2 is due to the fact that we are folding by 4 - stack.extend_from_slice(&[current_depth - 2, current_domain_size]); - current_depth -= 2; - } - - stack.push(remainder_length as u64 / 2); - for i in 0..remainder_length / 2 { - let mut remainder_4 = vec![0; 4]; - remainder_4[0] = remainder[4 * i]; - remainder_4[1] = remainder[4 * i + 1]; - remainder_4[2] = remainder[4 * i + 2]; - remainder_4[3] = remainder[4 * i + 3]; - - stack.extend_from_slice(&remainder_4); - } - - stack -} diff --git a/crates/lib/core/tests/pcs/fri/verifier_fri_e2f4.rs b/crates/lib/core/tests/pcs/fri/verifier_fri_e2f4.rs deleted file mode 100644 index acbf25ad73..0000000000 --- a/crates/lib/core/tests/pcs/fri/verifier_fri_e2f4.rs +++ /dev/null @@ -1,476 +0,0 @@ -use core::{marker::PhantomData, mem}; - -use miden_core::{EMPTY_WORD, Felt, FieldElement, QuadFelt, Word}; -use miden_processor::crypto::{Hasher, RandomCoin, WinterRandomCoin}; -use miden_utils_testing::{ - MerkleTreeVC, PrimeField64, - crypto::{MerklePath, NodeIndex, PartialMerkleTree, Poseidon2 as MidenHasher}, - group_slice_elements, - math::fft, -}; -use winter_fri::{ - DefaultProverChannel, FriOptions, FriProof, FriProver, VerifierError, folding::fold_positions, -}; - -use super::channel::{MidenFriVerifierChannel, UnBatch}; - -const MAX_REMAINDER_POLY_DEGREE: usize = 128; -const FRI_FOLDING_FACTOR: usize = 4; -const BLOWUP_FACTOR: usize = 8; -const NUM_FRI_QUERIES: usize = 32; - -type AdvMap = Vec<(Word, Vec)>; - -pub struct FriResult { - /// A vector containing the Merkle authentication paths used to authenticate the queries. - pub partial_trees: Vec, - - /// A map used to unhash Merkle nodes to a sequence of field elements representing the - /// query-values. - pub advice_maps: AdvMap, - - /// A vector of consecutive quadruples of the form (poe, p, e1, e0) where p is index of the - /// query at the first layer and (e1, e0) is its corresponding evaluation and poe is g^p with g - /// being the initial domain generator. - pub positions: Vec, - - /// A vector of tuples representing the folding challenges. - pub alphas: Vec, - - /// A vector of consecutive quadruples (c3, c2, c1, c0) representing the Merkle tree layer - /// commitments. - pub commitments: Vec, - - /// The remainder codeword as consecutive (r0, r1). - pub remainder: Vec, - - /// The number of queries contained in the current FRI proof. - pub num_queries: usize, -} - -// This function proves and then verifies a FRI proof with the following fixed parameters: -// 1) Max remainder codeword (1 << 6). -// 2) Blow up factor 8. -// 3) Folding factor 4. -// -// The main purpose of this function is to build the non-deterministic inputs needed to verify -// a FRI proof inside the Miden VM. -// The output is organized as follows: -pub fn fri_prove_verify_fold4_ext2(trace_length_e: usize) -> Result { - let trace_length = 1 << trace_length_e; - let lde_blowup = BLOWUP_FACTOR; - let max_remainder_size = MAX_REMAINDER_POLY_DEGREE; - let folding_factor = FRI_FOLDING_FACTOR; - let nonce = 0_u64; - - let options = FriOptions::new(lde_blowup, folding_factor, max_remainder_size); - let mut channel = build_prover_channel(trace_length, &options); - let evaluations = build_evaluations(trace_length, lde_blowup); - - // instantiate the prover and generate the proof - let mut prover = FriProver::<_, _, _, MerkleTreeVC>::new(options.clone()); - prover.build_layers(&mut channel, evaluations.clone()); - let positions = channel.draw_query_positions(nonce); - let proof = prover.build_proof(&positions); - - // make sure the proof can be verified - let commitments = channel.layer_commitments().to_vec(); - - let max_degree = trace_length - 1; - let result = verify_proof( - proof.clone(), - commitments.clone(), - &evaluations, - max_degree, - trace_length * lde_blowup, - &positions, - &options, - ); - let commitments: Vec = channel - .layer_commitments() - .to_vec() - .iter() - .flat_map(|digest| digest.as_elements().iter().map(|e| e.as_canonical_u64())) - .collect(); - - let remainder_poly: Vec = - proof.parse_remainder().expect("should return remainder polynomial"); - let remainder: Vec = QuadFelt::slice_as_base_elements(&remainder_poly[..]) - .to_owned() - .iter() - .map(|a| a.as_canonical_u64()) - .collect(); - - match result { - Ok(((partial_trees, advice_maps), all_position_evaluation, alphas)) => Ok(FriResult { - partial_trees, - advice_maps, - positions: all_position_evaluation, - alphas, - commitments, - remainder, - num_queries: positions.len(), - }), - Err(err) => Err(err), - } -} - -// HELPER UTILS -// ================================================================================================ - -pub fn build_prover_channel( - trace_length: usize, - options: &FriOptions, -) -> DefaultProverChannel> { - DefaultProverChannel::new(trace_length * options.blowup_factor(), NUM_FRI_QUERIES) -} - -pub fn build_evaluations(trace_length: usize, lde_blowup: usize) -> Vec { - let mut p = (0..trace_length as u32) - .map(|i| (i, i)) - .map(|(i, j)| QuadFelt::new([i.into(), j.into()])) - .collect::>(); - let domain_size = trace_length * lde_blowup; - p.resize(domain_size, QuadFelt::ZERO); - - let twiddles = fft::get_twiddles::(domain_size); - - fft::evaluate_poly(&mut p, &twiddles); - p -} - -#[expect(clippy::type_complexity)] -fn verify_proof( - proof: FriProof, - commitments: Vec<::Digest>, - evaluations: &[QuadFelt], - max_degree: usize, - domain_size: usize, - positions: &[usize], - options: &FriOptions, -) -> Result<((Vec, AdvMap), Vec, Vec), VerifierError> { - let mut channel = MidenFriVerifierChannel::::new( - proof, - commitments.clone(), - domain_size, - options.folding_factor(), - ) - .unwrap(); - let mut coin = WinterRandomCoin::new(&[]); - - let miden_verifier = - FriVerifierFold4Ext2::new(&mut channel, &mut coin, options.clone(), max_degree)?; - - let queried_evaluations = positions.iter().map(|&p| evaluations[p]).collect::>(); - - let result = - miden_verifier.verify_fold_4_ext_2(&mut channel, &queried_evaluations, positions)?; - - Ok(result) -} - -/// Partial implementation for verification in the case of folding factor 4 -pub struct FriVerifierFold4Ext2 { - domain_size: usize, - domain_generator: Felt, - layer_commitments: Vec, - layer_alphas: Vec, - options: FriOptions, - _channel: PhantomData>, -} - -impl FriVerifierFold4Ext2 { - pub fn new( - channel: &mut MidenFriVerifierChannel, - public_coin: &mut WinterRandomCoin, - options: FriOptions, - max_poly_degree: usize, - ) -> Result { - assert_eq!(options.blowup_factor(), BLOWUP_FACTOR); - assert_eq!(options.folding_factor(), FRI_FOLDING_FACTOR); - - // infer evaluation domain info - let domain_size = max_poly_degree.next_power_of_two() * options.blowup_factor(); - let domain_generator = Felt::get_root_of_unity(domain_size.ilog2()); - - // read layer commitments from the channel and use them to build a list of alphas - let layer_commitments = channel.read_fri_layer_commitments(); - let mut layer_alphas = Vec::with_capacity(layer_commitments.len()); - let mut max_degree_plus_1 = max_poly_degree + 1; - for (depth, commitment) in layer_commitments.iter().enumerate() { - public_coin.reseed(*commitment); - let alpha = public_coin.draw().map_err(VerifierError::RandomCoinError)?; - layer_alphas.push(alpha); - - // make sure the degree can be reduced by the folding factor at all layers - // but the remainder layer - if depth != layer_commitments.len() - 1 - && !max_degree_plus_1.is_multiple_of(options.folding_factor()) - { - return Err(VerifierError::DegreeTruncation( - max_degree_plus_1 - 1, - options.folding_factor(), - depth, - )); - } - max_degree_plus_1 /= options.folding_factor(); - } - - Ok(FriVerifierFold4Ext2 { - domain_size, - domain_generator, - layer_commitments, - layer_alphas, - options, - _channel: PhantomData, - }) - } - - // PUBLIC ACCESSORS - // -------------------------------------------------------------------------------------------- - - /// Returns size of the domain over which a polynomial commitment checked by this verifier - /// has been evaluated. - /// - /// The domain size can be computed by rounding `max_poly_degree` to the next power of two - /// and multiplying the result by the `blowup_factor` from the protocol options. - pub fn domain_size(&self) -> usize { - self.domain_size - } - - /// Verifier in the setting of (folding_factor, blowup_factor, extension_degree) = (4, (1 << 3), - /// 2) - #[expect(clippy::type_complexity)] - fn verify_fold_4_ext_2( - &self, - channel: &mut MidenFriVerifierChannel, - evaluations: &[QuadFelt], - positions: &[usize], - ) -> Result<((Vec, AdvMap), Vec, Vec), VerifierError> { - // 1 ----- verify the recursive components of the FRI proof ------------------------------- - let positions = positions.to_vec(); - let evaluations = evaluations.to_vec(); - let mut final_pos_eval: Vec<(usize, QuadFelt)> = vec![]; - let advice_provider = - channel.unbatch::<4, 3>(&positions, self.domain_size(), self.layer_commitments.clone()); - - let mut d_generator = self.domain_generator; - let mut all_alphas = vec![]; - let mut all_position_evaluation = vec![]; - for (index, &position) in positions.iter().enumerate() { - d_generator = self.domain_generator; - let (cur_pos, evaluation, position_evaluation, alphas) = iterate_query_fold_4_quad_ext( - &self.layer_alphas, - &advice_provider.0, - &advice_provider.1, - position, - self.options.num_fri_layers(self.domain_size()), - self.domain_size(), - &evaluations[index], - &mut d_generator, - )?; - all_position_evaluation.extend_from_slice(&position_evaluation[..]); - all_alphas = alphas; - - final_pos_eval.push((cur_pos, evaluation)); - } - - // 2 ----- verify the remainder of the FRI proof ------------------------------------------ - - // read the remainder from the channel and make sure it matches with the columns - // of the previous layer - let remainder_commitment = self.layer_commitments.last().unwrap(); - let remainder_poly = channel.read_remainder(remainder_commitment)?; - let offset = Felt::GENERATOR; - for &(final_pos, final_eval) in final_pos_eval.iter() { - let comp_eval = eval_horner_rev( - &remainder_poly, - offset * d_generator.exp_vartime(final_pos as u64), - ); - if comp_eval != final_eval { - return Err(VerifierError::InvalidRemainderFolding); - } - } - - Ok((advice_provider, all_position_evaluation, all_alphas)) - } -} - -fn iterate_query_fold_4_quad_ext( - layer_alphas: &[QuadFelt], - partial_trees: &[PartialMerkleTree], - key_val_map: &[(Word, Vec)], - position: usize, - number_of_layers: usize, - initial_domain_size: usize, - evaluation: &QuadFelt, - domain_generator: &mut Felt, -) -> Result<(usize, QuadFelt, Vec, Vec), VerifierError> { - let mut cur_pos = position; - let mut evaluation = *evaluation; - let mut domain_size = initial_domain_size; - let get_domain_offset = Felt::GENERATOR; - - let initial_domain_generator = *domain_generator; - let norm_cst = Felt::get_root_of_unity(2).inv(); - let mut init_exp = initial_domain_generator.exp(position as u64); - - let arr = vec![evaluation]; - let a = QuadFelt::slice_as_base_elements(&arr); - - let position_evaluation = vec![ - a[0].as_canonical_u64(), - a[1].as_canonical_u64(), - position as u64, - init_exp.as_canonical_u64(), - ]; - - let mut alphas = vec![]; - for depth in 0..number_of_layers { - let target_domain_size = domain_size / FRI_FOLDING_FACTOR; - - let folded_pos = cur_pos % target_domain_size; - - // Assumes the num_partitions == 1 - let position_index = folded_pos; - - let tree_depth = target_domain_size.ilog2(); - - let query_nodes = partial_trees[depth] - .get_node(NodeIndex::new(tree_depth as u8, position_index as u64).unwrap()) - .unwrap(); - let query_values = &key_val_map - .iter() - .find(|(k, _)| *k == query_nodes) - .expect("must contain the leaf values") - .1; - - let query_values = [ - QuadFelt::new([query_values[0], query_values[1]]), - QuadFelt::new([query_values[2], query_values[3]]), - QuadFelt::new([query_values[4], query_values[5]]), - QuadFelt::new([query_values[6], query_values[7]]), - ]; - - let query_value = query_values[cur_pos / target_domain_size]; - - if evaluation != query_value { - return Err(VerifierError::InvalidLayerFolding(depth)); - } - - let xs_new = match cur_pos / target_domain_size { - 0 => init_exp, - 1 => init_exp * norm_cst, - 2 => init_exp * (norm_cst * norm_cst), - _ => init_exp * (norm_cst * norm_cst * norm_cst), - } * get_domain_offset; - - init_exp = init_exp * init_exp * init_exp * init_exp; - - evaluation = { - let f_minus_x = query_values[2]; - let f_x = query_values[0]; - let x_star = QuadFelt::from(xs_new); - let alpha = layer_alphas[depth]; - - let tmp0 = fri_2(f_x, f_minus_x, x_star, alpha); - - let f_minus_x = query_values[3]; - let f_x = query_values[1]; - let alpha = layer_alphas[depth]; - - let tmp1 = fri_2(f_x, f_minus_x, x_star * QuadFelt::from(norm_cst.inv()), alpha); - - fri_2(tmp0, tmp1, x_star * x_star, alpha * alpha) - }; - - let arr = vec![layer_alphas[depth]]; - let a = QuadFelt::slice_as_base_elements(&arr); - alphas.push(a[0].as_canonical_u64()); - alphas.push(a[1].as_canonical_u64()); - alphas.push(0); - alphas.push(0); - - *domain_generator = (*domain_generator).exp((FRI_FOLDING_FACTOR as u32).into()); - cur_pos = folded_pos; - domain_size /= FRI_FOLDING_FACTOR; - } - - Ok((cur_pos, evaluation, position_evaluation, alphas)) -} - -impl UnBatch for MidenFriVerifierChannel { - fn unbatch( - &mut self, - positions_: &[usize], - domain_size: usize, - layer_commitments: Vec, - ) -> (Vec, Vec<(Word, Vec)>) { - let queries = self.layer_queries().clone(); - let mut current_domain_size = domain_size; - let mut positions = positions_.to_vec(); - let depth = layer_commitments.len() - 1; - - let mut adv_key_map = vec![]; - let mut partial_trees = vec![]; - let mut layer_proofs = self.layer_proofs(); - for query in queries.iter().take(depth) { - let mut folded_positions = fold_positions(&positions, current_domain_size, N); - - let layer_proof = layer_proofs.remove(0); - - let x = group_slice_elements::(query); - let leaves: Vec = x.iter().map(|row| MidenHasher::hash_elements(row)).collect(); - let unbatched_proof = layer_proof.into_openings(&leaves, &folded_positions).unwrap(); - assert_eq!(x.len(), unbatched_proof.len()); - - let nodes: Vec<[Felt; 4]> = - leaves.iter().map(|leaf| [leaf[0], leaf[1], leaf[2], leaf[3]]).collect(); - - let paths: Vec = - unbatched_proof.into_iter().map(|list| list.1.into()).collect(); - - let iter_pos = folded_positions.iter_mut().map(|a| *a as u64); - let nodes_tmp = nodes.clone(); - let iter_nodes = nodes_tmp.iter(); - let iter_paths = paths.into_iter(); - let mut tmp_vec = vec![]; - for (p, (node, path)) in iter_pos.zip(iter_nodes.zip(iter_paths)) { - tmp_vec.push((p, Word::from(*node), path)); - } - - let new_set = - PartialMerkleTree::with_paths(tmp_vec).expect("should not fail from paths"); - partial_trees.push(new_set); - - nodes.into_iter().zip(x.iter()).for_each(|(a, b)| { - let mut value = QuadFelt::slice_as_base_elements(b).to_owned(); - value.extend(EMPTY_WORD); - adv_key_map.push((a.to_owned().into(), value)); - }); - - mem::swap(&mut positions, &mut folded_positions); - current_domain_size /= N; - } - - (partial_trees, adv_key_map) - } -} - -// HELPER FUNCTIONS -// ================================================================================================ - -fn fri_2(f_x: E, f_minus_x: E, x_star: E, alpha: E) -> E -where - B: PrimeField64, - E: FieldElement, -{ - (f_x + f_minus_x + ((f_x - f_minus_x) * alpha / x_star)) / E::ONE.double() -} - -pub fn eval_horner_rev(p: &[E], x: E::BaseField) -> E -where - E: FieldElement, -{ - p.iter().fold(E::ZERO, |acc, &coeff| acc * E::from(x) + coeff) -} diff --git a/crates/lib/core/tests/pcs/mod.rs b/crates/lib/core/tests/pcs/mod.rs deleted file mode 100644 index 4bb3590380..0000000000 --- a/crates/lib/core/tests/pcs/mod.rs +++ /dev/null @@ -1 +0,0 @@ -mod fri; diff --git a/crates/mast-package/Cargo.toml b/crates/mast-package/Cargo.toml index 943cbc80e7..15a66b578e 100644 --- a/crates/mast-package/Cargo.toml +++ b/crates/mast-package/Cargo.toml @@ -48,5 +48,3 @@ proptest.workspace = true proptest-derive.workspace = true serde_json.workspace = true -[package.metadata.cargo-shear] -ignored = ["serde_json"] diff --git a/crates/precompiles/benches/precompiles_bench/support.rs b/crates/precompiles/benches/precompiles_bench/support.rs index 9b6223633f..514665d5eb 100644 --- a/crates/precompiles/benches/precompiles_bench/support.rs +++ b/crates/precompiles/benches/precompiles_bench/support.rs @@ -38,7 +38,7 @@ impl PrecompileFixture { } fn generate_stack_inputs(workload: PrecompileWorkload) -> StackInputs { - assert!(workload.keccaks <= u32::MAX as usize, "Keccak workload count must fit in u32"); + assert!(u32::try_from(workload.keccaks).is_ok(), "Keccak workload count must fit in u32"); StackInputs::new(&[Felt::new_unchecked(workload.keccaks as u64)]) .expect("single Keccak count should fit on the operand stack") } diff --git a/processor/src/trace/chiplets/ace/trace.rs b/processor/src/trace/chiplets/ace/trace.rs index db8bb444bd..6abdbbf4bd 100644 --- a/processor/src/trace/chiplets/ace/trace.rs +++ b/processor/src/trace/chiplets/ace/trace.rs @@ -216,7 +216,8 @@ impl CircuitEvaluation { eval.m_0 = m_0; } - debug_assert!(multiplicities_iter.next().is_none()); + let next = multiplicities_iter.next(); + debug_assert!(next.is_none()); } /// Returns the output value, if the circuit has finished evaluating. diff --git a/processor/src/trace/chiplets/hasher/mod.rs b/processor/src/trace/chiplets/hasher/mod.rs index e4901aaf8e..15d453e1d3 100644 --- a/processor/src/trace/chiplets/hasher/mod.rs +++ b/processor/src/trace/chiplets/hasher/mod.rs @@ -259,7 +259,8 @@ impl Hasher { // Last batch: boundary output only. let last_batch = batch_groups.next().expect("multi-batch block has a final op batch"); - debug_assert!(batch_groups.next().is_none()); + let next = batch_groups.next(); + debug_assert!(next.is_none()); absorb_into_state(&mut state, last_batch); let permuted = self.append_controller_permutation( LINEAR_HASH, diff --git a/processor/src/trace/parallel/tests.rs b/processor/src/trace/parallel/tests.rs index 2f8a806004..065394ac03 100644 --- a/processor/src/trace/parallel/tests.rs +++ b/processor/src/trace/parallel/tests.rs @@ -1320,7 +1320,7 @@ fn collect_end_flags(trace: &ExecutionTrace) -> Vec { fn read_opcode(main_trace: &MainTrace, row_idx: RowIndex) -> u8 { let opcode = main_trace.get_op_code(row_idx).as_canonical_u64(); - assert!(opcode <= u8::MAX as u64, "invalid opcode"); + assert!(u8::try_from(opcode).is_ok(), "invalid opcode"); opcode as u8 } diff --git a/processor/src/trace/trace_state.rs b/processor/src/trace/trace_state.rs index c0b726f707..07a0bb8e55 100644 --- a/processor/src/trace/trace_state.rs +++ b/processor/src/trace/trace_state.rs @@ -387,6 +387,11 @@ pub struct NodeFlags { impl NodeFlags { /// Creates a new instance of `NodeFlags`. + /// + /// The four booleans are independent flags packed into the hasher state's + /// second word for an END operation; there is no natural smaller grouping, + /// so we accept the bool parameters explicitly. + #[allow(clippy::fn_params_excessive_bools)] pub fn new(is_loop_body: bool, is_loop: bool, is_call: bool, is_syscall: bool) -> Self { Self { is_loop_body, diff --git a/tools/miden-core-fuzz/Cargo.lock b/tools/miden-core-fuzz/Cargo.lock index ec4e418bce..0379830857 100644 --- a/tools/miden-core-fuzz/Cargo.lock +++ b/tools/miden-core-fuzz/Cargo.lock @@ -756,7 +756,6 @@ dependencies = [ "miden-debug-types", "miden-rowan", "miden-utils-diagnostics", - "thiserror", ] [[package]]