Update openvm to 1.4.0 #3291

qwang98 · 2025-09-17T02:49:43Z

This PR updated the openvm dependency to 1.4.0. A lot of changes were introduced, hence the size of this PR.

Not covered in this PR:

gpu: this PR brings us to parity with existing functionality, so only CPU is supported. See [Ovm GPU] WIP feature branch #3347 for GPU support.
plonk: since the main proving route for openvm is using a single row, plonk support was deprioritized for now and will be eventually fixed in another PR.

Changes to openvm here
Changes to stark-backend here

Tasks:

make compile
pil export test
implement all extensions (Circuit, Executor, (Prover))
make tracegen work
re-enable Send + Sync bound on Executor in ovm (instead, removed the need for that bound in ovm)
support tco flag

… without executor

…f, but can't get column names

[Ovm 1.4] get original airs

openvm/src/extraction_utils.rs

[OVM-1.4] patch bus map

openvm/src/extraction_utils.rs

openvm/extensions/hints-circuit/src/lib.rs

openvm/src/powdr_extension/chip.rs

openvm/src/powdr_extension/executor/mod.rs

[OVM 1.4] `VmConfig` related trait implementations

pacheco

Nice! Still doing the review, these are some initial comments

pacheco · 2025-10-13T19:38:12Z

.github/workflows/nightly-tests.yml

        run: |
          rustup toolchain install 1.88
-          cargo +1.88 install --git 'http://github.com/powdr-labs/openvm.git' --rev fbd69da cargo-openvm
+          cargo +1.88 install --git 'http://github.com/openvm-org/openvm.git' cargo-openvm


no need for the fork here anymore?

No, I don't think it was really needed before either. cargo-openvm is used to compile the guest, and our fork does not touch that part of openvm.

pacheco · 2025-10-13T19:39:21Z

.github/workflows/nightly-tests.yml

        with:
          repository: powdr-labs/openvm-reth-benchmark
-          ref: main
+          ref: 15a1cb1533036f1b5284ddf79b3cadd644778398


i guess this needs to get in first, then reth updated, then this can go back to main?

Yeah it's gonna be a few commits to merge all of this..

pacheco · 2025-10-13T19:41:23Z

Cargo.toml

+openvm-stark-backend = { git = "https://github.com/powdr-labs/stark-backend.git", rev = "e27de8b", default-features = false, features = [
  "parallel",
  "jemalloc",
-  "bench-metrics",


bench-metrics not needed anymore for metrics.json?

pacheco · 2025-10-13T19:42:42Z

Cargo.toml

  "jemalloc",
  "nightly-features",
-  "bench-metrics",
+  "evm-prove",


what's this feature for?

It enables verifying in evm, so the prover and verifier for the last recursion step which outputs a bn128 snark iirc.

pacheco · 2025-10-13T20:13:20Z

openvm/src/extraction_utils.rs

+        .iter()
+        .fold(HashMap::new(), |mut acc, instruction| {
+            let (air_name, _) = air_by_opcode_id.get_instruction_air_and_id(instruction);
+            // TODO: main_columns might not be correct, as the RA::with_capacity() uses the following `main_width()`


for now or later PR?

It's all working properly now, so I'll say for a later PR where we EITHER investigate this and find it to be proper and just remove this comment OR find a "proper" solution.

Still keeping this comment here to flag a potential issue though.

pacheco · 2025-10-13T20:24:06Z

openvm/src/extraction_utils.rs

+                .or_insert(RecordArenaDimension {
+                    height: 0,
+                    width: air_by_opcode_id
+                        .get_instruction_air_stats(instruction)


minor: i think it doesn't matter much, but the calls of get_instruction_air_and_id and get_instruction_air_stats are both searching for the same entry in the OriginalAirs maps... not sure how feasible it is to avoid this due to the trait though

Good point. We don't need to go through the trait as we have access to the concrete type. I refactored to accessing the maps inside OriginalAirs directly.

pacheco · 2025-10-14T01:11:12Z

openvm/src/extraction_utils.rs

-            .filter(|op| {
-                // Filter out the opcode that we are not interested in
-                instruction_allowlist.contains(op)
+        let res = instruction_allowlist


no need for the let res i think since we're immediately returning it

pacheco · 2025-10-14T01:44:24Z

openvm/src/lib.rs


    // Generate an AppProvingKey
-    let app_pk = Arc::new(sdk.app_keygen(app_config)?);
+    sdk.app_keygen();


is it right to ignore the returned values here?

this was called for caching, but it's not required, removed!

pacheco · 2025-10-14T02:04:27Z

openvm/src/lib.rs

+    // Create the SDK
+    let sdk: GenericSdk<_, SpecializedConfigCpuBuilder, _> = GenericSdk::new(app_config).unwrap();
+
+    let mut app_prover = sdk.app_prover(exe.clone())?;


can be moved inside the else?

pacheco · 2025-10-14T02:07:38Z

openvm/src/lib.rs


    // All gate constraints should be satisfied, but bus interactions are not implemented yet.
    #[test]
+    #[ignore = "Pending bug fix"]


for a future PR? does this need a TODO?

Yeah. This is PLONK and the bug is described in #3333. We decided to make it run first before fixing PLONK, which is not our default path.

refined the bug in this PR. mock works, but verification fails.

actually mock does not work with debug_assert on. Refined and still ignored in this PR.

Yeah...--release flag can be kind of confusing...

…pc_call

pacheco · 2025-10-14T18:11:17Z

openvm/src/powdr_extension/chip.rs

+        original_airs: OriginalAirs<BabyBear>,
+        base_config: OriginalVmConfig,
        periphery: PowdrPeripheryInstances,
+        record_arena_by_air_name: Rc<RefCell<OriginalArenas>>,


this parameter is already inside the precompile being passed?

very good catch...

pacheco · 2025-10-14T18:31:35Z

openvm/src/powdr_extension/executor/mod.rs

+impl OriginalArenas {
+    /// Given an estimate of how many times the APC is called in this segment, and the original airs and apc,
+    /// initializes the arenas iff not already initialized.
+    fn ensure_initialized(


is this gonna be called multiple times? could it just be initialize and fail if already init?

It is called each time the apc instruction is executed, but actually does the initialization only at the beginning of each segment.
The state machine is

Uninitialized execute_preflight_seg_0 execute_apc_call (this is where init happens) Initialized execute_apc_call (here it just adds to the arena) ... (more calls to execute_apc_call) trace_gen_seg_0 (read from the arenas and create the final table, then reset to Uninitialized) Uninitialized execute_preflight_seg_1 (now we need to initialize again for seg_1) ...

We do need to check if it's initialized at each execute_apc_call because we have no way to know we just started a new segment based on PreflightExecutor::execute. So this check happens in ensure_initialized.

pacheco · 2025-10-14T18:34:10Z

openvm/src/powdr_extension/executor/mod.rs

-            apc,
+    pub fn arenas(&self) -> &HashMap<String, MatrixRecordArena<BabyBear>> {
+        match self {
+            OriginalArenas::Uninitialized => unreachable!(),


i'd use panic since this is not unreachable by construction, its more like an "unwrap"

pacheco · 2025-10-14T18:35:02Z

openvm/src/powdr_extension/executor/mod.rs

+/// - read from during trace generation
+/// - reset to uninitialized after trace generation
+#[derive(Default)]
+pub enum OriginalArenas {


minor: could be a struct wrapping an Option?

Yes I agree it is basically an option, but I thought it was clearer to name the possible states explicitly rather than relying on Some/None

pacheco · 2025-10-14T18:37:49Z

openvm/src/powdr_extension/executor/mod.rs

-    apc: Arc<Apc<F, Instr<F>>>,
-}
+    /// Returns a mutable reference to the arenas.
+    /// Should only be called after `initialize` is called.


no method named initialize

good catch!

pacheco · 2025-10-14T19:19:00Z

openvm/src/powdr_extension/executor/mod.rs

-    if let Some(ref ecc) = this.sdk_vm_config.ecc {
-        complex = complex.extend(ecc)?;
+
+    fn get_opcode_name(&self, opcode: usize) -> String {


unused i think

It's required for the trait and is used when the metrics flag is on iirc

pacheco · 2025-10-14T19:20:40Z

openvm/src/powdr_extension/plonk/chip.rs

+        original_airs: OriginalAirs<BabyBear>,
+        base_config: OriginalVmConfig,
        periphery: PowdrPeripheryInstances,
+        record_arena_by_air_name: Rc<RefCell<OriginalArenas>>,


already part of precompile i think

pacheco · 2025-10-14T21:36:13Z

openvm/src/powdr_extension/trace_generator/periphery.rs

+        assert!(inventory
+            .find_air::<VariableRangeCheckerAir>()
+            .nth(1)
+            .is_none());


this doesn't ensure the air is present, only that there is at most one instance?

pacheco · 2025-10-14T21:36:29Z

openvm/src/powdr_extension/trace_generator/periphery.rs

+        assert!(inventory
+            .find_chip::<SharedVariableRangeCheckerChip>()
+            .nth(1)
+            .is_none());


openvm/src/powdr_extension/trace_generator/mod.rs

Schaeff and others added 4 commits September 16, 2025 12:54

fixed hints-circuit

e1e783a

wip

2cfe4e3

first commit but chip inventory doesn't seem to work

21ad1e7

add exeutor complex, but seems that we can get airs from chip complex…

26e31c4

… without executor

qwang98 marked this pull request as draft September 17, 2025 05:51

qwang98 and others added 6 commits September 17, 2025 17:04

managed to get airs via executor inventory -> air inventory -> air re…

04508d5

…f, but can't get column names

fix sdk calls

33efc8e

Merge pull request #3292 from powdr-labs/ovm-1.4-get-original-airs

a36b3a2

[Ovm 1.4] get original airs

Merge branch 'ovm-1.4' of github.com:powdr-labs/powdr into ovm-1.4

dcb0293

add flag

0c3dd22

patch bus map

9596cc7

qwang98 commented Sep 17, 2025

View reviewed changes

openvm/src/extraction_utils.rs Outdated Show resolved Hide resolved

Schaeff and others added 6 commits September 17, 2025 12:43

Merge pull request #3294 from powdr-labs/ovm-1.4-extraction-utils

c0da8f8

[OVM-1.4] patch bus map

make compile

14229da

pull main, make compile

3269e55

export_pil wip

d1e2a22

pull main

aab0dfd

make pil export test pass

85f35d0

qwang98 commented Sep 18, 2025

View reviewed changes

openvm/src/extraction_utils.rs Outdated Show resolved Hide resolved

qwang98 commented Sep 18, 2025

View reviewed changes

openvm/extensions/hints-circuit/src/lib.rs Outdated Show resolved Hide resolved

qwang98 added 6 commits September 18, 2025 13:42

implement VmExecutionExtension and VmExecutionConfig for some types

3724813

add some vmcircuit implementations

e276b89

solve AnyRap trait bounds

3f37dc2

add asref<systemconfig> and asmut<systemconfig> impls

7c3d7aa

remove executor_inventory() function in place of create_executors()

ad65a11

give PowdrExtension access to ChipComplex via OriginalVmConfig

78752fd

qwang98 commented Sep 18, 2025

View reviewed changes

openvm/src/powdr_extension/chip.rs Outdated Show resolved Hide resolved

qwang98 commented Sep 18, 2025

View reviewed changes

openvm/src/powdr_extension/executor/mod.rs Show resolved Hide resolved

qwang98 commented Sep 18, 2025

View reviewed changes

openvm/src/powdr_extension/executor/mod.rs Show resolved Hide resolved

Merge pull request #3298 from powdr-labs/ovm-1.4-extend-executor

d2571c8

[OVM 1.4] `VmConfig` related trait implementations

Schaeff added 2 commits October 10, 2025 16:07

update k256

63191ff

update dep

9de513b

Schaeff force-pushed the ovm-1.4 branch from c544a33 to 9de513b Compare October 11, 2025 12:20

Schaeff added 9 commits October 11, 2025 14:21

run only one test

e854c9f

run cargo test

5885b92

enable logs and debug, run only one test

70169b3

run more tests, use pr-test profile

26fe978

revert, single job matrix

ffa9419

remove all-features

2ee1142

remove tco

21bd511

point to reth without tco

ca3ff63

revert ci

055b6fb

Schaeff changed the title ~~[DO NOT MERGE] discussion board for OVM-1.4~~ Update openvm to 1.4.0 Oct 13, 2025

Schaeff marked this pull request as ready for review October 13, 2025 10:42

Schaeff added 3 commits October 13, 2025 16:35

enable tco

e5d9993

update reth

3a1dacd

update k256

2139f4e

pacheco reviewed Oct 14, 2025

View reviewed changes

Schaeff added 6 commits October 14, 2025 10:22

address review comment about record_arena_dimension_by_air_name_per_a…

6b3e266

…pc_call

typo

5b1f359

address review comments

f69b675

remove ignore as test passes

7d2d9a0

refine plonk error, add test, fix height change

9975ee6

Merge branch 'main' of github.com:powdr-labs/powdr into ovm-1.4

e4730ef

Schaeff mentioned this pull request Oct 14, 2025

[Ovm GPU] WIP feature branch #3347

Draft

7 tasks

ignore tests, add reason

a1a84d9

pacheco reviewed Oct 14, 2025

View reviewed changes

Schaeff added 3 commits October 15, 2025 10:11

pull main

d3da73c

fix review comments

d0e6c3c

pull main

99651ba

Update openvm to 1.4.0 #3291

Are you sure you want to change the base?

Update openvm to 1.4.0 #3291

Conversation

qwang98 commented Sep 17, 2025 • edited by Schaeff Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pacheco left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

qwang98 Oct 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Schaeff Oct 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

qwang98 Oct 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Schaeff Oct 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

qwang98 commented Sep 17, 2025 •

edited by Schaeff

Loading

qwang98 Oct 14, 2025 •

edited

Loading

Schaeff Oct 14, 2025 •

edited

Loading

qwang98 Oct 14, 2025 •

edited

Loading

Schaeff Oct 15, 2025 •

edited

Loading