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

Improve interface for specifying teardown function #6277

Closed
Tracked by #5917
just-mitch opened this issue May 8, 2024 · 1 comment · Fixed by #12499
Closed
Tracked by #5917

Improve interface for specifying teardown function #6277

just-mitch opened this issue May 8, 2024 · 1 comment · Fixed by #12499
Assignees

Comments

@just-mitch
Copy link
Contributor

just-mitch commented May 8, 2024

Go from:

context.set_public_teardown_function(
    context.this_address(),
    FunctionSelector::from_signature("pay_fee_with_shielded_rebate(Field,(Field),Field)"),
    [amount, asset.to_field(), secret_hash]
);

to

FPC::at(context.this_address()).pay_fee_with_shielded_rebate(amount, asset, secret_hash).set_public_teardown_function(&mut context)
@github-project-automation github-project-automation bot moved this to Todo in A3 May 8, 2024
just-mitch added a commit that referenced this issue May 9, 2024
### Deviations from [the
spec](https://docs.aztec.network/protocol-specs/gas-and-fees/kernel-tracking):

I needed to create a new stack for processing the teardown calls,
instead of storing a single call. I.e.
```diff
class PublicKernelCircuitPublicInputs {
    // ... other fields
--- +CallRequest public_teardown_call_request
+++ +CallRequest[MAX_PUBLIC_CALL_STACK_LENGTH_PER_TX] public_teardown_call_stack
}
```

This is because a teardown function can call a nested function, and,
similar to the current design for public/private calls,
we need a way to keep track of our execution stack.

Further, in order to pass in the CallRequest to the private kernel
circuits, I needed to add a new parameter to the PrivateCallData.

### Overview

We designate a function to be run for teardown as:

```
context.set_public_teardown_function(
    context.this_address(),
    FunctionSelector::from_signature("pay_fee_with_shielded_rebate(Field,(Field),Field)"),
    [amount, asset.to_field(), secret_hash]
);
```

As I note in a comment, I created #6277 for getting back to something
like:
```
FPC::at(context.this_address()).pay_fee_with_shielded_rebate(amount, asset, secret_hash).set_public_teardown_function(&mut context)
```

This sets `publicTeardownFunctionCall: PublicCallRequest` in the
encapsulating `ClientExecutionContext`, which defaults to
`PublicCallRequest.empty()`.

When private simulation is finished, we collect an array of all the
public teardown functions that were set during the simulation.

We assert that the length of that array is 0 or 1.

When proving, we convert the `publicTeardownFunctionCall` to a
`CallRequest` if it is not empty, otherwise we use
`CallRequest.empty()`.

This is specified in the `PrivateCallData` which is passed to the
private kernel circuit.

In the private kernel circuits, we assert that if the
`public_teardown_function_hash` is not zero on the
`PrivateCircuitPublicInputs`, then it matches the hash of the
`publicTeardownFunctionCall` in the `PrivateCallData`.

Further, we assert that if the teardown call request in the
`PrivateCallData` is not empty, then the teardown call request from the
previous kernel *is* empty.

In the private kernel tail, we assert that the public teardown call
request is empty.

In private kernel tail to public, we initialize the teardown call stack
to have the single element corresponding to the call request if it is
not empty, and initialize it to an empty array otherwise.

Since teardown now has its own stack, we update the logic for how to
know when we are in the different phases to simply look at each of their
stacks:
- setup uses end_non_revertible.public_call_stack
- app logic uses end.public_call_stack
- teardown uses public_teardown_call_stack

### Note:

This does not change the fact that teardown is still non-revertible.
That is covered by #5924
@iAmMichaelConnor
Copy link
Contributor

iAmMichaelConnor commented Dec 17, 2024

If the intention is for this syntax to set the function pay_fee_with_shielded_rebate as the teardown function, perhaps call the method set_as_public_teardown_function (note the "as"). Otherwise I'd be confused as to when the function will actually be called.

My brain might prefer something like:

context.set_public_teardown_function(
    FPC::at(context.this_address()).pay_fee_with_shielded_rebate(amount, asset, secret_hash)
);

@benesjan benesjan self-assigned this Mar 5, 2025
@github-project-automation github-project-automation bot moved this from Todo to Done in A3 Mar 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants