Skip to content

Conversation

@mmagician
Copy link
Collaborator

@mmagician mmagician commented Dec 19, 2025

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:

  • add a simple wrapper over storage map, s.t. the map keys are [index, 0, 0, 0]
  • export procedures 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: Word
  • tests that show how other account components have an easy interface for operating on the array

Usage:

use miden::core::word
use miden::standards::data_structures::array

const ARRAY_SLOT = word("test::array::data")

#! Wrapper for array::get that uses exec internally.
#! Inputs:  [index, pad(15)]
#! Outputs: [VALUE, pad(12)]
pub proc test_get
    push.ARRAY_SLOT[0..2]
    exec.array::get
end

#! Wrapper for array::set that uses exec internally.
#! Inputs:  [index, VALUE, pad(11)]
#! Outputs: [OLD_VALUE, pad(12)]
pub proc test_set
    push.ARRAY_SLOT[0..2]
    exec.array::set
end
let data_slot = StorageSlotName::new("test::array::data").unwrap();

// Create the wrapper account component
let wrapper_component = AccountComponent::new(
    wrapper_library.clone(),
    vec![StorageSlot::with_map(
        data_slot.clone(),
        StorageMap::with_entries([(
            Word::from([Felt::ZERO, Felt::ZERO, Felt::ZERO, Felt::ZERO]),
            Word::from([42u32, 42, 42, 42]),
        )])?,
    )],
)?
.with_supports_all_types();

@mmagician mmagician force-pushed the mmagician-array-via-map branch from 505aa25 to 5479b77 Compare December 19, 2025 15:35
@mmagician
Copy link
Collaborator Author

@copilot add change log entry for this PR 2203

Copy link
Contributor

Copilot AI commented Dec 19, 2025

@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.

@mmagician mmagician force-pushed the mmagician-array-via-map branch 2 times, most recently from b5f4cdb to e708a51 Compare January 5, 2026 10:33
@mmagician mmagician requested a review from Copilot January 5, 2026 10:33
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 Array struct with methods for creating arrays with initial elements or from slices
  • Implemented MASM procedures (get and set) 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.

@mmagician mmagician marked this pull request as ready for review January 5, 2026 12:35
@mmagician mmagician requested a review from Copilot January 5, 2026 12:35
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

partylikeits1983
partylikeits1983 approved these changes Jan 5, 2026
@partylikeits1983
Copy link
Contributor

Looks great! This is a great feature!

When reviewing the implementation, it reminded me of this issue for KEY => vec![] issue I opened: #2085

Copy link
Contributor

@Fumuran Fumuran left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Aside from the decision of whether we need to make this component a utility, I have just a small style nit inline.

mmagician and others added 8 commits January 17, 2026 10:02
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>
mmagician and others added 2 commits January 17, 2026 10:04
Co-authored-by: Andrey Khmuro <andrey@polygon.technology>
chore: remove component code
@mmagician mmagician force-pushed the mmagician-array-via-map branch from 34ccf83 to 6bd6069 Compare January 17, 2026 10:04
@mmagician mmagician changed the title feat: Single-word Array component feat: Single-word Array abstraction Jan 17, 2026
@mmagician mmagician requested a review from bobbinth January 17, 2026 10:53
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>
Copy link
Contributor

@PhilippGackstatter PhilippGackstatter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! Sorry for the late review, it slipped through my notifications.

let wrapper_component_code = format!(
r#"
use miden::core::word
use miden::standards::data_structures::array
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we end up introducing a "storage array" (#2176 or #383) next to the "storage map", naming this "array" may be confusing. First question is whether we would keep this data structure if we'd add a protocol-level "array" and if so, maybe it should be called smt_array or something to differentiate it from "storage array".

Nit: I think miden::standards::(smt_)array would also be sufficient (motivation).

Copy link
Collaborator Author

@mmagician mmagician Jan 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First question is whether we would keep this data structure if we'd add a protocol-level "array"

no, probably we'd not keep both around, but for now we need an array concept (really the double-word array) for the AggLayer integration.

I would avoid smt_array, because the underlying data structure should not matter to the user (as long as this is the only array impl. we have).

So I'd leave this as-if for now, and remove if/when storage arrays land. Do you think we could merge with the current naming & module placement?

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.

6 participants