Conversation
505aa25 to
5479b77
Compare
|
@copilot add change log entry for this PR 2203 |
|
@mmagician I've opened a new pull request, #2204, to work on those changes. Once the pull request is ready, I'll request review from you. |
b5f4cdb to
e708a51
Compare
There was a problem hiding this comment.
Pull request overview
This PR introduces a single-word Array account component that provides a simple, reusable array data structure for Miden accounts. The implementation wraps a StorageMap where array indices are mapped to keys of the form [index, 0, 0, 0].
Key changes:
- Added
Arraystruct with methods for creating arrays with initial elements or from slices - Implemented MASM procedures (
getandset) for array access with configurable storage slots - Added comprehensive unit tests and integration tests demonstrating the component's usage
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
crates/miden-standards/src/account/array.rs |
Core implementation of the Array component with constructors, MASM generation, and conversion to AccountComponent |
crates/miden-standards/asm/account_components/array.masm |
MASM template defining get and set procedures for array operations |
crates/miden-standards/src/account/mod.rs |
Adds array module to the account components public API |
crates/miden-testing/src/kernel_tests/tx/test_array.rs |
Integration test verifying get/set operations work correctly in transaction context |
crates/miden-testing/src/kernel_tests/tx/mod.rs |
Includes the new test module |
CHANGELOG.md |
Documents the addition of the Array component |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Looks great! This is a great feature! When reviewing the implementation, it reminded me of this issue for |
Fumuran
left a comment
There was a problem hiding this comment.
Looks good! Aside from the decision of whether we need to make this component a utility, I have just a small style nit inline.
chore: simplify test chore: be explicit about padding
* Initial plan * chore: add changelog entry for PR 2203 Co-authored-by: mmagician <8402446+mmagician@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: mmagician <8402446+mmagician@users.noreply.github.com>
Co-authored-by: Andrey Khmuro <andrey@polygon.technology>
chore: remove component code
34ccf83 to
6bd6069
Compare
Array componentArray abstraction
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
PhilippGackstatter
left a comment
There was a problem hiding this comment.
Looks good to me! Sorry for the late review, it slipped through my notifications.
* feat: working array chore: simplify test chore: be explicit about padding * chore: add changelog entry for Array component (#2204) * Initial plan * chore: add changelog entry for PR 2203 Co-authored-by: mmagician <8402446+mmagician@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: mmagician <8402446+mmagician@users.noreply.github.com> * fix: set_map_item no longer returns old root * chore: change Try to TryFrom for AccountComponent * chore: correct the docs * chore: masm doc corrections * chore: use BeWord instead of Word * chore: adjust masm comment about max len * Update crates/miden-standards/asm/account_components/array.masm Co-authored-by: Andrey Khmuro <andrey@polygon.technology> * chore: turn array into utility chore: remove component code * chore: lint, simplify test & comments * chore: remove unnecessary comments * chore: remove duplicated changelog entry * Apply suggestions from code review Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: Andrey Khmuro <andrey@polygon.technology> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
* feat: working array chore: simplify test chore: be explicit about padding * chore: add changelog entry for Array component (#2204) * Initial plan * chore: add changelog entry for PR 2203 Co-authored-by: mmagician <8402446+mmagician@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: mmagician <8402446+mmagician@users.noreply.github.com> * fix: set_map_item no longer returns old root * chore: change Try to TryFrom for AccountComponent * chore: correct the docs * chore: masm doc corrections * chore: use BeWord instead of Word * chore: adjust masm comment about max len * Update crates/miden-standards/asm/account_components/array.masm Co-authored-by: Andrey Khmuro <andrey@polygon.technology> * chore: turn array into utility chore: remove component code * chore: lint, simplify test & comments * chore: remove unnecessary comments * chore: remove duplicated changelog entry * Apply suggestions from code review Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: Andrey Khmuro <andrey@polygon.technology> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
We eventually want to have double-word arrays & double-word queues. Once we agreed on the structure and where the files live, it's going to be trivial to extend to double-word array. A queue will require an extra storage slot to hold the metadata, though.
Features:
[index, 0, 0, 0]set(slot_id_prefix: felt, slot_id_suffix: felt, index: Felt, VALUE: Word) -> OLD_VALUE: Word&get(slot_id_prefix: felt, slot_id_suffix: felt, index: Felt) -> VALUE: WordUsage: