Skip to content
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

Refactor StoragePlaceholder #1109

Closed
igamigo opened this issue Jan 27, 2025 · 8 comments
Closed

Refactor StoragePlaceholder #1109

igamigo opened this issue Jan 27, 2025 · 8 comments
Assignees
Milestone

Comments

@igamigo
Copy link
Collaborator

igamigo commented Jan 27, 2025

For making the component template API more usable, we could add more context to the templating aspect of the storage layout. Specifically, we could move from having generic keys to something more oriented toward describing the "type" or "kind" of data requirements.

I think ideally we'd iterate over all slots that require user input, show slot name/description, and allow user to enter the required values (there may be more than one value needed per slot). What we now call placeholder may need to be treated more like a "type" of the data. For example, we may have 3 slots each expecting Falcon public keys and so all 3 would have type auth::rpo_falcon512 (or something like this) - but the user should be able to provide 3 different public keys.

(Link)

@bobbinth bobbinth added this to the v0.8 milestone Jan 27, 2025
@igamigo
Copy link
Collaborator Author

igamigo commented Jan 29, 2025

@bobbinth some points/questions about this:

  • In your example case for public keys, the CLI could identify these and generate keys automatically so as to not require any other input. Was this the idea you had in mind?
  • How should these be identified in serialized formats? What types do we know we want to support? I can't think of many, other than perhaps a string-like type for things such as a token ticker symbol, although this might not be the direction you had in mind. For arbitrary values that do not fit a more specific type, I guess we would need to add a type for felts, words and maps as well.
  • Do we expect that one value could be used as part of multiple slots? That an "arbitrary" Felt is part of multiple slots is something that is supported with the current StoragePlaceholder type, for instance, but it might not be necessary/possible with the approach you had in mind.

@bobbinth
Copy link
Contributor

In your example case for public keys, the CLI could identify these and generate keys automatically so as to not require any other input. Was this the idea you had in mind?

Not necessarily generate but maybe identify that we need a public key from the user and then give user options to either select one of existing public keys that the client is tracking or generate a new one (we could start simple and then make it more sophisticated).

Going back to my original example, I think we may need to modify it to look something like this:

# simple value slot with the value initialized to 0
[[storage]]
name = "my_slot_1"
description = "some description of what this slot is for"
slot = 0
value = ["0", "0", "0", "0"]

# simple value slot which requires initialization of the first element to a u32 value
[[storage]]
name = "my_slot_2"
slot = 1
value = ["{{u32}}", "0", "0", "0"]

# simple value slot which requires initialization to an RpoFalcon512 public key
[[storage]]
name = "my_slot_3"
slot = 2
value = "{{auth::rpo_falcon512::pub_key}}"

# simple value slot which requires initialization to an RpoFalcon512 public key,
# but this could be a different public key from slot 2
[[storage]]
name = "my_slot_4"
slot = 2
value = "{{auth::rpo_falcon512::pub_key}}"

The last two examples are the tricky ones: somehow we'll need to map auth::rpo_falcon512::pub_key string to the specific type of PublicKey from the rpo_falcon512 module. This mapping could be hard-coded, but maybe there are better ways to do it.

Assuming we can do this mapping, we'll need to make sure this type can be converted into a Word (if it can't be, then a simple value slot would not be suitable for it).

Beyond that, in the CLI we'll need to figure out where to get the data. There are a few options here:

  • This could be a "free-entry" by the user. In this case, we'll need to make sure we can parse the user input (e.g., a string) into the PublicKey struct (maybe by implementing something like TryFrom<String> on it).
  • This could be an optional selection from the Falcon public keys the CLI is already aware of for the user.
  • This could be a suggestion to generate a new Falcon key-value pair.

But I think these are conceptually easy assuming we know that auth::rpo_falcon512::pub_key maps to miden_crypto::dsa::rpo_falcon512::PublicKey.

To summarize: for slots that require inputs, we could have 2 scenarios:

  • "native" input type - e.g., felt, word, u32.
  • "custom" input type - e.g., auth::rpo_falcon512::pub_key.

For custom types, we'll need to figure out how to make the system extensible - but maybe for now we just stat with the hard-coded approach.


Not addressed in this comment is how to handle multi-slot types and map slots. I haven't thought through these too much yet, but I'm hoping the methodology outlined above could also be applied to them.

@igamigo
Copy link
Collaborator Author

igamigo commented Jan 30, 2025

Does this negate InitStorageData? Or at least, the idea of initializing it from something like a TOML file. Or maybe keys could be identified by the slot's name and element index (eg, my_slot.3 = "20") or something, but this probably gets complicated pretty fast for multi-slot values.

I think an alternative that combines both aspects could be one of the following:

Alternative 1: Specifying placeholder info

There could be a [placeholders] table that exposes, by key, some metadata for each placeholder. This would allow for specific descriptions and maybe things such as default values (this implies being able to parse the type from strings). So you could have something like this:

# ...

[[storage]]
name = "token_metadata"
description = "Contains metadata about the token associated to the faucet account"
slot = 1
value = ["{{metadata.max_supply}}","{{metadata.token_symbol}}","{{metadata.decimals}}","0x0"] 

# ...

[placeholders]
metadata.max_supply = { type = "u64",  default_value = "0xFFFFFFFFFFFFFFFF", description = "Total amount of tokens that can be minted over the account's lifetime" }
metadata.token_symbol = { type = "String", description = "..." }
metadata.decimals = { type = "u8", description = "..." }

Alternative 2: Specifying inline placeholder type

We could specify inline types as well if we still wanted to have keyed values:

# ...

[[storage]]
name = "token_metadata"
description = "Contains metadata about the token associated to the faucet account"
slot = 1
value = ["{{metadata.max_supply: u64}}","{{metadata.token_symbol: String}}","{{metadata.decimals: u8}}","0x0"] 

Both of these approaches would also allow for using keys in more than one location, although it might make the templates and validations unnecessarily more complex. In your approach the storage slot description can describe each of the inner templated values, for example, so there might not be a need to extend the format.

As for the type information per se, I think we are interested in knowing to what type the type identifier maps, but also that the type can be converted to Felt and/or Word. There could be a trait for each of these two conversions, maybe with some blanket implementation for types that implement or TryInto<Felt/Word>, but then there is also the parsing from string into the native type. I agree that having a hardcoded approach is a good start, and then later there could be some type registry that maps identifiers to native types that implement TryFrom<&str>.

@bobbinth
Copy link
Contributor

bobbinth commented Jan 30, 2025

Or maybe keys could be identified by the slot's name and element index (eg, my_slot.3 = "20") or something, but this probably gets complicated pretty fast for multi-slot values.

This is kind of what I had in mind - though, I can see how this could be more complicated then we'd like. For multi-slot values though, I was thinking there would be just a single value. Specifically, for simple slots (i.e., not maps), we have 3 ways of how to initialize things:

  1. We want to initialize a specific element.
  2. We want to initialize a full word.
  3. We want to initialize multiple words.

Full word

Full word is the simplest case and it would look like so:

# simple value slot initialize with a hex value
[[storage]]
name = "my_slot_1"
slot = 0
value = "0x123456"

# simple value slot with individually initialized elements
[[storage]]
name = "my_slot_2"
slot = 1
value = [{ value = "1" }, { value =  "2" }, { value = "3" }, { value = "4" }]

# simple value slot which requires user-provided value which can be any word
[[storage]]
name = "my_slot_3"
slot = 2
value = "{{word}}"

# simple value slot which requires user-provided value which should be Falcon public key
[[storage]]
name = "my_slot_4"
slot = 3
value = "{{auth::rpo_falcon512::pub_key}}"

The data file for these could look like:

my_slot_3 = ["5", "6", "7", "8"]  # or could be a single hex encoded string
my_slot_4 = "0x123456789"

Individual elements

For individual elements, I think you are right that my proposal had some limitations. What we probably want to do is provide name/description for individual elements. Maybe something like this could work:

[[storage]]
name = "token_metadata"
description = "Contains metadata about the token associated to the faucet account"
slot = 0
value = [
    { name = "max_supply", description = "Maximum supply of the token in base units", value = "{{felt}}" },
    { name = "symbol", description = "Ticker symbol used to identify the token", value = "{{TokenSymbol}}" },
    { name = "decimals", description = "Number of decimal places", value = "{{u32}}" },
    { value = "0" },
]

The data file for these could look like:

token_metadata.max_supply = "1000000000"
token_metadata.symbol = "ETH"
token_metadata.decimals = "9"

Or maybe even:

[token_metadata]
max_supply = "1000000000"
symbol = "ETH"
decimals = "9"

Multi-word values

For multi-word values, we could provide just one name/description for the whole value. For example:

# A two-word value that can be initialized with any valid words
[[storage]]
name = "some_data"
slots = [0, 1]
value = "{{word}}" # or maybe better use {{any}}?

# A 4-word value where the type should be an ECDSA public key (64 bytes)
[[storage]]
name = "pub_key"
slots = [0, 1, 3, 4]
value = "{{auth::ecdsa::secp256k1::pub_key}}"

The data file for these could look like:

some_data = "0x124567..."
pub_key = """
-----BEGIN PUBLIC KEY-----
MFYwEAYHKoZIzj0CAQYFK4EEAAoDQgAEeYRv0S7Zf5CNh5/APxiT6xoY+z521DHT
FgLdUPUP2e/3jkYDuZTbCHP8zEHm7nhG6AUOpJCbTF2J2vWkC1i3Yg==
-----END PUBLIC KEY-----
"""

@igamigo
Copy link
Collaborator Author

igamigo commented Jan 31, 2025

I think that works but it might have some usability problems. For values that are hardcoded, in your specs it's as if we were assuming the type is felt for pre-defined elements, and word for pre-defined words. So for instance, what if I want to specify a hardcoded TokenSymbol within the TOML specification?

[[storage]]
name = "token_metadata"
description = "Contains metadata about the token associated to the faucet account"
slot = 0
value = [
    # ...
    { name = "symbol", description = "...", value = "0x12345" }, # we can't do `value="TST"` because we can't automatically infer the type
    # ...
]

In your ECDSA example, you could not hardcode the key within the original TOML because you don't know how to parse the string the correct way into a set of words.
So the user would be forced to convert the symbol into a felt and then a decimal/hex representation and paste it in. And this might be fine, but also if we want to make this system extensible, it might not. Although the first approach would likely involve hardcoded matching as you suggested anyway.

@igamigo
Copy link
Collaborator Author

igamigo commented Jan 31, 2025

Something like this could encompass all variants:

[[storage]]
id = "token_metadata"
description = "Contains metadata about the token associated to the faucet account"
slot = 0
value = [
    { type = "felt", id = "max_supply", description = "Maximum supply of the token in base units" }, # placeholder
    { type = "TokenSymbol", value = "TST" }, # hardcoded non-felt type
    { type = "u8", id = "decimals", description = "Number of decimal places" }, # placeholder
    { value = "0" }, # could also be just "0"?  maybe type can be inferred to either felt or word depending on "position"
]


[[storage]]
id = "faucet_pubkey"
description = "Contains the falcon public key"
slot = 1
type = "auth::rpo_falcon512::pub_key" # placeholder

[[storage]]
id = "owner_pubkey"
description = "Contains the ECDSA public key"
slots = [2,3,4,5]
type = "auth::ecdsa::secp256k1::pub_key" # hardcoded non-word type
value = """
-----BEGIN PUBLIC KEY-----
MFYwEAYHKoZIzj0CAQYFK4EEAAoDQgAEeYRv0S7Zf5CNh5/APxiT6xoY+z521DHT
FgLdUPUP2e/3jkYDuZTbCHP8zEHm7nhG6AUOpJCbTF2J2vWkC1i3Yg==
-----END PUBLIC KEY-----
"""

In this example, value not being present means it's a placeholder. But maybe this all is just being too complicated.
In general, I think being as explicit as possible makes it more understandable plus easier to parse and validate.

@bobbinth
Copy link
Contributor

I like this approach! Yes, let's add type to specify the value type and have value field for slots that are initialized in the template.

One question is how to specify "generic" type for multi-slot values. Maybe something like this would work?

[[storage]]
name = "some_data"
slots = [0, 1]
type = "[word; 2]"

Or maybe we should just have any instead of word and felt to basically mean any value that fits into this slot/element.

@igamigo
Copy link
Collaborator Author

igamigo commented Feb 20, 2025

Closed by #1124

@igamigo igamigo closed this as completed Feb 20, 2025
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

No branches or pull requests

2 participants