Skip to content

chore: remove native extension-specific code#2590

Merged
stephenh-axiom-xyz merged 3 commits intodevelop-v2.0.0-betafrom
chore/remove-native-vestiges
Mar 19, 2026
Merged

chore: remove native extension-specific code#2590
stephenh-axiom-xyz merged 3 commits intodevelop-v2.0.0-betafrom
chore/remove-native-vestiges

Conversation

@stephenh-axiom-xyz
Copy link
Contributor

Resolves INT-6435. Documentation update will be part of a larger change.

Summary

Remove remaining code-level references to the Native extension, which was deleted in an earlier commit (4d54346f6). This cleans up leftover Native-specific APIs, types, and helper code that are no longer used now that the deferral extension handles the deferral address space directly.

1. Remove KvStore and hint-load-by-key

  • Delete KvStore trait and its HashMap impl from Streams (crates/vm/src/arch/vm.rs)
  • Remove kv_store field from Streams and StdIn, along with add_key_value helper (crates/sdk/src/stdin.rs)
  • Delete Rv32HintLoadByKeySubEx phantom sub-executor and its encode/decode helpers (extensions/rv32im/circuit/src/extension/mod.rs)
  • Remove HintLoadByKey variant from PhantomImm enum and Rv32Phantom enum (extensions/rv32im/guest/src/lib.rs, extensions/rv32im/transpiler/src/instructions.rs)
  • Delete hint_load_by_key guest intrinsic (extensions/rv32im/guest/src/io.rs) and its re-export (crates/toolchain/openvm/src/io/mod.rs)
  • Remove transpiler handling for PhantomImm::HintLoadByKey (extensions/rv32im/transpiler/src/lib.rs)
  • Delete hint_load_by_key example program and its integration test (extensions/rv32im/tests/)

2. Remove native store-to-deferral-AS from rv32im loadstore

  • Remove NATIVE_STOREW_FUNCT3, NATIVE_STOREW_FUNCT7 constants and store_to_native! macro from guest (extensions/rv32im/guest/src/lib.rs, extensions/rv32im/guest/src/io.rs)
  • Delete store_u32_to_native wrapper (crates/toolchain/openvm/src/io/mod.rs)
  • Remove transpiler handling for NATIVE_STOREW_FUNCT3 R-type instruction (extensions/rv32im/transpiler/src/lib.rs)
  • Reject DEFERRAL_AS as an invalid address space in loadstore execution and trace generation, rather than handling it as a special case (extensions/rv32im/circuit/src/loadstore/execution.rs, extensions/rv32im/circuit/src/adapters/loadstore.rs)
  • Remove is_native_store variable and all F-typed (field-element) store dispatch branches from the loadstore executor; stores are now always U8-typed (extensions/rv32im/circuit/src/loadstore/execution.rs)
  • Simplify AOT: is_aot_supported now unconditionally returns true; delete is_aot_supported_impl which checked for deferral-AS stores (extensions/rv32im/circuit/src/loadstore/aot.rs)

3. Move deferral memory helpers from rv32im to deferral crate

  • Delete extensions/rv32im/circuit/src/adapters/deferral.rs (199 lines) and its re-export
  • Create extensions/deferral/circuit/src/adapters.rs with the subset of helpers still needed: memory_read_deferral, memory_write_deferral, timed_read_deferral, timed_write_deferral, tracing_read_deferral, tracing_write_deferral
  • Functions only needed by the old Native extension (memory_read_or_imm_deferral, *_from_state, tracing_read_or_imm_deferral, tracing_write_deferral_inplace) are dropped
  • Update import in extensions/deferral/circuit/src/call/trace.rs

4. Rename Native terminology in types and comments

  • MemoryCellType::Native { size } -> MemoryCellType::F { size } and native32() -> field32() (crates/vm/src/arch/config.rs + all match sites)
  • DEFAULT_NATIVE_BLOCK_SIZE -> DEFAULT_FIELD_BLOCK_SIZE
  • Delete MemoryConfig::aggregation() (only used for native-extension testing) and VmChipTestBuilder::default_native()
  • Update comments from "native address space" to "deferral address space" in AIR docs and assertions
  • Update Poseidon2 chip module doc to remove "native kernel" language

5. Delete CUDA native adapter

  • Delete crates/vm/cuda/include/system/native_adapter.cuh
  • Remove native:: C++ namespace from crates/circuits/primitives/cuda/include/primitives/constants.h
  • Update comment in extensions/rv32im/circuit/cuda/src/loadstore.cu

@github-actions

This comment was marked as outdated.

@github-actions

This comment was marked as outdated.

@github-actions
Copy link

github-actions bot commented Mar 19, 2026

Claude finished @stephenh-axiom-xyz's task in 1m 54s —— View job


  • Gather context and review diff
  • Review changes in detail
  • Post final review

Code Review: Remove Native Extension-Specific Code

This is a thorough and well-structured removal of dead code. The changes are consistent across Rust, CUDA, guest, and transpiler layers. A few items to consider:

Issues

1. Dead LoadStoreOp<F> impls (medium)

extensions/rv32im/circuit/src/loadstore/execution.rs:401-444 — The three impl<F: PrimeField32> LoadStoreOp<F> blocks for StoreWOp, StoreHOp, and StoreBOp are now dead code. The dispatch! macro (line 86-104) exclusively uses the U8 type parameter for all arms — no arm instantiates LoadStoreOp<F>. These impls were only needed when native stores wrote field elements (F-typed) to the deferral address space, which has been removed.

Fix this →

2. Stale prev_data comments (minor)

Now that DEFERRAL_AS is rejected as InvalidInstruction in pre_compute_impl (execution.rs:54), prev_data can no longer come from the deferral address space through the loadstore path. The comments were updated from "native" to "deferral" but the stated rationale no longer applies:

  • extensions/rv32im/circuit/src/loadstore/core.rs:244: // Note: prev_data can be a field, so we need to use u32
  • extensions/rv32im/circuit/cuda/src/loadstore.cu:28: same comment

Since stores are now always U8-typed, prev_data values will always fit in u8. The u32 representation may still be justified for uniformity with the run_write_data return type, but the "can be a field" explanation is wrong. Consider updating or removing these comments.

Fix this →

3. Stale comment in common/mod.rs (nit)

extensions/rv32im/circuit/src/common/mod.rs:160: The comment says "If we ignore the deferral address space..." — but now that loadstore rejects DEFERRAL_AS, there's nothing to "ignore." The comment could be simplified to just state the fact directly.

4. AOT assert_ne! vs returning error (nit)

In extensions/rv32im/circuit/src/loadstore/aot.rs:115, the code returns Err(AotError::InvalidInstruction) for RV32_IMM_AS, which is consistent. But the DEFERRAL_AS check on the same line uses the same pattern — good. However, there's no unused_parameter lint for the inst param in is_aot_supported (line 21-23). The parameter is now completely ignored since the function unconditionally returns true. A leading underscore (_inst) would be more idiomatic and suppress any future lints.

Looks Good

  • KvStore removal is thorough — trait, impl, fields, helpers, phantom sub-executor, encode/decode, guest intrinsic, transpiler handling, example program, and test are all removed.

  • Deferral helpers migration to extensions/deferral/circuit/src/adapters.rs correctly retains only the subset still used. The dropped functions were Native-extension-only. Import update in call/trace.rs is correct.

  • NativeF rename is applied consistently: MemoryCellType::NativeF, native32()field32(), DEFAULT_NATIVE_BLOCK_SIZEDEFAULT_FIELD_BLOCK_SIZE.

  • Loadstore simplification — dispatch macro reduced from 18 arms to 12 by removing the is_native_store dimension. Adding debug_assert_ne!(e, DEFERRAL_AS) at the adapter level provides defense-in-depth.

  • CUDA cleanupnative_adapter.cuh deleted, native:: namespace removed from constants.h.

  • AOT simplificationis_aot_supported now unconditionally returns true, removing the dead is_aot_supported_impl.

  • New adapters.rs in the deferral crate is well-structured with correct safety comments for the unsafe memory operations.

@stephenh-axiom-xyz stephenh-axiom-xyz merged commit 9f6721a into develop-v2.0.0-beta Mar 19, 2026
28 checks passed
@stephenh-axiom-xyz stephenh-axiom-xyz deleted the chore/remove-native-vestiges branch March 19, 2026 16:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants