Skip to content

feature(sdk): High-level account storage API #436

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 28 commits into
base: next
Choose a base branch
from

Conversation

greenhat
Copy link
Contributor

@greenhat greenhat commented Mar 14, 2025

Close #431
Close #471

Note: This PR updates VM to a certain commit on the next branch

This PR adds the component macros that generate the additional code for account storage API and the AccountComponentMetadata which is stored in the compiled Miden package.

The added storage-example project:

#[component]
struct MyAccount {
    #[storage(
        slot(0),
        description = "test value",
        type = "auth::rpo_falcon512::pub_key"
    )]
    owner_public_key: Value,

    #[storage(slot(1), description = "test map")]
    asset_qty_map: StorageMap,
}

// // generated by the `component` and `storage` attribute macros
// impl Default for MyAccount {
//     fn default() -> Self {
//         Self {
//             owner_public_key: Value { slot: 0 },
//             asset_qty_map: StorageMap { slot: 1 },
//         }
//     }
// }

impl foo::Guest for MyAccount {
    fn set_asset_qty(pub_key: Word, asset: CoreAsset, qty: Felt) {
        let my_account = MyAccount::default();
        let owner_key: Word = my_account.owner_public_key.read();
        if pub_key == owner_key {
            my_account.asset_qty_map.write(asset, qty);
        }
    }

    fn get_asset_qty(asset: CoreAsset) -> Felt {
        let my_account = MyAccount::default();
        my_account.asset_qty_map.read(&asset)
    }
}

via https://github.com/0xMiden/compiler/blob/i431-acc-storage-high/examples/storage-example/src/lib.rs#L28-L64

I've ditched the MyAccount.owner_public_key. in favour of Default implementation after @bitwalker showed me that rustc optimizes away all the overhead.
I also made Value and StorageMap polymorphic (see asset passed as Word).

The generated AccountComponentMetadata looks:

name = "storage-example"
description = "A simple example of a Miden account storage API"
version = "0.1.0"
supported-types = []

[[storage]]
name = "owner_public_key"
description = "test value"
slot = 0
type = "auth::rpo_falcon512::pub_key"

[[storage]]
name = "asset_qty_map"
description = "test map"
slot = 1
values = []

see https://github.com/0xMiden/compiler/blob/i431-acc-storage-high/tests/integration/src/rust_masm_tests/examples.rs#L24-L39

@greenhat greenhat force-pushed the greenhat/i430-acc-storage-low branch 2 times, most recently from d81e4d7 to 599ce97 Compare March 24, 2025 08:33
Base automatically changed from greenhat/i430-acc-storage-low to next March 24, 2025 18:30
@greenhat greenhat force-pushed the i431-acc-storage-high branch from fa04e2c to 411a3a5 Compare March 25, 2025 15:41
@greenhat greenhat changed the base branch from next to greenhat/i438-remove-old-frontend March 25, 2025 15:42
@greenhat greenhat force-pushed the i431-acc-storage-high branch from 411a3a5 to c086c49 Compare March 25, 2025 15:55
Base automatically changed from greenhat/i438-remove-old-frontend to next March 25, 2025 15:55
@greenhat
Copy link
Contributor Author

@bobbinth @bitwalker
So far, the DX I came up with is captured in the storage-example (see the _high suffixed methods) - https://github.com/0xPolygonMiden/compiler/blob/i431-acc-storage-high/examples/storage-example/src/lib.rs#L28-L77

The #[component] attribute macros generates the MyAccount instance with the same name as the type itself (see the commented const). I'm still on the fence about the instance name. We have to have the instance somewhere, but using the same name as the type might seem like "far too magic" for the users. Alternatively, we can put the instance as:

impl MyAccount {
  const instance: Self = ...
}

// and use it like

MyAccount::instance.owner_public_key.write(value);

Another option is to ditch the instance entirely and rely on exposing storage fields as static methods:

MyAccount::owner_public_key().write(value);

But it introduces confusion. Meaning the user defines a struct field but accesses it through the static method.

@greenhat greenhat force-pushed the i431-acc-storage-high branch from c086c49 to 4b95dc4 Compare March 26, 2025 10:33
@bobbinth
Copy link
Contributor

The #[component] attribute macros generates the MyAccount instance with the same name as the type itself (see the commented const).

Question: do we actually need the constant? For example, if we didn't generate the constant, could we do something like:

impl MyAccount {
    pub fn set_key(&self, key: Word) {
        self.owner_public_key.write(key);
    }
}

// maybe a note script in the future
pub fn foo(account: MyAccount) {
    account.foo(Word::default());
}

@greenhat
Copy link
Contributor Author

The #[component] attribute macros generates the MyAccount instance with the same name as the type itself (see the commented const).

Question: do we actually need the constant? For example, if we didn't generate the constant, could we do something like:

impl MyAccount {
    pub fn set_key(&self, key: Word) {
        self.owner_public_key.write(key);
    }
}

// maybe a note script in the future
pub fn foo(account: MyAccount) {
    account.foo(Word::default());
}

set_key defined as an interface function in WIT, so it cannot have self. In Rust code, we're implementing the WIT interface (as a trait) generated by wit-bindgen from the WIT interface file. We'd probably need to fork it and maintain it ourselves. I'd rather leave it as a last resort since it's quite actively developed and patching from upstream would require some effort.

For foo to work, we'd need to define account parameter in WIT somehow, since WIT function cannot have an interface as a parameter. If we introduce some token type for the account, we probably confuse the developer, and then we would have to recognize and erase/transform its use during our compilation. We are essentially trying to pass a note script compilation dependency (account) at runtime. I don't think we would be able to do it nicely. Plus, note script can have multiple accounts as dependencies, which complicates it even more.

@bitwalker I also tried to define MyAccount as a Wasm resource in the WIT but was quickly discouraged by the amount of glue rust code generated. If we figure out how to erase it and nicely materialize the account resource, it might be a promising direction.

@greenhat greenhat force-pushed the i431-acc-storage-high branch from 4b95dc4 to 7e965ab Compare March 27, 2025 13:28
@bobbinth
Copy link
Contributor

set_key defined as an interface function in WIT, so it cannot have self. In Rust code, we're implementing the WIT interface (as a trait) generated by wit-bindgen from the WIT interface file. We'd probably need to fork it and maintain it ourselves. I'd rather leave it as a last resort since it's quite actively developed and patching from upstream would require some effort.

What would be the approach to define a new procedures on an account component? For example, for the BasicWallet component we have 3 procedures (receive_asset, create_note, and move_asset_to_note). These don't access storage, but if they did - how would they be expressed in Rust?

For foo to work, we'd need to define account parameter in WIT somehow, since WIT function cannot have an interface as a parameter.

How would we write a function which could manipulate an account component via its interface (i.e., this could be a note script, tx script, or for foreign account invocation). I think going through traits is the right approach, but would it look something like this?:

// MyAccount trait somehow is defined from WIT such that it has foo() function

/// entrypoint of a note
pub fn execute() {
    MyAccount::foo(Word::default());
}

I think we may even have this somewhere already, but how something like a P2ID note script could look like in Rust?

@greenhat
Copy link
Contributor Author

greenhat commented Mar 28, 2025

set_key defined as an interface function in WIT, so it cannot have self. In Rust code, we're implementing the WIT interface (as a trait) generated by wit-bindgen from the WIT interface file. We'd probably need to fork it and maintain it ourselves. I'd rather leave it as a last resort since it's quite actively developed and patching from upstream would require some effort.

What would be the approach to define a new procedures on an account component? For example, for the BasicWallet component we have 3 procedures (receive_asset, create_note, and move_asset_to_note). These don't access storage, but if they did - how would they be expressed in Rust?

So far, my plan is to do it like this:

use miden::{
    blake3_hash_1to1, component, felt, CoreAsset, Felt, NoteType, Recipient, StorageMap,
    StorageMapAccess, Tag, Value, ValueAccess,
};

#[component]
struct MyAccount {
    #[storage(
        slot(0),
        description = "test value",
        type = "auth::rpo_falcon512::pub_key"
    )]
    owner_public_key: Value,

    #[storage(slot(1), description = "test map")]
    foo_map: StorageMap,
}

impl my_account::Guest for MyAccount {
    fn receive_asset(asset: CoreAsset) {
        miden::account::add_asset(asset);

        let mut some_status = MyAccount.foo_map.read(asset.into());
        // imagine updating some_status here
        let _ = MyAccount.foo_map.write(asset.into(), some_status);
    }

    fn send_asset(asset: CoreAsset, tag: Tag, note_type: NoteType, recipient: Recipient) {
        let owner = MyAccount.owner_public_key.read();
        // imagine some checks of the owner key here

        let asset = miden::account::remove_asset(asset);
        miden::tx::create_note(asset, tag, note_type, recipient);
    }
}

With WIT interface defined as:

interface my-account {
    use core-types.{core-asset, tag, recipient, note-type, felt};

    receive-asset: func(core-asset: core-asset);
    send-asset: func(core-asset: core-asset, tag: tag, note-type: note-type, recipient: recipient);
}

For foo to work, we'd need to define account parameter in WIT somehow, since WIT function cannot have an interface as a parameter.

How would we write a function which could manipulate an account component via its interface (i.e., this could be a note script, tx script, or for foreign account invocation). I think going through traits is the right approach, but would it look something like this?:

// MyAccount trait somehow is defined from WIT such that it has foo() function

/// entrypoint of a note
pub fn execute() {
    MyAccount::foo(Word::default());
}

I think we may even have this somewhere already, but how something like a P2ID note script could look like in Rust?

The P2ID script that calls the above account looks like this:

use bindings::{
    exports::miden::base::note_script::Guest,
    miden::my_account::{aux::process_list_felt, my_account::receive_asset},
};
use miden::*;

struct MyNote;

impl Guest for MyNote {
    fn note_script() {
        let inputs = miden::note::get_inputs();

        let target_account_id_felt = inputs[0];
        let account_id = miden::account::get_id();
        assert_eq(account_id.as_felt(), target_account_id_felt);
        let assets = miden::note::get_assets();
        for asset in assets {
            receive_asset(asset);
        }
    }
}

via our test at https://github.com/0xPolygonMiden/compiler/blob/i431-acc-storage-high/tests/rust-apps-wasm/rust-sdk/p2id-note/src/lib.rs#L24-L47

EDIT: I renamed basic_wallet to my_account

@bobbinth
Copy link
Contributor

Thank you! This looks good! I think once this is working, we can consider some next steps to make it even more ergonomic. For example, we could have something like:

#[note_script]
pub fn main() {
    // note script content
}

And it would get transformed into

struct MyNote;
impl Guest for MyNote {
    fn note_script() {
        // note script content
    }
}

And similarly, for accounts something like:

#[component]
impl MyAccount {
    fn receive_asset(&self, asset: CoreAsset) {
        self.add_asset(asset);

        let mut some_status = self.foo_map.read(asset.into());
        // imagine updating some_status here
        let _ = self.foo_map.write(asset.into(), some_status);
    }

    fn send_asset(&self, asset: CoreAsset, tag: Tag, note_type: NoteType, recipient: Recipient) {
        let owner = self.owner_public_key.read();
        // imagine some checks of the owner key here

        let asset = self.remove_asset(asset);
        miden::tx::create_note(asset, tag, note_type, recipient);
    }
}

Would get transformed into the code snippet that you have above.

Not sure what from this is possible (or even desirable), but I think once the basic things are working, we can improve ergonomics one step at a time.

@greenhat
Copy link
Contributor Author

greenhat commented Apr 2, 2025

@bobbinth @bitwalker I'm wrapping up this PR and here is how the Rust code looks:

#[component]
struct MyAccount {
    #[storage(
        slot(0),
        description = "test value",
        type = "auth::rpo_falcon512::pub_key"
    )]
    owner_public_key: Value,

    #[storage(slot(1), description = "test map")]
    asset_qty_map: StorageMap,
}

// // generated by the `component` and `storage` attribute macros
// impl Default for MyAccount {
//     fn default() -> Self {
//         Self {
//             owner_public_key: Value { slot: 0 },
//             asset_qty_map: StorageMap { slot: 1 },
//         }
//     }
// }

impl foo::Guest for MyAccount {
    fn set_asset_qty(pub_key: Word, asset: CoreAsset, qty: Felt) {
        let my_account = MyAccount::default();
        let owner_key: Word = my_account.owner_public_key.read();
        if pub_key == owner_key {
            my_account.asset_qty_map.write(asset, qty);
        }
    }

    fn get_asset_qty(asset: CoreAsset) -> Felt {
        let my_account = MyAccount::default();
        my_account.asset_qty_map.read(&asset)
    }
}

via https://github.com/0xMiden/compiler/blob/i431-acc-storage-high/examples/storage-example/src/lib.rs#L28-L64

I've ditched the MyAccount.owner_public_key. in favour of Default implementation after @bitwalker showed me that rustc optimizes away all the overhead.
I also made Value and StorageMap polymorphic (see asset passed as Word).

The generated AccountComponentMetadata looks:

name = "storage-example"
description = "A simple example of a Miden account storage API"
version = "0.1.0"
supported-types = []

[[storage]]
name = "owner_public_key"
description = "test value"
slot = 0
type = "auth::rpo_falcon512::pub_key"

[[storage]]
name = "asset_qty_map"
description = "test map"
slot = 1
values = []

see https://github.com/0xMiden/compiler/blob/i431-acc-storage-high/tests/integration/src/rust_masm_tests/examples.rs#L24-L39

@greenhat greenhat marked this pull request as ready for review April 2, 2025 14:51
@greenhat greenhat requested a review from bitwalker April 2, 2025 14:51
@greenhat
Copy link
Contributor Author

greenhat commented Apr 2, 2025

All the unimplemented parts I made into sub-issues in #340

@bobbinth
Copy link
Contributor

bobbinth commented Apr 3, 2025

Looks very good for the initial approach!

A couple of small comments (which can be addressed in subsequent PRs):

  • For StorageMap we could probably use get() and set() methods instead of read() and write().
  • Is the a reason to name asset CoreAsset rather than just Asset?

In the longer run, we'll need to try to abstract away some of the extra boilerplate (e.g., so that the user can use self instead of MyAccount::default() and would not need to do foo::Guest etc.).

Also, cc @igamigo in case he has any comments on the storage metadata front.

@greenhat
Copy link
Contributor Author

greenhat commented Apr 3, 2025

  • For StorageMap we could probably use get() and set() methods instead of read() and write().

Agreed.

  • Is the a reason to name asset CoreAsset rather than just Asset?

Great question! CoreAsset is the type from the low-level API - the thin wrapper around Word. I reserved the name Asset for the high-level API type. However, I completely missed the fact that this is our first high-level API. :) So, yeah, it's time to make the Asset type and use it here.

@greenhat greenhat marked this pull request as draft April 3, 2025 12:19
greenhat and others added 24 commits April 18, 2025 18:06
only and to represent more realistic account code.
needs a Cargo.toml with custom Miden fields.
support in `component` attribute macro parsed from Cargo.toml.
@greenhat greenhat force-pushed the i431-acc-storage-high branch from 37bb8ab to 117bfe4 Compare April 18, 2025 15:21
@greenhat greenhat marked this pull request as ready for review April 18, 2025 15:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants