Skip to content

Commit

Permalink
feat: Use actual tx fee in gas token when charging fee (#6166)
Browse files Browse the repository at this point in the history
Uses the actual `transaction_fee` injected during teardown for `pay_fee`
in the `GasToken`.

This PR also refactors the e2e_fees and e2e_dapp_subscription tests to
use snapshots, and changes dapp_subscription so each test can be run
independently. It also modifies the `AppSubscription` contract so the
max fee it covers is configurable during initialization, instead of
hardcoded to 42. It also lowers the severity of some random logs to make
e2e output more readable (mostly around prover). Last, it includes
bugfix for the snapshot manager when it runs in "disabled mode", where
it was recreating the context multiple times, and refactors it by
splitting it into different classes depending on the mode it runs.
  • Loading branch information
spalladino authored May 3, 2024
1 parent 3ccc6ac commit 8418eac
Show file tree
Hide file tree
Showing 38 changed files with 1,514 additions and 1,253 deletions.
5 changes: 5 additions & 0 deletions noir-projects/aztec-nr/aztec/src/context/avm_context.nr
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,11 @@ impl PublicContextInterface for AvmContext {
0
}

fn transaction_fee(self) -> Field {
assert(false, "'transaction_fee' not implemented!");
0
}

fn nullifier_exists(self, unsiloed_nullifier: Field, address: AztecAddress) -> bool {
nullifier_exists(unsiloed_nullifier, address.to_field()) == 1
}
Expand Down
1 change: 1 addition & 0 deletions noir-projects/aztec-nr/aztec/src/context/interface.nr
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ trait PublicContextInterface {
args: [Field]
) -> FunctionReturns<RETURNS_COUNT>;
fn nullifier_exists(self, unsiloed_nullifier: Field, address: AztecAddress) -> bool;
fn transaction_fee(self) -> Field;
}

struct PrivateCallInterface<T> {
Expand Down
4 changes: 4 additions & 0 deletions noir-projects/aztec-nr/aztec/src/context/public_context.nr
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,10 @@ impl PublicContextInterface for PublicContext {
self.inputs.public_global_variables.gas_fees.fee_per_l2_gas
}

fn transaction_fee(self) -> Field {
self.inputs.transaction_fee
}

fn nullifier_exists(self, unsiloed_nullifier: Field, address: AztecAddress) -> bool {
// Current public can only check for settled nullifiers, so we always silo.
let siloed_nullifier = silo_nullifier(address, unsiloed_nullifier);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ contract AppSubscription {
subscription_price: SharedImmutable<Field>,
subscriptions: Map<AztecAddress, PrivateMutable<SubscriptionNote>>,
gas_token_address: SharedImmutable<AztecAddress>,
gas_token_limit_per_tx: SharedImmutable<Field>,
}

global SUBSCRIPTION_DURATION_IN_BLOCKS = 5;
Expand All @@ -47,7 +48,8 @@ contract AppSubscription {
note.remaining_txs -= 1;
storage.subscriptions.at(user_address).replace(&mut note, true);

GasToken::at(storage.gas_token_address.read_private()).pay_fee(42).enqueue(&mut context);
let gas_limit = storage.gas_token_limit_per_tx.read_private();
GasToken::at(storage.gas_token_address.read_private()).pay_fee(gas_limit).enqueue(&mut context);

context.end_setup();

Expand All @@ -63,14 +65,15 @@ contract AppSubscription {
subscription_recipient_address: AztecAddress,
subscription_token_address: AztecAddress,
subscription_price: Field,
gas_token_address: AztecAddress
gas_token_address: AztecAddress,
gas_token_limit_per_tx: Field
) {
storage.target_address.initialize(target_address);
storage.subscription_token_address.initialize(subscription_token_address);
storage.subscription_recipient_address.initialize(subscription_recipient_address);
storage.subscription_price.initialize(subscription_price);

storage.gas_token_address.initialize(gas_token_address);
storage.gas_token_limit_per_tx.initialize(gas_token_limit_per_tx);
}

#[aztec(public)]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ use dep::aztec::prelude::{AztecAddress, EthAddress};
use dep::aztec::context::interface::PublicContextInterface;
use dep::aztec::protocol_types::hash::sha256_to_field;

pub fn calculate_fee<TPublicContext>(_context: TPublicContext) -> U128 where TPublicContext: PublicContextInterface {
U128::from_integer(1)
pub fn calculate_fee<TPublicContext>(context: TPublicContext) -> Field where TPublicContext: PublicContextInterface {
context.transaction_fee()
}

pub fn get_bridge_gas_msg_hash(owner: AztecAddress, amount: Field) -> Field {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,11 @@ contract GasToken {
#[aztec(public)]
fn pay_fee(fee_limit: Field) -> Field {
let fee_limit_u128 = U128::from_integer(fee_limit);
let fee = calculate_fee(context);
let fee = U128::from_integer(calculate_fee(context));
dep::aztec::oracle::debug_log::debug_log_format(
"Gas token: paying fee {0} (limit {1})",
[fee.to_field(), fee_limit]
);
assert(fee <= fee_limit_u128, "Fee too high");

let sender_new_balance = storage.balances.at(context.msg_sender()).read() - fee;
Expand Down
2 changes: 1 addition & 1 deletion yarn-project/aztec-node/src/aztec-node/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -676,7 +676,7 @@ export class AztecNodeService implements AztecNode {
this.log.warn(`Simulated tx ${tx.getTxHash()} reverts: ${reverted[0].revertReason}`);
throw reverted[0].revertReason;
}
this.log.info(`Simulated tx ${tx.getTxHash()} succeeds`);
this.log.debug(`Simulated tx ${tx.getTxHash()} succeeds`);
const [processedTx] = processedTxs;
return {
constants: processedTx.data.constants,
Expand Down
2 changes: 0 additions & 2 deletions yarn-project/circuit-types/src/interfaces/configs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,8 @@ export interface SequencerConfig {
acvmWorkingDirectory?: string;
/** The path to the ACVM binary */
acvmBinaryPath?: string;

/** The list of functions calls allowed to run in setup */
allowedFunctionsInSetup?: AllowedFunction[];

/** The list of functions calls allowed to run teardown */
allowedFunctionsInTeardown?: AllowedFunction[];
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,10 @@ import * as fs from 'fs/promises';

import { waitRegisteredAccountSynced } from '../benchmarks/utils.js';
import {
SnapshotManager,
type ISnapshotManager,
type SubsystemsContext,
addAccounts,
createSnapshotManager,
publicDeployAccounts,
} from '../fixtures/snapshot_manager.js';
import { getBBConfig, setupPXEService } from '../fixtures/utils.js';
Expand All @@ -42,7 +43,7 @@ export class ClientProverTest {
static TOKEN_NAME = 'Aztec Token';
static TOKEN_SYMBOL = 'AZT';
static TOKEN_DECIMALS = 18n;
private snapshotManager: SnapshotManager;
private snapshotManager: ISnapshotManager;
logger: DebugLogger;
keys: Array<[Fr, Fq]> = [];
wallets: AccountWalletWithSecretKey[] = [];
Expand All @@ -59,7 +60,7 @@ export class ClientProverTest {

constructor(testName: string) {
this.logger = createDebugLogger(`aztec:client_prover_test:${testName}`);
this.snapshotManager = new SnapshotManager(`client_prover_integration/${testName}`, dataPath);
this.snapshotManager = createSnapshotManager(`client_prover_integration/${testName}`, dataPath);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,10 @@ import {
import { DocsExampleContract, TokenBlacklistContract, type TokenContract } from '@aztec/noir-contracts.js';

import {
SnapshotManager,
type ISnapshotManager,
type SubsystemsContext,
addAccounts,
createSnapshotManager,
publicDeployAccounts,
} from '../fixtures/snapshot_manager.js';
import { TokenSimulator } from '../simulators/token_simulator.js';
Expand Down Expand Up @@ -54,7 +55,7 @@ export class BlacklistTokenContractTest {
// This value MUST match the same value that we have in the contract
static DELAY = 2;

private snapshotManager: SnapshotManager;
private snapshotManager: ISnapshotManager;
logger: DebugLogger;
wallets: AccountWallet[] = [];
accounts: CompleteAddress[] = [];
Expand All @@ -68,7 +69,7 @@ export class BlacklistTokenContractTest {

constructor(testName: string) {
this.logger = createDebugLogger(`aztec:e2e_blacklist_token_contract:${testName}`);
this.snapshotManager = new SnapshotManager(`e2e_blacklist_token_contract/${testName}`, dataPath);
this.snapshotManager = createSnapshotManager(`e2e_blacklist_token_contract/${testName}`, dataPath);
}

async mineBlocks(amount: number = BlacklistTokenContractTest.DELAY) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ describe('e2e_blacklist_token_contract mint', () => {
await t.setup();
// Have to destructure again to ensure we have latest refs.
({ asset, tokenSim, wallets, blacklisted } = t);
}, 200_000);
}, 300_000);

afterAll(async () => {
await t.teardown();
Expand Down
Loading

0 comments on commit 8418eac

Please sign in to comment.