Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support public function calls from private functions #447

Closed
spalladino opened this issue May 3, 2023 · 5 comments · Fixed by #548
Closed

Support public function calls from private functions #447

spalladino opened this issue May 3, 2023 · 5 comments · Fixed by #548
Assignees

Comments

@spalladino
Copy link
Collaborator

spalladino commented May 3, 2023

Private functions should be able to "enqueue" a call to a private function. These are different to calls from private to private or public to public, since they won't be synchronous nor return any value, but rather just populate the public call stack to be emptied by the sequencer.

To implement this, we'd need a new oracle function that takes care of enqueuing the function call. Note that it'll have to be different from the one for #430 since this one won't return anything. This will just populate the public_call_stack of the PrivateCircuitPublicInputs. The private kernel circuit should just forward the contents of the public call stack from its inputs to its outputs, so they can be later picked up on the sequencer.

We'll also need to augment the Tx object sent to the P2P module to include the public call stack preimages. Right now, the private kernel circuit output only includes the hashes of the call stack items, but in order to process the public calls, we'll need the preimages. These preimages are only known to the private simulator, so they'll need to be pushed from the client to the sequencer.

Once the PublicProcessor picks up a tx with a public_call_stack, it should run those calls through the public function simulator, and then through the public kernel circuit.

@spalladino
Copy link
Collaborator Author

spalladino commented May 9, 2023

@iAmMichaelConnor I think that the semantics of private_call_stack and public_call_stack in PrivateCircuitPublicInputs will have to diverge.

private_call_stack now is the hash of every PrivateCallStackItem, which includes the PrivateCircuitPublicInputs (output of the private circuit execution). In other words, private_call_stack contains the result of every nested call as they were pushed onto the stack.

However, we cannot get the results of nested public executions as outputs of the private circuit, since these are enqueued to be run later in the sequencer. Instead, public_call_stack would be the hash of the requests to execute these enqueued public functions, which look like this:

interface PublicExecution {
  /** Address of the contract being executed. */
  contractAddress: AztecAddress;
  /** Function of the contract being called. */
  functionData: FunctionData;
  /** Arguments for the call. */
  args: Fr[];
  /** Context of the call. */
  callContext: CallContext;
}

(note we can possible trim down many of these fields and recalculate them on the sequencer, but that's another story)

These request preimages would be sent to the sequencer, which should run them through the VM public circuit and get the actual public_call_stack hashes and items, which are then fed into the public kernel circuit.

Does this make sense? If so, should we rename PrivateCircuitPublicInputs.public_call_stack to PrivateCircuitPublicInputs.enqueued_public_calls to avoid confusion?

@iAmMichaelConnor
Copy link
Contributor

iAmMichaelConnor commented May 10, 2023

My mental model was that the PublicCallStackItem would always look the same, regardless of whether the call was made by a public function or a private function. That way, a single public_call_stack -- which is maintained by the Public Kernel Circuit -- could contain PublicCallStackItems from both private and public functions. The only difference would be -- as you rightly point out -- that lots of the PublicCircuitPublicInputs fields of a PublicCallStackItem would be 0 for a private -> public call, because the return values, events, calls, etc. aren't known ahead of time. The Private Kernel Circuit could assert that such fields are indeed 0.


Although, re-reading, your suggestion also has a single public_call_stack by the time we reach the Public Kernel Circuit?

These request preimages would be sent to the sequencer, which should run them through the VM public circuit and get the actual public_call_stack hashes and items, which are then fed into the public kernel circuit.

I'm still not sure on some details of your proposal.

Suppose a private function A enqueues public function requests B, C, D. Then suppose B makes pulic function calls to E, F, G (just so that there are both kinds of calls on the stack).

The Public Kernel Circuit will need to copy the enqueued_public_calls for C, D from the PrivateKernelPublicinputs onto its own stack, because it can only process one of B, C, or D (let's say B, in this example) inside public_kernel_circuit_no_previous_kernel. So I guess that initial kernel circuit would need to either:

  • maintain a separate enqueued_public_calls array for C & D (which is perhaps a bit messy, because we'd need branching logic in every public kernel circuit to pop from this array, or some new public kernel circuits); or
  • mutate the enqueued_public_calls_preimages of C & D into public_call_stack_preimages... but it doesn't know the return values/events/calls of functions C and D yet, because it can only process one function at a time. So I guess it would then need to pad those values with 0, at this stage, and then we're back to my mental model?

We don't have this problem for calls to E, F, G, because the return values can be populated by their calling function B.


I suppose my approach has 'bugs', though. If we unified the public_call_stack to contain calls made by both private and public functions, how would a particular iteration of the Public Kernel Circuit distinguish between "this public call stack preimage has been padded with 0" and "this public call stack preimage is complete"?

Maybe a boolean called_from_a_private_function: bool (name should be improved!) needs to be added as a data member to the PublicCallStackItem, so that the Public Kernel Circuit can conditionally overwrite the 0 values when processing each of the functions B, C, D?

I rambled. Good topic for our call later. I do like your suggestion of only sending the data that's actually needed, because it makes the code much easier to follow. I just have some 'question marks' over how it'll work.

@spalladino
Copy link
Collaborator Author

Summarizing today's discussion with @iAmMichaelConnor: out of the private circuit, we'll get public_call_stack_items where most of the struct is empty. Only contract_address, function_signature, args, and call_context should be set, which should be equivalent to the PublicExecution listed above.

This will reduce the number of conditionals in the circuit, and get an initial version working faster since we won't need to alter the structs. We'll need to dig into the public kernel circuit to see what changes would be needed to handle this case, if any.

Note that removing the "outputs" of an execution (return_values, etc) from all call stack items is not viable, since a call may use the return values from a call stack item, so these are needed for the general case of a public function calling another public function (or private calling private). The only scenario in which they can be safely ignored is this one, since return values from async enqueued public calls cannot be used.

@PhilWindle
Copy link
Collaborator

So this is effectively going down the road of storing the hashes of the 'requests' where the struct is bridging between private and public executions? But in a way that doesn't mean we introduce a load of new data structures solely for that purpose.

@spalladino
Copy link
Collaborator Author

Yup. We are using the same data structures as we have today, only we change their semantic when it is an enqueued public function call request.

@spalladino spalladino moved this from In Progress to In Review in A3 May 14, 2023
@github-project-automation github-project-automation bot moved this from In Review to Done in A3 May 16, 2023
@iAmMichaelConnor iAmMichaelConnor removed this from A3 Jul 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants