Skip to content

Commit

Permalink
feat: public refunds via FPC (#4750)
Browse files Browse the repository at this point in the history
Enable and test public refunds. 

Specifically, when Alice uses an FPC to pay her fees, and she *overpays*
(e.g. pays 3 bananas when only 1 is required), she has a public balance
transmitted to her as part of TX teardown (2 bananas in the example).

In order to accomplish this, we needed to fix #4736. 

Thus, this PR squashes writes performed in the non-revertible phases of
a TX.

Further, when all phases were completed, we squash across phases when we
recombine the `end` and `endNonRevertible`.

This presented a problem, because on current master that "squash across
phases" being performed during recombine in TS would need to be mirrored
in the base rollup.

Instead of doing that, this PR changes the private inputs to the base
rollup circuit to accept a `CombinedAccumulatedData`, i.e. effects that
have already been recombined. This affords the reduction in
gates/opcodes in the base_rollup.

This approach does not introduce any new trust assumptions because as
mentioned in #4736, the base rollup circuit [trusts that the writes it
receives are
valid](#2521 (comment)).
  • Loading branch information
just-mitch committed Mar 1, 2024
1 parent 9633b0f commit 30502c9
Show file tree
Hide file tree
Showing 31 changed files with 816 additions and 386 deletions.
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
mod subscription_note;
mod dapp_payload;

contract AppSubscriptionContract {
contract AppSubscription {
use dep::std;
use dep::std::option::Option;
use crate::dapp_payload::DAppPayload;
Expand Down
18 changes: 10 additions & 8 deletions noir-projects/noir-contracts/contracts/fpc_contract/src/main.nr
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,9 @@ contract FPC {
);

let _void = context.call_public_function(
storage.fee_asset.read_private(),
FunctionSelector::from_signature("pay_fee(Field)"),
[amount.to_field()]
context.this_address(),
FunctionSelector::from_signature("pay_fee((Field),Field,(Field))"),
[context.msg_sender().to_field(), amount, asset.to_field()]
);
}

Expand All @@ -67,12 +67,14 @@ contract FPC {
}

#[aztec(public)]
internal fn pay_fee(from: AztecAddress, amount: Field, asset: AztecAddress) {
let _void = context.call_public_function(
internal fn pay_fee(refund_address: AztecAddress, amount: Field, asset: AztecAddress) {
let refund = context.call_public_function(
storage.fee_asset.read_public(),
FunctionSelector::from_signature("pay_fee(Field)"),
[amount.to_field()]
);
// TODO handle refunds
[amount]
)[0];

// Just do public refunds for the present
Token::at(asset).transfer_public(context, context.this_address(), refund_address, refund, 0)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,17 @@ contract GasToken {
storage.balances.at(to).write(new_balance);
}

// TODO(@just-mitch): remove this function before mainnet deployment
// convenience function for testing
// the true canonical gas token contract will not have this function
#[aztec(public)]
fn mint_public(to: AztecAddress, amount: Field) {
let amount = U128::from_integer(amount);
let new_balance = storage.balances.at(to).read().add(amount);

storage.balances.at(to).write(new_balance);
}

#[aztec(public)]
fn check_balance(fee_limit: Field) {
let fee_limit = U128::from_integer(fee_limit);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,13 @@ impl PublicKernelTeardownCircuitPrivateInputs {
&mut public_inputs
);

public_inputs.to_inner()
let mut output = public_inputs.to_inner();

// If we enqueued multiple public functions as part of executing the teardown circuit,
// continue to treat them as part of teardown.
output.needs_setup = false;

output
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use dep::types::{
PublicDataMembershipWitness
},
nullifier_leaf_preimage::NullifierLeafPreimage, public_data_update_request::PublicDataUpdateRequest,
public_data_read::PublicDataRead, kernel_data::PublicKernelData,
public_data_read::PublicDataRead, kernel_data::RollupKernelData,
side_effect::{SideEffect, SideEffectLinkedToNoteHash}, accumulated_data::CombinedAccumulatedData
},
constants::{
Expand All @@ -38,7 +38,7 @@ use dep::types::{
};

struct BaseRollupInputs {
kernel_data: PublicKernelData,
kernel_data: RollupKernelData,
start: PartialStateReference,

state_diff_hints: StateDiffHints,
Expand Down Expand Up @@ -72,18 +72,12 @@ impl BaseRollupInputs {
== self.constants.global_variables.version, "kernel version does not match the rollup version"
);

// recombine the accumulated data
let combined = CombinedAccumulatedData::recombine(
self.kernel_data.public_inputs.end_non_revertible,
self.kernel_data.public_inputs.end
);

// First we compute the contract tree leaves
let contract_leaves = self.calculate_contract_leaves(combined);
let contract_leaves = self.calculate_contract_leaves();

let contracts_tree_subroot = self.calculate_contract_subtree_root(contract_leaves);

let commitments_tree_subroot = self.calculate_commitments_subtree(combined);
let commitments_tree_subroot = self.calculate_commitments_subtree();

let empty_commitments_subtree_root = calculate_empty_tree_root(NOTE_HASH_SUBTREE_HEIGHT);

Expand All @@ -106,13 +100,13 @@ impl BaseRollupInputs {
);

// Insert nullifiers:
let end_nullifier_tree_snapshot = self.check_nullifier_tree_non_membership_and_insert_to_tree(combined);
let end_nullifier_tree_snapshot = self.check_nullifier_tree_non_membership_and_insert_to_tree();

// Validate public public data reads and public data update requests, and update public data tree
let end_public_data_tree_snapshot = self.validate_and_process_public_state(combined);
let end_public_data_tree_snapshot = self.validate_and_process_public_state();

// Calculate the overall calldata hash
let calldata_hash = BaseRollupInputs::components_compute_kernel_calldata_hash(combined);
let calldata_hash = BaseRollupInputs::components_compute_kernel_calldata_hash(self.kernel_data.public_inputs.end);

// Perform membership checks that the notes provided exist within the historical trees data
self.perform_archive_membership_checks();
Expand All @@ -135,9 +129,9 @@ impl BaseRollupInputs {
}
}

fn calculate_contract_leaves(self, combined: CombinedAccumulatedData) -> [Field; MAX_NEW_CONTRACTS_PER_TX] {
fn calculate_contract_leaves(self) -> [Field; MAX_NEW_CONTRACTS_PER_TX] {
let mut contract_leaves = [0; MAX_NEW_CONTRACTS_PER_TX];
let new_contracts = combined.new_contracts;
let new_contracts = self.kernel_data.public_inputs.end.new_contracts;

// loop over the new contracts
for i in 0..new_contracts.len() {
Expand All @@ -164,14 +158,14 @@ impl BaseRollupInputs {

// TODO(Kev): This should say calculate_commitments_subtree_root
// Cpp code says calculate_commitments_subtree, so I'm leaving it as is for now
fn calculate_commitments_subtree(self, combined: CombinedAccumulatedData) -> Field {
calculate_subtree(combined.new_note_hashes.map(|c: SideEffect| c.value))
fn calculate_commitments_subtree(self) -> Field {
calculate_subtree(self.kernel_data.public_inputs.end.new_note_hashes.map(|c: SideEffect| c.value))
}

fn check_nullifier_tree_non_membership_and_insert_to_tree(self, combined: CombinedAccumulatedData) -> AppendOnlyTreeSnapshot {
fn check_nullifier_tree_non_membership_and_insert_to_tree(self) -> AppendOnlyTreeSnapshot {
indexed_tree::batch_insert(
self.start.nullifier_tree,
combined.new_nullifiers.map(|nullifier: SideEffectLinkedToNoteHash| nullifier.value),
self.kernel_data.public_inputs.end.new_nullifiers.map(|nullifier: SideEffectLinkedToNoteHash| nullifier.value),
self.state_diff_hints.sorted_nullifiers,
self.state_diff_hints.sorted_nullifier_indexes,
self.state_diff_hints.nullifier_subtree_sibling_path,
Expand Down Expand Up @@ -219,7 +213,7 @@ impl BaseRollupInputs {
calculate_subtree(leaves.map(|leaf:NullifierLeafPreimage| leaf.hash()))
}

fn validate_and_process_public_state(self, combined: CombinedAccumulatedData) -> AppendOnlyTreeSnapshot {
fn validate_and_process_public_state(self) -> AppendOnlyTreeSnapshot {
// TODO(#2521) - data read validation should happen against the current state of the tx and not the start state.
// Blocks all interesting usecases that read and write to the same public state in the same tx.
// https://aztecprotocol.slack.com/archives/C02M7VC7TN0/p1695809629015719?thread_ts=1695653252.007339&cid=C02M7VC7TN0
Expand All @@ -233,7 +227,7 @@ impl BaseRollupInputs {

let end_public_data_tree_snapshot = insert_public_data_update_requests(
self.start.public_data_tree,
combined.public_data_update_requests.map(
self.kernel_data.public_inputs.end.public_data_update_requests.map(
|request: PublicDataUpdateRequest| {
PublicDataTreeLeaf {
slot: request.leaf_slot,
Expand Down Expand Up @@ -566,7 +560,7 @@ mod tests {
membership_witness::{ArchiveRootMembershipWitness, NullifierMembershipWitness, PublicDataMembershipWitness},
new_contract_data::NewContractData, nullifier_leaf_preimage::NullifierLeafPreimage,
public_data_read::PublicDataRead, public_data_update_request::PublicDataUpdateRequest,
kernel_data::PublicKernelData, side_effect::SideEffect,
kernel_data::{PublicKernelData, RollupKernelData}, side_effect::SideEffect,
accumulated_data::CombinedAccumulatedData
},
address::{AztecAddress, EthAddress},
Expand Down Expand Up @@ -620,7 +614,7 @@ mod tests {

fn update_public_data_tree<EXISTING_LEAVES, SUBTREE_SIBLING_PATH_LENGTH, SUBTREE_HEIGHT>(
public_data_tree: &mut NonEmptyMerkleTree<EXISTING_LEAVES, PUBLIC_DATA_TREE_HEIGHT, SUBTREE_SIBLING_PATH_LENGTH, SUBTREE_HEIGHT>,
kernel_data: &mut PublicKernelData,
kernel_data: &mut RollupKernelData,
snapshot: AppendOnlyTreeSnapshot,
public_data_writes: BoundedVec<(u64, PublicDataTreeLeaf), 2>,
mut pre_existing_public_data: [PublicDataTreeLeafPreimage; EXISTING_LEAVES]
Expand Down Expand Up @@ -744,7 +738,7 @@ mod tests {
fn update_nullifier_tree_with_new_leaves(
mut self,
nullifier_tree: &mut NonEmptyMerkleTree<MAX_NEW_NULLIFIERS_PER_TX, NULLIFIER_TREE_HEIGHT, NULLIFIER_SUBTREE_SIBLING_PATH_LENGTH, NULLIFIER_SUBTREE_HEIGHT>,
kernel_data: &mut PublicKernelData,
kernel_data: &mut RollupKernelData,
start_nullifier_tree_snapshot: AppendOnlyTreeSnapshot
) -> ([NullifierLeafPreimage; MAX_NEW_NULLIFIERS_PER_TX], [NullifierMembershipWitness; MAX_NEW_NULLIFIERS_PER_TX], [Field; MAX_NEW_NULLIFIERS_PER_TX], [u64; MAX_NEW_NULLIFIERS_PER_TX]) {
let mut nullifier_predecessor_preimages: [NullifierLeafPreimage; MAX_NEW_NULLIFIERS_PER_TX] = dep::std::unsafe::zeroed();
Expand Down Expand Up @@ -801,12 +795,7 @@ mod tests {
}

fn build_inputs(mut self) -> BaseRollupInputs {
let mut kernel_data = self.kernel_data.to_public_kernel_data();

let combined = CombinedAccumulatedData::recombine(
kernel_data.public_inputs.end_non_revertible,
kernel_data.public_inputs.end
);
let mut kernel_data = self.kernel_data.to_rollup_kernel_data();

let start_note_hash_tree = NonEmptyMerkleTree::new(
self.pre_existing_notes,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,3 +43,10 @@ struct PublicKernelData {
vk_path : [Field; VK_TREE_HEIGHT],
}

struct RollupKernelData {
public_inputs : RollupKernelCircuitPublicInputs,
proof : Proof,
vk : VerificationKey,
vk_index : u32,
vk_path : [Field; VK_TREE_HEIGHT],
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use crate::{
PrivateKernelInnerCircuitPublicInputs, PrivateKernelTailCircuitPublicInputs,
PublicKernelCircuitPublicInputs, RollupKernelCircuitPublicInputs
},
kernel_data::{PrivateKernelInnerData, PrivateKernelTailData, PublicKernelData},
kernel_data::{PrivateKernelInnerData, PrivateKernelTailData, PublicKernelData, RollupKernelData},
public_data_read::PublicDataRead, public_data_update_request::PublicDataUpdateRequest,
read_request::ReadRequestContext, side_effect::{SideEffect, SideEffectLinkedToNoteHash}
},
Expand Down Expand Up @@ -292,4 +292,14 @@ impl PreviousKernelDataBuilder {

PublicKernelData { public_inputs, proof: self.proof, vk: self.vk, vk_index: self.vk_index, vk_path: self.vk_path }
}

pub fn to_rollup_kernel_data(self) -> RollupKernelData {
let public_inputs = RollupKernelCircuitPublicInputs {
aggregation_object: AggregationObject {},
end: self.end.finish(),
constants: CombinedConstantData { historical_header: self.historical_header, tx_context: self.tx_context }
};

RollupKernelData { public_inputs, proof: self.proof, vk: self.vk, vk_index: self.vk_index, vk_path: self.vk_path }
}
}
12 changes: 12 additions & 0 deletions yarn-project/circuits.js/jest.config.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import type { Config } from 'jest';

const config: Config = {
preset: 'ts-jest/presets/default-esm',
moduleNameMapper: {
'^(\\.{1,2}/.*)\\.[cm]?js$': '$1',
},
testRegex: './src/.*\\.test\\.(js|mjs|ts)$',
rootDir: './src',
};

export default config;
11 changes: 0 additions & 11 deletions yarn-project/circuits.js/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,17 +29,6 @@
"remake-constants": "node --loader ts-node/esm src/scripts/constants.in.ts && prettier -w src/constants.gen.ts && cd ../../l1-contracts && ./.foundry/bin/forge fmt",
"test": "NODE_NO_WARNINGS=1 node --experimental-vm-modules $(yarn bin jest) --passWithNoTests"
},
"inherits": [
"../package.common.json"
],
"jest": {
"preset": "ts-jest/presets/default-esm",
"moduleNameMapper": {
"^(\\.{1,2}/.*)\\.[cm]?js$": "$1"
},
"testRegex": "./src/.*\\.test\\.(js|mjs|ts)$",
"rootDir": "./src"
},
"dependencies": {
"@aztec/bb.js": "portal:../../barretenberg/ts",
"@aztec/foundation": "workspace:^",
Expand Down
4 changes: 3 additions & 1 deletion yarn-project/circuits.js/src/structs/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,13 @@ export * from './aggregation_object.js';
export * from './call_context.js';
export * from './call_request.js';
export * from './complete_address.js';
export * from './content_commitment.js';
export * from './contract_deployment_data.js';
export * from './contract_storage_read.js';
export * from './contract_storage_update_request.js';
export * from './function_data.js';
export * from './function_leaf_preimage.js';
export * from './global_variables.js';
export * from './content_commitment.js';
export * from './header.js';
export * from './kernel/combined_accumulated_data.js';
export * from './kernel/combined_constant_data.js';
Expand All @@ -26,6 +26,8 @@ export * from './kernel/public_call_data.js';
export * from './kernel/public_kernel_circuit_private_inputs.js';
export * from './kernel/public_kernel_circuit_public_inputs.js';
export * from './kernel/public_kernel_data.js';
export * from './kernel/rollup_kernel_circuit_public_inputs.js';
export * from './kernel/rollup_kernel_data.js';
export * from './l2_to_l1_message.js';
export * from './membership_witness.js';
export * from './nullifier_key_validation_request.js';
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import { makeAccumulatedData, makeFinalAccumulatedData } from '../../tests/factories.js';
import { makeCombinedAccumulatedData, makeFinalAccumulatedData } from '../../tests/factories.js';
import { CombinedAccumulatedData, PrivateAccumulatedRevertibleData } from './combined_accumulated_data.js';

describe('CombinedAccumulatedData', () => {
it('Data after serialization and deserialization is equal to the original', () => {
const original = makeAccumulatedData();
const original = makeCombinedAccumulatedData();
const afterSerialization = CombinedAccumulatedData.fromBuffer(original.toBuffer());
expect(original).toEqual(afterSerialization);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,10 @@ export class PublicDataRead {
toFriendlyJSON() {
return `Leaf=${this.leafSlot.toFriendlyJSON()}: ${this.value.toFriendlyJSON()}`;
}

equals(other: PublicDataRead) {
return this.leafSlot.equals(other.leafSlot) && this.value.equals(other.value);
}
}

/**
Expand Down Expand Up @@ -128,6 +132,10 @@ export class PublicDataUpdateRequest {
return this.leafSlot.isZero() && this.newValue.isZero();
}

static isEmpty(x: PublicDataUpdateRequest) {
return x.isEmpty();
}

equals(other: PublicDataUpdateRequest) {
return this.leafSlot.equals(other.leafSlot) && this.newValue.equals(other.newValue);
}
Expand Down Expand Up @@ -325,8 +333,22 @@ export class CombinedAccumulatedData {
MAX_PUBLIC_CALL_STACK_LENGTH_PER_TX,
);

const nonSquashedWrites = [
...revertible.publicDataUpdateRequests,
...nonRevertible.publicDataUpdateRequests,
].filter(x => !x.isEmpty());

const squashedWrites = Array.from(
nonSquashedWrites
.reduce<Map<string, PublicDataUpdateRequest>>((acc, curr) => {
acc.set(curr.leafSlot.toString(), curr);
return acc;
}, new Map())
.values(),
);

const publicDataUpdateRequests = padArrayEnd(
[...nonRevertible.publicDataUpdateRequests, ...revertible.publicDataUpdateRequests].filter(x => !x.isEmpty()),
squashedWrites,
PublicDataUpdateRequest.empty(),
MAX_PUBLIC_DATA_UPDATE_REQUESTS_PER_TX,
);
Expand Down

0 comments on commit 30502c9

Please sign in to comment.