[Tracking Issue] Identify and throw errors for currently-unsupported (but planned) features.

[iAmMichaelConnor]

There are many features we would like to add, and there are many problems that we're aware of. Not all of these features can be built, nor problems resolved, before a first release.

Nevertheless, we can strive to identify unsupported features, and let the user know that they're attempting to do something which is currently unsupported, but planned. I.e. we should add code which identifies known pitfalls and gives the user a human-readable and contextual error, instead of some opaque, technical error.

This tracking issue can be an ever-growing list of such unsupported features. Each item in the list should be an issue which seeks to programmatically identify unsupported patterns, and throw an informative error in such cases.

For each case, we should assess whether the time & effort needed to identify and throw an error is more or less time than implementing the feature properly.

• The simulator identifies contract re-entrancy.
  • If Contract A calls contract B, but later in the simulation Contract A is called by B (or C or D etc), this could cause re-entrancy errors.
  • If a private function of Contract A calls a private function of Contract A (e.g. a recursive call), the simulator should identify cases where further state changes are made after the call (because such state changes could cause state ordering errors). "State changes" includes getting, nullifying or inserting notes after the call.
  • If a public function of Contract A calls a public function of Contract A (e.g. a recursive call), the simulator should identify cases where further state changes are made after the call (because such state changes could cause state ordering errors). "State changes" includes getting, nullifying or inserting notes, and reading and writing public state, after the call.
  • Note: Contract A should still be able to make a private -> internal public call to itself, and make further private state changes after such a call.
  • The eventual fix will be to track the execution order of state transitions, logs, and enqueued function calls, but this 'ordering' work will take some time.
  • Until we do the eventual fix, we should identify this pattern and throw an error.

Document limitations

Preview Give feedback

1. Document limitation: ordering of notes, noteHashes, logs, etc will not be in proper order in block #1652
   A-documentation +1
   
2. Document limitation: no nonces for publicly created commitments #1654
   A-documentation +1
   
3. Document confusing naming: noteHash == commitment #1679
   A-documentation +1
   

Add informative error messages

Preview Give feedback

1. Add simulator error when reentrancy occurs #1653
   +0
2. Improve logging when a chopped transient note is not found in private data tree #1603
   +0
   

Tests to replicate bugs/limitations

Preview Give feedback

1. Test to replicate: private kernel outputs public function calls in wrong order #1615
   T-bug T-testing +2
   
2. Test to replicate: public kernel rejects a perfectly valid sequence of public state updates #1616
   T-bug T-testing +2
   
3. Test to replicate: public kernel outputs wrong final state for insertion into public data tree #1617
   T-bug T-testing +2
   

Working around these bugs

Preview Give feedback

1. Temporary hack: overwrite private ordering kernel's final public_call_stack with simulator's output #1624
   C-protocol-circuits +1
   
2. Temporary hack: remove public kernel checks that state updates are in correct order #1622
   C-protocol-circuits +1
   
3. Temporary hack: overwrite public kernel's final state update using simulator's output #1623
   C-protocol-circuits +1
   
4. Spike: are there serious issues with L2->L1 message ordering in sandbox? #1629
   spike +1
   
5. Hack around bad UX for nullifying publicly-created notes #1655
   A-ux/devex C-aztec.nr +2
   

[dbanks12]

If a private function of Contract A calls a private function of Contract A (e.g. a recursive call), the simulator should identify cases where further state changes are made after the call (because such state changes could cause state ordering errors). "State changes" includes getting, nullifying or inserting notes after the call.

After discussing with @spalladino, I don't think this is an issue that a user could experience today because while the kernel is not strict on private state ordering, the simulator is. The simulator will never let you get or nullify a note that does not exist yet.

[dbanks12]

If a public function of Contract A calls a public function of Contract A (e.g. a recursive call), the simulator should identify cases where further state changes are made after the call (because such state changes could cause state ordering errors). "State changes" includes getting, nullifying or inserting notes, and reading and writing public state, after the call.

I think the same is true for the simulator in public (the simulator is as strict as it needs to be), but the problem is that the public kernel does have constraints to ensure public state updates make sense in-sequence. Since the public kernel doesn't always execute things in execution order, it may think that it is enforcing proper ordering of public state updates, but what it is actually doing is rejecting a perfectly valid sequence of public state updates.

So, could we just temporarily remove the public kernel constraints that ensure a series of public state updates make sense in sequence? Maybe, but it is also the job of the final public kernel to turn the "final" state value that should be injected in the public data tree. That value must still be correct.

[iAmMichaelConnor]

Closing, as all but one complete, and we can just track that one issue.