Skip to content

feat(StateVariables): Extend SharedMutable to work with multi-field values #5491

Closed

Description

@nventuro

#5490 introduced a version of SharedMutable<T> where T is required to implement FromField, meaning it must fit inside a Field wrapper value. While this is fine for values such as AztecAddress, we'll eventually want to store multi-field structs, and so this constraint will need to be relaxed.

We may implement this by relying solely on Serialize and Deserialize, but are likely going to run into trouble until noir-lang/noir#4633 is fixed, since creating an array out of multiple single-field elements is the core issue there.

Activity

  1. changed the title [-]Extend SharedMutable to work with multi-field values[/-] [+]feat(StateVariables): Extend SharedMutable to work with multi-field values[/+] on Apr 2, 2024
  2. added a commit that references this issue on Apr 10, 2024
  3. added a commit that references this issue on Apr 11, 2024
  4. nventuro commented on Apr 18, 2024

    @nventuro
    ContributorAuthor

    This will likely result in ScheduledValueChange requiring that T implements the serde traits (instead of to/from field).

  5. added a commit that references this issue on Jul 1, 2024
  6. LHerskind commented on Jul 2, 2024

    @LHerskind
    Contributor

    Napkin math of saving on the Token::transfer function with this.

    We need to lookup 2 users keys.

    One lookup should cost ~17K
    Currently a lookup of a key is 27K, but we are really doing two lookups of 10k + 6.5K for the case where it is in the address).

    The cost should therefore be ~34K.

    What is our current cost:

    • We lookup npk, ovpk, ivpk for the sender
    • We lookup npk, ivpk for the recipient

    Meaning that we have 5 lookups of 27K -> ~135K spent on key rotation.

    We should save 101K constraints by having this in.

    @iAmMichaelConnor, not sure where you would ideally have some of these numbers 🤷

  7. iAmMichaelConnor commented on Jul 2, 2024

    @iAmMichaelConnor
    Contributor

    Can a sharedMutable be hashed into a single field?

    I.e. store the following in a single storage slot:

    • Current values of:
      • Ivpk.x
      • Ovpk.x
      • Npk.x
      • Encoding of their y-coord signs
    • Scheduled changed values of them
    • Time of change

    (I forget the naming and mechanics of how sharedMutable has been implemented, but hopefully you get the gist).

    If that's all stored in a single slot, for a particular user address, then to read that slot from the archive tree, it's ~80 constraints for poseidon2 of 2 fields * 40 height = 3200 constraints.
    To unpack all that data listed above, something like a poseidon2 of 10 fields - maybe ~400 constraints.
    To unpack the signs, a few constraints.
    To recompute the y-coords: 4 or 5 constraints per point. Although there might need to be a range check, to check the 'negative-ness', in which case all this messing around with signs might be unnecessary and similar in cost to poseidon-hashing the y-coords directly into the slot's hash value. Meh, for an estimate, let's say the coordinates aren't compressed, then instead of ~400 constraints, it's more like a poseidon2 of 16 fields - maybe ~640 constraints.
    So 3200 + 640 = 4000 constraints to read the keys from the registry for 1 address.
    8000 for two addresses.
    And if we're in the case where the address isn't-yet registered in the registry, we should be re-purposing the poseidon hashes that were used for unpacking above. I.e. we read the registry, find that the value is 0 (or maybe some known hash of 0's if we're reading a hash of some empty shared mutable slot), and then pass the address's preimage data to the existing poseidon hashes. So this case should add approx 0 extra gates.

    So I reckon 8000 to read all keys for sender & recipient.

  8. LHerskind commented on Jul 2, 2024

    @LHerskind
    Contributor

    Can a sharedMutable be hashed into a single field?

    Yes, that is done as part of #7169. So we have the logic. But you still need the values to be there such that you can actually read them and open (using oracles). This issue is just to fix the latter, but is blocked by noir atm.

  9. self-assigned this
    on Feb 4, 2025
  10. moved this from Todo to Done in A3on Feb 6, 2025
  11. added a commit that references this issue on Feb 6, 2025
    952615b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

Type

No type

Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions