Switch witness construction to VM event logs #41
Merged
+931
−109
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
branche name: feat/multi_arch_ci
## 背景
- 本ワークスペースの SIMD 最適化（AVX2/AVX‑512/NEON）コードパスの劣化検知が必要
。
- GitHub ホストランナーは AVX‑512 を保証せず、ARM64 も環境により未提供のため、実行戦略を分離。
- AVX2: x86_64 ランナーでビルド＋テストを実行。
- AVX‑512: x86_64 ランナーでは未対応の可能性が高く、ビルドのみで担保。
- NEON: ARM64 ランナー（例: `ubuntu-24.04-arm64` または `self-hosted, Linux, ARM64`）でビルド＋テストを実行。
## スコープ
- `.github/workflows/rust.yml` の Build & Test を CPU 機能マトリクス化。
- x86_64/AVX2 は実行テストまで、x86_64/AVX‑512 はビルドのみ、ARM64/NEON は実行テストまで。
- 既存の Clippy/Fmt ジョブは維持。
- すべて nightly、`Swatinem/rust-cache@v2`、`RUST_BACKTRACE=1` を適用。
## 受け入れ条件
- Push/PR で以下が動作:
- x64/AVX2: リリースビルド＋テストが成功。
- x64/AVX‑512: リリースビルドが成功（テストはスキップ）。
- ARM64/NEON: リリースビルド＋テストが成功（ARM64 ランナーが利用可能な場合）。
- Clippy/Fmt は従来通り、警告・未整形で CI が失敗。
- 2 回目以降の実行でビルドキャッシュが効く。
## 注意点
- AVX‑512 はホストランナーで未対応の可能性が高く、テスト実行は SIGILL 回避のためスキップ。
- ARM64 ランナーが未提供の場合は NEON 行を一時的にコメントアウト、または `self-hosted` ラベル運用に切替。
- 実行テストは `is_x86_feature_detected!` や `cfg(target_feature)` で条件化し、誤実行を防止。
## 提案する rust.yml（差し替え案）
```yaml
name: Rust CI
on:
push:
branches: [ "main" ]
pull_request:
branches: [ "main" ]
workflow_dispatch:
permissions:
contents: read
concurrency:
group: rust-ci-${{ github.ref }}
cancel-in-progress: true
env:
CARGO_TERM_COLOR: always
RUST_BACKTRACE: 1
jobs:
build-test-matrix:
name: Build & Test (${{ matrix.name }})
runs-on: ${{ matrix.os }}
strategy:
fail-fast: false
matrix:
include:
- name: x64-avx2
os: ubuntu-latest
rustflags: -C target-cpu=haswell
run_tests: true
- name: x64-avx512-build
os: ubuntu-latest
rustflags: -C target-cpu=skylake-avx512
run_tests: false # build only to avoid SIGILL
- name: arm64-neon
# 利用可能な場合は GitHub ホスト ARM64 ランナーに置換
# 例: ubuntu-24.04-arm64（組織設定依存）
# 未提供時は self-hosted を使用: [self-hosted, Linux, ARM64]
os: ubuntu-24.04-arm64
rustflags: -C target-feature=+neon
run_tests: true
steps:
- uses: actions/checkout@v4
- name: Install nightly toolchain
uses: actions-rust-lang/setup-rust-toolchain@v1
with:
toolchain: nightly
- name: Cache cargo build
uses: Swatinem/rust-cache@v2
- name: Build (release)
run: cargo build --workspace --release --verbose
env:
RUSTFLAGS: ${{ matrix.rustflags }}
- name: Test (release)
if: ${{ matrix.run_tests == true }}
run: cargo test --workspace --release --verbose
env:
RUSTFLAGS: ${{ matrix.rustflags }}
clippy:
runs-on: ubuntu-latest
name: Clippy
steps:
- uses: actions/checkout@v4
- name: Install nightly toolchain
uses: actions-rust-lang/setup-rust-toolchain@v1
with:
toolchain: nightly
components: clippy
- name: Cache cargo build
uses: Swatinem/rust-cache@v2
- name: Clippy (deny warnings)
run: cargo clippy --workspace --all-targets --all-features -- -D warning
s
fmt:
runs-on: ubuntu-latest
name: Rustfmt
steps:
- uses: actions/checkout@v4
- name: Install nightly toolchain
uses: actions-rust-lang/setup-rust-toolchain@v1
with:
toolchain: nightly
components: rustfmt
- name: Cache cargo build
uses: Swatinem/rust-cache@v2
- name: Check formatting
run: cargo fmt --all -- --check
```
What I changed - VM events: Added `VmPoseidon16Event`, `VmPoseidon24Event`, `VmDotProductEvent`, `VmMultilinearEvalEvent` in `crates/vm/src/lib.rs`. Events store cycle, addresses, sizes, and values (inputs/outputs). - ExecutionResult: Extended with event vectors to export collected events to consumers: - `vm_poseidon16_events`, `vm_poseidon24_events`, `vm_dot_product_events`, `vm_multilinear_eval_events` - Runner integration: In `crates/vm/src/runner.rs` - Recorded events during final execution only (`final_execution == true`) right after each precompile executes. - Captured input and output values for reproducibility. - Added debug assertions to ensure call counts match event counts in final execution. - Trace construction: In `crates/zk_vm/zk_vm_trace/src/execution_trace.rs` - Removed the memory rescanning logic for precompile witness reconstruction. - Built `WitnessPoseidon*`, `WitnessDotProduct`, and `WitnessMultilinearEval` directly from the VM events. - Kept Poseidon padding and dummy row logic unchanged. Notes - First pass does not record events; final pass does. Debug asserts guard count consistency. - Poseidon tables retain 2^k padding and dummy rows exactly as before. - No fallback to rescanning remains; witness creation is event-only. Validation - Ran `cargo test`: all tests passed. - Existing trace columns generation remains intact and still uses instruction encodings and memory reads; only precompile witness collection switched to events. Next steps - Do you want me to add unit tests for event correctness (cycle, addresses, lengths, values) and padding invariants? - If you’d like, I can run a release test and a quick timing comparison to check trace build time improvements.
…f the Rust workflow
Contributor
Author
|
I'll add unit tests. But first, fix the Clippy issue on another PR. |
Contributor
Author
|
- `crates/lean_vm/src/runner.rs`: Added two comprehensive tests (`vm_precompile_events_capture_expected_data`, `vm_precompile_events_only_final_pass`) covering event contents, pointer/address assertions, value integrity for all precompile types, and ensuring only the final execution populates event vectors. - `crates/lean_prover/witness_generation/src/execution_trace.rs`: Added end-to-end test (`execution_trace_uses_vm_events`) asserting witness construction directly from VM events, including poseidon padding invariants, dot-product/multilinear data, and evaluation outputs; auxiliary guard skips the sentinel `ending_pc` to avoid panics when iterating PCs. **Build/Test Adjustments** - Ensured helper data uses disjoint memory regions and power-of-two frame growth so tests run deterministically. - `get_execution_trace` now skips the final `pc == ending_pc` entry to prevent out-of-bounds access during witness generation. **Tests Run** - `cargo test -p lean_vm -q` - `cargo test -p witness_generation -q` (`cargo test --workspace` still aborts on existing `test_zkvm` stack overflow, so I validated the new coverage with targeted crate runs.)
Collaborator
|
Ok this is super cool |
…ation details within a new RowMultilinearEval struct. Update related tests to reflect this change and ensure correctness of assertions.
…mproved clarity and consistency.
…ectly at ending program counter
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Previously,
get_execution_tracerebuilt witness data by pattern-matching each instruction and rereading VM memory, while ExecutionResult only exposed the memory snapshot and control-flow history. This PR updates the runnerso it records each Poseidon16/24, dot-product, and multilinear-eval invocation as self-contained
Vm*Eventsand stores them onExecutionResult; the witness generator now consumes those events directly, eliminating theinstruction match fallback and avoiding redundant computation.
Changes
crates/lean_prover/witness_generation/src/execution_trace.rscrates/lean_vm/src/lib.rscrates/lean_vm/src/runner.rsRelated todo:
leanMultisig/TODO.md
Line 11 in 645797e