From e23fd0dd95f193070e4a646ce5953d22e8a09476 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Bene=C5=A1?= Date: Wed, 18 Dec 2024 09:56:44 -0600 Subject: [PATCH] refactor: fpc optimization, cleanup + docs (#10555) --- cspell.json | 1 + .../contracts/amm_contract/src/main.nr | 2 +- .../contracts/fpc_contract/src/config.nr | 23 +++ .../contracts/fpc_contract/src/lib.nr | 10 -- .../contracts/fpc_contract/src/main.nr | 158 ++++++++++++++---- .../contracts/fpc_contract/src/settings.nr | 23 --- .../contracts/nft_contract/src/main.nr | 2 +- .../contracts/token_contract/src/main.nr | 90 +++++----- .../token_contract/src/test/refunds.nr | 8 +- .../noir-contracts/scripts/flamegraph.sh | 4 +- .../contract/contract_function_interaction.ts | 21 ++- .../src/fee/fee_juice_payment_method.ts | 2 +- .../aztec.js/src/fee/fee_payment_method.ts | 2 +- .../aztec.js/src/fee/no_fee_payment_method.ts | 2 +- .../src/fee/private_fee_payment_method.ts | 62 +++++-- .../src/fee/public_fee_payment_method.ts | 55 ++++-- .../src/contract/contract_class.ts | 2 +- .../cli-wallet/src/utils/options/fees.ts | 15 +- .../cli/src/cmds/devnet/bootstrap_network.ts | 10 +- .../src/benchmarks/bench_prover.test.ts | 3 +- .../src/benchmarks/bench_tx_size_fees.test.ts | 9 +- .../src/e2e_fees/account_init.test.ts | 17 +- .../src/e2e_fees/dapp_subscription.test.ts | 24 +-- .../end-to-end/src/e2e_fees/failures.test.ts | 52 +++--- .../end-to-end/src/e2e_fees/fees_test.ts | 8 +- .../src/e2e_fees/gas_estimation.test.ts | 2 +- .../src/e2e_fees/private_payments.test.ts | 84 +++------- .../src/e2e_fees/public_payments.test.ts | 2 +- .../pxe/src/pxe_service/error_enriching.ts | 2 +- 29 files changed, 394 insertions(+), 301 deletions(-) create mode 100644 noir-projects/noir-contracts/contracts/fpc_contract/src/config.nr delete mode 100644 noir-projects/noir-contracts/contracts/fpc_contract/src/lib.nr delete mode 100644 noir-projects/noir-contracts/contracts/fpc_contract/src/settings.nr diff --git a/cspell.json b/cspell.json index 0b8d2a27549..3ed9bc44cf4 100644 --- a/cspell.json +++ b/cspell.json @@ -11,6 +11,7 @@ "asyncify", "auditability", "authwit", + "authwits", "Automine", "autonat", "autorun", diff --git a/noir-projects/noir-contracts/contracts/amm_contract/src/main.nr b/noir-projects/noir-contracts/contracts/amm_contract/src/main.nr index fe405512cf4..b14258f9d0f 100644 --- a/noir-projects/noir-contracts/contracts/amm_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/amm_contract/src/main.nr @@ -120,7 +120,7 @@ contract AMM { // We then complete the flow in public. Note that the type of operation and amounts will all be publicly known, // but the identity of the caller is not revealed despite us being able to send tokens to them by completing the - // partial notees. + // partial notes. AMM::at(context.this_address()) ._add_liquidity( config, diff --git a/noir-projects/noir-contracts/contracts/fpc_contract/src/config.nr b/noir-projects/noir-contracts/contracts/fpc_contract/src/config.nr new file mode 100644 index 00000000000..00c1473d34b --- /dev/null +++ b/noir-projects/noir-contracts/contracts/fpc_contract/src/config.nr @@ -0,0 +1,23 @@ +use dep::aztec::protocol_types::{address::AztecAddress, traits::{Deserialize, Serialize}}; + +global CONFIG_LENGTH: u32 = 2; + +pub struct Config { + pub accepted_asset: AztecAddress, // Asset the FPC accepts (denoted as AA below) + 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.admin.to_field()] + } +} + +impl Deserialize for Config { + fn deserialize(fields: [Field; CONFIG_LENGTH]) -> Self { + Config { + accepted_asset: AztecAddress::from_field(fields[0]), + admin: AztecAddress::from_field(fields[1]), + } + } +} diff --git a/noir-projects/noir-contracts/contracts/fpc_contract/src/lib.nr b/noir-projects/noir-contracts/contracts/fpc_contract/src/lib.nr deleted file mode 100644 index e9aad0aeb73..00000000000 --- a/noir-projects/noir-contracts/contracts/fpc_contract/src/lib.nr +++ /dev/null @@ -1,10 +0,0 @@ -use dep::aztec::context::PublicContext; - -pub fn compute_rebate(context: PublicContext, initial_amount: Field) -> Field { - let actual_fee = context.transaction_fee(); - assert( - !initial_amount.lt(actual_fee), - "Initial amount paid to the paymaster does not cover actual fee", - ); - initial_amount - actual_fee -} 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 96ca07817a2..d61d242a1d7 100644 --- a/noir-projects/noir-contracts/contracts/fpc_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/fpc_contract/src/main.nr @@ -1,12 +1,16 @@ -mod lib; -mod settings; +mod config; 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. +/// +/// ***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. #[aztec] contract FPC { - use crate::lib::compute_rebate; - use crate::settings::Settings; + use crate::config::Config; use dep::aztec::{ macros::{functions::{initializer, internal, private, public}, storage::storage}, protocol_types::{abis::function_selector::FunctionSelector, address::AztecAddress}, @@ -16,61 +20,153 @@ contract FPC { #[storage] struct Storage { - settings: PublicImmutable, + config: PublicImmutable, } + /// 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(other_asset: AztecAddress, admin: AztecAddress) { - let settings = Settings { other_asset, admin }; - storage.settings.initialize(settings); + fn constructor(accepted_asset: AztecAddress, admin: AztecAddress) { + let config = Config { accepted_asset, admin }; + storage.config.initialize(config); } + /// Pays for the tx fee with msg_sender's private balance of accepted asset (AA). The maximum fee a user is willing + /// to pay is defined by `max_fee` and is denominated in AA. + /// + /// ## Overview + /// Uses partial notes to implement a refund flow which works as follows: + /// Setup Phase: + /// 1. This `setup_refund` function: + /// - Calls the AA token contract, which: + /// - subtracts the `max_fee` from the user's balance; + /// - prepares a partial note for the user (which will be used to later refund the user any unspent fee); + /// - sets a public teardown function (within the same AA token contract), where at the end of the tx + /// a fee (denominated in AA) will be transferred to the FPC in public, and a partial note will be finalized + /// with the refund amount (also denominated in AA). + /// - Sets itself as the `fee_payer` of the tx; meaning this contract will be responsible for ultimately + /// transferring the `tx_fee` -- denominated in fee juice -- to the protocol, during the later "teardown" + /// phase of this tx. + /// + /// Execution Phase: + /// 2. Then the private and public functions of the tx get executed. + /// + /// Teardown Phase: + /// 3. By this point, the protocol has computed the `tx_fee` (denominated in "fee juice"). So now we can + /// execute the "teardown function" which was lined-up during the earlier "setup phase". + /// Within the teardown function, we: + /// - compute how much of the `max_fee` (denominated in AA) the user needs to pay to the FPC, + /// and how much of it will be refunded back to the user. Since the protocol-calculated `tx_fee` is + /// denominated in fee juice, and not in this FPC's AA, an equivalent value of AA is computed based + /// on an exchange rate between AA and fee juice. + /// - finalize the refund note with a value of `max_fee - tx_fee` for the user; + /// - send the tx fee to the FPC in public. + /// + /// Protocol-enshrined fee-payment phase: + /// 4. The protocol deducts the protocol-calculated `tx_fee` (denominated in fee juice) from the `fee_payer`'s + /// balance (which in this case is this FPC's balance), which is a special storage slot in a protocol-controlled + /// "fee juice" contract. + /// + /// With this scheme a user has privately paid for the tx fee with an arbitrary AA (e.g. could be a stablecoin), + /// by paying this FPC. This FPC has in turn paid the protocol-mandated `tx_fee` (denominated in fee + /// juice). + /// + /// ***Note:*** + /// This flow allows us to pay for the tx with msg_sender's private balance of AA and hence msg_sender's identity + /// is not revealed. We do, however, reveal: + /// - the `max_fee`; + /// - which FPC has been used to make the payment; + /// - the asset which was used to make the payment. #[private] - fn fee_entrypoint_private(amount: Field, asset: AztecAddress, nonce: Field) { + fn fee_entrypoint_private(max_fee: Field, nonce: Field) { // TODO(PR #8022): Once PublicImmutable performs only 1 merkle proof here, we'll save ~4k gates - let settings = storage.settings.read(); + let config = storage.config.read(); - assert(asset == settings.other_asset); - - Token::at(asset).setup_refund(settings.admin, context.msg_sender(), amount, nonce).call( + Token::at(config.accepted_asset).setup_refund(context.msg_sender(), max_fee, nonce).call( &mut context, ); context.set_as_fee_payer(); } + /// Pays for the tx fee with msg_sender's public balance of accepted asset (AA). The maximum fee a user is willing + /// to pay is defined by `max_fee` and is denominated in AA. + /// + /// ## Overview + /// The refund flow works as follows: + /// Setup phase: + /// 1. This `fee_entrypoint_public` function: + /// - Transfers the `max_fee` from the user's balance of the accepted asset to this contract. + /// - Sets itself as the `fee_payer` of the tx. + /// - Sets a public teardown function in which the refund will be paid back to the user in public. + /// + /// Execution phase: + /// 2. Then the private and public functions of the tx get executed. + /// + /// Teardown phase: + /// 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. We send the refund back to the user in public. + /// + /// Protocol-enshrined fee-payment phase: + /// 4. The protocol deducts the actual fee denominated in fee juice from the FPC's balance. #[private] - fn fee_entrypoint_public(amount: Field, asset: AztecAddress, nonce: Field) { - FPC::at(context.this_address()) - .prepare_fee(context.msg_sender(), amount, asset, nonce) + 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 + let config = storage.config.read(); + + // We pull the max fee from the user's balance of the accepted asset to this contract. + // docs:start:public_call + Token::at(config.accepted_asset) + .transfer_in_public(context.msg_sender(), context.this_address(), max_fee, nonce) .enqueue(&mut context); + // docs:end:public_call + context.set_as_fee_payer(); // TODO(#6277) for improving interface: - // FPC::at(context.this_address()).pay_refund(context.msg_sender(), amount, asset).set_public_teardown_function(&mut context); + // FPC::at(context.this_address()).pay_refund(...).set_public_teardown_function(&mut context); context.set_public_teardown_function( context.this_address(), comptime { FunctionSelector::from_signature("pay_refund((Field),Field,(Field))") }, - [context.msg_sender().to_field(), amount, asset.to_field()], + [context.msg_sender().to_field(), max_fee, config.accepted_asset.to_field()], ); } + /// Pays the refund to the `refund_recipient`. The refund is the difference between the `max_fee` and + /// the actual fee. `accepted_asset` is the asset in which the refund is paid. It's passed as an argument + /// to avoid the need for another read from public storage. #[public] #[internal] - fn prepare_fee(from: AztecAddress, amount: Field, asset: AztecAddress, nonce: Field) { - // docs:start:public_call - Token::at(asset).transfer_in_public(from, context.this_address(), amount, nonce).call( - &mut context, - ); - // docs:end:public_call + fn pay_refund(refund_recipient: AztecAddress, max_fee: Field, accepted_asset: AztecAddress) { + let actual_fee = context.transaction_fee(); + assert(!max_fee.lt(actual_fee), "Max fee paid to the paymaster does not cover actual fee"); + // TODO(#10805): Introduce a real exchange rate + let refund = max_fee - actual_fee; + + Token::at(accepted_asset) + .transfer_in_public(context.this_address(), refund_recipient, refund, 0) + .call(&mut context); } + /// Pulls all the accepted asset funds from this contract to the `to` address. Only the admin can call + /// this function. #[public] - #[internal] - fn pay_refund(refund_address: AztecAddress, amount: Field, asset: AztecAddress) { - // Just do public refunds for the present - let refund = compute_rebate(context, amount); - Token::at(asset).transfer_in_public(context.this_address(), refund_address, refund, 0).call( - &mut context, - ); + 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] + fn get_accepted_asset() -> AztecAddress { + storage.config.read().accepted_asset } } diff --git a/noir-projects/noir-contracts/contracts/fpc_contract/src/settings.nr b/noir-projects/noir-contracts/contracts/fpc_contract/src/settings.nr deleted file mode 100644 index c3717ca1a7e..00000000000 --- a/noir-projects/noir-contracts/contracts/fpc_contract/src/settings.nr +++ /dev/null @@ -1,23 +0,0 @@ -use dep::aztec::protocol_types::{address::AztecAddress, traits::{Deserialize, Serialize}}; - -global SETTINGS_LENGTH: u32 = 2; - -pub struct Settings { - other_asset: AztecAddress, - admin: AztecAddress, -} - -impl Serialize for Settings { - fn serialize(self: Self) -> [Field; SETTINGS_LENGTH] { - [self.other_asset.to_field(), self.admin.to_field()] - } -} - -impl Deserialize for Settings { - fn deserialize(fields: [Field; SETTINGS_LENGTH]) -> Self { - Settings { - other_asset: AztecAddress::from_field(fields[0]), - admin: AztecAddress::from_field(fields[1]), - } - } -} diff --git a/noir-projects/noir-contracts/contracts/nft_contract/src/main.nr b/noir-projects/noir-contracts/contracts/nft_contract/src/main.nr index 22d12012c5a..109fe6a57f4 100644 --- a/noir-projects/noir-contracts/contracts/nft_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/nft_contract/src/main.nr @@ -152,7 +152,7 @@ contract NFT { // We prepare the private balance increase. let hiding_point_slot = _prepare_private_balance_increase(to, &mut context, storage); - // At last we finalize the transfer. Usafe of the `unsafe` method here is safe because we set the `from` + // At last we finalize the transfer. Usage of the `unsafe` method here is safe because we set the `from` // function argument to a message sender, guaranteeing that he can transfer only his own NFTs. nft._finalize_transfer_to_private_unsafe(from, token_id, hiding_point_slot).enqueue( &mut context, 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 642244cf251..f1ec6266f7e 100644 --- a/noir-projects/noir-contracts/contracts/token_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/token_contract/src/main.nr @@ -505,6 +505,9 @@ contract Token { // docs:end:finalize_transfer_to_private // docs:start:finalize_transfer_to_private_unsafe + /// This is a wrapper around `_finalize_transfer_to_private` placed here so that a call + /// to `_finalize_transfer_to_private` can be enqueued. Called unsafe as it does not check `from` (this has to be + /// done in the calling function). #[public] #[internal] fn _finalize_transfer_to_private_unsafe( @@ -541,7 +544,7 @@ contract Token { // docs:start:mint_to_private /// Mints token `amount` to a private balance of `to`. Message sender has to have minter permissions (checked - /// in the enqueud call). + /// in the enqueued call). #[private] fn mint_to_private( from: AztecAddress, // sender of the tag: TODO(#9887): this is not great? @@ -613,64 +616,44 @@ 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_payer_point - G_npk * fee_payer_npk = - /// = (G_npk * fee_payer_npk + G_rnd * randomness + G_slot * fee_payer_slot) - /// - G_npk * fee_payer_npk - G_slot * fee_payer_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_payer_npk is part of the fee_payer address preimage derivation, and is assumed to be known. So - // if we have a known set of fee payer contract addresses getting fee_payer_npk and fee_payer_slot is - // trivial (slot is derived in a `Map<...>` as a hash of balances map slot and a fee payer address). - /// Note 2: fee_payer_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_payer: AztecAddress, // Address of the entity which will receive the fee note. user: AztecAddress, // A user for which we are setting up the fee refund. - funded_amount: Field, // The amount the user funded the fee payer with (represents fee limit). + max_fee: Field, // The maximum fee a user is willing to pay for the tx. nonce: Field, // A nonce to make authwitness unique. ) { - // 1. This function is called by fee paying contract (fee_payer) when setting up a refund so we need to support - // the authwit flow here and check that the user really permitted fee_payer to set up a refund on their behalf. + // 1. This function is called by FPC when setting up a refund so we need to support the authwit flow here + // and check that the user really permitted fee_recipient to set up a refund on their behalf. assert_current_call_valid_authwit(&mut context, user); - // 3. Deduct the funded amount from the user's balance - this is a maximum fee a user is willing to pay - // (called fee limit in aztec spec). The difference between fee limit and the actual tx fee will be refunded - // to the user in the `complete_refund(...)` function. + // 2. Deduct the max fee from user's balance. The difference between max fee and the actual tx fee will + // be refunded to the user in the `complete_refund(...)` function. let change = subtract_balance( &mut context, storage, user, - U128::from_integer(funded_amount), + 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, )); - // 4. We prepare the partial notes - let fee_payer_point_slot = - _prepare_private_balance_increase(user, fee_payer, &mut context, storage); + // 3. Prepare the partial note for the refund. let user_point_slot = _prepare_private_balance_increase(user, user, &mut context, storage); - // 5. Set the public teardown function to `complete_refund(...)`. Public teardown is the only time when a public + // 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_payer_point_slot, user_point_slot, funded_amount], + comptime { FunctionSelector::from_signature("complete_refund((Field),Field,Field)") }, + [fee_recipient.to_field(), user_point_slot, max_fee], ); } // docs:end:setup_refund @@ -691,32 +674,34 @@ contract Token { context.storage_write(slot + aztec::protocol_types::point::POINT_LENGTH as Field, setup_log); } - // TODO(#7728): even though the funded_amount should be a U128, we can't have that type in a contract interface due + // 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_payer_slot: Field, user_slot: Field, funded_amount: Field) { + fn complete_refund(fee_recipient: AztecAddress, user_slot: Field, max_fee: Field) { // TODO(#7728): Remove the next line - let funded_amount = U128::from_integer(funded_amount); + let max_fee = U128::from_integer(max_fee); let tx_fee = U128::from_integer(context.transaction_fee()); // 1. We check that user funded the fee payer contract with at least the transaction fee. // TODO(#7796): we should try to prevent reverts here - assert(funded_amount >= tx_fee, "funded amount not enough to cover tx fee"); + 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 the tx fee. + // TODO(#10805): Introduce a real exchange rate + let refund_amount = max_fee - tx_fee; - // 2. We compute the refund amount as the difference between funded amount and tx fee. - let refund_amount = funded_amount - tx_fee; + // 3. We send the tx fee to the fee recipient in public. + _increase_public_balance_inner(fee_recipient, tx_fee.to_field(), storage); - // 3. We construct the note finalization payloads with the correct amounts and hiding points to get the note - // hashes and unencrypted logs. - let fee_payer_finalization_payload = - UintNote::finalization_payload().new(&mut context, fee_payer_slot, tx_fee); + // 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_payer_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. } @@ -724,13 +709,24 @@ contract Token { /// Internal /// // docs:start:increase_public_balance + /// TODO(#9180): Consider adding macro support for functions callable both as an entrypoint and as an internal + /// function. #[public] #[internal] fn _increase_public_balance(to: AztecAddress, amount: Field) { + _increase_public_balance_inner(to, amount, storage); + } + // docs:end:increase_public_balance + + #[contract_library_method] + fn _increase_public_balance_inner( + 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 d8797b3ac8c..5e01965e8a2 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, @@ -71,7 +71,7 @@ unconstrained fn setup_refund_success() { // This test should be reworked when final support for partial notes is in // Once that happens, the expected error message is commented out below -//#[test(should_fail_with="funded amount not enough to cover tx fee")] +//#[test(should_fail_with="max fee not enough to cover tx fee")] #[test(should_fail_with = "Balance too low")] unconstrained fn setup_refund_insufficient_funded_amount() { let (env, token_contract_address, owner, recipient, _mint_amount) = @@ -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, @@ -96,6 +96,6 @@ unconstrained fn setup_refund_insufficient_funded_amount() { env.impersonate(fee_payer); - // The following should fail with "funded amount not enough to cover tx fee" because funded amount is 0 + // The following should fail with "max fee not enough to cover tx fee" because funded amount is 0 setup_refund_from_call_interface.call(&mut env.private()) } diff --git a/noir-projects/noir-contracts/scripts/flamegraph.sh b/noir-projects/noir-contracts/scripts/flamegraph.sh index 8db540142e6..ca5ae33c18b 100755 --- a/noir-projects/noir-contracts/scripts/flamegraph.sh +++ b/noir-projects/noir-contracts/scripts/flamegraph.sh @@ -63,10 +63,10 @@ FUNCTION_ARTIFACT="${ARTIFACT_NAME}-${FUNCTION}.json" mkdir -p "$SCRIPT_DIR/../dest" # At last, generate the flamegraph -$PROFILER gates-flamegraph --artifact-path "$SCRIPT_DIR/../target/$FUNCTION_ARTIFACT" --backend-path "$SCRIPT_DIR/../../../barretenberg/cpp/build/bin/bb" --backend-gates-command "gates_mega_honk" --output "$SCRIPT_DIR/../dest" +$PROFILER gates --artifact-path "$SCRIPT_DIR/../target/$FUNCTION_ARTIFACT" --backend-path "$SCRIPT_DIR/../../../barretenberg/cpp/build/bin/bb" --backend-gates-command "gates_mega_honk" --output "$SCRIPT_DIR/../dest" # serve the file over http -echo "Serving flamegraph at http://0.0.0.0:8000/main_gates.svg" +echo "Serving flamegraph at http://0.0.0.0:8000/main::gates.svg" python3 -m http.server --directory "$SCRIPT_DIR/../dest" 8000 # Clean up before exiting diff --git a/yarn-project/aztec.js/src/contract/contract_function_interaction.ts b/yarn-project/aztec.js/src/contract/contract_function_interaction.ts index 71117bdf280..35d2231522d 100644 --- a/yarn-project/aztec.js/src/contract/contract_function_interaction.ts +++ b/yarn-project/aztec.js/src/contract/contract_function_interaction.ts @@ -103,13 +103,20 @@ export class ContractFunctionInteraction extends BaseContractInteraction { const txRequest = await this.create(); const simulatedTx = await this.wallet.simulateTx(txRequest, true, options?.from, options?.skipTxValidation); - // As account entrypoints are private, for private functions we retrieve the return values from the first nested call - // since we're interested in the first set of values AFTER the account entrypoint - // For public functions we retrieve the first values directly from the public output. - const rawReturnValues = - this.functionDao.functionType == FunctionType.PRIVATE - ? simulatedTx.getPrivateReturnValues().nested?.[0].values - : simulatedTx.getPublicReturnValues()?.[0].values; + let rawReturnValues; + if (this.functionDao.functionType == FunctionType.PRIVATE) { + if (simulatedTx.getPrivateReturnValues().nested.length > 0) { + // The function invoked is private and it was called via an account contract + // TODO(#10631): There is a bug here: this branch might be triggered when there is no-account contract as well + rawReturnValues = simulatedTx.getPrivateReturnValues().nested[0].values; + } else { + // The function invoked is private and it was called directly (without account contract) + rawReturnValues = simulatedTx.getPrivateReturnValues().values; + } + } else { + // For public functions we retrieve the first values directly from the public output. + rawReturnValues = simulatedTx.getPublicReturnValues()?.[0].values; + } return rawReturnValues ? decodeFromAbi(this.functionDao.returnTypes, rawReturnValues) : []; } diff --git a/yarn-project/aztec.js/src/fee/fee_juice_payment_method.ts b/yarn-project/aztec.js/src/fee/fee_juice_payment_method.ts index 94a3dc954ba..c5a8edfad28 100644 --- a/yarn-project/aztec.js/src/fee/fee_juice_payment_method.ts +++ b/yarn-project/aztec.js/src/fee/fee_juice_payment_method.ts @@ -11,7 +11,7 @@ export class FeeJuicePaymentMethod implements FeePaymentMethod { constructor(protected sender: AztecAddress) {} getAsset() { - return ProtocolContractAddress.FeeJuice; + return Promise.resolve(ProtocolContractAddress.FeeJuice); } getFunctionCalls(): Promise { diff --git a/yarn-project/aztec.js/src/fee/fee_payment_method.ts b/yarn-project/aztec.js/src/fee/fee_payment_method.ts index c27710efedb..6cdf795cd56 100644 --- a/yarn-project/aztec.js/src/fee/fee_payment_method.ts +++ b/yarn-project/aztec.js/src/fee/fee_payment_method.ts @@ -7,7 +7,7 @@ import { type AztecAddress } from '@aztec/foundation/aztec-address'; */ export interface FeePaymentMethod { /** The asset used to pay the fee. */ - getAsset(): AztecAddress; + getAsset(): Promise; /** * Creates a function call to pay the fee in the given asset. * @param gasSettings - The gas limits and max fees. diff --git a/yarn-project/aztec.js/src/fee/no_fee_payment_method.ts b/yarn-project/aztec.js/src/fee/no_fee_payment_method.ts index e9ed5809bc7..3692e990328 100644 --- a/yarn-project/aztec.js/src/fee/no_fee_payment_method.ts +++ b/yarn-project/aztec.js/src/fee/no_fee_payment_method.ts @@ -10,7 +10,7 @@ export class NoFeePaymentMethod implements FeePaymentMethod { constructor() {} getAsset() { - return AztecAddress.ZERO; + return Promise.resolve(AztecAddress.ZERO); } getFunctionCalls(): Promise { diff --git a/yarn-project/aztec.js/src/fee/private_fee_payment_method.ts b/yarn-project/aztec.js/src/fee/private_fee_payment_method.ts index e505901da36..717f5aec04d 100644 --- a/yarn-project/aztec.js/src/fee/private_fee_payment_method.ts +++ b/yarn-project/aztec.js/src/fee/private_fee_payment_method.ts @@ -5,17 +5,17 @@ import { type AztecAddress } from '@aztec/foundation/aztec-address'; import { Fr } from '@aztec/foundation/fields'; import { type Wallet } from '../account/wallet.js'; +import { ContractFunctionInteraction } from '../contract/contract_function_interaction.js'; +import { SignerlessWallet } from '../wallet/signerless_wallet.js'; import { type FeePaymentMethod } from './fee_payment_method.js'; /** * Holds information about how the fee for a transaction is to be paid. */ export class PrivateFeePaymentMethod implements FeePaymentMethod { + private assetPromise: Promise | null = null; + constructor( - /** - * The asset used to pay the fee. - */ - private asset: AztecAddress, /** * Address which will hold the fee payment. */ @@ -26,11 +26,6 @@ export class PrivateFeePaymentMethod implements FeePaymentMethod { */ private wallet: Wallet, - /** - * Address that the FPC sends notes it receives to. - */ - private feeRecipient: AztecAddress, - /** * If true, the max fee will be set to 1. * TODO(#7694): Remove this param once the lacking feature in TXE is implemented. @@ -42,8 +37,43 @@ export class PrivateFeePaymentMethod implements FeePaymentMethod { * The asset used to pay the fee. * @returns The asset used to pay the fee. */ - getAsset() { - return this.asset; + getAsset(): Promise { + if (!this.assetPromise) { + // We use signer-less wallet because this function could be triggered before the associated account is deployed. + const signerlessWallet = new SignerlessWallet(this.wallet); + + const interaction = new ContractFunctionInteraction( + signerlessWallet, + this.paymentContract, + { + name: 'get_accepted_asset', + functionType: FunctionType.PRIVATE, + isInternal: false, + isStatic: false, + parameters: [], + returnTypes: [ + { + kind: 'struct', + path: 'authwit::aztec::protocol_types::address::aztec_address::AztecAddress', + fields: [ + { + name: 'inner', + type: { + kind: 'field', + }, + }, + ], + }, + ], + errorTypes: {}, + isInitializer: false, + }, + [], + ); + + this.assetPromise = interaction.simulate(); + } + return this.assetPromise!; } getFeePayer(): Promise { @@ -65,11 +95,11 @@ export class PrivateFeePaymentMethod implements FeePaymentMethod { caller: this.paymentContract, action: { name: 'setup_refund', - args: [this.feeRecipient.toField(), this.wallet.getAddress().toField(), maxFee, nonce], - selector: FunctionSelector.fromSignature('setup_refund((Field),(Field),Field,Field)'), + args: [this.wallet.getAddress().toField(), maxFee, nonce], + selector: FunctionSelector.fromSignature('setup_refund((Field),Field,Field)'), type: FunctionType.PRIVATE, isStatic: false, - to: this.asset, + to: await this.getAsset(), returnTypes: [], }, }); @@ -78,10 +108,10 @@ export class PrivateFeePaymentMethod implements FeePaymentMethod { { name: 'fee_entrypoint_private', to: this.paymentContract, - selector: FunctionSelector.fromSignature('fee_entrypoint_private(Field,(Field),Field)'), + selector: FunctionSelector.fromSignature('fee_entrypoint_private(Field,Field)'), type: FunctionType.PRIVATE, isStatic: false, - args: [maxFee, this.asset.toField(), nonce], + args: [maxFee, nonce], returnTypes: [], }, ]; diff --git a/yarn-project/aztec.js/src/fee/public_fee_payment_method.ts b/yarn-project/aztec.js/src/fee/public_fee_payment_method.ts index 144e307d7ea..fddcf5e8daf 100644 --- a/yarn-project/aztec.js/src/fee/public_fee_payment_method.ts +++ b/yarn-project/aztec.js/src/fee/public_fee_payment_method.ts @@ -4,18 +4,18 @@ import { FunctionSelector, FunctionType } from '@aztec/foundation/abi'; import { type AztecAddress } from '@aztec/foundation/aztec-address'; import { Fr } from '@aztec/foundation/fields'; +import { ContractFunctionInteraction } from '../contract/contract_function_interaction.js'; import { type AccountWallet } from '../wallet/account_wallet.js'; +import { SignerlessWallet } from '../wallet/signerless_wallet.js'; import { type FeePaymentMethod } from './fee_payment_method.js'; /** * Holds information about how the fee for a transaction is to be paid. */ export class PublicFeePaymentMethod implements FeePaymentMethod { + private assetPromise: Promise | null = null; + constructor( - /** - * The asset used to pay the fee. - */ - protected asset: AztecAddress, /** * Address which will hold the fee payment. */ @@ -30,8 +30,43 @@ export class PublicFeePaymentMethod implements FeePaymentMethod { * The asset used to pay the fee. * @returns The asset used to pay the fee. */ - getAsset() { - return this.asset; + getAsset(): Promise { + if (!this.assetPromise) { + // We use signer-less wallet because this function could be triggered before the associated account is deployed. + const signerlessWallet = new SignerlessWallet(this.wallet); + + const interaction = new ContractFunctionInteraction( + signerlessWallet, + this.paymentContract, + { + name: 'get_accepted_asset', + functionType: FunctionType.PRIVATE, + isInternal: false, + isStatic: false, + parameters: [], + returnTypes: [ + { + kind: 'struct', + path: 'authwit::aztec::protocol_types::address::aztec_address::AztecAddress', + fields: [ + { + name: 'inner', + type: { + kind: 'field', + }, + }, + ], + }, + ], + errorTypes: {}, + isInitializer: false, + }, + [], + ); + + this.assetPromise = interaction.simulate(); + } + return this.assetPromise!; } getFeePayer(): Promise { @@ -43,7 +78,7 @@ export class PublicFeePaymentMethod implements FeePaymentMethod { * @param gasSettings - The gas settings. * @returns The function call to pay the fee. */ - getFunctionCalls(gasSettings: GasSettings): Promise { + async getFunctionCalls(gasSettings: GasSettings): Promise { const nonce = Fr.random(); const maxFee = gasSettings.getFeeLimit(); @@ -58,7 +93,7 @@ export class PublicFeePaymentMethod implements FeePaymentMethod { selector: FunctionSelector.fromSignature('transfer_in_public((Field),(Field),Field,Field)'), type: FunctionType.PUBLIC, isStatic: false, - to: this.asset, + to: await this.getAsset(), returnTypes: [], }, }, @@ -68,10 +103,10 @@ export class PublicFeePaymentMethod implements FeePaymentMethod { { name: 'fee_entrypoint_public', to: this.paymentContract, - selector: FunctionSelector.fromSignature('fee_entrypoint_public(Field,(Field),Field)'), + selector: FunctionSelector.fromSignature('fee_entrypoint_public(Field,Field)'), type: FunctionType.PRIVATE, isStatic: false, - args: [maxFee, this.asset.toField(), nonce], + args: [maxFee, nonce], returnTypes: [], }, ]); diff --git a/yarn-project/circuits.js/src/contract/contract_class.ts b/yarn-project/circuits.js/src/contract/contract_class.ts index 8809a2bbe02..78c49020d92 100644 --- a/yarn-project/circuits.js/src/contract/contract_class.ts +++ b/yarn-project/circuits.js/src/contract/contract_class.ts @@ -35,7 +35,7 @@ export function getContractClassFromArtifact( ); if (!dispatchFunction) { throw new Error( - 'A contract with public functions should define a public_dispatch(Field) function as its public entrypoint.', + `A contract with public functions should define a public_dispatch(Field) function as its public entrypoint. Contract: ${artifact.name}`, ); } packedBytecode = dispatchFunction.bytecode; diff --git a/yarn-project/cli-wallet/src/utils/options/fees.ts b/yarn-project/cli-wallet/src/utils/options/fees.ts index 829a2c8c67b..fbb6afb53ad 100644 --- a/yarn-project/cli-wallet/src/utils/options/fees.ts +++ b/yarn-project/cli-wallet/src/utils/options/fees.ts @@ -135,15 +135,10 @@ export function parsePaymentMethod( if (!parsed.asset) { throw new Error('Missing "asset" in payment option'); } - if (!parsed.feeRecipient) { - // Recipient of a fee in the refund flow - throw new Error('Missing "feeRecipient" in payment option'); - } const fpc = aliasedAddressParser('contracts', parsed.fpc, db); - const feeRecipient = AztecAddress.fromString(parsed.feeRecipient); - return [AztecAddress.fromString(parsed.asset), fpc, feeRecipient]; + return [AztecAddress.fromString(parsed.asset), fpc]; }; return async (sender: AccountWallet) => { @@ -182,13 +177,13 @@ export function parsePaymentMethod( const [asset, fpc] = getFpcOpts(parsed, db); log(`Using public fee payment with asset ${asset} via paymaster ${fpc}`); const { PublicFeePaymentMethod } = await import('@aztec/aztec.js/fee'); - return new PublicFeePaymentMethod(asset, fpc, sender); + return new PublicFeePaymentMethod(fpc, sender); } case 'fpc-private': { - const [asset, fpc, feeRecipient] = getFpcOpts(parsed, db); - log(`Using private fee payment with asset ${asset} via paymaster ${fpc} with rebate secret ${feeRecipient}`); + const [asset, fpc] = getFpcOpts(parsed, db); + log(`Using private fee payment with asset ${asset} via paymaster ${fpc}`); const { PrivateFeePaymentMethod } = await import('@aztec/aztec.js/fee'); - return new PrivateFeePaymentMethod(asset, fpc, sender, feeRecipient); + return new PrivateFeePaymentMethod(fpc, sender); } case undefined: throw new Error('Missing "method" in payment option'); diff --git a/yarn-project/cli/src/cmds/devnet/bootstrap_network.ts b/yarn-project/cli/src/cmds/devnet/bootstrap_network.ts index 9ba166fad9d..fe8059e0203 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 @@ -222,14 +222,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/FPC'); - 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_prover.test.ts b/yarn-project/end-to-end/src/benchmarks/bench_prover.test.ts index 5cfda3ff9d6..b4151bd06fa 100644 --- a/yarn-project/end-to-end/src/benchmarks/bench_prover.test.ts +++ b/yarn-project/end-to-end/src/benchmarks/bench_prover.test.ts @@ -204,13 +204,12 @@ describe('benchmarks/proving', () => { const feeFnCall0 = { gasSettings, - paymentMethod: new PublicFeePaymentMethod(initialTokenContract.address, initialFpContract.address, wallet), + paymentMethod: new PublicFeePaymentMethod(initialFpContract.address, wallet), }; // const feeFnCall1 = { // gasSettings, // paymentMethod: new PrivateFeePaymentMethod( - // initialTokenContract.address, // initialFpContract.address, // await getWalletOnPxe(1), // ), 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 a998193f7f8..02b8f858f30 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 @@ -49,7 +49,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 @@ -91,7 +94,7 @@ describe('benchmarks/tx_size_fees', () => { ], [ 'public fee', - () => new PublicFeePaymentMethod(token.address, fpc.address, aliceWallet), + () => new PublicFeePaymentMethod(fpc.address, aliceWallet), // DA: // non-rev: 1 nullifiers, overhead; rev: 2 note hashes, 1 nullifier, 1168 B enc note logs, 0 B enc logs,0 B unenc logs, teardown // L2: @@ -100,7 +103,7 @@ describe('benchmarks/tx_size_fees', () => { ], [ 'private fee', - () => new PrivateFeePaymentMethod(token.address, fpc.address, aliceWallet, sequencerAddress), + () => new PrivateFeePaymentMethod(fpc.address, aliceWallet), // DA: // non-rev: 3 nullifiers, overhead; rev: 2 note hashes, 1168 B enc note logs, 0 B enc logs, 0 B unenc logs, teardown // L2: diff --git a/yarn-project/end-to-end/src/e2e_fees/account_init.test.ts b/yarn-project/end-to-end/src/e2e_fees/account_init.test.ts index f4464e972c4..e997538a6d1 100644 --- a/yarn-project/end-to-end/src/e2e_fees/account_init.test.ts +++ b/yarn-project/end-to-end/src/e2e_fees/account_init.test.ts @@ -66,13 +66,11 @@ describe('e2e_fees account_init', () => { // Seeded by initBalances below in a beforeEach hook let fpcsInitialGas: bigint; let fpcsInitialPublicBananas: bigint; - let sequencerInitialPrivateBananas: bigint; async function initBalances() { - [[fpcsInitialGas], [fpcsInitialPublicBananas], [sequencerInitialPrivateBananas]] = await Promise.all([ + [[fpcsInitialGas], [fpcsInitialPublicBananas]] = await Promise.all([ t.getGasBalanceFn(bananaFPC.address), t.getBananaPublicBalanceFn(bananaFPC.address), - t.getBananaPrivateBalanceFn(t.sequencerAddress), ]); } @@ -117,12 +115,7 @@ describe('e2e_fees account_init', () => { await t.mintPrivateBananas(mintedBananas, bobsAddress); // Bob deploys his account through the private FPC - const paymentMethod = new PrivateFeePaymentMethod( - bananaCoin.address, - bananaFPC.address, - await bobsAccountManager.getWallet(), - t.sequencerAddress, // Sequencer is the recipient of the refund fee notes because it's the FPC admin. - ); + const paymentMethod = new PrivateFeePaymentMethod(bananaFPC.address, await bobsAccountManager.getWallet()); const tx = await bobsAccountManager.deploy({ fee: { gasSettings, paymentMethod } }).wait(); const actualFee = tx.transactionFee!; @@ -132,8 +125,8 @@ describe('e2e_fees account_init', () => { await expect(t.getBananaPrivateBalanceFn(bobsAddress)).resolves.toEqual([mintedBananas - actualFee]); // the FPC admin (set to sequencer) got the banana fee note so his private balance should have increased by the actual fee - await expect(t.getBananaPrivateBalanceFn(t.sequencerAddress)).resolves.toEqual([ - sequencerInitialPrivateBananas + actualFee, + await expect(t.getBananaPublicBalanceFn(t.bananaFPC.address)).resolves.toEqual([ + fpcsInitialPublicBananas + actualFee, ]); // the FPC should have been the fee payer @@ -144,7 +137,7 @@ describe('e2e_fees account_init', () => { const mintedBananas = FEE_FUNDING_FOR_TESTER_ACCOUNT; await bananaCoin.methods.mint_to_public(bobsAddress, mintedBananas).send().wait(); - const paymentMethod = new PublicFeePaymentMethod(bananaCoin.address, bananaFPC.address, bobsWallet); + const paymentMethod = new PublicFeePaymentMethod(bananaFPC.address, bobsWallet); const tx = await bobsAccountManager .deploy({ skipPublicDeployment: false, diff --git a/yarn-project/end-to-end/src/e2e_fees/dapp_subscription.test.ts b/yarn-project/end-to-end/src/e2e_fees/dapp_subscription.test.ts index 262272d194c..d3bd333d2e7 100644 --- a/yarn-project/end-to-end/src/e2e_fees/dapp_subscription.test.ts +++ b/yarn-project/end-to-end/src/e2e_fees/dapp_subscription.test.ts @@ -26,7 +26,6 @@ describe('e2e_fees dapp_subscription', () => { let aliceAddress: AztecAddress; // Dapp subscriber. let bobAddress: AztecAddress; // Dapp owner. let sequencerAddress: AztecAddress; - let feeRecipient: AztecAddress; // Account that receives the fees from the fee refund flow. let bananaCoin: BananaCoin; let counterContract: CounterContract; @@ -59,9 +58,6 @@ describe('e2e_fees dapp_subscription', () => { counterContract, pxe, } = await t.setup()); - - // We like sequencer so we send him the fees. - feeRecipient = sequencerAddress; }); afterAll(async () => { @@ -112,9 +108,7 @@ describe('e2e_fees dapp_subscription', () => { the FPC finalizes the partial notes for the fee and the refund */ - const { transactionFee } = await subscribe( - new PrivateFeePaymentMethod(bananaCoin.address, bananaFPC.address, aliceWallet, feeRecipient), - ); + const { transactionFee } = await subscribe(new PrivateFeePaymentMethod(bananaFPC.address, aliceWallet)); // We let Alice see Bob's notes because the expect uses Alice's wallet to interact with the contracts to "get" state. aliceWallet.setScopes([aliceAddress, bobAddress]); @@ -127,7 +121,7 @@ describe('e2e_fees dapp_subscription', () => { // alice, bob, fpc await expectBananasPrivateDelta(-t.SUBSCRIPTION_AMOUNT - transactionFee!, t.SUBSCRIPTION_AMOUNT, 0n); - await expectBananasPublicDelta(0n, 0n, 0n); + await expectBananasPublicDelta(0n, 0n, transactionFee!); // REFUND_AMOUNT is a transparent note note }); @@ -144,9 +138,7 @@ describe('e2e_fees dapp_subscription', () => { PUBLIC TEARDOWN the FPC finalizes the partial notes for the fee and the refund */ - const { transactionFee } = await subscribe( - new PublicFeePaymentMethod(bananaCoin.address, bananaFPC.address, aliceWallet), - ); + const { transactionFee } = await subscribe(new PublicFeePaymentMethod(bananaFPC.address, aliceWallet)); await expectMapping( t.getGasBalanceFn, @@ -165,7 +157,7 @@ describe('e2e_fees dapp_subscription', () => { it('should call dapp subscription entrypoint', async () => { // Subscribe again, so this test does not depend on the previous ones being run. - await subscribe(new PrivateFeePaymentMethod(bananaCoin.address, bananaFPC.address, aliceWallet, feeRecipient)); + await subscribe(new PrivateFeePaymentMethod(bananaFPC.address, aliceWallet)); expect(await subscriptionContract.methods.is_initialized(aliceAddress).simulate()).toBe(true); @@ -187,18 +179,14 @@ describe('e2e_fees dapp_subscription', () => { it('should reject after the sub runs out', async () => { // Subscribe again. This will overwrite the previous subscription. - await subscribe(new PrivateFeePaymentMethod(bananaCoin.address, bananaFPC.address, aliceWallet, feeRecipient), 0); + await subscribe(new PrivateFeePaymentMethod(bananaFPC.address, aliceWallet), 0); // TODO(#6651): Change back to /(context.block_number()) as u64 < expiry_block_number as u64/ when fixed await expect(dappIncrement()).rejects.toThrow(/Note encrypted logs hash mismatch/); }); it('should reject after the txs run out', async () => { // Subscribe again. This will overwrite the previous subscription. - await subscribe( - new PrivateFeePaymentMethod(bananaCoin.address, bananaFPC.address, aliceWallet, feeRecipient), - 5, - 1, - ); + await subscribe(new PrivateFeePaymentMethod(bananaFPC.address, aliceWallet), 5, 1); await expect(dappIncrement()).resolves.toBeDefined(); await expect(dappIncrement()).rejects.toThrow(/note.remaining_txs as u64 > 0/); }); diff --git a/yarn-project/end-to-end/src/e2e_fees/failures.test.ts b/yarn-project/end-to-end/src/e2e_fees/failures.test.ts index 7efc18bc661..ecc938877dc 100644 --- a/yarn-project/end-to-end/src/e2e_fees/failures.test.ts +++ b/yarn-project/end-to-end/src/e2e_fees/failures.test.ts @@ -40,12 +40,11 @@ describe('e2e_fees failures', () => { const outrageousPublicAmountAliceDoesNotHave = t.ALICE_INITIAL_BANANAS * 5n; const privateMintedAlicePrivateBananas = t.ALICE_INITIAL_BANANAS; - const [initialAlicePrivateBananas, initialSequencerPrivateBananas] = await t.getBananaPrivateBalanceFn( - aliceAddress, - sequencerAddress, - ); + const [initialAlicePrivateBananas] = await t.getBananaPrivateBalanceFn(aliceAddress, sequencerAddress); const [initialAliceGas, initialFPCGas] = await t.getGasBalanceFn(aliceAddress, bananaFPC.address); + const [initialFPCPublicBananas] = await t.getBananaPublicBalanceFn(bananaFPC.address); + await t.mintPrivateBananas(privateMintedAlicePrivateBananas, aliceAddress); // if we simulate locally, it throws an error @@ -56,12 +55,7 @@ describe('e2e_fees failures', () => { .send({ fee: { gasSettings, - paymentMethod: new PrivateFeePaymentMethod( - bananaCoin.address, - bananaFPC.address, - aliceWallet, - t.sequencerAddress, - ), + paymentMethod: new PrivateFeePaymentMethod(bananaFPC.address, aliceWallet), }, }) .wait(), @@ -85,12 +79,7 @@ describe('e2e_fees failures', () => { skipPublicSimulation: true, fee: { gasSettings, - paymentMethod: new PrivateFeePaymentMethod( - bananaCoin.address, - bananaFPC.address, - aliceWallet, - t.sequencerAddress, - ), + paymentMethod: new PrivateFeePaymentMethod(bananaFPC.address, aliceWallet), }, }) .wait({ dontThrowOnRevert: true }); @@ -107,16 +96,19 @@ describe('e2e_fees failures', () => { // and thus we paid the fee await expectMapping( t.getBananaPrivateBalanceFn, - [aliceAddress, sequencerAddress], + [aliceAddress], [ // Even with the revert public teardown function got successfully executed so Alice received the refund note // and hence paid the actual fee. initialAlicePrivateBananas + privateMintedAlicePrivateBananas - feeAmount, - // Sequencer is the FPC admin/fee recipient and hence he should have received the fee amount note - initialSequencerPrivateBananas + feeAmount, ], ); + // FPC should have received the fee in public + await expect(t.getBananaPublicBalanceFn(t.bananaFPC.address)).resolves.toEqual([ + initialFPCPublicBananas + feeAmount, + ]); + // Gas balance of Alice should have stayed the same as the FPC paid the gas fee and not her (she paid bananas // to FPC admin). await expectMapping( @@ -152,7 +144,7 @@ describe('e2e_fees failures', () => { .send({ fee: { gasSettings, - paymentMethod: new PublicFeePaymentMethod(bananaCoin.address, bananaFPC.address, aliceWallet), + paymentMethod: new PublicFeePaymentMethod(bananaFPC.address, aliceWallet), }, }) .wait(), @@ -182,7 +174,7 @@ describe('e2e_fees failures', () => { skipPublicSimulation: true, fee: { gasSettings, - paymentMethod: new PublicFeePaymentMethod(bananaCoin.address, bananaFPC.address, aliceWallet), + paymentMethod: new PublicFeePaymentMethod(bananaFPC.address, aliceWallet), }, }) .wait({ dontThrowOnRevert: true }); @@ -218,7 +210,7 @@ describe('e2e_fees failures', () => { .send({ fee: { gasSettings, - paymentMethod: new BuggedSetupFeePaymentMethod(bananaCoin.address, bananaFPC.address, aliceWallet), + paymentMethod: new BuggedSetupFeePaymentMethod(bananaFPC.address, aliceWallet), }, }) .wait(), @@ -232,7 +224,7 @@ describe('e2e_fees failures', () => { skipPublicSimulation: true, fee: { gasSettings, - paymentMethod: new BuggedSetupFeePaymentMethod(bananaCoin.address, bananaFPC.address, aliceWallet), + paymentMethod: new BuggedSetupFeePaymentMethod(bananaFPC.address, aliceWallet), }, }) .wait(), @@ -272,7 +264,7 @@ describe('e2e_fees failures', () => { .send({ fee: { gasSettings: badGas, - paymentMethod: new PublicFeePaymentMethod(bananaCoin.address, bananaFPC.address, aliceWallet), + paymentMethod: new PublicFeePaymentMethod(bananaFPC.address, aliceWallet), }, }) .wait(), @@ -284,7 +276,7 @@ describe('e2e_fees failures', () => { skipPublicSimulation: true, fee: { gasSettings: badGas, - paymentMethod: new PublicFeePaymentMethod(bananaCoin.address, bananaFPC.address, aliceWallet), + paymentMethod: new PublicFeePaymentMethod(bananaFPC.address, aliceWallet), }, }) .wait({ @@ -317,12 +309,14 @@ describe('e2e_fees failures', () => { }); class BuggedSetupFeePaymentMethod extends PublicFeePaymentMethod { - override getFunctionCalls(gasSettings: GasSettings): Promise { + override async getFunctionCalls(gasSettings: GasSettings): Promise { const maxFee = gasSettings.getFeeLimit(); const nonce = Fr.random(); const tooMuchFee = new Fr(maxFee.toBigInt() * 2n); + const asset = await this.getAsset(); + return Promise.resolve([ this.wallet .setPublicAuthWit( @@ -334,7 +328,7 @@ class BuggedSetupFeePaymentMethod extends PublicFeePaymentMethod { selector: FunctionSelector.fromSignature('transfer_in_public((Field),(Field),Field,Field)'), type: FunctionType.PUBLIC, isStatic: false, - to: this.asset, + to: asset, returnTypes: [], }, }, @@ -344,10 +338,10 @@ class BuggedSetupFeePaymentMethod extends PublicFeePaymentMethod { { name: 'fee_entrypoint_public', to: this.paymentContract, - selector: FunctionSelector.fromSignature('fee_entrypoint_public(Field,(Field),Field)'), + selector: FunctionSelector.fromSignature('fee_entrypoint_public(Field,Field)'), type: FunctionType.PRIVATE, isStatic: false, - args: [tooMuchFee, this.asset.toField(), nonce], + args: [tooMuchFee, nonce], returnTypes: [], }, ]); 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 427816c6d9a..518f545c8de 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 @@ -60,7 +60,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; @@ -140,8 +140,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); @@ -223,7 +223,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(); diff --git a/yarn-project/end-to-end/src/e2e_fees/gas_estimation.test.ts b/yarn-project/end-to-end/src/e2e_fees/gas_estimation.test.ts index 522004039e4..1d2a7427cc1 100644 --- a/yarn-project/end-to-end/src/e2e_fees/gas_estimation.test.ts +++ b/yarn-project/end-to-end/src/e2e_fees/gas_estimation.test.ts @@ -96,7 +96,7 @@ describe('e2e_fees gas_estimation', () => { it('estimates gas with public payment method', async () => { const teardownFixedFee = gasSettings.teardownGasLimits.computeFee(gasSettings.maxFeesPerGas).toBigInt(); - const paymentMethod = new PublicFeePaymentMethod(bananaCoin.address, bananaFPC.address, aliceWallet); + const paymentMethod = new PublicFeePaymentMethod(bananaFPC.address, aliceWallet); const estimatedGas = await makeTransferRequest().estimateGas({ fee: { gasSettings, paymentMethod, estimatedGasPadding: 0 }, }); diff --git a/yarn-project/end-to-end/src/e2e_fees/private_payments.test.ts b/yarn-project/end-to-end/src/e2e_fees/private_payments.test.ts index e9518e69f08..39420fd44b8 100644 --- a/yarn-project/end-to-end/src/e2e_fees/private_payments.test.ts +++ b/yarn-project/end-to-end/src/e2e_fees/private_payments.test.ts @@ -39,7 +39,6 @@ describe('e2e_fees private_payment', () => { let initialBobPrivateBananas: bigint; let initialFPCPublicBananas: bigint; - let initialSequencerPrivateBananas: bigint; let initialFPCGas: bigint; let initialSequencerGas: bigint; @@ -53,11 +52,11 @@ describe('e2e_fees private_payment', () => { initialSequencerL1Gas = await t.getCoinbaseBalance(); [ - [initialAlicePrivateBananas, initialBobPrivateBananas, initialSequencerPrivateBananas], + [initialAlicePrivateBananas, initialBobPrivateBananas], [initialAlicePublicBananas, initialBobPublicBananas, initialFPCPublicBananas], [initialAliceGas, initialFPCGas, initialSequencerGas], ] = await Promise.all([ - t.getBananaPrivateBalanceFn(aliceAddress, bobAddress, sequencerAddress), + t.getBananaPrivateBalanceFn(aliceAddress, bobAddress), t.getBananaPublicBalanceFn(aliceAddress, bobAddress, bananaFPC.address), t.getGasBalanceFn(aliceAddress, bananaFPC.address, sequencerAddress), ]); @@ -94,12 +93,7 @@ describe('e2e_fees private_payment', () => { const settings = { fee: { gasSettings, - paymentMethod: new PrivateFeePaymentMethod( - bananaCoin.address, - bananaFPC.address, - aliceWallet, - sequencerAddress, - ), + paymentMethod: new PrivateFeePaymentMethod(bananaFPC.address, aliceWallet), }, }; const localTx = await interaction.prove(settings); @@ -140,15 +134,13 @@ describe('e2e_fees private_payment', () => { await expectMapping( t.getBananaPrivateBalanceFn, - [aliceAddress, bobAddress, bananaFPC.address, sequencerAddress], - [ - initialAlicePrivateBananas - feeAmount - transferAmount, - transferAmount, - 0n, - initialSequencerPrivateBananas + feeAmount, - ], + [aliceAddress, bobAddress], + [initialAlicePrivateBananas - feeAmount - transferAmount, transferAmount], ); + // FPC should have received fee amount of bananas + await expectMapping(t.getBananaPublicBalanceFn, [bananaFPC.address], [initialFPCPublicBananas + feeAmount]); + await expectMapping( t.getGasBalanceFn, [aliceAddress, bananaFPC.address, sequencerAddress], @@ -181,12 +173,7 @@ describe('e2e_fees private_payment', () => { .send({ fee: { gasSettings, - paymentMethod: new PrivateFeePaymentMethod( - bananaCoin.address, - bananaFPC.address, - aliceWallet, - sequencerAddress, - ), + paymentMethod: new PrivateFeePaymentMethod(bananaFPC.address, aliceWallet), }, }) .wait(); @@ -195,9 +182,13 @@ describe('e2e_fees private_payment', () => { await expectMapping( t.getBananaPrivateBalanceFn, - [aliceAddress, bananaFPC.address, sequencerAddress], - [initialAlicePrivateBananas - feeAmount + newlyMintedBananas, 0n, initialSequencerPrivateBananas + feeAmount], + [aliceAddress], + [initialAlicePrivateBananas - feeAmount + newlyMintedBananas], ); + + // FPC should have received fee amount of bananas + await expectMapping(t.getBananaPublicBalanceFn, [bananaFPC.address], [initialFPCPublicBananas + feeAmount]); + await expectMapping( t.getGasBalanceFn, [aliceAddress, bananaFPC.address, sequencerAddress], @@ -230,12 +221,7 @@ describe('e2e_fees private_payment', () => { .send({ fee: { gasSettings, - paymentMethod: new PrivateFeePaymentMethod( - bananaCoin.address, - bananaFPC.address, - aliceWallet, - sequencerAddress, - ), + paymentMethod: new PrivateFeePaymentMethod(bananaFPC.address, aliceWallet), }, }) .wait(); @@ -244,17 +230,13 @@ describe('e2e_fees private_payment', () => { await expectMapping( t.getBananaPrivateBalanceFn, - [aliceAddress, bananaFPC.address, sequencerAddress], - [ - initialAlicePrivateBananas - feeAmount + amountTransferredToPrivate, - 0n, - initialSequencerPrivateBananas + feeAmount, - ], + [aliceAddress], + [initialAlicePrivateBananas - feeAmount + amountTransferredToPrivate], ); await expectMapping( t.getBananaPublicBalanceFn, - [aliceAddress, bananaFPC.address, sequencerAddress], - [initialAlicePublicBananas - amountTransferredToPrivate, initialFPCPublicBananas, 0n], + [aliceAddress, bananaFPC.address], + [initialAlicePublicBananas - amountTransferredToPrivate, initialFPCPublicBananas + feeAmount], ); await expectMapping( t.getGasBalanceFn, @@ -293,12 +275,7 @@ describe('e2e_fees private_payment', () => { .send({ fee: { gasSettings, - paymentMethod: new PrivateFeePaymentMethod( - bananaCoin.address, - bananaFPC.address, - aliceWallet, - sequencerAddress, - ), + paymentMethod: new PrivateFeePaymentMethod(bananaFPC.address, aliceWallet), }, }) .wait(); @@ -307,18 +284,16 @@ describe('e2e_fees private_payment', () => { await expectMapping( t.getBananaPrivateBalanceFn, - [aliceAddress, bobAddress, bananaFPC.address, sequencerAddress], + [aliceAddress, bobAddress], [ initialAlicePrivateBananas - feeAmount - amountTransferredInPrivate + amountTransferredToPrivate, initialBobPrivateBananas + amountTransferredInPrivate, - 0n, - initialSequencerPrivateBananas + feeAmount, ], ); await expectMapping( t.getBananaPublicBalanceFn, - [aliceAddress, bananaFPC.address, sequencerAddress], - [initialAlicePublicBananas - amountTransferredToPrivate, initialFPCPublicBananas, 0n], + [aliceAddress, bananaFPC.address], + [initialAlicePublicBananas - amountTransferredToPrivate, initialFPCPublicBananas + feeAmount], ); await expectMapping( t.getGasBalanceFn, @@ -342,12 +317,7 @@ describe('e2e_fees private_payment', () => { skipPublicSimulation: true, fee: { gasSettings, - paymentMethod: new PrivateFeePaymentMethod( - bananaCoin.address, - bankruptFPC.address, - aliceWallet, - aliceAddress, - ), + paymentMethod: new PrivateFeePaymentMethod(bankruptFPC.address, aliceWallet), }, }) .wait(), @@ -362,14 +332,12 @@ describe('e2e_fees private_payment', () => { fee: { gasSettings: t.gasSettings, paymentMethod: new PrivateFeePaymentMethod( - bananaCoin.address, bananaFPC.address, aliceWallet, - sequencerAddress, // Sequencer is the recipient of the refund fee notes because it's the FPC admin. true, // We set max fee/funded amount to 1 to trigger the error. ), }, }), - ).rejects.toThrow('funded amount not enough to cover tx fee'); + ).rejects.toThrow('max fee not enough to cover tx fee'); }); }); diff --git a/yarn-project/end-to-end/src/e2e_fees/public_payments.test.ts b/yarn-project/end-to-end/src/e2e_fees/public_payments.test.ts index 22e4fbcb46a..8f1175fb0b0 100644 --- a/yarn-project/end-to-end/src/e2e_fees/public_payments.test.ts +++ b/yarn-project/end-to-end/src/e2e_fees/public_payments.test.ts @@ -64,7 +64,7 @@ describe('e2e_fees public_payment', () => { .send({ fee: { gasSettings, - paymentMethod: new PublicFeePaymentMethod(bananaCoin.address, bananaFPC.address, aliceWallet), + paymentMethod: new PublicFeePaymentMethod(bananaFPC.address, aliceWallet), }, }) .wait(); diff --git a/yarn-project/pxe/src/pxe_service/error_enriching.ts b/yarn-project/pxe/src/pxe_service/error_enriching.ts index cb71823fe13..2c18f72b1e2 100644 --- a/yarn-project/pxe/src/pxe_service/error_enriching.ts +++ b/yarn-project/pxe/src/pxe_service/error_enriching.ts @@ -39,7 +39,7 @@ export async function enrichSimulationError(err: SimulationError, db: PxeDatabas ); } else { logger.warn( - `Could not function artifact in contract ${contract.name} for function '${fnName}' when enriching error callstack`, + `Could not find function artifact in contract ${contract.name} for function '${fnName}' when enriching error callstack`, ); } });