feat!: shared mutable configurable delays

[nventuro]

Closes #5493, follow up of #6085.

This makes the delay in SharedMutable not be fixed and instead configurable by users throughout the lifetime of the contract. This is however more complicated than it sounds at first: because private proofs are being created relying on the public values being stable until a future point in time, it must not be possible to cause for a shared value to change before some delay.

Two scenarios are particularly tricky:

• if the delay is reduced, then it is possible to schedule a value change with a shorter delay, violating the original delay's constraints. The solution to this is to make delay changes be scheduled actions themselves, so that the total delay (wait time for the new delay to come into effect plus the new reduced delay) equals the original delay. Note that increasing a delay cna be done instantly.
• if we schedule delay changes as per the above, then we must consider a scenario in which a delay reduction is scheduled in the near future. It may happen that waiting for the reduction to come into effect and then scheduling results in a shorter delay than if the scheduling were to happen immediately - this lower 'effective delay' is the value that must be used in private proofs.

How

I had originally considered creating a sort of wrapper state variable that held two SharedMutables, one for the value and one for the delay, or alternatively two ScheduledValueChanges, but ultimately I realized that a scheduled value change is significantly different from a scheduled delay change. Namely:

• the notion of the 'current' delay is meaningless in private - we only care about the 'effective' delay
• there's no use for the block horizon of a delay change
• scheduling a delay change requires setting a delay depending on the current and new values, not an externally defined one

Due to these differences, I introduced ScheduledDelayChange, which is essentially a variant of the value change, but with these considerations baked in. I think this is a reasonable way to do things, even if at first this may seem to introduce too many concepts. It also helps with the fact that there's so many values involved (pre, post and block of change for value and delays, as well as current, effective, historical values, etc.), and with language becoming weird - we need to describe the delay for scheduling a delay change, which will later affect the delays of scheduling value changes.

With ScheduledDelayChange, extending the functionality of SharedMutable was relatively straightforward. The unit tests became a bit more complicated due to there bieng more scenarios, so I also used this as an opportunity to try to create slightly more complex Noir tests. I didn't go too crazy here, but they seem to be right at the point where we'd want to introduce something like a Test struct with custom impls for setup, common assertions, etc.

Problems

An uninitialized SharedMutable has both delay and value of 0. A zero delay transforms SharedMutable into PublicMutable: scheduled value changes become effective immediately, and it is not possible to read from private since tx.max_block_number would equal a historical block (i.e. an already mined one). Delay initialization is therefore required, and this is typically fine: since the initial delay is 0 any change will be an increase, and therefore instant.

The problem arises when we cannot have explicit initialization and instead wish to rely on defaults. This happens e.g. when we put a SharedMutable inside a Map: we can't initialize all entries for all keys, and we run into trouble. This is a pattern followed by KeyRegistry and TokenBlacklist: we have per-user configuration, and cant really ask users to initialize their state before interacting with the system.

Solution?

A possible solution would be to have a default value for the delay, and to store e.g. Option<u32> instead of plain integers and using unwrap_or(DEFAULT). We could then make this a type parameter for SharedMutable, e.g. registry: Map<Address, SharedMutable<Key, DEFAULT_DELAY>>.

This would make certain things more complicated, particularly the effective delay and delay change block of change computations, but it should all be containable within ScheduledDelayChange, which sounds just about right.

---

I'm keeping this is a draft so we can discuss the current approach and wether we think the above or an alternative solution would be reasonable to attempt. Note that this PR won't pass CI as some of the contracts won't build.

[LHerskind]

Before looking at any code, just based on the description:

"if we schedule delay changes as per the above, then we must consider a scenario in which a delay reduction is scheduled in the near future. It may happen that waiting for the reduction to come into effect and then scheduling results in a shorter delay than if the scheduling were to happen immediately - this lower 'effective delay' is the value that must be used in private proofs."

This is not clear to me, how? My intuition is that it is something with different delays that is I'm getting mixed up in the explanation, but if same delay then it is not clear to me.

"there's no use for the block horizon of a delay change"

Again, would be great with a bit info on why here, I might just be an idiot though.

• For stuff like the maps, it seems like it would make sense for the value to be shared at the map level and not individually?
  • For the blacklist token for example, (in my mind), you would want all the actors to have a similar delay for the roles.

[nventuro]

This is not clear to me, how? My intuition is that it is something with different delays that is I'm getting mixed up in the explanation, but if same delay then it is not clear to me.

Imagine you're building a historical proof off of block 200. The delay then was 50, so you may think the earliest the value could change would be 251 (if someone scheduled at 201), and set max_block_number to 250.

However, if a delay reduction was scheduled to take place at block 210, reducing the delay to 20, then someone could wait until 210, schedule then, and have the value change at block 230, way before you imagined. These 30 blocks are the 'effective delay' in this case.

There's multiple mentions of this concept as well as diagrams throughout the code.

"there's no use for the block horizon of a delay change"

The meaning of the block horizon is 'what's the latest block in which you still know the current value will remain, regardless of what anyone else does'. We don't care about what the delay might be in the future, what we are interested in is how it can be used to schedule value changes, which is what we're actually reading.

[nventuro]

I gave the Option approach some more thought and think it'd be a very good fit. We'd setup a SharedMutable<T, INITIAL_DELAY> and use that for the initial value, before any delay change is scheduled. When changing the delay, users would always do Option::some, and never go back to this 'default'. ScheduledDelayChange neatly encapsulates the entire thing and doesn't expose any of it. We can also pack the is_some bits to not use extra storage.

For stuff like the maps, it seems like it would make sense for the value to be shared at the map level and not individually? For the blacklist token for example, (in my mind), you would want all the actors to have a similar delay for the roles.

The initial value would be shared by all map entries, yes, as it'll be part of the type in the map:

roles: Map<AztecAdddress, SharedMutable<UserRoles, INITIAL_USER_ROLE_CHANGE_DELAY>>,

If your application does not require the per-user delays to be different, then you can just never call schedule_delay_change.

[nventuro]

I followed the pattern described above and run into no major issues, other than some minor syntax annoyance due to noir-lang/noir#4710. The serde implementation is also a nice example of bitpacking in a single field.

[AztecBot]

Docs Preview

Hey there! 👋 You can check your preview at https://663cffb0143d031c9c976b18--aztec-docs-dev.netlify.app

[nventuro]

I just realized I also had to update SharedMutablePrivateGetter to reflect these changes. Seems quite dangerous that these two can easily go out of sync.

[Thunkar]

LGTM, also great explanations in the comments. Great job! 🚀

[LHerskind]

This is great. Have added a few comments mainly to comments and me being a simpleton needing more hand-holding.

[LHerskind]

There are a lot of / in here. Inconsistent number of / for comments throughout the code.

[LHerskind]

The scheduling is always a little weird in my head. So for this case, if I have an initial delay of 3 days, and do any reduction, that change will take effect such that the value can be changed after 3 days.

Can you explain again the reason of doing that in here. Just that when I read this, my simply mind is like "why?", if you had the original 3 day delay as the block of change that seems easier to follow.

The benefit was that you avoid having this weird "almost change" period where if it is queued and I insert a change at block N, it might take 3 days before effective and N+1 might take 1 day (with your example). The comment is inside the function, but when I just read here I go "huh" and then write all this blabber before I see it 😆

[nventuro]

I'm not sure if you're asking 'why are we waiting the minimum amount of time instead of always simply waiting a longer time?', or if you're not sure why this is the correct amount of time to wait 🤔

[LHerskind]

Kinda both actually. When I read:

when reducing the delay, the change will take effect after a delay equal to the difference between old and new delay

It is not instantly clear to me why we use that and why that is the shortest time (i might be idiot) and I had to think about it 🤯.

[nventuro]

From past experience it can be very annoying to be forced to wait extra time when you don't need to (e.g. if decreasing a delay from a month to two weeks, having to wait a total of 4 vs 6 weeks is a big difference). Hopefully most people will be relatively satisfied with the behavior and not be nerd-sniped by the API reference?

[LHerskind]

Sure, I get why, was just not something that instantly clicked for me just from the comment. But think that might just be me reading comprehension 😆

[benesjan]

Beautiful and very thorough work. Added just some nits

[sklppy88]

Love the explanations. 🙌 🙏

[LHerskind]

Kinda both actually. When I read:

when reducing the delay, the change will take effect after a delay equal to the difference between old and new delay

It is not instantly clear to me why we use that and why that is the shortest time (i might be idiot) and I had to think about it 🤯.