Skip to content

Commit

Permalink
refactor: nuking pay_refund_with_shielded_rebate flow (#9639)
Browse files Browse the repository at this point in the history
  • Loading branch information
benesjan authored Nov 8, 2024
1 parent e06b192 commit 2e13938
Show file tree
Hide file tree
Showing 21 changed files with 255 additions and 547 deletions.
1 change: 0 additions & 1 deletion noir-projects/noir-contracts/Nargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ members = [
"contracts/parent_contract",
"contracts/pending_note_hashes_contract",
"contracts/price_feed_contract",
"contracts/private_fpc_contract",
"contracts/router_contract",
"contracts/schnorr_account_contract",
"contracts/schnorr_hardcoded_account_contract",
Expand Down
46 changes: 15 additions & 31 deletions noir-projects/noir-contracts/contracts/fpc_contract/src/main.nr
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
mod lib;
mod settings;

use dep::aztec::macros::aztec;

#[aztec]
contract FPC {
use crate::lib::compute_rebate;
use crate::settings::Settings;
use dep::aztec::{
macros::{functions::{initializer, internal, private, public}, storage::storage},
protocol_types::{abis::function_selector::FunctionSelector, address::AztecAddress},
Expand All @@ -14,38 +16,27 @@ contract FPC {

#[storage]
struct Storage<Context> {
other_asset: SharedImmutable<AztecAddress, Context>,
settings: SharedImmutable<Settings, Context>,
}

#[public]
#[initializer]
fn constructor(other_asset: AztecAddress) {
storage.other_asset.initialize(other_asset);
fn constructor(other_asset: AztecAddress, admin: AztecAddress) {
let settings = Settings { other_asset, admin };
storage.settings.initialize(settings);
}

#[private]
fn fee_entrypoint_private(
amount: Field,
asset: AztecAddress,
secret_hash: Field,
nonce: Field,
) {
assert(asset == storage.other_asset.read_private());
Token::at(asset)
.transfer_to_public(context.msg_sender(), context.this_address(), amount, nonce)
.call(&mut context);
context.set_as_fee_payer();
// Would like to get back to
// FPC::at(context.this_address()).pay_refund_with_shielded_rebate(amount, asset, secret_hash).set_public_teardown_function(&mut context);
context.set_public_teardown_function(
context.this_address(),
comptime {
FunctionSelector::from_signature(
"pay_refund_with_shielded_rebate(Field,(Field),Field)",
)
},
[amount, asset.to_field(), secret_hash],
fn fee_entrypoint_private(amount: Field, asset: AztecAddress, nonce: Field) {
// TODO(PR #8022): Once SharedImmutable performs only 1 merkle proof here, we'll save ~4k gates
let settings = storage.settings.read_private();

assert(asset == settings.other_asset);

Token::at(asset).setup_refund(settings.admin, context.msg_sender(), amount, nonce).call(
&mut context,
);
context.set_as_fee_payer();
}

#[private]
Expand Down Expand Up @@ -82,11 +73,4 @@ contract FPC {
&mut context,
);
}

#[public]
#[internal]
fn pay_refund_with_shielded_rebate(amount: Field, asset: AztecAddress, secret_hash: Field) {
let refund = compute_rebate(context, amount);
Token::at(asset).shield(context.this_address(), refund, secret_hash, 0).call(&mut context);
}
}

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -675,6 +675,7 @@ contract Token {
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).
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.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use crate::{test::utils, Token};
use aztec::oracle::random::random;

use dep::authwit::cheatcodes as authwit_cheatcodes;
use std::test::OracleMock;
Expand All @@ -14,14 +15,16 @@ unconstrained fn setup_refund_success() {

let funded_amount = 1_000;

// We use the same randomness for both the fee payer and the user as we currently don't have `OracleMock::mock_once()`
// We use the same randomness for both the fee payer, the user and the nonce as we currently don't have
// `OracleMock::mock_once()`
let fee_payer_randomness = 123;
let user_randomness = fee_payer_randomness;
let nonce = fee_payer_randomness;

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);
Token::at(token_contract_address).setup_refund(fee_payer, user, funded_amount, nonce);

authwit_cheatcodes::add_private_authwit_from_call_interface(
user,
Expand Down Expand Up @@ -70,9 +73,10 @@ unconstrained fn setup_refund_insufficient_funded_amount() {

// We set funded amount to 0 to make the transaction fee higher than the funded amount
let funded_amount = 0;
let nonce = random();

let setup_refund_from_call_interface =
Token::at(token_contract_address).setup_refund(fee_payer, user, funded_amount);
Token::at(token_contract_address).setup_refund(fee_payer, user, funded_amount, nonce);

authwit_cheatcodes::add_private_authwit_from_call_interface(
user,
Expand Down
1 change: 0 additions & 1 deletion scripts/ci/get_e2e_jobs.sh
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ allow_list=(
"e2e_fees_failures"
"e2e_fees_gas_estimation"
"e2e_fees_private_payments"
"e2e_fees_private_refunds"
"e2e_max_block_number"
"e2e_nested_contract"
"e2e_ordering"
Expand Down
29 changes: 17 additions & 12 deletions yarn-project/aztec.js/src/fee/private_fee_payment_method.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { type FunctionCall } from '@aztec/circuit-types';
import { type GasSettings } from '@aztec/circuits.js';
import { computeSecretHash } from '@aztec/circuits.js/hash';
import { FunctionSelector, FunctionType } from '@aztec/foundation/abi';
import { type AztecAddress } from '@aztec/foundation/aztec-address';
import { Fr } from '@aztec/foundation/fields';
Expand Down Expand Up @@ -28,10 +27,15 @@ export class PrivateFeePaymentMethod implements FeePaymentMethod {
private wallet: Wallet,

/**
* A secret to shield the rebate amount from the FPC.
* Use this to claim the shielded amount to private balance
* Address that the FPC sends notes it receives to.
*/
private rebateSecret = Fr.random(),
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.
*/
private setMaxFeeToOne = false,
) {}

/**
Expand All @@ -52,31 +56,32 @@ export class PrivateFeePaymentMethod implements FeePaymentMethod {
* @returns The function call to pay the fee.
*/
async getFunctionCalls(gasSettings: GasSettings): Promise<FunctionCall[]> {
// We assume 1:1 exchange rate between fee juice and token. But in reality you would need to convert feeLimit
// (maxFee) to be in token denomination.
const maxFee = this.setMaxFeeToOne ? Fr.ONE : gasSettings.getFeeLimit();
const nonce = Fr.random();
const maxFee = gasSettings.getFeeLimit();

await this.wallet.createAuthWit({
caller: this.paymentContract,
action: {
name: 'transfer_to_public',
args: [this.wallet.getCompleteAddress().address, this.paymentContract, maxFee, nonce],
selector: FunctionSelector.fromSignature('transfer_to_public((Field),(Field),Field,Field)'),
name: 'setup_refund',
args: [this.feeRecipient, this.wallet.getAddress(), maxFee, nonce],
selector: FunctionSelector.fromSignature('setup_refund((Field),(Field),Field,Field)'),
type: FunctionType.PRIVATE,
isStatic: false,
to: this.asset,
returnTypes: [],
},
});

const secretHashForRebate = computeSecretHash(this.rebateSecret);

return [
{
name: 'fee_entrypoint_private',
to: this.paymentContract,
selector: FunctionSelector.fromSignature('fee_entrypoint_private(Field,(Field),Field,Field)'),
selector: FunctionSelector.fromSignature('fee_entrypoint_private(Field,(Field),Field)'),
type: FunctionType.PRIVATE,
isStatic: false,
args: [maxFee, this.asset, secretHashForRebate, nonce],
args: [maxFee, this.asset, nonce],
returnTypes: [],
},
];
Expand Down
16 changes: 10 additions & 6 deletions yarn-project/cli-wallet/src/utils/options/fees.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ export class FeeOpts implements IFeeOpts {

static paymentMethodOption() {
return new Option(
'--payment <method=name,asset=address,fpc=address,claimSecret=string,claimAmount=string,rebateSecret=string>',
'--payment <method=name,asset=address,fpc=address,claimSecret=string,claimAmount=string,feeRecipient=string>',
'Fee payment method and arguments. Valid methods are: none, fee_juice, fpc-public, fpc-private.',
);
}
Expand Down Expand Up @@ -141,10 +141,15 @@ 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];
return [AztecAddress.fromString(parsed.asset), fpc, feeRecipient];
};

return async (sender: AccountWallet) => {
Expand Down Expand Up @@ -180,12 +185,11 @@ export function parsePaymentMethod(
return new PublicFeePaymentMethod(asset, fpc, sender);
}
case 'fpc-private': {
const [asset, fpc] = getFpcOpts(parsed, db);
const rebateSecret = parsed.rebateSecret ? Fr.fromString(parsed.rebateSecret) : Fr.random();
const [asset, fpc, feeRecipient] = getFpcOpts(parsed, db);
log(
`Using private fee payment with asset ${asset} via paymaster ${fpc} with rebate secret ${rebateSecret.toString()}`,
`Using private fee payment with asset ${asset} via paymaster ${fpc} with rebate secret ${feeRecipient.toString()}`,
);
return new PrivateFeePaymentMethod(asset, fpc, sender, rebateSecret);
return new PrivateFeePaymentMethod(asset, fpc, sender, feeRecipient);
}
case undefined:
throw new Error('Missing "method" in payment option');
Expand Down
11 changes: 8 additions & 3 deletions yarn-project/cli/src/cmds/devnet/bootstrap_network.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,8 @@ export async function bootstrapNetwork(

await initPortal(pxe, l1Clients, erc20Address, portalAddress, bridge.address);

const fpc = await deployFPC(wallet, token.address);
const feeRecipient = wallet.getAddress();
const fpc = await deployFPC(wallet, token.address, feeRecipient);

const counter = await deployCounter(wallet);
// NOTE: Disabling for now in order to get devnet running
Expand Down Expand Up @@ -193,11 +194,15 @@ async function initPortal(
await publicClient.waitForTransactionReceipt({ hash });
}

async function deployFPC(wallet: Wallet, tokenAddress: AztecAddress): Promise<ContractDeploymentInfo> {
async function deployFPC(
wallet: Wallet,
tokenAddress: AztecAddress,
feeRecipient: AztecAddress,
): Promise<ContractDeploymentInfo> {
// 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)
const fpc = await FPCContract.deploy(wallet, tokenAddress, feeRecipient)
.send({ universalDeploy: true })
.deployed({ proven: true, provenTimeout: 600 });
const info: ContractDeploymentInfo = {
Expand Down
2 changes: 0 additions & 2 deletions yarn-project/end-to-end/scripts/e2e_test_config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,6 @@ tests:
test_path: 'e2e_fees/gas_estimation.test.ts'
e2e_fees_private_payments:
test_path: 'e2e_fees/private_payments.test.ts'
e2e_fees_private_refunds:
test_path: 'e2e_fees/private_refunds.test.ts'
e2e_keys: {}
e2e_l1_with_wall_time: {}
e2e_lending_contract: {}
Expand Down
6 changes: 5 additions & 1 deletion yarn-project/end-to-end/src/benchmarks/bench_prover.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ describe('benchmarks/proving', () => {
let schnorrWalletAddress: CompleteAddress;

let recipient: CompleteAddress;
let feeRecipient: CompleteAddress; // The address that receives the fees from the fee refund flow.

let initialGasContract: FeeJuiceContract;
let initialTestContract: TestContract;
Expand Down Expand Up @@ -89,7 +90,9 @@ describe('benchmarks/proving', () => {
.send()
.deployed();
initialGasContract = await FeeJuiceContract.at(ProtocolContractAddress.FeeJuice, initialSchnorrWallet);
initialFpContract = await FPCContract.deploy(initialSchnorrWallet, initialTokenContract.address).send().deployed();
initialFpContract = await FPCContract.deploy(initialSchnorrWallet, initialTokenContract.address, feeRecipient)
.send()
.deployed();

const feeJuiceBridgeTestHarness = await FeeJuicePortalTestingHarnessFactory.create({
aztecNode: ctx.aztecNode,
Expand All @@ -112,6 +115,7 @@ describe('benchmarks/proving', () => {
]);

recipient = CompleteAddress.random();
feeRecipient = CompleteAddress.random();
});

// remove the fake prover and setup the real one
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ 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).send().deployed();
fpc = await FPCContract.deploy(aliceWallet, token.address, sequencerAddress).send().deployed();
});

// mint tokens
Expand Down Expand Up @@ -94,7 +94,7 @@ describe('benchmarks/tx_size_fees', () => {
],
[
'private fee',
() => new PrivateFeePaymentMethod(token.address, fpc.address, aliceWallet),
() => new PrivateFeePaymentMethod(token.address, fpc.address, aliceWallet, sequencerAddress),
// 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:
Expand Down
Loading

0 comments on commit 2e13938

Please sign in to comment.