Instantiate accounts from account components

[bobbinth]

Currently, new accounts are crated using Account::new() constructor which looks like so:

impl Account {
    pub fn new(
        seed: Word,
        code: AccountCode,
        storage: AccountStorage,
    ) -> Result<Self, AccountError> {
        ...
    }
}

Here, the assumption is that AccountCode and AccountStorage have been constructed to be in-sync with each other. This creates some complications. For example, when constructing AccountCode, we need to know how to assign storage offsets to procedures - and currently, we don't have a way to do this (so, right now the we just have placeholder code).

One way to improve the structure is to instantiate accounts from AccountComponent's (initially discussed in 0xPolygonMiden/compiler#333 (reply in thread)). This could look like so:

impl Account {
    pub fn new(
        seed: Word,
        components: AccountComponent,
    ) -> Result<Self, AccountError> {
        // instantiate AccountCode from components
        // combine all storage slots from components and instantiate AccountStorage
        ...
    }
}

pub struct AccountComponent {
    code: Library, // or maybe just MastForest
    storage: Vec<StorageSlot>,
}

The AccountCode constructor can then be modified to look like so:

impl AccountCode {
    pub fn new(components: &[AccountComponent], storage_offset: u8) -> Result<Self, AccountError> {
        /// combine all component libraries into a single library, apply storage offsets etc.
    }
}

In the above, when initializing regular account code, storage_offset would be set to 0, but for faucets, it would be set to 1.

Once the above is implemented, we would also be able to remove some temporary code from the transaction kernel (e.g., here).

This change will also cover most of #550.

[PhilippGackstatter]

1. At first I thought that the storage_size of any procedure associated with a certain module is the number of storage slots that component may access, e.g. storage.len().

So say we have two components:

• Component A with slots: [Value, Value].
• Component B with slots: [Value, Value, Value].

All AccountProcedureInfo's of component A would be instantiated with

• storage_offset: 0
• storage_size: 2
  i.e. it can access slots 0 and 1.

All AccountProcedureInfo's of component B would be instantiated with

• storage_offset: 2
• storage_size: 3
  i.e. it can access slots 2, 3 and 4.

But judging from this comment it seems the we'd want to specify not only the offest but also the size for a procedure? Can you clarify which one of these models we need?

2. You suggested this snippet:

pub fn new(components: &[AccountComponent], storage_offset: u8) -> Result<Self, AccountError> {

The storage_offset here is just to differentiate between regular and faucet accounts and not used for anything else? If so, I would rather use an enum here, e.g. AccountCode::new(&[...], AccountType::FungibleFaucet) which is more easily readable at the call-site and we can hide the mapping to 0 or 1 as an implementation detail in the new function.

3. The way I understand it is that the storage offsets are implemented by mapping the MAST root of a procedure to its procedure info consisting of its storage offset and storage size. One question I have in regards to this is whether this would cause problems if two procedures from different components have the same code and thus the same MAST root.
   For example, if component A and B both happen to have the same "helper procedure", e.g. like this (which doesn't seem unlikely to me):

miden-base/miden-tx/src/tests/kernel_tests/test_account.rs

Lines 470 to 475 in 0d28607

	export.foo_write
	push.1.2.3.4.0
	exec.account::set_item
	dropw dropw
	end

Then they would have the same MAST root. And during MAST Forest merging, we would deduplicate them and we'd only be able to set one storage offset/size for this procedure and one component would access the wrong slot as a consequence.
I couldn't find it immediately in 0xPolygonMiden/miden-vm#1510 but I'm guessing that the annotations do not affect the MAST root, so even when specified in MASM code, these two would be the same:

use.miden::account

@storage_offset(1)
export.foo_write
    push.5.6.7.8.0
    exec.account::set_item

    dropw dropw
end

@storage_offset(0)
export.bar_write
    push.5.6.7.8.0
    exec.account::set_item

    dropw dropw
end

[bobbinth]

But judging from this comment it seems the we'd want to specify not only the offest but also the size for a procedure? Can you clarify which one of these models we need?

The description in this issue supersedes the approach with procedure decorators. So, the description you have with components A and B is exactly correct.

The storage_offset here is just to differentiate between regular and faucet accounts and not used for anything else? If so, I would rather use an enum here, e.g. AccountCode::new(&[...], AccountType::FungibleFaucet) which is more easily readable at the call-site and we can hide the mapping to 0 or 1 as an implementation detail in the new function.

Yep - I'm fine with this approach too.

The way I understand it is that the storage offsets are implemented by mapping the MAST root of a procedure to its procedure info consisting of its storage offset and storage size. One question I have in regards to this is whether this would cause problems if two procedures from different components have the same code and thus the same MAST root.
For example, if component A and B both happen to have the same "helper procedure", e.g. like this (which doesn't seem unlikely to me):

This should not be a problem as only procedures in the public interface would get the offset (helper procedures do not get their own ProcedureInfo). Thus, the storage offset will be set at the time when a public procedure is called and so the helper procedures will automatically use the offsets of their corresponding "entrypoint" public procedures.

There could be a problem, however, if two components expose exactly the same public procedure. But I think this should probably be detected and treated as an error.

[greenhat]

Sounds good! With Vec<StorageSlotType> in the Miden package, plus some "storage initializers" on the side (miden-client?) we will get Vec<StorageSlot>.

[bobbinth]

"storage initializers" on the side (miden-client?) we will get Vec<StorageSlot>.

I think where "storage initializers" should go is still an open question. I don't have a great solution for this yet.

[bobbinth]

Closed by #941.