Storage slot allocation for SharedMutable #5492
Labels
S-blocked
Status: Blocked
Comments
nventuro
added a commit
that referenced
this issue
Apr 10, 2024
(Large) part of #4761. This is an initial implementation of `SharedMutableStorage`, with some limitations. I think those are best worked on in follow-up PRs, once we have the bones working. The bulk of the SharedMutable pattern is in `ScheduledValueChange`, a pure Noir struct that has all of the block number related logic. `SharedMutable` then makes a state variable out of that struct, adding public storage access both in public and private (via historical reads - see #5379), and using the new `request_max_block_number` function (from #5251). I made an effort to test as much as I could of these in Noir, with partial success in the case of `SharedMutable` due to lack of certain features, notably noir-lang/noir#4652. There is also an end-to-end test that goes through two scheuled value changes, showing that scheduled values do not affect the current one. I added some inline docs but didn't include proper docsite pages yet so that we can discuss the implementation, API, etc., and make e.g. renamings less troublesome. ### Notable implementation details I chose to make the delay a type parameter instead of a value mostly because of two reasons: - it lets us nicely serialize and deserialize `ScheduledValueChange` without including this field (which we are not currently interested in storing) - it lets us declare a state variable of type `SharedMutable<T, DELAY>` without having to change the signature of the `new` function, which is automatically injected by the macro. Overall I think this is fine, especially since we may later make the delay mutable (see below), but still worth noting. Additionally, I created a simple `public_storage` module to get slightly nicer API and encapsulation. This highlighted a Noir issue (noir-lang/noir#4633), which currently only affects public historical reads but will also affect current reads once we migrate to using the AVM opcodes. ### Future work - #5491 - #5492 (this takes care of padding during storage slot allocation) - #5501 - #5493 --------- Co-authored-by: Jan Beneš <[email protected]>
AztecBot
pushed a commit
to AztecProtocol/aztec-nr
that referenced
this issue
Apr 11, 2024
(Large) part of AztecProtocol/aztec-packages#4761. This is an initial implementation of `SharedMutableStorage`, with some limitations. I think those are best worked on in follow-up PRs, once we have the bones working. The bulk of the SharedMutable pattern is in `ScheduledValueChange`, a pure Noir struct that has all of the block number related logic. `SharedMutable` then makes a state variable out of that struct, adding public storage access both in public and private (via historical reads - see #5379), and using the new `request_max_block_number` function (from #5251). I made an effort to test as much as I could of these in Noir, with partial success in the case of `SharedMutable` due to lack of certain features, notably noir-lang/noir#4652. There is also an end-to-end test that goes through two scheuled value changes, showing that scheduled values do not affect the current one. I added some inline docs but didn't include proper docsite pages yet so that we can discuss the implementation, API, etc., and make e.g. renamings less troublesome. ### Notable implementation details I chose to make the delay a type parameter instead of a value mostly because of two reasons: - it lets us nicely serialize and deserialize `ScheduledValueChange` without including this field (which we are not currently interested in storing) - it lets us declare a state variable of type `SharedMutable<T, DELAY>` without having to change the signature of the `new` function, which is automatically injected by the macro. Overall I think this is fine, especially since we may later make the delay mutable (see below), but still worth noting. Additionally, I created a simple `public_storage` module to get slightly nicer API and encapsulation. This highlighted a Noir issue (noir-lang/noir#4633), which currently only affects public historical reads but will also affect current reads once we migrate to using the AVM opcodes. ### Future work - AztecProtocol/aztec-packages#5491 - AztecProtocol/aztec-packages#5492 (this takes care of padding during storage slot allocation) - AztecProtocol/aztec-packages#5501 - AztecProtocol/aztec-packages#5493 --------- Co-authored-by: Jan Beneš <[email protected]>
nventuro
changed the title
|
We likely won't make SharedMutable serializable but instead implement #5736. Once noir also implements noir-lang/noir#4784 we should be able to then do the following and have automatic allocation: impl<T> Storage<2 * N + 1> for SharedMutable<T> where T: Serialize<N> + Deserialize<N> {}It'd be nice if we could derive the |
LHerskind
added a commit
that referenced
this issue
Jul 1, 2024
This PR changes public write functions so that they not only store the new delay and/or value, but also a hash of the combined serialization of them. This lets us produce smaller proofs in private, since we only need to prove inclusion of the hash and not each of the individual 4 values. This will result in even larger savings once we do #5491. The only annoying bit is that producing the unconstrained hint so that we can contrain the hash and prove inclusion involves performing reads, and hence computation of storage slots. But because we cannot pass mutable references to unconstrained functions, we cannot have methods for the `&mut PrivateContext` impl, resulting in a bit of a hack to create a dummy state variable that I can call methods on. This is all going to go away regardless once #5492 is done. I also restored the tests deleted in #6985, updating the existing ones and adding new cases. --------- Co-authored-by: LHerskind <[email protected]>
superstar0402
added a commit
to superstar0402/aztec-nr
that referenced
this issue
Aug 16, 2024
(Large) part of AztecProtocol/aztec-packages#4761. This is an initial implementation of `SharedMutableStorage`, with some limitations. I think those are best worked on in follow-up PRs, once we have the bones working. The bulk of the SharedMutable pattern is in `ScheduledValueChange`, a pure Noir struct that has all of the block number related logic. `SharedMutable` then makes a state variable out of that struct, adding public storage access both in public and private (via historical reads - see #5379), and using the new `request_max_block_number` function (from #5251). I made an effort to test as much as I could of these in Noir, with partial success in the case of `SharedMutable` due to lack of certain features, notably noir-lang/noir#4652. There is also an end-to-end test that goes through two scheuled value changes, showing that scheduled values do not affect the current one. I added some inline docs but didn't include proper docsite pages yet so that we can discuss the implementation, API, etc., and make e.g. renamings less troublesome. ### Notable implementation details I chose to make the delay a type parameter instead of a value mostly because of two reasons: - it lets us nicely serialize and deserialize `ScheduledValueChange` without including this field (which we are not currently interested in storing) - it lets us declare a state variable of type `SharedMutable<T, DELAY>` without having to change the signature of the `new` function, which is automatically injected by the macro. Overall I think this is fine, especially since we may later make the delay mutable (see below), but still worth noting. Additionally, I created a simple `public_storage` module to get slightly nicer API and encapsulation. This highlighted a Noir issue (noir-lang/noir#4633), which currently only affects public historical reads but will also affect current reads once we migrate to using the AVM opcodes. ### Future work - AztecProtocol/aztec-packages#5491 - AztecProtocol/aztec-packages#5492 (this takes care of padding during storage slot allocation) - AztecProtocol/aztec-packages#5501 - AztecProtocol/aztec-packages#5493 --------- Co-authored-by: Jan Beneš <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
SharedMutablecurrently requires that the inner typeTfits in a singleField(until we implement #5491), but despite that stores multiple values in storage: twoT(preandpost) and oneu32(the block of change). To avoid storage collisions, it produces a derived storage slot by hashing the assinged one with some constant, so that it can freely use as much storage as needed.We should change this so that it implements the serde traits and can therefore enjoy auto-allocation with no collisions by the macro. This may uncover Noir issues (such as noir-lang/noir#4633).
The text was updated successfully, but these errors were encountered: