diff --git a/noir-projects/noir-contracts/contracts/fpc_contract/src/config.nr b/noir-projects/noir-contracts/contracts/fpc_contract/src/config.nr index b000f3b94b1c..00c1473d34bf 100644 --- a/noir-projects/noir-contracts/contracts/fpc_contract/src/config.nr +++ b/noir-projects/noir-contracts/contracts/fpc_contract/src/config.nr @@ -4,12 +4,12 @@ global CONFIG_LENGTH: u32 = 2; pub struct Config { pub accepted_asset: AztecAddress, // Asset the FPC accepts (denoted as AA below) - pub private_fee_recipient: AztecAddress, // Address to which AA is sent during the private fee payment flow + pub admin: AztecAddress, // Address to which AA is sent during the private fee payment flow } impl Serialize for Config { fn serialize(self: Self) -> [Field; CONFIG_LENGTH] { - [self.accepted_asset.to_field(), self.private_fee_recipient.to_field()] + [self.accepted_asset.to_field(), self.admin.to_field()] } } @@ -17,7 +17,7 @@ impl Deserialize for Config { fn deserialize(fields: [Field; CONFIG_LENGTH]) -> Self { Config { accepted_asset: AztecAddress::from_field(fields[0]), - private_fee_recipient: AztecAddress::from_field(fields[1]), + admin: AztecAddress::from_field(fields[1]), } } } diff --git a/noir-projects/noir-contracts/contracts/fpc_contract/src/main.nr b/noir-projects/noir-contracts/contracts/fpc_contract/src/main.nr index d0af0f5dd4a2..fd5b3e2751b0 100644 --- a/noir-projects/noir-contracts/contracts/fpc_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/fpc_contract/src/main.nr @@ -4,11 +4,14 @@ use dep::aztec::macros::aztec; /// Fee Payment Contract (FPC) allows users to pay for the transaction fee with an arbitrary asset. Supports private /// and public fee payment flows. -#[aztec] +/// +/// ***Note:*** +/// Accepted asset funds sent by the users to this contract stay in this contract and later on can +/// be pulled by the admin using the `pull_funds` function. contract FPC { use crate::config::Config; use dep::aztec::{ - macros::{functions::{initializer, internal, private, public, view}, storage::storage}, + macros::{functions::{initializer, internal, private, public}, storage::storage}, protocol_types::{abis::function_selector::FunctionSelector, address::AztecAddress}, state_vars::PublicImmutable, }; @@ -19,12 +22,12 @@ contract FPC { config: PublicImmutable, } - /// Initializes the contract with an accepted asset (AA) and a private fee recipient (address that receives AA - /// from users during the private fee payment flow). + /// Initializes the contract with an accepted asset (AA) and an admin (address that can pull accumulated AA funds + /// from this contract). #[public] #[initializer] - fn constructor(accepted_asset: AztecAddress, private_fee_recipient: AztecAddress) { - let config = Config { accepted_asset, private_fee_recipient }; + fn constructor(accepted_asset: AztecAddress, admin: AztecAddress) { + let config = Config { accepted_asset, admin }; storage.config.initialize(config); } @@ -33,13 +36,13 @@ contract FPC { /// /// ## Overview /// Uses partial notes to implement a refund flow which works as follows: - /// 1. `setup_refund` function subtracts the `max_fee` from user's balance of AA, prepares partial notes - /// for the `private_fee_recipient` (to obtain the payment in AA for the fee) and for the msg_sender (for refund - /// note of the AA) and sets a public teardown function (in which the partial notes will be finalized), + /// 1. `setup_refund` function subtracts the `max_fee` from user's balance of AA and prepares partial note + /// for the msg_sender (for refund note of the AA) and sets a public teardown function (in which the partial note + /// will be finalized and the fee will be paid to FPC), /// 2. then the private and public functions of a tx get executed, /// 3. at this point we know the tx fee so we can compute in the teardown function how much of AA the user needs - /// to pay to the `private_fee_recipient` and how much of it will be refunded back. Note that this is computed - /// based on an exchange rate between AA and fee juice. + /// to pay to the FPC and how much of it will be refunded back (this is computed based on an exchange rate between + /// AA and fee juice). Now we finalize the refund note for the user and we send the fee to the FPC in public, /// 4. the protocol deducts the actual fee denominated in fee juice from the FPC's balance. /// /// With this scheme a user has privately paid for the tx fee with an arbitrary AA (e.g. could be a stablecoin). @@ -52,9 +55,9 @@ contract FPC { // TODO(PR #8022): Once PublicImmutable performs only 1 merkle proof here, we'll save ~4k gates let config = storage.config.read(); - Token::at(config.accepted_asset) - .setup_refund(config.private_fee_recipient, context.msg_sender(), max_fee, nonce) - .call(&mut context); + Token::at(config.accepted_asset).setup_refund(context.msg_sender(), max_fee, nonce).call( + &mut context, + ); context.set_as_fee_payer(); } @@ -65,16 +68,9 @@ contract FPC { /// The refund flow works as follows: /// 1. We pull the `max_fee` from the user's balance of the AA to this contract (requires setting an authwit), /// 2. then the private and public functions of a tx get executed, - /// 3. at this point we know the tx fee so we can compute how much of AA the user needs to pay to - /// the `private_fee_recipient` and how much of it will be refunded back. Note that this is computed based on - /// an exchange rate between AA and fee juice. + /// 3. at this point we know the tx fee so we can compute how much of AA the user needs to pay to FPC and how much + /// of it will be refunded back. Note that this is computed based on an exchange rate between AA and fee juice. /// 4. the protocol deducts the actual fee denominated in fee juice from the FPC's balance. - /// - /// ***Note:*** - /// AA funds sent by the users to this contract stay in this contract and are not transferred - /// to the `private_fee_recipient`. In the private flow we needed a separate `private_fee_recipient` as this - /// contract does not have keys associated with it. In production we would want to have a method allowing for - /// pulling of these funds from this contract. #[private] fn fee_entrypoint_public(max_fee: Field, nonce: Field) { // TODO(PR #8022): Once PublicImmutable performs only 1 merkle proof here, we'll save ~4k gates @@ -112,6 +108,22 @@ contract FPC { .call(&mut context); } + /// Pulls all the accepted asset funds from this contract to the `to` address. Only the admin can call + /// this function. + #[public] + fn pull_funds(to: AztecAddress) { + // TODO(PR #8022): Once PublicImmutable performs only 1 merkle proof here, we'll save ~4k gates + let config = storage.config.read(); + + assert(context.msg_sender() == config.admin, "Only admin can pull funds"); + + let token = Token::at(config.accepted_asset); + + // We send the full balance to `to`. + let balance = token.balance_of_public(context.this_address()).view(&mut context); + token.transfer_in_public(context.this_address(), to, balance, 0).call(&mut context); + } + /// Note: Not marked as view as we need it to be callable as an entrypoint since in some places we need to obtain /// this value before we have access to an account contract (kernels do not allow for static entrypoints). #[private] diff --git a/noir-projects/noir-contracts/contracts/token_contract/src/main.nr b/noir-projects/noir-contracts/contracts/token_contract/src/main.nr index 6a204633929f..5470cdefc5bb 100644 --- a/noir-projects/noir-contracts/contracts/token_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/token_contract/src/main.nr @@ -255,7 +255,9 @@ contract Token { storage.balances.at(from).sub(from, U128::from_integer(amount)).emit( encode_and_encrypt_note_unconstrained(&mut context, from, from), ); - Token::at(context.this_address())._increase_public_balance(to, amount).enqueue(&mut context); + Token::at(context.this_address())._increase_public_balance_unsafe(to, amount).enqueue( + &mut context, + ); } // docs:end:transfer_to_public @@ -616,30 +618,10 @@ contract Token { finalization_payload.emit(); } - /// We need to use different randomness for the user and for the fee payer notes because if the randomness values - /// were the same we could fingerprint the user by doing the following: - /// 1) randomness_influence = fee_recipient_point - G_npk * fee_recipient_npk = - /// = (G_npk * fee_recipient_npk + G_rnd * randomness + G_slot * fee_recipient_slot) - /// - G_npk * fee_recipient_npk - G_slot * fee_recipient_slot = - /// = G_rnd * randomness - /// 2) user_fingerprint = user_point - randomness_influence = - /// = (G_npk * user_npk + G_rnd * randomness + G_slot * user_slot) - G_rnd * randomness = - /// = G_npk * user_npk + G_slot * user_slot - /// 3) Then the second time the user would use this fee paying contract we would recover the same fingerprint - /// and link that the 2 transactions were made by the same user. Given that it's expected that only - /// a limited set of fee paying contracts will be used and they will be known, searching for fingerprints - /// by trying different fee payers is a feasible attack. - /// - /// Note 1: fee_recipient_npk is part of the fee_recipient address preimage derivation, and is assumed to be known. - /// So if we have a known set of fee payer contract addresses getting fee_recipient_npk - /// and fee_recipient_slot is trivial (slot is derived in a `Map<...>` as a hash of balances map slot - /// and a fee payer address). - /// Note 2: fee_recipient_point and user_point above are public information because they are passed as args to - /// the public `complete_refund(...)` function. // docs:start:setup_refund + /// Called by fee payer contract (FPC) to set up a refund for a user during the private fee payment flow. #[private] fn setup_refund( - fee_recipient: AztecAddress, // Receives the fee note (set by the fee payer contract (FPC)) user: AztecAddress, // A user for which we are setting up the fee refund. max_fee: Field, // The maximum fee a user is willing to pay for the tx (denominated in FPC's accepted asset) nonce: Field, // A nonce to make authwitness unique. @@ -657,23 +639,23 @@ contract Token { U128::from_integer(max_fee), INITIAL_TRANSFER_CALL_MAX_NOTES, ); + // Emit the change note. storage.balances.at(user).add(user, change).emit(encode_and_encrypt_note_unconstrained( &mut context, user, user, )); - // 3. We prepare the partial notes - let fee_recipient_point_slot = - _prepare_private_balance_increase(user, fee_recipient, &mut context, storage); + // 3. Prepare the partial note for the refund. let user_point_slot = _prepare_private_balance_increase(user, user, &mut context, storage); // 4. Set the public teardown function to `complete_refund(...)`. Public teardown is the only time when a public // function has access to the final transaction fee, which is needed to compute the actual refund amount. + let fee_recipient = context.msg_sender(); // FPC is the fee recipient. context.set_public_teardown_function( context.this_address(), - comptime { FunctionSelector::from_signature("complete_refund(Field,Field,Field)") }, - [fee_recipient_point_slot, user_point_slot, max_fee], + comptime { FunctionSelector::from_signature("complete_refund((Field),Field,Field)") }, + [fee_recipient.to_field(), user_point_slot, max_fee], ); } // docs:end:setup_refund @@ -697,9 +679,11 @@ contract Token { // TODO(#7728): even though the max_fee should be a U128, we can't have that type in a contract interface due // to serialization issues. // docs:start:complete_refund + /// Executed as a public teardown function and is responsible for completing the refund in a private fee payment + /// flow. #[public] #[internal] - fn complete_refund(fee_recipient_slot: Field, user_slot: Field, max_fee: Field) { + fn complete_refund(fee_recipient: AztecAddress, user_slot: Field, max_fee: Field) { // TODO(#7728): Remove the next line let max_fee = U128::from_integer(max_fee); let tx_fee = U128::from_integer(context.transaction_fee()); @@ -708,18 +692,17 @@ contract Token { // TODO(#7796): we should try to prevent reverts here assert(max_fee >= tx_fee, "max fee not enough to cover tx fee"); - // 2. We compute the refund amount as the difference between funded amount and tx fee. + // 2. We compute the refund amount as the difference between funded amount and the tx fee. let refund_amount = max_fee - tx_fee; - // 3. We construct the note finalization payloads with the correct amounts and hiding points to get the note - // hashes and unencrypted logs. - let fee_recipient_finalization_payload = - UintNote::finalization_payload().new(&mut context, fee_recipient_slot, tx_fee); + // 3. We send the tx fee to the fee recipient in public. + _increase_public_balance(fee_recipient, tx_fee.to_field(), storage); + + // 4. We construct the user note finalization payload with the refund amount. let user_finalization_payload = UintNote::finalization_payload().new(&mut context, user_slot, refund_amount); - // 4. At last we emit the note hashes and the final note logs. - fee_recipient_finalization_payload.emit(); + // 5. At last we emit the user finalization note hash and the corresponding note log. user_finalization_payload.emit(); // --> Once the tx is settled user and fee recipient can add the notes to their pixies. } @@ -729,11 +712,20 @@ contract Token { // docs:start:increase_public_balance #[public] #[internal] - fn _increase_public_balance(to: AztecAddress, amount: Field) { + fn _increase_public_balance_unsafe(to: AztecAddress, amount: Field) { + _increase_public_balance(to, amount, storage); + } + // docs:end:increase_public_balance + + #[contract_library_method] + fn _increase_public_balance( + to: AztecAddress, + amount: Field, + storage: Storage<&mut PublicContext>, + ) { let new_balance = storage.public_balances.at(to).read().add(U128::from_integer(amount)); storage.public_balances.at(to).write(new_balance); } - // docs:end:increase_public_balance // docs:start:reduce_total_supply #[public] diff --git a/noir-projects/noir-contracts/contracts/token_contract/src/test/refunds.nr b/noir-projects/noir-contracts/contracts/token_contract/src/test/refunds.nr index ef1555684978..5e01965e8a22 100644 --- a/noir-projects/noir-contracts/contracts/token_contract/src/test/refunds.nr +++ b/noir-projects/noir-contracts/contracts/token_contract/src/test/refunds.nr @@ -34,7 +34,7 @@ unconstrained fn setup_refund_success() { let _ = OracleMock::mock("getRandomField").returns(fee_payer_randomness); let setup_refund_from_call_interface = - Token::at(token_contract_address).setup_refund(fee_payer, user, funded_amount, nonce); + Token::at(token_contract_address).setup_refund(user, funded_amount, nonce); authwit_cheatcodes::add_private_authwit_from_call_interface( user, @@ -86,7 +86,7 @@ unconstrained fn setup_refund_insufficient_funded_amount() { let nonce = random(); let setup_refund_from_call_interface = - Token::at(token_contract_address).setup_refund(fee_payer, user, funded_amount, nonce); + Token::at(token_contract_address).setup_refund(user, funded_amount, nonce); authwit_cheatcodes::add_private_authwit_from_call_interface( user, diff --git a/yarn-project/cli/src/cmds/devnet/bootstrap_network.ts b/yarn-project/cli/src/cmds/devnet/bootstrap_network.ts index 462ae4649bd9..08961e12b4fe 100644 --- a/yarn-project/cli/src/cmds/devnet/bootstrap_network.ts +++ b/yarn-project/cli/src/cmds/devnet/bootstrap_network.ts @@ -61,8 +61,8 @@ export async function bootstrapNetwork( await initPortal(pxe, l1Clients, erc20Address, portalAddress, bridge.address); - const feeRecipient = wallet.getAddress(); - const fpc = await deployFPC(wallet, token.address, feeRecipient); + const fpcAdmin = wallet.getAddress(); + const fpc = await deployFPC(wallet, token.address, fpcAdmin); const counter = await deployCounter(wallet); // NOTE: Disabling for now in order to get devnet running @@ -219,14 +219,12 @@ async function initPortal( async function deployFPC( wallet: Wallet, tokenAddress: AztecAddress, - feeRecipient: AztecAddress, + admin: AztecAddress, ): Promise { // eslint-disable-next-line @typescript-eslint/ban-ts-comment // @ts-ignore - Importing noir-contracts.js even in devDeps results in a circular dependency error. Need to ignore because this line doesn't cause an error in a dev environment const { FPCContract } = await import('@aztec/noir-contracts.js'); - const fpc = await FPCContract.deploy(wallet, tokenAddress, feeRecipient) - .send({ universalDeploy: true }) - .deployed(waitOpts); + const fpc = await FPCContract.deploy(wallet, tokenAddress, admin).send({ universalDeploy: true }).deployed(waitOpts); const info: ContractDeploymentInfo = { address: fpc.address, initHash: fpc.instance.initializationHash, diff --git a/yarn-project/end-to-end/src/benchmarks/bench_tx_size_fees.test.ts b/yarn-project/end-to-end/src/benchmarks/bench_tx_size_fees.test.ts index 464773af6c7e..afd6cf567d11 100644 --- a/yarn-project/end-to-end/src/benchmarks/bench_tx_size_fees.test.ts +++ b/yarn-project/end-to-end/src/benchmarks/bench_tx_size_fees.test.ts @@ -47,7 +47,10 @@ describe('benchmarks/tx_size_fees', () => { beforeAll(async () => { feeJuice = await FeeJuiceContract.at(ProtocolContractAddress.FeeJuice, aliceWallet); token = await TokenContract.deploy(aliceWallet, aliceWallet.getAddress(), 'test', 'test', 18).send().deployed(); - fpc = await FPCContract.deploy(aliceWallet, token.address, sequencerAddress).send().deployed(); + + // We set Alice as the FPC admin to avoid the need for deployment of another account. + const fpcAdmin = aliceWallet.getAddress(); + fpc = await FPCContract.deploy(aliceWallet, token.address, fpcAdmin).send().deployed(); }); // mint tokens diff --git a/yarn-project/end-to-end/src/e2e_fees/fees_test.ts b/yarn-project/end-to-end/src/e2e_fees/fees_test.ts index ffbf4d01f665..270a8a1f1c05 100644 --- a/yarn-project/end-to-end/src/e2e_fees/fees_test.ts +++ b/yarn-project/end-to-end/src/e2e_fees/fees_test.ts @@ -62,7 +62,7 @@ export class FeesTest { public sequencerAddress!: AztecAddress; public coinbase!: EthAddress; - public feeRecipient!: AztecAddress; // Account that receives the fees from the fee refund flow. + public fpcAdmin!: AztecAddress; public gasSettings!: GasSettings; @@ -142,8 +142,8 @@ export class FeesTest { [this.aliceWallet, this.bobWallet] = this.wallets.slice(0, 2); [this.aliceAddress, this.bobAddress, this.sequencerAddress] = this.wallets.map(w => w.getAddress()); - // We like sequencer so we send him the fees. - this.feeRecipient = this.sequencerAddress; + // We set Alice as the FPC admin to avoid the need for deployment of another account. + this.fpcAdmin = this.aliceAddress; this.feeJuiceContract = await FeeJuiceContract.at(getCanonicalFeeJuice().address, this.aliceWallet); const bobInstance = await this.bobWallet.getContractInstance(this.bobAddress); @@ -225,7 +225,7 @@ export class FeesTest { expect(await context.pxe.isContractPubliclyDeployed(feeJuiceContract.address)).toBe(true); const bananaCoin = this.bananaCoin; - const bananaFPC = await FPCContract.deploy(this.aliceWallet, bananaCoin.address, this.feeRecipient) + const bananaFPC = await FPCContract.deploy(this.aliceWallet, bananaCoin.address, this.fpcAdmin) .send() .deployed();