Together or not together

[iFrostizz]

I spec'ed an extension of the state_keys attribute macro to be able to generate getter functions for the chosen state keys as part of #944
As the current state of this PR, we would need to write

/// The program state keys.
#[state_keys]
pub enum StateKeys {
    /// The total supply of the token. Key prefix 0x0.
    #[state_keys(getter = i64)]
    TotalSupply,
    /// The name of the token. Key prefix 0x1.
    Name,
    /// The symbol of the token. Key prefix 0x2.
    Symbol,
    /// The balance of the token by address. Key prefix 0x3 + address.
    Balance(Address),
}

And a method would be automatically expanded with

#[public]
pub fn total_supply(Context { program, .. }: Context<StateKeys>) -> i64 {
    program
        .state()
        .get(StateKeys::TotalSupply)
        .expect("failed to get total supply")
}

The look of the macro may still be a bit rough on the edges as pointed by @richardpringle on #944 (comment) which is also why this discussion was opened.
We should also decide wether or not we want this feature to land on main, since we may have longer term plans regarding how we threat storage retrieval. For instance, the current design of state keys may change because we need to specify the const generic value of the Context (which is in most cases the state keys).

[richardpringle]

I've got changes coming that might break this a bit (mainly that expect).

Ignoring the above fact for a second, one of the things I don't like about this proposal is that it's caught between being strict and loose with what's stored at the specified state-key.

I could do this:

#[state_keys]
pub enum StateKeys {
    #[state_keys(getter = u64)] // u64 here
    TotalSupply,
}

#[public]
pub fn get_total_supply(Context { program, .. }: Context<StateKeys>) -> i64 { // but this is an i64
    program
        .state()
        .get(StateKeys::TotalSupply)
        .expect("failed to get total supply")
}

I'm generally in favour of making things more strict, but regardless, it shouldn't be somewhere in the middle.

I do like pre-defined getters and setters, but maybe, instead of an enum, each key gets its own type. If we defined types as something like I suggested in #944 (comment) (but a little different):

state_schema! {
    MyUnitStruct => i64,
    MyTupleStruct(Address) => String,
}

we could output separate types for keys and values such that calling get looks like the following:

state.get(state_keys::MyUnitStruct) // Result<i64, StateError>
state.get(state_keys::MyTupleStruct(Address)) // Result<String, StateError>

Of course, this would necessarily change how we do caching, but there are several ways that could be accomplished.

[richardpringle]

This is effectively like defining our own DSL for defining the state-schema.

[iFrostizz]

I agree with your first example, although it might be off if I understand correctly because the getter"variable" inside the attribute macro only sets the type of the return type in the total_supply function and doesn't really change the semantics of the expanded state keys. Meaning that in this case, we could just write #[state_keys(getter = i64)].

There is another example that couldn't be covered by such a simplistic macro

#[state_keys]
pub enum StateKeys {
    #[state_keys(getter = u64)]
    Balance(Address),
}

Which, under no specific rule, expands to:

#[public]
pub fn balance(Context { program, .. }: Context<StateKeys>, address: Address) ->u64 {
    program
        .state()
        .get(StateKeys::Address(address))
        .expect("failed to get balance")
}

In this case, we probably would want to replace the .expect("...") by a .unwrap_or_default() by writing #[state_keys(getter = u64, default)] because otherwise the program would panic if the user never had any of these tokens.

This macro is probably not meant to be bent too much in order to satisfy every use-case. This is just about having a macro that generates a simple getter method. Overcomplicating it with tons of options may not be a good idea, though if we cannot think about other of these 2 cases it might be something to do.

[richardpringle]

I like the getter for its convenience, but the thing I don't like about the getter is that it implies the type stored at that key without enforcing it.

I think no matter the direction we decide, we should go all the way: either no implied value type and we let the user do as they will, or we define a full schema and enforce it. It's the grey zones between the extremes that cause the most problems.

[patrick-ogrady]

If we can do this (feel like getting/setting "global vars"), it would be super cool.

Get:

if state_keys::MyTupleStruct(Address) == XXX {
  ...
}

OR

if state_keys::MyTupleStruct(Address).Get() == XXX {
  ...
}

Set:

state_keys::MyBalance(Address) = 10

OR

state_keys::MyBalance(Address).Set(10)

[richardpringle]

So far, this seems pretty straightforward to implement with one exception, composition. How should we compose state-keys such that code that requires state-access can be pulled into a different program.

The classic example is ownership.

state_schema! {
    pub OwnerKey => Address,
}

fn is_owner(context: &Context) -> bool {
    let Some(owner) = context.program().state().get(OwnerKey) else {
        return false;
    };

    owner == context.actor()
}

(or something along those lines)

The state-key here is OwnerKey, but how might I pull this in to multiple different programs?

I could do something like:

use owner_lib::OwnerKey;

state_schema! {
    MyKey => MyValue,
    OwnerKey,
}

#[public]
pub fn do_something(ctx: Context) {
    if !is_owner(&ctx) {
        return;
    }

    // do something 
}

OR

we force composable code to use traits

///! `owned/lib.rs` 
use wasmlanche_sdk::{Address, Context};

pub trait OwnerKey: Schema<Value = Address> {}

pub fn is_owner<Key: OwnerKey>(context: &Context, owner_key: Key) -> bool {
    let Some(owner): Option<Address> = context.program().state().get(owner_key) else {
        return false;
    };

    owner == context.actor()
}

Where you use the trait as follows:

use owned::{is_owner, OwnerKey};

state_schema! {
    Owner => Address,
}

impl OwnerKey for Owner {}

#[public]
pub fn do_something(ctx: Context) {
    if !is_owner::<Owner>(&ctx) {
        return;
    }

    // do something 
}

[richardpringle]

Or... we just don't compose state-keys at all?

[iFrostizz]

Both solutions are looking great, but if they are feasible, my preference would be to keep the first one because it is a bit less verbose. It may not be possible to "hide" the trait implementation for the 2nd case and in the case of very big programs, the state keys implementations may look at bit overwhelming.

On the other side, it seems odd to me that we need to import every single key. For instance, what would happen if we forget to import a key while calling a function ? It probably depends on what the expanded code does, but how would it resolves the key and knows that it has been "called" from the top program ? That may mean that implementors need to read the library source code and lookup for these state keys, another question that comes to my mind is that if the library itself is importing another library, would the top-level program have to specify all the "low-level" state keys of the first library ?

Likewise, wouldn't it make more sense to be able to "pack" the state keys to be able to import them all, or would that make the program too loosely defined ?

state_schema! {
    pub OwnerKey => Address,
}

fn is_owner(context: &Context) -> bool {
    let Some(owner) = context.program().state().get(OwnerKey) else {
        return false;
    };

    owner == context.actor()
}

use owner_lib::OwnerLibStateKeys;

state_schema! {
    MyKey => MyValue,
    OwnerLibStateKeys, // this could resolve to a namespaced state keys pack
}

#[public]
pub fn do_something(ctx: Context) {
    if !is_owner(&ctx) {
        return;
    }

    // do something 
}