Reduce CAccountMap NIF surface with map-owned storage and frozen-only lock#43
Merged
Conversation
…oot. Speculative paths use clone_lazy; Chain.State drops :store in favor of an internal state_trie with map-level freeze and native difference/apply. Co-authored-by: Cursor <cursoragent@cursor.com>
OptionParser treated child flags like --no-start as watchdog options; parse manually like the deadlock watchdog so the nightly CI command works. Co-authored-by: Cursor <cursoragent@cursor.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors state management by moving the state root trie into C++ via the CAccountMap NIF, removing the separate :store field from Elixir's Chain.State, and introducing a speculative clone_lazy mechanism for RPC and EdgeV2 execution. Critical feedback on the C++ implementation highlights a refcounting leak and potential Use-After-Free (UAF) vulnerability with merkletree resources, a BEAM VM crash risk from using enif_make_existing_atom on uninitialized atoms, and dead code in the destructor of SharedAccountMap that would cause a double-decrement of has_clone if activated.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Remove dead SharedAccountMap COW fields, share one fork helper for eager/lazy clone, reject writes on frozen maps, and own materialized storage with a single map ref so uncompact no longer over-keeps. Co-authored-by: Cursor <cursoragent@cursor.com>
…lock. Stop exporting live storage tries from account_map_get, batch EVM writes via storage_put_map, and drop clone_lazy / per-trie seal so speculative paths and lock cost stay simpler. Co-authored-by: Cursor <cursoragent@cursor.com>
dominicletz
changed the title
Export state root hashes instead of a live state_trie, compact via one account_map_compact NIF call, and make map-backed accounts explicit with map_backed on Chain.Account. Co-authored-by: Cursor <cursoragent@cursor.com>
Merge duplicate account_map NIFs, gate bare-tree exports behind CMERKLE_TEST_NIFS, drop dead Account storage APIs, and keep EVM storage reads on the normal scheduler. Co-authored-by: Cursor <cursoragent@cursor.com>
dominicletz
changed the title
Normalize ids the same way as ensure_account so cache_account and other EVM paths type-check. Co-authored-by: Cursor <cursoragent@cursor.com>
Drop put_tree and live-tree root_hash, make root_hash a struct field, require genesis_storage/0 on all chains, and route Edge account roots through Account.root_hash. Co-authored-by: Cursor <cursoragent@cursor.com>
…nce. Startup purge of sync.sq3 was SELECT-ing every block into memory; batch and background it, and add MemoryGuard so absolute/rapid RSS growth stops the VM. Co-authored-by: Cursor <cursoragent@cursor.com>
…ows at once." This reverts commit ae3ec1f.
Document Phases A–E (cached compact roots, SharedState equality, NIF-returned roots, CompactStorage COW, state_trie-driven diff) and add a bench that reproduces the multi-second compact_small_delta hotspot. Co-authored-by: Cursor <cursoragent@cursor.com>
Production already uses account-map APIs only; remove gated bare-tree exports, legacy merkle benches, and update fuzz/stress/tests to the map-native surface. Co-authored-by: Cursor <cursoragent@cursor.com>
Lock in compact/uncompact root, clone isolation, difference tuple shape, and nif_stats coverage so Phases A–E stay regression-tested. Co-authored-by: Cursor <cursoragent@cursor.com>
P10/P11 (and nightly P4/P5) no longer exist; use P12/P14 (and P12–P16 nightly). Also slim the state-diff contract tests and drop the redundant NIF surface file. Co-authored-by: Cursor <cursoragent@cursor.com>
CompactStorage caches storage roots; difference_full is state_trie-driven and returns 6-tuples so Elixir skips a second storage_root_hash pass. Clone shares compact slots via shared_ptr. Align docs/LOCK_ORDER/bench with shipping behavior. Co-authored-by: Cursor <cursoragent@cursor.com>
Reproduce the >2s Model.ChainSql.state/1 paths without starting the node, and document them next to the existing State.difference bench. Co-authored-by: Cursor <cursoragent@cursor.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
CAccountMap: EVM/RPC/Edge usestorage_put_map/storage_*/ proofs instead of livemerkletreeresources fromget.CMERKLE_TEST_NIFS(on unlessMIX_ENV=prod).from_binary; remove deadAccount.tree/storage_*/compactAPIs. Speculative paths useState.clone/1;account_map_lockis O(1)frozenonly.state_trieexport — Edge usesstate_root_hashes/1.State.compact/1is oneaccount_map_compactNIF. Storage get/size/range stay on the normal scheduler.docs/caccount-map-nif.md,AGENTS.md,LOCK_ORDER.md,SECURITY_REVIEW.md.SELECTs the entiresync.sq3into memory (batched + background purge). AddMemoryGuardto halt on absolute/rapid RSS growth.Test plan
mix test test/cmerkle_*.exs test/chain_state_*.exs test/caccount_map*.exs test/chain_account_hash_nif_test.exs test/cmerkletree_test.exs --exclude external --no-startMIX_ENV=prod mix compile(bare-tree NIFs omitted)mix test test/memory_guard_test.exs./run/ prod start: RSS stays ~flat during SyncSql purge (no immediate OOM)mix run --no-start scripts/cmerkle_fuzz.exs -- --iterations 2000 --seed 1mix run --no-start scripts/cmerkle_parallel_stress.exs -- --waves 5 --tasks 48eth_call/ sendtransaction speculative path on a local./devnode