Skip to content

feat/dma memcpy - #874

Open
jotabulacios wants to merge 7 commits into
mainfrom
feat/dma-memcpy
Open

feat/dma memcpy#874
jotabulacios wants to merge 7 commits into
mainfrom
feat/dma-memcpy

Conversation

@jotabulacios

@jotabulacios jotabulacios commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

What

The guest's out-of-line memcpy becomes a DMA ecall: the executor performs the copy
natively and a dedicated AIR table proves it. Copies are chunked at 256 bytes.

Why

A copy ran as a RISC-V loop — per eight bytes a load, a store, two pointer increments and
a branch, each one a CPU row plus its memory operations. The DMA table replaces all of
that with one row per eight bytes: the source read at T+1 and the destination write at T+2
share the same value columns, so a copied byte cannot change without unbalancing the memory
bus. Both sides enforce the 256-byte bound — the executor rejects larger ecalls, the AIR
proves count < 257 on every first row — so one guest instruction cannot add an unbounded
number of rows to a continuation epoch.

Impact (ethrex 20-tx block, distinct senders)

  • Guest cycles: 9,073,592 → 8,183,048 (−9.8%); per transfer, empty block subtracted,
    415,544 → 379,600 (−8.7%)
  • Prove: 25.309 s → 23.831 s (−5.8%); peak heap 50,856 → 48,584 MB (−4.5%)
  • Smaller blocks gain more (−22.5% on an empty block, −11.0% at 10 tx): the copy traffic
    sits mostly in per-block setup, so the ratio converges to ~8.7% rather than to zero
  • Cycles are deterministic — same figures on a laptop and on the bench machine

Breaking

Adds a fixed table, so the set of tables in a proof changes: prover and verifier must be
deployed together, and earlier binaries cannot verify these proofs.

Validation

Both DMA guests prove and verify. Forgeries are rejected: altered copied byte, source row
skipped forward, early end, wrong row width. Plus a 256-case differential fuzz over
overlap, alignment and page crossings, a guest walking lengths 0–256 and a multi-chunk
copy, the length-drift test with a non-empty DMA table, and make lint.

Changes (32 files)

  • executor/src/vm/instruction/execution.rs: the ecall — operand validation, 256-byte
    bound, copy through a fixed scratch (snapshot semantics on overlap).
  • prover/src/tables/dma.rs: the table — rows chained through DmaNext, Zero for end
    detection, LT for the 1-vs-8-byte width and the per-call bound.
  • prover/src/constraints/templates.rs: emit_add_pair_no_overflow, so address
    transitions cannot wrap modulo 2^64.
  • syscalls/src/syscalls.rs: strong assembly memcpy symbol that chunks into ecalls and
    preserves the C return value.
  • Tests and two guests (dma_memcpy_min, dma_memcpy_cases).

@jotabulacios

Copy link
Copy Markdown
Collaborator Author

/bench

@github-actions

github-actions Bot commented Jul 28, 2026

Copy link
Copy Markdown

Benchmark — ethrex 20 transfers (median of 3)

Table parallelism: auto (cores / 3)

Metric main PR Δ
Peak heap 50856 MB 48584 MB -2272 MB (-4.5%) ⚪
Prove time 25.309s 23.831s -1.478s (-5.8%) 🟢

🎉 Improvement detected — heap or time decreased by more than 5%.

✅ Low variance (time: 1.4%, heap: 1.8%)

Commit: f9fdd03 · Baseline: cached · Runner: self-hosted bench

@github-actions

github-actions Bot commented Jul 28, 2026

Copy link
Copy Markdown

Benchmark Results for modified programs 🚀

Command Mean [ms] Min [ms] Max [ms] Relative
head ecsm 3.4 ± 0.0 3.3 3.4 1.00
Command Mean [ms] Min [ms] Max [ms] Relative
head hashmap 114.3 ± 2.6 110.0 119.6 1.00
Command Mean [ms] Min [ms] Max [ms] Relative
head keccak 131.1 ± 2.9 126.2 134.8 1.00
Command Mean [ms] Min [ms] Max [ms] Relative
head syscall_commit 93.4 ± 1.0 92.0 94.7 1.00

@jotabulacios

Copy link
Copy Markdown
Collaborator Author

/bench

@github-actions

Copy link
Copy Markdown

Benchmark Results for unmodified programs 🚀

Command Mean [ms] Min [ms] Max [ms] Relative
base binary_search 53.6 ± 10.8 49.4 84.3 1.03 ± 0.22
head binary_search 51.8 ± 2.6 49.6 58.2 1.00
Command Mean [ms] Min [ms] Max [ms] Relative
base bitwise_ops 50.5 ± 0.6 50.0 51.5 1.00
head bitwise_ops 50.8 ± 0.6 50.2 51.8 1.01 ± 0.02
Command Mean [ms] Min [ms] Max [ms] Relative
base fibonacci_26 52.3 ± 0.6 51.5 53.1 1.00 ± 0.02
head fibonacci_26 52.3 ± 0.6 51.6 53.3 1.00
Command Mean [ms] Min [ms] Max [ms] Relative
base matrix_multiply 52.5 ± 0.8 51.7 54.0 1.00
head matrix_multiply 52.5 ± 0.6 51.9 53.5 1.00 ± 0.02
Command Mean [ms] Min [ms] Max [ms] Relative
base modular_exp 50.3 ± 0.8 49.5 51.6 1.00
head modular_exp 50.6 ± 0.7 49.6 51.4 1.01 ± 0.02
Command Mean [ms] Min [ms] Max [ms] Relative
base quicksort 52.8 ± 0.4 52.5 53.7 1.00
head quicksort 53.3 ± 0.6 52.7 54.6 1.01 ± 0.01
Command Mean [ms] Min [ms] Max [ms] Relative
base sieve 54.0 ± 0.5 53.5 54.8 1.00
head sieve 56.8 ± 3.4 53.7 63.3 1.05 ± 0.06
Command Mean [ms] Min [ms] Max [ms] Relative
base sum_array 61.8 ± 0.5 61.2 62.4 1.00
head sum_array 62.2 ± 0.6 61.5 63.0 1.01 ± 0.01

@jotabulacios jotabulacios changed the title experimental/dma memcpy feat/dma memcpy Jul 29, 2026
@jotabulacios

Copy link
Copy Markdown
Collaborator Author

/ai-review

@github-actions

Copy link
Copy Markdown

Codex Code Review

No actionable issues found in the PR changes. Static review only; no builds or tests run per instructions.

@jotabulacios
jotabulacios marked this pull request as ready for review July 29, 2026 15:30
@github-actions

Copy link
Copy Markdown

AI Review

PR #874 · 32 changed files

Findings

Status Sev Location Finding Found by
confirmed medium executor/src/vm/instruction/execution.rs:479 DMA ecall address overflow check is off-by-one kimi
openrouter/moonshotai/kimi-k2.7-code
confirmed low prover/src/constraints/templates.rs:376 Misleading docstring in emit_add_pair_no_overflow kimi
openrouter/moonshotai/kimi-k2.7-code

Status column reflects the verdict from the verifier: deepseek-verifier (openrouter/deepseek/deepseek-v4-pro).

AI-001: DMA ecall address overflow check is off-by-one
  • Status: confirmed
  • Severity: medium
  • Location: executor/src/vm/instruction/execution.rs:479
  • Found by: kimi:openrouter/moonshotai/kimi-k2.7-code
  • Verified by: deepseek-verifier:openrouter/deepseek/deepseek-v4-pro
  • Rejected by: -

Claim

The DMA memcpy ecall rejects valid copies where the final accessed byte is exactly u64::MAX. It checks dst.checked_add(n) and src.checked_add(n), which require one-past-the-end to fit, but only bytes in [addr, addr+n) are accessed. The correct condition is that addr + (n-1) does not overflow.

Evidence

Lines 479-480 use dst.checked_add(n).ok_or(MemoryError::AddressOverflow)? and src.checked_add(n).ok_or(MemoryError::AddressOverflow)?. For n>0, the last byte accessed is at addr+n-1, so a copy with addr = u64::MAX - n + 1 is valid but is rejected because addr + n overflows. This matches the prover's per-row no-overflow constraints, so it is a completeness/consistency edge-case bug rather than a soundness issue.

Suggested fix

Change both checks to dst.checked_add(n.saturating_sub(1)).ok_or(MemoryError::AddressOverflow)? (and similarly for src), which is equivalent for n>0 and passes for n=0.

AI-003: Misleading docstring in emit_add_pair_no_overflow
  • Status: confirmed
  • Severity: low
  • Location: prover/src/constraints/templates.rs:376
  • Found by: kimi:openrouter/moonshotai/kimi-k2.7-code
  • Verified by: deepseek-verifier:openrouter/deepseek/deepseek-v4-pro
  • Rejected by: -

Claim

The docstring says the constraint fires "while active - end == 1", but the implementation computes active = main(active_column) - main(end_column) and callers pass active_column = MU, end_column = END. The phrase collides with the local variable name and does not describe the actual arithmetic condition (mu - end == 1).

Evidence

Lines 376-381 describe the condition as active - end == 1; lines 401-402 compute let active = b.main(0, active_column) - b.main(0, end_column); and in prover/src/tables/dma.rs the helper is invoked with active_column = cols::MU and end_column = cols::END.

Suggested fix

Reword the docstring to describe the actual condition, e.g. "while mu - end == 1 (i.e. active, non-terminal rows)", or rename the parameter/variable to avoid the collision.

Reviewer Lanes

Lane Model Prompt Status Findings
glm openrouter/z-ai/glm-5.2 general success 0
kimi openrouter/moonshotai/kimi-k2.7-code general success 2
minimax minimax/MiniMax-M3 general error: opencode failed (provider/auth/runtime error) and no findings were submitted 0
moonmath zro/minimax-m3 general error: agentic lane timed out after 1800s 0
nemotron openrouter/nvidia/nemotron-3-ultra-550b-a55b general success 3

Verification Lanes

Lane Model Status Confirmed Rejected Uncertain
deepseek-verifier openrouter/deepseek/deepseek-v4-pro success 2 3 0

Native Codex and Claude reviews run separately and post their own comments. They are not included in this structured provenance report.

Discarded candidates (3) — rejected by the verifier
  • New emit_add_pair_no_overflow constraint critical for address wrap prevention (prover/src/constraints/templates.rs:330, found by nemotron:openrouter/nvidia/nemotron-3-ultra-550b-a55b) — This finding does not identify any actual bug or issue. It merely describes what the new emit_add_pair_no_overflow constraint does ('constrains high carry to zero on active non-terminal rows'), notes it is security-critical, and states it 'must be correct.' Both the claim and evidence acknowledge the logic 'appears correct.' This is an observation, not an actionable finding.
  • Register timestamp advancement in collect_dma_memcpy_ops may affect subsequent register accesses (prover/src/tables/trace_builder.rs:1080, found by nemotron:openrouter/nvidia/nemotron-3-ultra-550b-a55b) — The finding acknowledges the behavior is correct: the register_state.write calls update timestamps properly for the MEMW register read protocol. The evidence even states 'This is correct for MEMW register read protocol.' The CPU's collect_register_ops_from_cpu does not generate duplicate MEMW ops for ecall arguments because decode doesn't mark them as read_register — so DMA's explicit reads and timestamp updates are the sole source of these MEMW entries, making the design consistent. No bug is identified; this is a design remark, not an issue.
  • Assembly memcpy const interpolation relies on compile-time constant evaluation (syscalls/src/syscalls.rs:185, found by nemotron:openrouter/nvidia/nemotron-3-ultra-550b-a55b) — This is a statement about Rust language semantics, not an actual code issue. The DMA_MEMCPY_SYSCALL_NUMBER and DMA_MEMCPY_MAX_BYTES are both defined as const usize at lines 147-148, and the global_asm! macro at lines 235-236 correctly uses const interpolation to reference them. The claim that 'any change to make them non-const would break the assembly' is a tautology about Rust's const evaluation — it's not identifying a problem with the current code, which is correct as written.

Raw lane outputs, candidates, final issues, and model metrics are uploaded as workflow artifacts.

@nicole-graus

Copy link
Copy Markdown
Collaborator

/bench-verify

@github-actions

Copy link
Copy Markdown

Benchmark started on the bench server. The recursion-guest cycle comparison adds guest builds on top of the verifier bench, longer on a cold runner. The bench server is occupied until it finishes.

@github-actions

github-actions Bot commented Jul 29, 2026

Copy link
Copy Markdown

Verifier benchmark — adb267b620 vs main (20 pairs)

=== Verify ABBA result ===

Metric main PR Δ
Verify time (per-side) 2.853s 2.862s +0.34% ⚪
Proof size 156.30 MiB 156.64 MiB +0.22% 🔴

Per-side (⚠️ PR REJECTS the baseline's valid proof — likely a VERIFY REGRESSION, not a format change): A/B/B/A cancels machine drift but not proof-specific variance — read the Verify-time Δ as approximate.

  pairs: 20   mean A (PR): 2.862s   mean B (main): 2.853s
  [parametric] paired-t   mean +0.34%   sd 0.74%   se 0.16%
               95% CI: [-0.01%, +0.68%]   (t df=19 = 2.093)
  [robust]     median +0.25%   Wilcoxon W+=130 W-=41  p(exact)=0.0539  (z=+1.92)

  run-to-run jitter:    A CV 0.60%   B CV 0.36%        (lower = steadier)
  within-session drift: +0.11% over the run, 1st->2nd half -0.11%

INCONCLUSIVE — effect not separable from 0 at n=20 (point estimate ~+0.25%). Add pairs to resolve.

Drift-free interleaved A/B/B/A measurement. - = PR faster. Trust the verdict when paired-t and Wilcoxon agree.


Recursion guest cycles (main vs PR)

=== Recursion-guest cycle comparison — single query (blowup=2, 1 query) — deterministic to ~±100k cycles ===
REF_B (baseline) origin/main e0add1d guest=recursion-min.elf
REF_A (PR) adb267b adb267b guest=recursion-min.elf

Metric REF_B (baseline) REF_A (PR) Δ (A-B)
Guest cycles 39.8M 39.5M -0.3M (-0.79%)
Keccak calls 3025 3057 +32

note: cycles reproduce to ~±100k (build codegen + proof nondeterminism); treat sub-100k deltas as noise, not signal.

raw (exact integer counts)
ref_b_sha=e0add1d5291d2229fbd868d317aa8edfe8021c44 ref_b_elf=recursion-min.elf ref_b_cycles=39770941 ref_b_keccak=3025 ref_b_execute_wall_s=2
ref_a_sha=adb267b620c213025b132c4a9f2f1021d5caaff8 ref_a_elf=recursion-min.elf ref_a_cycles=39456071 ref_a_keccak=3057 ref_a_execute_wall_s=1
delta_cycles=-314870 delta_keccak=32

(blowup2 regime unavailable for these refs — see the workflow log.)

(blowup4 regime unavailable for these refs — see the workflow log.)

=== Recursion-guest cycle comparison — 128-bit (blowup=4, 110 queries) — real ethrex block, 4 transfers — deterministic to ~±100k cycles ===
REF_B (baseline) origin/main e0add1d guest=recursion-cont-blowup4.elf
REF_A (PR) adb267b adb267b guest=recursion-cont-blowup4.elf

Metric REF_B (baseline) REF_A (PR) Δ (A-B)
Guest cycles 1019.5M 1012.5M -7.0M (-0.69%)
Keccak calls 1530913 1542262 +11349

note: cycles reproduce to ~±100k (build codegen + proof nondeterminism); treat sub-100k deltas as noise, not signal.

raw (exact integer counts)
ref_b_sha=e0add1d5291d2229fbd868d317aa8edfe8021c44 ref_b_elf=recursion-cont-blowup4.elf ref_b_cycles=1019535779 ref_b_keccak=1530913 ref_b_execute_wall_s=16
ref_a_sha=adb267b620c213025b132c4a9f2f1021d5caaff8 ref_a_elf=recursion-cont-blowup4.elf ref_a_cycles=1012535467 ref_a_keccak=1542262 ref_a_execute_wall_s=17
delta_cycles=-7000312 delta_keccak=11349

@nicole-graus

Copy link
Copy Markdown
Collaborator

/bench-verify

@github-actions

Copy link
Copy Markdown

Benchmark started on the bench server. The recursion-guest cycle comparison adds guest builds on top of the verifier bench, longer on a cold runner. The bench server is occupied until it finishes.

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