chore: move rvr files into openvm-circuit to avoid circular dependency

[gdmlcjs]

Moves the rvr files related to compiling and execution into openvm-circuit. Those rvr files previously depended on openvm-circuit and in order to enable rvr execution through the openvm pipeline, they had to be made a part of openvm-circuit to prevent circular dependencies.

closes INT-7537

[github-actions]

Claude finished @gdmlcjs's task in 1m 52s —— View job

---

Code Review: Move RVR files into openvm-circuit

• Gather context and read changed files
• Review Cargo.toml dependency changes
• Review moved/renamed source files
• Review new modules and test structure
• Post review findings

---

Overall this is a clean structural migration that correctly breaks the circular dependency between rvr-openvm and openvm-circuit. The compile/execute/metered/state/debug/io modules move into crates/vm/src/arch/rvr/ behind #[cfg(feature = "rvr")], and previously hardcoded constants are now parameterized through CProject. The approach is sound and well-organized.

Findings

1. #[allow(dead_code)] on the entire rvr module — crates/vm/src/arch/rvr/mod.rs:2

#![allow(dead_code)]

This suppresses all dead-code warnings for the new module tree. Since this code is behind the rvr feature flag and much of it is pub-exported, it would be better to either remove this and address any actual dead-code warnings, or scope the #[allow] more narrowly to specific items. Blanket allow(dead_code) risks masking genuinely unused code during refactoring.

2. Duplicate build_metered_config / build_metered_cost_config implementations — Both crates/vm/src/arch/rvr/metered.rs and crates/vm/tests/rvr_openvm/utils.rs contain build_metered_config / build_metered_cost_config functions with overlapping logic. The library version (metered.rs:152-216) takes an ExecutorInventory and resolves opcodes to AIR indices, while the test version (utils.rs:137-239) takes an RvrExtensionCtx and does the same opcode-to-AIR resolution independently. This duplication is a maintenance risk — if segmentation logic changes, both need updating. Consider having the test helper delegate to the library function, or extract the shared opcode→chip mapping into a single utility.

3. Constants header function extracted without removing the old one — crates/rvr/rvr-openvm/src/constants.rs introduces a new constants_header() function, and crates/rvr/rvr-openvm/src/emit/project.rs:247 now calls crate::constants::constants_header(...) instead of the old crate::metered::constants_header(...). Please verify the old metered::constants_header is fully removed/dead. If there are still references from the base branch, they could cause compilation issues.

4. CProject::new parameter explosion — crates/rvr/rvr-openvm/src/emit/project.rs:82-97 now takes 8 parameters:

pub fn new(output_dir, name, tracer_mode, memory_bits, page_bits, chunk_bits, segment_check_insns, public_values_as)

Already behind #[allow(clippy::too_many_arguments)]. Consider bundling the new tracer-constant parameters (page_bits, chunk_bits, segment_check_insns, public_values_as) into a small config struct. This is low priority though — the callers are all internal.

5. Feature flag dependency surface — crates/vm/Cargo.toml:142-153: The rvr feature now pulls in 8 dependencies (plus libloading, tempfile, bincode, sha2). This is a significant expansion of openvm-circuit's dependency graph when rvr is enabled. The tradeoff is acceptable given the circular-dependency constraint, but worth noting for future maintainers — if the rvr execution path is ever optional at a higher level, consider whether some of these can be split out again.

6. Test dev-dependency bloat — crates/vm/Cargo.toml:71-110: The [dev-dependencies] section now includes the full extension test matrix (rvr-openvm-ext-algebra, rvr-openvm-ext-bigint, etc.) plus many openvm-*-circuit/openvm-*-transpiler crates, plus eyre, halo2curves-axiom, num-bigint, etc. This is the consequence of migrating the integration tests. It means any developer running cargo test -p openvm-circuit will need all of these even without the rvr feature. Consider gating the heavier test dependencies behind a feature (e.g., rvr-tests) or documenting this.

7. unsafe FFI code moved as-is — The FFI code in execute.rs, io.rs, metered.rs, and state.rs involves substantial unsafe (raw pointer dereferences, extern "C" callbacks). This is unchanged from the original location and appears correct, but since it's now part of openvm-circuit (a core crate), it deserves extra scrutiny in future audits. The metered_periodic_check callback (metered.rs) and register_and_execute (execute.rs) are particularly sensitive.

8. pub mod rvr visibility — crates/vm/src/arch/mod.rs:14:

pub mod rvr;

This makes the entire rvr module publicly accessible from openvm_circuit::arch::rvr. Is this intentional? The test files already import via this path (use openvm_circuit::arch::rvr as rvr_openvm). If external crates shouldn't depend on these internals, consider pub(crate) instead.

Minor / Nit

• crates/vm/tests/rvr_openvm.rs uses #[path = "rvr_openvm/..."] directives for all test modules — this is a standard pattern for integration test organization, looks fine.
• The crates/vm/tests/programs/ruint/Cargo.toml rename from rvr-openvm to vm for the workspace member path is correct.
• debug_info.rs test adds use super::utils::*; — consistent with the other migrated test files.

Summary

This PR accomplishes its stated goal of resolving the circular dependency. The code moves are mechanical and correct. The main areas for improvement are reducing the duplicated config-building logic between library and test code, and considering the dev-dependency impact on the core VM crate. None of these are blocking.

[github-actions]

Claude finished @gdmlcjs's task in 2m 37s —— View job

---

Code Review: Move RVR files into openvm-circuit

• Gather context and read PR diff
• Review structural changes (moved/renamed files, module organization)
• Review Cargo.toml dependency changes
• Review source code changes in detail
• Review test migration and second commit
• Post review findings

---

This PR correctly breaks the circular dependency between rvr-openvm and openvm-circuit by moving the compile/execute/metered/state/debug/io modules into crates/vm/src/arch/rvr/ behind #[cfg(feature = "rvr")]. The second commit (1f5db7c) is a nice follow-up that removes duplicated build_metered_config / build_metered_cost_config implementations from the test utils, having the tests call the library functions directly via rvr_openvm::build_metered_config(...). The structural split between rvr-openvm (codegen/emit/toolchain — no openvm-circuit dependency) and openvm-circuit::arch::rvr (compile/execute/metered — depends on openvm-circuit internals) is clean and well-motivated.

Findings

1. Unused _system_config parameter in build_metered_cost_config — crates/vm/src/arch/rvr/metered_cost.rs:40

_system_config: &SystemConfig,

This was already unused in the base branch, but since this PR is refactoring the config-building functions, it's a good time to either remove the parameter or add a #[allow(unused)] with a note about future use. Currently callers must construct and pass a SystemConfig reference for no reason.

2. constants_header extraction is clean — The old metered::constants_header(text_start, memory_bits) (2 params) is replaced by constants::constants_header(text_start, memory_bits, chunk_bits, page_bits, segment_check_insns) (5 params), parameterizing values that were previously hardcoded from openvm-circuit constants. The generated C header now correctly receives TRACER_CHUNK_BITS, TRACER_PAGE_BITS, TRACER_SEGMENT_CHECK_INSNS, and per-AS buffer capacities. The old reference in project.rs:225 on the base (crate::metered::constants_header) is properly updated to crate::constants::constants_header with the new parameters. No dead code left.

3. openvm_compile_constants centralizes the values well — crates/vm/src/arch/rvr/compile.rs:425-432:

fn openvm_compile_constants() -> (u32, u32, u32, u32) {
    (
        DEFAULT_SEGMENT_CHECK_INSNS as u32,
        DEFAULT_PAGE_BITS as u32,
        CHUNK.ilog2(),
        PUBLIC_VALUES_AS,
    )
}

This tuple is used both for cache key computation and for constructing CProject. The unnamed tuple fields make the call sites harder to read (e.g., compile.rs:469 destructures as (segment_check_insns, page_bits, chunk_bits, public_values_as)). A named struct would be clearer, but this is minor.

4. Dev-dependency weight on openvm-circuit — crates/vm/Cargo.toml:71-110 adds ~30 dev-dependencies (full extension test matrix + SDK + halo2curves + etc.) unconditionally. These are always pulled in for cargo test -p openvm-circuit even without --features rvr, because the test binary rvr_openvm.rs is compiled regardless (it just has #![cfg(feature = "rvr")] at the top, which gates test execution but not compilation of dependent crates). This increases build times for all openvm-circuit developers. Consider either:

• Gating the test file with [[test]] required-features = ["rvr"] in Cargo.toml so the binary isn't compiled without the feature, or
• Moving the heavy dev-deps behind a rvr-tests feature

5. pub mod rvr visibility — crates/vm/src/arch/mod.rs:16: The module is pub, which means external crates can depend on openvm_circuit::arch::rvr::*. The test files already use this path (use openvm_circuit::arch::rvr as rvr_openvm). If the intent is only for internal consumption and tests, pub(crate) would prevent external crates from coupling to these internals. If external crates (like openvm-sdk) need access, pub is correct — just confirming this is intentional.

6. CProject::new parameter growth — crates/rvr/rvr-openvm/src/emit/project.rs:82-97 now takes 8 positional parameters (already behind #[allow(clippy::too_many_arguments)]). The new parameters (page_bits, chunk_bits, segment_check_insns, public_values_as) are tracer constants that always travel together. A small config struct would improve readability, but this is low priority given all callers are internal.

7. EmitContext::public_values_as threading — crates/rvr/rvr-openvm/src/emit/context.rs:15 adds a public_values_as: u32 field to EmitContext, replacing the hardcoded PUBLIC_VALUES_AS import from openvm-circuit in codegen.rs:126. This correctly decouples the code generator from the openvm-circuit constant. The codegen.rs change (ctx.public_values_as instead of PUBLIC_VALUES_AS) is clean.

8. rvr-openvm crate is now much thinner — The crate went from exporting compile/execute/metered/state/debug/io to just emit/toolchain/constants. The lib.rs re-exports are clean. The new backend_cache_stamp() function makes sense here since it covers the codegen/runtime files that live in this crate.

9. Test migration is mechanical and correct — All test files (algebra, bigint, ecc, keccak, etc.) have identical 2-line changes: use rvr_openvm as rvr_openvm → use openvm_circuit::arch::rvr as rvr_openvm and use super::utils → use super::utils. The rvr_openvm.rs test root uses #[path = "rvr_openvm/..."] directives — standard pattern.

10. Second commit addresses prior review feedback — The 1f5db7c commit removes the ~110-line duplicate build_metered_config / build_metered_cost_config from tests/rvr_openvm/utils.rs and updates callers to use the library versions. The deferral test (deferral.rs) is also updated to call the library functions directly. This eliminates the maintenance risk flagged in the earlier automated review.

Minor / Nit

• crates/vm/tests/programs/ruint/Cargo.toml path change from rvr-openvm to vm is correct.
• The #![cfg(feature = "rvr")] gate on rvr_openvm.rs correctly skips all tests when the feature isn't enabled.
• The rvr feature in Cargo.toml now includes dep:libloading and dep:tempfile, which were previously under the aot feature. This is correct since rvr compilation also needs these.

Summary

Clean structural migration that correctly resolves the circular dependency. The code moves are mechanical, the constant parameterization through CProject is well-done, and the second commit properly deduplicates the test config builders. The main actionable suggestion is gating the test binary compilation on the rvr feature (finding 4) to avoid pulling ~30 dev-deps for non-rvr developers.

[github-actions]

Claude finished @gdmlcjs's task in 2m 0s —— View job

---

Code Review: Move RVR files into openvm-circuit

• Gather context and read PR diff
• Review Cargo.toml and dependency changes
• Review moved/renamed source files and new module structure
• Review parameterization of constants and CProject changes
• Review test migration
• Post review findings

---

This is a well-executed structural migration. The split is clean: rvr-openvm retains codegen/emit/toolchain (no openvm-circuit dependency), while openvm-circuit::arch::rvr gets compile/execute/metered/state/debug/io (which need openvm-circuit internals). The second commit (1f5db7c) properly deduplicates the build_metered_config/build_metered_cost_config test helpers. The constant parameterization through CProject is done correctly.

Actionable findings

1. Dev-dependencies unconditionally compiled for the test binary — crates/vm/Cargo.toml:71-110, crates/vm/tests/rvr_openvm.rs:1

The test root file uses #![cfg(feature = "rvr")] which prevents running the tests, but cargo still compiles the test binary (and resolves all its use imports), meaning all ~30 dev-dependencies are always pulled in when building tests for openvm-circuit. Adding a [[test]] section with required-features would prevent compilation entirely:

[[test]]
name = "rvr_openvm"
required-features = ["rvr"]

This would let non-rvr developers avoid the heavy dependency pull. Fix this →

2. _system_config unused parameter in build_metered_cost_config — crates/vm/src/arch/rvr/metered_cost.rs:40

_system_config: &SystemConfig,

This parameter is unused (prefixed with _) and forces callers to construct a SystemConfig for no reason. Since this PR is refactoring these functions anyway, it's a good opportunity to remove it — or at minimum add a comment if it's reserved for future use.

3. _text_start unused parameter in constants_header — crates/rvr/rvr-openvm/src/constants.rs:30

pub fn constants_header(
    _text_start: u32,
    memory_bits: u8,
    ...

The _text_start parameter is accepted but never used in the generated header. Previously TEXT_START was emitted in the old version. If it's no longer needed, remove it to avoid confusion. If it's planned for future use, a brief comment would help.

4. pub mod rvr visibility — crates/vm/src/arch/mod.rs:16

The module is pub, meaning external crates can depend on openvm_circuit::arch::rvr::*. The tests already use this path. If the intent is only internal+test usage, pub(crate) would prevent external coupling. If external crates (like openvm-sdk) need access, pub is correct — just confirming intentionality.

Minor observations

• constants_header migration is clean: Old crate::metered::constants_header is fully removed; no dangling references remain.
• Second commit nicely deduplicates: The test utils no longer duplicate the library's build_metered_config/build_metered_cost_config — they call through rvr_openvm::build_metered_config(...) directly.
• backend_cache_stamp() in rvr-openvm/src/lib.rs: Good addition — hashing the codegen/runtime source files that live in the now-thinner rvr-openvm crate. Combined with the existing native_cache_stamp() in compile.rs (which covers the openvm-circuit-side sources), this gives correct cache invalidation across the split.
• Test migration is mechanical and correct: All test files have the same 2-line import change pattern. The #[path = "rvr_openvm/..."] integration test organization is standard.
• CProject::new parameter growth (8 params, behind #[allow(clippy::too_many_arguments)]): The new params (page_bits, chunk_bits, segment_check_insns, public_values_as) always travel together. A small config struct would improve readability but this is low priority with internal-only callers.

Summary

Clean migration that correctly resolves the circular dependency. The main actionable item is gating the test binary with required-features (finding 1) to avoid pulling ~30 dev-deps for non-rvr builds. The unused parameters (findings 2 & 3) are minor cleanup opportunities. None of these are blocking.

[shuklaayush]

i would avoid this whole file and add tests to respective extension crates instead. maybe vm crate can have a small test_utils or similar module that is shared

[gdmlcjs]

I put the test utils inside the rvr directory as a separate crate.

[github-actions]

Claude finished @gdmlcjs's task in 2m 56s —— View job

---

Code Review: Move RVR files into openvm-circuit (post-revision)

• Gather context and read the latest diff against base branch
• Review how prior review feedback (shuklaayush) was addressed
• Review structural changes and module organization
• Review Cargo.toml dependency changes
• Review source code changes in detail
• Review test migration and distribution
• Post review findings

---

This revision (6 commits on top of develop-v2.1.0-rvr) substantially addresses the earlier review feedback from @shuklaayush. The PR now has a much cleaner architecture than the first iteration. Here's what changed and what's left:

Prior feedback resolution

Feedback	Status
"revert variables for compile-time constants" (EmitContext::public_values_as, CProject params)	Addressed. Constants are now redefined in rvr-openvm-ext-ffi-common and imported directly. CProject::new dropped from 8 params back to 3. constants_header() takes zero params.
"delete backend_cache_stamp/caching code"	Addressed. All caching/hashing code removed (compile_cached, native_cache_key, native_cache_stamp, backend_cache_stamp, sha2 dep).
"remove unused parameters" (_system_config, _text_start)	Addressed. build_metered_cost_config no longer takes _system_config. constants_header() no longer takes _text_start or memory_bits.
"distribute tests to extension crates" (avoid bloating crates/vm dev-deps)	Addressed. Tests moved to rvr-openvm-tests, extensions/algebra/rvr/tests/, extensions/ecc/rvr/tests/, etc. crates/vm dev-deps are now minimal (just test-log, test-case, openvm-stark-*, openvm-circuit).
"avoid function wrapper for constants" (openvm_compile_constants)	Addressed. Constants are read directly from rvr_openvm_ext_ffi_common or from openvm-circuit internal modules. No wrapper function.

Architecture (post-revision)

The split is now:

• rvr-openvm (thin): codegen/emit/toolchain. No openvm-circuit dep.
• rvr-openvm-ext-ffi-common: shared constants (redefined with compile-time assertions in abi_consts.rs). Leaf crate, no cycles.
• openvm-circuit::arch::rvr (behind #[cfg(feature = "rvr")]): compile/execute/metered/state/debug/io. Has full access to openvm-circuit internals.
• rvr-openvm-test-utils: shared test harness used by rvr-openvm-tests and extension rvr test crates.
• Extension rvr/tests/: each extension (algebra, ecc, keccak, pairing, sha2) now owns its rvr integration tests.

Findings

1. abi_consts.rs drift guards are well-designed — crates/vm/src/arch/rvr/abi_consts.rs

The compile-time const _: () = assert!(...) pattern to catch drift between the redefined constants in ffi-common and the canonical constants in openvm-circuit is a good approach. One subtle issue: the DEFERRAL_DIGEST_SIZE assertion is gated on #[cfg(any(feature = "test-utils", feature = "cuda"))] (line 33), which means in a plain rvr-only build, this assertion doesn't fire. If DIGEST_SIZE were to change, the drift wouldn't be caught until someone runs with test-utils or cuda. Consider whether rvr should also enable the openvm-stark-sdk dep needed for this check, or if there's a way to define this constant in a leaf crate.

2. pub mod rvr visibility — crates/vm/src/arch/mod.rs:16

The module is pub, so external crates can depend on openvm_circuit::arch::rvr::*. The test-utils and extension test crates already depend on this path. If the intent is that external crates (outside the workspace) should not couple to these internals, pub(crate) would be safer — the test-utils crate could use openvm-circuit's re-exports instead. If external access is intentional, this is fine as-is; just confirming.

3. Phantom dispatch refactor is a correctness improvement — crates/rvr/rvr-openvm-lift/src/opcode.rs

The old code had hand-rolled phantom_disc constants (NOP: u32 = 0x00, RV32_HINT_INPUT: u32 = 0x20, etc.) that could silently drift from the authoritative SysPhantom / Rv32Phantom enums. The new code uses SysPhantom::from_repr() / Rv32Phantom::from_repr() which makes the dispatch exhaustive and tracks upstream changes automatically. This also eliminates the possibility of a new SysPhantom variant being added without being handled here (Rust would warn on non-exhaustive match). Nice improvement.

However, the dispatch in lift_instruction (line 48-54) has a subtlety: it first checks SysPhantom::from_repr(discriminant).is_some() || Rv32Phantom::from_repr(discriminant).is_some() to decide whether to call lift_phantom, but then also calls extensions.try_lift(insn, pc) for non-matching discriminants. If a future extension phantom discriminant happens to collide with a SysPhantom or Rv32Phantom value, it would be incorrectly handled by lift_phantom instead of the extension. This is unlikely given the discriminant ranges (0x00-0x03 for Sys, 0x20-0x22 for Rv32), but worth noting as a latent assumption.

4. build_blocks now hardcodes DEFAULT_SEGMENT_CHECK_INSNS as max_block_insns — crates/rvr/rvr-openvm-lift/src/cfg.rs:1067

Previously max_block_insns was a parameter; now it reads directly from ffi_common::DEFAULT_SEGMENT_CHECK_INSNS. This aligns with the "constants not variables" philosophy, but it means the block granularity is permanently tied to the segmentation check interval. If someone later wants to tune segmentation without changing block sizes (or vice versa), they'd need to decouple these again. The TODO in abi_consts.rs:7-8 about whether these should be consts or runtime-tunable is relevant here too.

5. DEFAULT_PC_STEP import replacing local INSTR_SIZE constant — crates/rvr/rvr-openvm-lift/src/cfg.rs:9

Good cleanup: replacing the local const INSTR_SIZE: u32 = 4 with openvm_instructions::program::DEFAULT_PC_STEP as INSTR_SIZE. This ties the block-building logic to the actual instruction step size from the canonical source.

6. NUM_REGS import replacing local constant — crates/rvr/rvr-openvm-lift/src/cfg.rs:10

Similarly, const NUM_REGS: usize = 32 is now imported from openvm_instructions::riscv::RV32_NUM_REGISTERS. Good.

7. Test harness design (rvr-openvm-test-utils) — crates/rvr/rvr-openvm-test-utils/src/lib.rs

The VmTestHarness<VB> generic over VmBuilder is well-structured and provides compare for all three modes (Pure, MeteredCost, Metered). The assert_segments function properly handles the inherent mismatch between rvr's block-boundary segmentation and OpenVM's per-instruction segmentation. The #![allow(dead_code)] on the test-utils crate (line 3) is reasonable since not every test file uses every helper.

8. Constants buffer capacity constants — crates/rvr/rvr-openvm/src/constants.rs:18-27

MEM_PAGE_BUF_CAP (65536), PV_PAGE_BUF_CAP (4096), DEFERRAL_PAGE_BUF_CAP (65536) are documented with their capacity justification. The comment on MEM_PAGE_BUF_CAP explaining the worst case (2000 insns × 10 pages = 20,000 < 65,536) is helpful. The DEFERRAL_PAGE_BUF_CAP has a TODO to audit — would be good to resolve before these buffers are used in production without bounds checks in C.

9. Minor: DEFAULT_INTERACTION_CONSTANT_OVERHEAD visibility change — crates/vm/src/arch/execution_mode/metered/segment_ctx.rs:47

Changed from const to pub const to be accessible from rvr/metered.rs. This is a minimal visibility expansion and appropriate.

Summary

This revision cleanly addresses all the prior review concerns. The constant duplication is now properly guarded by compile-time assertions. Tests are distributed to their respective extension crates, keeping crates/vm lean. The caching code is removed. The phantom dispatch refactor using from_repr is a correctness improvement. The overall architecture is clean and well-motivated.

No blocking issues. The main items for future consideration are: (1) whether pub mod rvr should be pub(crate), (2) resolving the DEFERRAL_PAGE_BUF_CAP capacity audit TODO, and (3) the DEFERRAL_DIGEST_SIZE drift guard not firing in rvr-only builds.

[github-actions]

group	app.proof_time_ms	app.cycles	leaf.proof_time_ms
fibonacci	3,823	12,000,265	953
keccak	18,666	18,655,329	3,324
sha2_bench	8,981	14,793,960	1,396
regex	1,429	4,137,067	375
ecrecover	647	123,583	275
pairing	906	1,745,757	283
kitchen_sink	2,118	2,579,903	434

Note: cells_used metrics omitted because CUDA tracegen does not expose unpadded trace heights.

Commit: f114146

Benchmark Workflow

[github-actions]

Claude finished @gdmlcjs's task in 1m 57s —— View job

---

Code Review: Move RVR files into openvm-circuit

• Gather context and read the full diff against base branch
• Review Cargo.toml and dependency changes
• Review moved/renamed source files and new module structure
• Review new code (abi_consts, compile, constants, mod.rs)
• Review test migration and distribution
• Review prior reviewer feedback resolution
• Post review findings

---

This PR cleanly resolves the circular dependency between rvr-openvm and openvm-circuit by splitting the crate along a natural boundary: codegen/emit/toolchain stays in rvr-openvm (no openvm-circuit dep), while compile/execute/metered/state/debug/io moves to openvm-circuit::arch::rvr (behind #[cfg(feature = "rvr")]). All 6 commits address prior reviewer feedback thoroughly — caching code removed, constants redefined with drift guards, tests distributed to extension crates, unused params cleaned up.

Prior feedback resolution

Feedback from @shuklaayush	Status
Revert variables for compile-time constants	Done. Constants redefined in rvr-openvm-ext-ffi-common, imported directly. CProject::new back to 3 params. constants_header() takes zero params.
Delete backend_cache_stamp/caching code	Done. All caching removed (compile_cached, native_cache_key, sha2 dep, etc).
Remove unused parameters	Done. _system_config and _text_start removed.
Distribute tests to extension crates	Done. Tests split across rvr-openvm-tests, extensions/algebra/rvr/tests/, extensions/ecc/rvr/tests/, etc. crates/vm dev-deps now minimal.
Don't wrap constants in a function	Done. openvm_compile_constants() removed; constants imported directly from rvr_openvm_ext_ffi_common.
Avoid dev-dependency bloat on crates/vm	Done. Heavy test deps moved to dedicated test crates.

Findings

1. DEFERRAL_DIGEST_SIZE drift guard doesn't fire in rvr-only builds — crates/vm/src/arch/rvr/abi_consts.rs:33-36

#[cfg(any(feature = "test-utils", feature = "cuda"))]
const _: () = assert!(
    ffi::DEFERRAL_DIGEST_SIZE == openvm_stark_sdk::config::baby_bear_poseidon2::DIGEST_SIZE,
);

The DEFERRAL_DIGEST_SIZE assertion is gated on test-utils or cuda, which means a plain rvr-only build never checks this constant. If DIGEST_SIZE drifts, the mismatch won't be caught until someone happens to build with test-utils. Consider whether there's a way to check this unconditionally — e.g., if DIGEST_SIZE could be moved to a leaf crate that rvr-openvm-ext-ffi-common can import directly, the constant could be a re-export rather than a redefinition.

2. pub mod rvr visibility — crates/vm/src/arch/mod.rs:14

The module is pub, so any external crate can depend on openvm_circuit::arch::rvr::*. The test-utils and extension test crates already import via this path. If external crates outside the workspace should not couple to these internals, pub(crate) would prevent that and force a more deliberate export surface. If external access is intentional (e.g., for openvm-sdk), this is fine — just confirming intentionality.

3. build_blocks now hardcodes DEFAULT_SEGMENT_CHECK_INSNS as max_block_insns — crates/rvr/rvr-openvm-lift/src/cfg.rs:1067

Previously max_block_insns was a parameter passed by the caller; now it reads rvr_openvm_ext_ffi_common::DEFAULT_SEGMENT_CHECK_INSNS directly inside build_block_list. This couples block granularity permanently to the segmentation interval. If someone later wants to tune segmentation without changing block sizes (or vice versa), they'd need to decouple these again. The TODO in abi_consts.rs:7-8 about whether these should be consts or runtime-tunable is relevant — worth a brief inline comment in build_block_list noting this intentional coupling.

4. DEFERRAL_PAGE_BUF_CAP audit TODO — crates/rvr/rvr-openvm/src/constants.rs:26-27

// TODO: justify this bound (audit max deferral pages per instruction).
pub const DEFERRAL_PAGE_BUF_CAP: usize = 1 << 16;

Since C code has no bounds checks on these page buffers, an undersize capacity would be a memory safety issue. The other two buffers (MEM_PAGE_BUF_CAP, PV_PAGE_BUF_CAP) have documented worst-case analysis. This TODO should be resolved before the buffers are used in production without bounds checks.

5. Phantom dispatch collision risk — crates/rvr/rvr-openvm-lift/src/opcode.rs:48-56

if SysPhantom::from_repr(discriminant).is_some()
    || Rv32Phantom::from_repr(discriminant).is_some()
{
    return Some(lift_phantom(insn, pc));
}
if let Some(lifted) = extensions.try_lift(insn, pc) {
    return Some(lifted);
}

Built-in phantom handlers are checked before extensions. If a future extension phantom discriminant collides with a SysPhantom or Rv32Phantom value, it would be incorrectly handled by lift_phantom instead of the extension. This is currently safe (discriminant ranges: 0x00–0x03 for Sys, 0x20–0x22 for Rv32), but the from_repr approach makes it implicit. A brief comment noting the assumed non-overlapping range would help future maintainers.

Minor / Nit

• Constant sourcing is clean: codegen.rs now uses rvr_openvm_ext_ffi_common::AS_PUBLIC_VALUES instead of importing from openvm_circuit. bigint/rvr/src/lib.rs uses openvm_instructions::program::DEFAULT_PC_STEP instead of a local constant. Both are correct.
• abi_consts.rs drift guard pattern is well-designed: compile-time const _: () = assert!(...) checks catch any drift between redefined constants and canonical sources without runtime overhead.
• Test harness (VmTestHarness<VB>) is well-structured: generic over VmBuilder, provides clean compare for all three modes, with proper relaxed comparison for metered mode (block vs instruction segmentation granularity).
• DEFAULT_INTERACTION_CONSTANT_OVERHEAD visibility change (segment_ctx.rs:47): const → pub const is a minimal, appropriate visibility expansion.
• Extension test migration is mechanical and correct: all test files follow the same import-path update pattern, now using rvr_openvm_test_utils for shared helpers.

Summary

Clean, well-executed migration that correctly resolves the circular dependency. All prior review feedback has been thoroughly addressed. The architecture split (thin rvr-openvm for codegen, openvm-circuit::arch::rvr for runtime, leaf ffi-common for shared constants) is sound and well-motivated. The compile-time drift guards in abi_consts.rs are a nice safety net.

No blocking issues. Main items for future consideration: (1) making the DEFERRAL_DIGEST_SIZE drift guard unconditional, (2) confirming pub mod rvr visibility intent, (3) resolving the DEFERRAL_PAGE_BUF_CAP capacity audit TODO.

[shuklaayush]

lgtm. some cleanup that can be handled later

[shuklaayush]

we should figure out a way to share these later. maybe there should be a common crate somewhere. not an issue for now since we already have too many crates

[shuklaayush]

fine for now but we should try to minimize these extra crates and try to consolidate the code