Closed bobbinth closed 2 weeks ago
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:
A
with slots: [Value, Value]
.B
with slots: [Value, Value, Value]
.All AccountProcedureInfo
's of component A
would be instantiated with
storage_offset
: 0storage_size
: 2
i.e. it can access slots 0 and 1.All AccountProcedureInfo
's of component B
would be instantiated with
storage_offset
: 2storage_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?
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.
A
and B
both happen to have the same "helper procedure", e.g. like this (which doesn't seem unlikely to me):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 https://github.com/0xPolygonMiden/miden-vm/pull/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
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
andB
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.
Sounds good! With Vec<StorageSlotType>
in the Miden package, plus some "storage initializers" on the side (miden-client?) we will get Vec<StorageSlot>
.
"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.
Closed by #941.
Currently, new accounts are crated using
Account::new()
constructor which looks like so:Here, the assumption is that
AccountCode
andAccountStorage
have been constructed to be in-sync with each other. This creates some complications. For example, when constructingAccountCode
, 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 https://github.com/0xPolygonMiden/compiler/discussions/333#discussioncomment-10767808). This could look like so:The
AccountCode
constructor can then be modified to look like so:In the above, when initializing regular account code,
storage_offset
would be set to0
, but for faucets, it would be set to1
.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 https://github.com/0xPolygonMiden/miden-base/issues/550.