diff --git a/CHANGELOG.md b/CHANGELOG.md index 3a32ec07d9..6b471ad001 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ ### Changes +- Renamed `make update-note-costs` to `make update-generated`, which regenerates the note cost tables and `bench-tx.json` together and is run only when the cost drift guard fails ([#3727](https://github.com/0xMiden/protocol/pull/3727)). - Keyed `bench-tx.json`'s per-note cycle counts by note label instead of note commitment, and added `total_cycles` to each scenario ([#3726](https://github.com/0xMiden/protocol/pull/3726)). - [BREAKING] Refactored `AccountVaultDelta` to track generic assets. `FungibleAssetDelta`, `NonFungibleAssetDelta` and `NonFungibleDeltaAction` were removed ([3485](https://github.com/0xMiden/protocol/pull/3485)). - [BREAKING] Moved the internal shared helpers of `miden::protocol::input_note`, `miden::protocol::active_note`, and the note memory-write helpers into private `input_note_internal` and `note_internal` modules ([#3501](https://github.com/0xMiden/protocol/pull/3501)). diff --git a/Makefile b/Makefile index e07ccbbf76..66ee899730 100644 --- a/Makefile +++ b/Makefile @@ -175,9 +175,9 @@ bench-tx: ## Run transaction benchmarks bench-note-checker: ## Run note checker benchmarks cargo bench --bin bench-note-checker --bench benches -.PHONY: update-note-costs -update-note-costs: ## Regenerate bench-tx.json and the checked-in note consumption cost tables - cargo run --bin bench-transaction --features concurrent -- update-note-costs +.PHONY: update-generated +update-generated: ## Regenerate bench-tx.json and the checked-in note consumption cost tables + cargo run --release --bin bench-transaction --features concurrent -- update-generated cargo +nightly fmt -p miden-standards -p miden-agglayer # --- installing ---------------------------------------------------------------------------------- diff --git a/bin/bench-transaction/README.md b/bin/bench-transaction/README.md index 6401573e08..17b098a940 100644 --- a/bin/bench-transaction/README.md +++ b/bin/bench-transaction/README.md @@ -34,13 +34,33 @@ The network-account auth procedure collects sponsored fees and answers sponsorsh The network-account scenarios feed two checked-in, generated cost tables: `crates/miden-standards/src/note/costs/table.rs` and `crates/miden-agglayer/src/costs/table.rs`. Each table entry is the note's consumption cost in VM cycles - the total cycle count of the canonical network-account transaction consuming it, taken as the maximum across the note's benchmarked execution paths. The values are estimates, not guaranteed worst cases - see the caveats in `miden_standards::note::costs` (e.g. asset counts are benchmarked at the planned, not current, protocol maximum). -Regenerate the tables (and `bench-tx.json`) with: +### Regenerating the Generated Artifacts + +There are two of them - the cost tables above and `bench-tx.json` - and they are regenerated +together, only when the drift guard asks for it: ```bash -make update-note-costs +make update-generated ``` -Freshness is enforced in CI: the `checked_in_note_costs_match_executed_scenarios` snapshot test in `src/note_costs.rs` re-executes every priced scenario during the regular test run and fails when a measured cost drifts more than 5% from its checked-in constant, or when a note's declared `created_notes` do not match what its scenarios actually create. It walks `PricedNote::all` - so all priced notes are covered. Drift within the tolerance (from unrelated changes landing on the base branch) is absorbed without regeneration - fee-wise this is safe, since the fee is logarithmic in cycles and the pricing safety margin dwarfs it. A PR that meaningfully changes cycle counts must run `make update-note-costs` and commit the updated tables - which doubles as review signal, since cost regressions show up as table diffs. +The guard is the `checked_in_note_costs_match_executed_scenarios` test in `src/note_costs.rs`. It +re-executes every priced scenario during the regular test run and fails when a measured cost drifts +more than 5% from its checked-in constant, or when a note's declared `created_notes` do not match +what its scenarios actually create. It walks `PricedNote::all` - so all priced notes are covered. + +Until that test fails, a PR regenerates nothing, including a PR that changes cycle counts. +Sub-threshold drift is deliberately absorbed. Fee-wise that is safe: the fee is logarithmic in +cycles and the pricing safety margin dwarfs the tolerated drift. It is also the cheaper trade for +review - regenerating on every cost-affecting change buries a couple of hundred lines of real diff +under a thousand lines of generated churn, which is what the tables and the snapshot produce +between them. + +When the guard does fail, run `make update-generated` and commit both artifacts in the same change. +Keeping them in lockstep is why there is one target rather than two. + +`bench-tx.json` is refreshed only as part of that regeneration, so between refreshes it is not +expected to match the current tree, and nothing enforces that it does. It is an observational +snapshot. ### Benchmark Groups diff --git a/bin/bench-transaction/src/main.rs b/bin/bench-transaction/src/main.rs index abc3eae897..8b7b6ef51f 100644 --- a/bin/bench-transaction/src/main.rs +++ b/bin/bench-transaction/src/main.rs @@ -30,10 +30,10 @@ async fn run_scenario( #[tokio::main(flavor = "current_thread")] async fn main() -> Result<()> { - let update_costs = match std::env::args().nth(1).as_deref() { + let update_cost_tables = match std::env::args().nth(1).as_deref() { None => false, - Some("update-note-costs") => true, - Some(other) => anyhow::bail!("unknown argument `{other}`; expected `update-note-costs`"), + Some("update-generated") => true, + Some(other) => anyhow::bail!("unknown argument `{other}`; expected `update-generated`"), }; // create a template file for benchmark results @@ -52,7 +52,7 @@ async fn main() -> Result<()> { // store benchmark results in the JSON file write_bench_results_to_json(path, benchmark_results)?; - if update_costs { + if update_cost_tables { bench_transaction::note_costs::update_cost_tables(&measured_cycles)?; } diff --git a/bin/bench-transaction/src/note_costs.rs b/bin/bench-transaction/src/note_costs.rs index 95c8ccdfcb..57eb396c72 100644 --- a/bin/bench-transaction/src/note_costs.rs +++ b/bin/bench-transaction/src/note_costs.rs @@ -17,7 +17,7 @@ use miden_standards::note::costs::NoteCost; use crate::cycle_counting_benchmarks::ExecutionBenchmark; /// Header line of the generated table files. -const GENERATED_HEADER: &str = "// This file is @generated by `make update-note-costs`. Do not edit by hand.\n\ +const GENERATED_HEADER: &str = "// This file is @generated by `make update-generated`. Do not edit by hand.\n\ // Values are maxima across the benchmarked paths; see `miden_standards::note::costs` for the\n\ // caveats on what they do and do not cover.\n"; @@ -478,8 +478,8 @@ mod tests { if !stale_costs.is_empty() { failures.push(format!( "cost table stale for {} note(s), each more than {DRIFT_TOLERANCE_PERCENT}% from \ - its checked-in constant: {}. Run `make update-note-costs` and commit the updated \ - tables", + its checked-in constant: {}. Run `make update-generated` and commit both \ + regenerated artifacts - the cost tables and bench-tx.json - together", stale_costs.len(), stale_costs.join("; "), )); diff --git a/crates/miden-agglayer/src/costs/mod.rs b/crates/miden-agglayer/src/costs/mod.rs index 0794076a51..61813ae2e8 100644 --- a/crates/miden-agglayer/src/costs/mod.rs +++ b/crates/miden-agglayer/src/costs/mod.rs @@ -7,7 +7,7 @@ //! The `NetworkNotePricer` in `miden-tx` turns cycle costs into fees, resolving the agglayer //! notes through [`AgglayerNote::note_cost`](crate::AgglayerNote::note_cost). //! -//! The table is regenerated with `make update-note-costs`; a snapshot test in +//! The table is regenerated with `make update-generated`; a snapshot test in //! `bench-transaction` fails CI when a checked-in value drifts more than 5% from the measured //! one (small drift from unrelated changes is tolerated - the pricing safety margin dwarfs //! it). diff --git a/crates/miden-agglayer/src/costs/table.rs b/crates/miden-agglayer/src/costs/table.rs index 535a1760aa..673093af87 100644 --- a/crates/miden-agglayer/src/costs/table.rs +++ b/crates/miden-agglayer/src/costs/table.rs @@ -1,4 +1,4 @@ -// This file is @generated by `make update-note-costs`. Do not edit by hand. +// This file is @generated by `make update-generated`. Do not edit by hand. // Values are maxima across the benchmarked paths; see `miden_standards::note::costs` for the // caveats on what they do and do not cover. diff --git a/crates/miden-standards/src/note/costs/mod.rs b/crates/miden-standards/src/note/costs/mod.rs index 0573300119..f846247ad9 100644 --- a/crates/miden-standards/src/note/costs/mod.rs +++ b/crates/miden-standards/src/note/costs/mod.rs @@ -20,7 +20,7 @@ //! Terminology: a note's *cost* is its measured cycle count; its *price* is the fee derived //! from that cost (and from the costs of the notes its consumption creates). //! -//! The table is regenerated with `make update-note-costs`; a snapshot test in +//! The table is regenerated with `make update-generated`; a snapshot test in //! `bench-transaction` fails CI when a checked-in value drifts more than 5% from the measured //! one (small drift from unrelated changes is tolerated - the pricing safety margin dwarfs //! it). diff --git a/crates/miden-standards/src/note/costs/table.rs b/crates/miden-standards/src/note/costs/table.rs index 8b925d0264..b804976bdb 100644 --- a/crates/miden-standards/src/note/costs/table.rs +++ b/crates/miden-standards/src/note/costs/table.rs @@ -1,4 +1,4 @@ -// This file is @generated by `make update-note-costs`. Do not edit by hand. +// This file is @generated by `make update-generated`. Do not edit by hand. // Values are maxima across the benchmarked paths; see `miden_standards::note::costs` for the // caveats on what they do and do not cover.