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

feat: public refunds via FPC #4750

Merged
merged 6 commits into from
Mar 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
just-mitch marked this conversation as resolved.
Show resolved Hide resolved
#[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_u120 = 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": [
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removing this, otheriwse it breaks yarn-project-base when merging to master. See here https://aztecprotocol.slack.com/archives/C03P17YHVK8/p1707752827905929

I think the solution is to just add jest.config.ts to all the yarn projects instead of configuring from package.json.

"../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
Loading
Loading