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

Add serde tests for circuit structs #6339

Closed
just-mitch opened this issue May 10, 2024 · 0 comments · Fixed by #6378
Closed

Add serde tests for circuit structs #6339

just-mitch opened this issue May 10, 2024 · 0 comments · Fixed by #6378

Comments

@just-mitch
Copy link
Collaborator

I got bit on a proving test because I forgot to add a field to the serialization function of PrivateKernelCircuitPublicInputs.

Add unit tests in noir to prevent this, and similar bugs.

This requires implementing deserialization methods for many structs.

@just-mitch just-mitch self-assigned this May 10, 2024
@github-project-automation github-project-automation bot moved this to Todo in A3 May 10, 2024
just-mitch added a commit that referenced this issue May 13, 2024
## Implemented

[See
spec](https://docs.aztec.network/protocol-specs/gas-and-fees/specifying-gas-fee-info).

A boolean flag `is_fee_payer` in the PrivateCircuitPublicInputs. The
private kernel circuits will check this flag for every call stack item.

When a call stack item is found with `is_fee_payer` set, the kernel
circuit will set `fee_payer` in its PrivateKernelCircuitPublicInputs to
be the callStackItem.contractAddress.

This is subsequently passed through the PublicKernelCircuitPublicInputs
to the KernelCircuitPublicInputs.

If a transaction attempts to set fee_payer multiple times, the
transaction will be considered invalid.

## Deviations

Whereas in the spec we said we would use `contract_address`, we have
updated that to use `storage_contract_address` because it correctly
handles delegate calls: if `is_fee_payer` gets set during delegate call,
the "delegator" should be the `fee_payer`, not the "delegatee".

## Remaining

Actually setting `fee_payer` in a contract, and making assertions.
(#5920)

If the fee_payer is not set, the transaction will be considered invalid.
(#6343)

## Elsewhere

Unit tests on SerDe
(#6339)
@just-mitch just-mitch linked a pull request May 13, 2024 that will close this issue
@just-mitch just-mitch removed their assignment May 13, 2024
AztecBot pushed a commit to AztecProtocol/aztec-nr that referenced this issue May 14, 2024
## Implemented

[See
spec](https://docs.aztec.network/protocol-specs/gas-and-fees/specifying-gas-fee-info).

A boolean flag `is_fee_payer` in the PrivateCircuitPublicInputs. The
private kernel circuits will check this flag for every call stack item.

When a call stack item is found with `is_fee_payer` set, the kernel
circuit will set `fee_payer` in its PrivateKernelCircuitPublicInputs to
be the callStackItem.contractAddress.

This is subsequently passed through the PublicKernelCircuitPublicInputs
to the KernelCircuitPublicInputs.

If a transaction attempts to set fee_payer multiple times, the
transaction will be considered invalid.

## Deviations

Whereas in the spec we said we would use `contract_address`, we have
updated that to use `storage_contract_address` because it correctly
handles delegate calls: if `is_fee_payer` gets set during delegate call,
the "delegator" should be the `fee_payer`, not the "delegatee".

## Remaining

Actually setting `fee_payer` in a contract, and making assertions.
(AztecProtocol/aztec-packages#5920)

If the fee_payer is not set, the transaction will be considered invalid.
(AztecProtocol/aztec-packages#6343)

## Elsewhere

Unit tests on SerDe
(AztecProtocol/aztec-packages#6339)
@github-project-automation github-project-automation bot moved this from Todo to Done in A3 May 14, 2024
iakovenkos pushed a commit that referenced this issue May 15, 2024
## Implemented

[See
spec](https://docs.aztec.network/protocol-specs/gas-and-fees/specifying-gas-fee-info).

A boolean flag `is_fee_payer` in the PrivateCircuitPublicInputs. The
private kernel circuits will check this flag for every call stack item.

When a call stack item is found with `is_fee_payer` set, the kernel
circuit will set `fee_payer` in its PrivateKernelCircuitPublicInputs to
be the callStackItem.contractAddress.

This is subsequently passed through the PublicKernelCircuitPublicInputs
to the KernelCircuitPublicInputs.

If a transaction attempts to set fee_payer multiple times, the
transaction will be considered invalid.

## Deviations

Whereas in the spec we said we would use `contract_address`, we have
updated that to use `storage_contract_address` because it correctly
handles delegate calls: if `is_fee_payer` gets set during delegate call,
the "delegator" should be the `fee_payer`, not the "delegatee".

## Remaining

Actually setting `fee_payer` in a contract, and making assertions.
(#5920)

If the fee_payer is not set, the transaction will be considered invalid.
(#6343)

## Elsewhere

Unit tests on SerDe
(#6339)
superstar0402 added a commit to superstar0402/aztec-nr that referenced this issue Aug 16, 2024
## Implemented

[See
spec](https://docs.aztec.network/protocol-specs/gas-and-fees/specifying-gas-fee-info).

A boolean flag `is_fee_payer` in the PrivateCircuitPublicInputs. The
private kernel circuits will check this flag for every call stack item.

When a call stack item is found with `is_fee_payer` set, the kernel
circuit will set `fee_payer` in its PrivateKernelCircuitPublicInputs to
be the callStackItem.contractAddress.

This is subsequently passed through the PublicKernelCircuitPublicInputs
to the KernelCircuitPublicInputs.

If a transaction attempts to set fee_payer multiple times, the
transaction will be considered invalid.

## Deviations

Whereas in the spec we said we would use `contract_address`, we have
updated that to use `storage_contract_address` because it correctly
handles delegate calls: if `is_fee_payer` gets set during delegate call,
the "delegator" should be the `fee_payer`, not the "delegatee".

## Remaining

Actually setting `fee_payer` in a contract, and making assertions.
(AztecProtocol/aztec-packages#5920)

If the fee_payer is not set, the transaction will be considered invalid.
(AztecProtocol/aztec-packages#6343)

## Elsewhere

Unit tests on SerDe
(AztecProtocol/aztec-packages#6339)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants