core/vm: memoize MULMOD modulus reciprocal#2310
Conversation
uint256.MulMod recomputes Reciprocal(m) on every call for full-width (4-word) moduli, and that reciprocal is ~60% of the call cost. Long runs of modular multiplication against a single fixed modulus are a common on-chain pattern, so cache (modulus, reciprocal) per EVM instance and reuse it via MulModWithReciprocal when the modulus repeats. The result is a pure function of (x, y, m) and identical to stock: on a miss the reciprocal is recomputed exactly as before; on a hit the cached value is what stock would have computed. Gas is unchanged, so no fork gate is needed. The memo is per-EVM scratch state like depth/returnData; the EVM is single-goroutine and not thread safe, so no synchronization. Mainnet fork experiment (58,867 blocks / 6.75M txs): execution 3.21 -> 2.75 ms/Mgas (~14%), MULMOD 223 -> 148 ns/op. Microbench hit path ~3x, zero allocations.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.
Tip: disable this comment in your organization's Code Review settings.
Add TestMulmodMemoCacheState asserting the memo is primed with the exact reciprocal on a full-width modulus, refreshed on a modulus switch, and never primed by a narrow modulus. The differential/fuzz tests verify output equivalence but cannot distinguish cached/recomputed/bypassed states, since the memo is behavior-neutral by design; this test pins that internal contract and brings changed-line mutation coverage to 100%.
|
Follow-up ( diffguard's mutation pass initially surfaced 3 survivors, all in the memo's cache-management logic (the The fix pins the internal caching contract via white-box assertions rather than adding more output cases: a full-width modulus primes the memo with the exact Side note validating the guard: |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #2310 +/- ##
========================================
Coverage 54.05% 54.06%
========================================
Files 907 908 +1
Lines 161982 161992 +10
========================================
+ Hits 87564 87582 +18
- Misses 69000 69004 +4
+ Partials 5418 5406 -12
... and 24 files with indirect coverage changes
🚀 New features to boost your workflow:
|
|
@claude review |
There was a problem hiding this comment.
This is a well-tested, low-risk performance optimization; approving with the inline nit noted below.
Extended reasoning...
Overview
The PR adds a one-entry (modulus, reciprocal) memo cache to the EVM to avoid recomputing uint256.Reciprocal(m) on every MULMOD for full-width moduli. It touches core/vm/evm.go (new mulmodMemo field on EVM), core/vm/instructions.go (opMulmod), core/vm/interpreter_dispatch.go (generated switch-dispatch MULMOD case), and adds core/vm/mulmod_memo.go plus an extensive differential/fuzz test file.
Security risks
None identified. This is a pure execution-speed optimization: no gas cost changes, no new externally-observable behavior, and the memo is proven to be output-equivalent to stock uint256.MulMod (same reciprocal, cached vs recomputed). The EVM type is single-goroutine/non-reused per its existing contract, so the new unsynchronized field is safe — consistent with existing fields like depth and returnData.
Level of scrutiny
This touches a consensus-critical hot path (the MULMOD opcode, both dispatch paths), so it warrants real scrutiny despite being "just a cache." I verified: the full-width guard (z[3] != 0) correctly excludes narrow/zero moduli (preserving m == 0 → 0), the cache correctly refreshes on modulus switch, and both call sites (opMulmod and the switch-dispatch runSwitch) were updated consistently. Test coverage is thorough — 500k+ differential cases, pointer-aliasing matrix, cache-state white-box tests, and a fuzz target — and mutation-testing survivors were addressed in a follow-up commit with principled reasoning (behavior-neutral caching logic can't be killed by output-only tests).
Other factors
The one finding from this run — that core/vm/gen_dispatch/main.go wasn't updated to match the hand-edited generated interpreter_dispatch.go, so a future go generate would silently revert the switch-dispatch memo call — is already posted as an inline comment. It's a maintenance/durability nit with no correctness or consensus impact (both code paths always produce identical output), not a blocker.
There was a problem hiding this comment.
Pull request overview
This PR introduces a per-EVM one-entry memoization of the modulus reciprocal used by the MULMOD opcode for full-width (4-word) moduli, allowing repeated modular multiplications with the same modulus to reuse uint256.Reciprocal(m) rather than recomputing it each time.
Changes:
- Added
modReciprocalmemo helper and wired it intoMULMODexecution (bothopMulmodand the switch-dispatch fast path). - Extended
EVMwith amulmodMemoscratch field to keep the cached(modulus, reciprocal)per instance. - Added differential/unit tests, fuzzing, and benchmarks validating memoized behavior against stock
uint256.MulMod.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| core/vm/mulmod_memo.go | Implements a 1-entry modulus→reciprocal memo and a memo-aware mulmod helper. |
| core/vm/mulmod_memo_test.go | Adds differential tests, aliasing coverage, fuzzing, and benchmarks for the memoized path. |
| core/vm/interpreter_dispatch.go | Switch-dispatch MULMOD path updated to call the memo helper (note: generated file). |
| core/vm/instructions.go | Non-switch MULMOD opcode implementation updated to use the memo helper. |
| core/vm/evm.go | Adds the mulmodMemo field to EVM to store per-instance cached reciprocal state. |
Files not reviewed (1)
- core/vm/interpreter_dispatch.go: Generated file
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The MULMOD case in the hand-written switch dispatch (interpreter_dispatch.go) was edited to call evm.mulmodMemo.mulmod, but the generator template still emitted z.MulMod. A future `go generate ./core/vm/` would silently revert the memoized call on the switch-dispatch hot path. Update the generator body so a regen is a no-op and the optimization is durable.
|
Summary
Memoize the modulus reciprocal used by the
MULMODopcode. For a full-width(4-word) modulus,
uint256.MulMod(x, y, m)recomputesReciprocal(m)on everycall, and that reciprocal is ~60% of the call's cost. A common on-chain pattern is
a long run of modular multiplications against a single fixed-prime modulus
(field arithmetic used broadly by cryptographic and verification contracts), so we
cache
(modulus, reciprocal)per EVM instance and reuse it viaMulModWithReciprocalwhen the modulus repeats.Measured on a mainnet-fork experiment: ~14% faster block execution (details below).
The change
core/vm/mulmod_memo.go: a one-entry(modulus, reciprocal)memo on the EVM.EVMgains amulmodMemofield (per-instance scratch state).MULMODexecution paths (opMulmodand the switch dispatcher) call the memoinstead of
uint256.MulModdirectly.Full width only: narrower moduli — and
m == 0— never reach the reciprocalreduction inside
MulMod, so they fall through to the stock call unchanged.Why this is consensus-safe
MulModfor a full-width modulus computesReciprocal(m)and then reduces with it. We cache that reciprocal.MulModWithReciprocal(x, y, m, Reciprocal(m))is defined to produce the identicalresult to
MulMod(x, y, m). The memo changes only how the reciprocal is obtained(cached vs recomputed), never the arithmetic.
(x, y, m), provably independent of cachecontents: on a miss we recompute the reciprocal exactly as stock would; on a hit the
cached value is bit-for-bit what stock would have produced. Two nodes always agree.
z[3] == 0guard), preservingm == 0 → 0.MULMOD's gas cost is untouched and noobservable state differs; this is a pure execution-speed optimization that is safe to
ship unconditionally.
depthandreturnData. TheEVMtype is documented as "should never be reused andis not thread safe", every construction site builds a fresh instance, and each EVM
executes on a single goroutine (verified: both parallel-execution paths construct
their own EVM per task/worker; no EVM is shared across goroutines). An unsynchronized
field is therefore both correct and the fastest option — a lock on the hottest opcode
would be pure overhead.
Measurements
Mainnet-fork experiment — improved binary vs stock, identical hardware/chain/tx-mix,
window of 58,867 blocks / 6.75M txs:
MULMODper opMULMODshare of in-EVM timeuint256.Reciprocalaccounted for ~5.15% of total node CPU before the change — theexact cost this memo removes on the hit path.
Microbenchmark (
BenchmarkMulmod, Apple M4 Pro, zero allocations):The miss path (modulus changes on every call) costs one extra 4-word compare plus the
same reciprocal stock computes anyway. Real modular sequences reuse a modulus, so the
hit path dominates.
Testing
core/vm/mulmod_memo_test.go, all differential against stockuint256.MulMod:moduli, and edge values.
dest==mod,x==dest==mod,y==dest==mod,x==y==dest==mod, each compared bit-for-bit to stock with identical aliasing.FuzzMulmodMemo— differential fuzz over arbitrary operands, exercising miss + hitper input (3.6M+ executions, no mismatch).
Scope notes
ADDMODis intentionally excluded:uint256.AddModreduces by conditionalsubtraction, not a division reciprocal, so it has no per-call recompute to memoize.
holiman/uint256; out of scopehere.