Pass &mut context to Storage::init and store a mutable ref to context in all state variables.

[iAmMichaelConnor]

This will clean up some of the noir contract syntax. You won't need to pass the &mut context to every method of a state variable, if each state variable has as a data member context: &mut Context.

[iAmMichaelConnor]

Note: this issue will be slightly fiddlier once #1890 is worked on (because modifications would be required to the noir macro code)

[benesjan]

I just tried implementing this and an issue arised in unconstrained functions where we don't have context available.

I can think of 4 options how to tackle this:

1. Separating the readonly/unconstrained and state modifying functionality into separate structs. This would result in us having Set and ReadonlySet etc. This would make the code of aztec library messier so I am not sure if this is a good idea. The private contract storage would then look like this:

struct Storage {
    balances: Map<Set<ValueNote, VALUE_NOTE_LEN>>,
}

struct ReadonlyStorage {
    balances: ReadonlyMap<ReadonlySet<ValueNote, VALUE_NOTE_LEN>>,
}

impl Storage {
    fn init(context: &mut PrivateContext) -> Self {
        Storage {
            balances: Map::new(context, 1, |context, slot| Set::new(context, slot, ValueNoteMethods)),
        }
    }
}

impl ReadonlyStorage {
    fn init() -> Self {
        ReadonlyStorage {
            balances:Readonly Map::new(1, |slot| ReadonlySet::new(slot, ValueNoteMethods)),
        }
    }
}

DISGUSTING🤮🤮🤮🤮🤮🤮🤮🤮
2. Create a new method Storage::init_unconstrained() then somehow mock a dummy context and then pass that to Storage::init(...). This seems incorrect and and I am not sure if we can somehow easily create the mock.
3. Move to unconstrained functionality out of the state var structs into standalone functions. E.g. create a derive_storage_slot function and call it from map here and from contract's unconstrained functions directly. Also 🤮🤮🤮🤮.
4. Give up on this issue.

@LeilaWang @iAmMichaelConnor @spalladino what does everybody think?

[LeilaWang]

Thanks for the detailed writeup! And somehow it's very fun reading your comments haha.

This actually is an issue for all types of functions. Because some apis in Set take PrivateContext, but one of them takes PublicContext.
We can pass Option and Option to storage. And initialise the state variables with optional context(s). And when a method of a state variable is called, we can just unwrap the optional context. If the context is none, it means it's not being called from the correct type of function and unwrapping it will fail.

[benesjan]

That sounds like a great approach @LeilaWang. I will give it a try. Thank you for the feedback

[Maddiaa0]

Because some apis in Set take PrivateContext, but one of them takes PublicContext.

Will this be nicely solved with the incoming implementation of traits?

[spalladino]

Will this be nicely solved with the incoming implementation of traits?

Yep. We'll still need to use an Option though, since some methods do not require a context at all, but at least we'll be relying on a single Context.

How "incoming" is traits, btw...?