feat: enforce semantics for random access memory ram#1962
Open
OlivierBBB wants to merge 2 commits into
Open
Conversation
Signed-off-by: Olivier Bégassat <olivier.begassat.cours@gmail.com>
… RAM The RandomAccess struct needs to store values as TimestampedCell's and so needs a different generic for its StaticArray's data field than for the geometry field. We add fields into RandomAccess to track accesses + a timestamp. Accesses are logged as a list of AccessData which logs timestamps, value etc ... Claude also added some GobDecode/GobEncode methods + various access methods for the RandomAccess / AccessData Signed-off-by: Olivier Bégassat <olivier.begassat.cours@gmail.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Adds stricter “timestamped RAM” semantics to the VM memory layer, likely to support consistent trace/lookup constraints for RAM accesses.
Changes:
- Introduces a timestamped-cell backing representation for
RandomAccessRAM and logs each read/write asAccessData. - Generalizes
StaticArrayto store an arbitrary cell type (C) rather than always storingW. - Adds/extends RAM semantics documentation in
ram.md.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| ram.md | New documentation describing RAM constraints and implementation notes. |
| pkg/zkc/vm/internal/memory/wom.go | Updates WOM to embed the new 2-parameter StaticArray. |
| pkg/zkc/vm/internal/memory/static_array.go | Generalizes StaticArray to store []C and updates signatures accordingly. |
| pkg/zkc/vm/internal/memory/rom.go | Updates ROM to embed and construct the new 2-parameter StaticArray. |
| pkg/zkc/vm/internal/memory/ram.go | Implements timestamped RAM cells plus per-access logging and gob support for timestamped cells. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+87
to
+90
| timestampWrite := uint64(ram.timestamp) | ||
| // Re-stamp the cell with this access's timestamp. | ||
| ram.data = expand(ram.data, address+1) | ||
| ram.data[address] = TimestampedCell[W]{timestamp: timestampWrite, value: valueWritten} |
Comment on lines
+35
to
+38
| // timestamp; increments by 1 with every memory access | ||
| timestamp int | ||
| // accesses logs memory accesses in chronological order | ||
| accesses []AccessData[W] |
Comment on lines
+60
to
+63
| if len(newValue) > 1 || (isWrite && len(newValue) != 1) || (!isWrite && len(newValue) != 0) { | ||
| var ignored W | ||
| return ignored, fmt.Errorf("invalid access call; isWrite = %v, len(newValues) = %d", isWrite, len(newValue)) | ||
| } |
Comment on lines
+160
to
+163
| What to do in case of empty RAM ? It shouldn't be an issue in our RV-interpreter. | ||
| In the "you only initialize / finalize those cells that you touched" approach | ||
| you can force an interaction with address `[]0`, for instance | ||
| what |
| you can force an interaction with address `[]0`, for instance | ||
| what | ||
|
|
||
| # Claude additions |
Comment on lines
30
to
31
| // D is the AddressDecoder strategy that encodes the layout of rows within the | ||
| // flat slice. |
Comment on lines
31
to
33
| // The type parameter W is the word type (e.g. a field element or big.Int), and | ||
| // D is the AddressDecoder strategy that encodes the layout of rows within the | ||
| // flat slice. |
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
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.
No description provided.