Remove PrivateContextInputs as a required parameter to a private noir contract function

[iAmMichaelConnor]

This is a subjective one, seeking a minor prettification of Noir Contract code. Tagging @sirasistant , @LeilaWang , @spalladino for thoughts, as I think we've touched on this topic before (but anyone please feel welcome to comment).

Currently, every private function needs inputs: PrivateContextInputs as a parameter:

    fn transfer(
        //*********************************/
        // Should eventually be hidden:
        inputs: PrivateContextInputs,
        //*********************************/
        amount: u120, 
        sender: Field, 
        recipient: Field,
    ) -> distinct pub abi::PrivateCircuitPublicInputs {
        let mut context = Context::new(inputs, abi::hash_args([amount as Field, sender, recipient]));

As a reminder, here's that struct:

struct PrivateContextInputs {
    call_context : CallContext,
    roots: CommitmentTreesRoots,

    contract_deployment_data: ContractDeploymentData,

    private_global_variables: PrivateGlobalVariables,
}

An alternative (which I think has been considered in the past) would be to make an oracle call at the time of instantiating the Context:

    fn transfer(
        amount: u120, 
        sender: Field, 
        recipient: Field,
    ) -> distinct pub abi::PrivateCircuitPublicInputs {
        let mut context = Context::new(abi::hash_args([amount as Field, sender, recipient]));

impl Context {
    fn new(inputs: abi::PrivateContextInputs, args_hash: Field) -> Context {
        Context {
            inputs: oracle::get_private_context_inputs(), // <---- suggestion

            args_hash: args_hash,
            return_values: BoundedVec::new(0),

            read_requests: BoundedVec::new(0),

            new_commitments: BoundedVec::new(0),
            // ---- etc -----
        }
    }

I won't show an example for what the oracle function would look like.

The simulator, upon making a call to a function, would need to 'keep hold of' the PrivateContextInputs data, awaiting an oracle call which would then fetch that data and feed it into the circuit.

[spalladino]

I love it, thanks so much for working on making Noir contracts easier to write.

I'm trying to think if there's any downside security-wise, like if a malicious oracle could cheat in the inputs response, but I believe the circuit trusts that oracle result the same way it trusts the arguments it receives, right?

[iAmMichaelConnor]

Yes, I believe the oracle results are identical to parameters, from the perspective of a circuit, where all variables (inputs, oracle inputs, intermediate computated values, public inputs) are mapped to witness indices.

[iAmMichaelConnor]

Whilst it's in my head:

We have an asymmetry between private functions and public functions. I don't have a complete proposal, so I'll just list pedantic things:

Private functions have PrivateContextInputs and a Context which consumes those inputs. The context then contains convenient methods for calling external functions, and getting metadata about the function call (i.e. for getting private context inputs data). With everything being gettable from the context, it gives a nice ux, imo.

E.g.

fn foo(
    inputs: PrivateContextInputs,
) -> distinct pub abi::PrivateCircuitPublicInputs {
    let mut context = Context::new(inputs, 0);

    let msg_sender = context.msg_sender(); // I introduced a new method for this, and for other data which is provided via the `inputs`. It feels clean for the context to give meta info about the tx context and call context.
    // We could alternatively do:
    // let msg_sender = context.inputs.call_context.msg_sender;
    // or
    // let msg_sender = inputs.call_context.msg_sender;
    // or
    // let msg_sender = inputs.msg_sender(); // if we introduce a new method
    // But if we get rid of the `inputs`, then the `context` is the only place to 
    // get this stuff from.
 
    let return_values = context.call_public_function(...);
    // We previously had PublicCallStackItem::call(...), but that felt clunky, and an ordinary dev
    // shouldn't have to know about `PublicCallStackItem` at all.

Public functions have PublicContextInputs, but no Context. This is because the public vm circuit will populate the 'public inputs' for a public function. But this means public -> public function calls can't be made via methods of a "context", because no such context exists in the public setting. Instead, pure functions (not methods) are called.

E.g.

open fn bar(
    inputs: PublicContextInputs,
) -> distinct pub abi::PrivateCircuitPublicInputs {
    // There is no context, but we could introduce one:
    // let mut context = Context::new(inputs, 0);
    // or better still, in keeping with the suggestion of this Github Issue:
    // let mut context = Context::new(0); // inputs are 'got' via an oracle call.

    let msg_sender = inputs.call_context.msg_sender;
    // let msg_sender = context.msg_sender(); // I would like to introduce this kind of syntax for data which is provided via the `inputs`. It feels clean for a context to give meta info about the tx context and call context; especially if that's how we decide to do it in private functions.
 
    let return_values = call_public_function(...); // notice, there's no `context` at the moment; it's a pure function.
    // But this is asymmetrical from the private version of making a function call.
    // For symmetry, it would be nice if we could do:
    // let return_values = context.call_public_function(...);
    // But we wouldn't have any use for `self` in such a method, so the syntax would become:
    // let return_values = Context::call_public_function(...);
    // Which still isn't very symmetrical... :(

[spalladino]

I don't have a complete proposal, so I'll just list pedantic things

Feels like I'm reading my own writing!

With everything being gettable from the context, it gives a nice ux, imo.

Agree. Do we envision a future version in which context does not need to be initialized manually by the user, and is rather "magically" available in each contract function?

// For symmetry, it would be nice if we could do:
// let return_values = context.call_public_function(...);
// But we wouldn't have any use for `self` in such a method, so the syntax would become:
// let return_values = Context::call_public_function(...);
// Which still isn't very symmetrical

Why not require a context instance in public land anyway? Even if we have no use for self, we can still define call_public_function (and others) as instance methods, just for the sake of symmetry. And if at some point we make context magically available (without the user having to initialize it), then we get a nice "global" object that the dev uses for contract actions, whether it's public or private.

[spalladino]

An alternative would be to keep everything as pure functions, and just accumulate the data needed for context as a singleton hidden from the user. So even in private we could do:

fn foo() {
  call_private_function(...);
}

Which under the hood would do something like:

global context = Context::new();

fn call_private_function(...) {
  context.initialize_if_needed();
  context.call_private_function(...);
}

But I prefer having less magic and having the user deal directly with the context instance.

[sirasistant]

Yes, we should be able to get the inputs in Context::new without issue in private functions, and the functions would look cleaner! It should be equivalent as you already discussed, oracle inputs should be the same as private inputs to the function.

Regarding public functions we might just instead have a new function in PublicContextInputs that calls the oracle to get the underlying data, so the public function reads it as needed

And regarding globals with initialization function AFAIK noir doesn't support globals that run functions on initialization right now, given that it doesn't support arithmetic operations either noir-lang/noir#1734

[iAmMichaelConnor]

I guess this is stale, given @Maddiaa0 introduced macros to hide such boilerplate? Closing.