From b9a0d9336432b2becb4bf26b804f7c49084307fe Mon Sep 17 00:00:00 2001 From: Miranda Wood Date: Tue, 28 May 2024 14:49:41 +0100 Subject: [PATCH 1/4] refactor: chopping transient logs in ts (#6708) This PR moves where we chop transient note logs in ts from `private_execution -> executePrivateFunction()` to `pxe_service -> collectSortedNoteEncryptedLogs()`. It is required as note logs emitted in a completed nested execution may be chopped later in the parent execution, and the `tx` will still contain incorrect logs. Since the `tx` struct does not contain `.notes` or `.nullifiers`, we only need to do this for note logs. This change means the logs in `ExecutionResult.noteEncryptedLogs` and `.callStackItem` are in sync which otherwise may be confusing. It also uses the existing `nullifiedNoteHashCounters` to filter chopped logs. --- .../src/client/client_execution_context.ts | 35 ++--------------- .../src/client/execution_note_cache.ts | 39 +++++++------------ .../simulator/src/client/execution_result.ts | 20 ++++++++-- .../src/client/private_execution.test.ts | 10 ++--- .../simulator/src/client/private_execution.ts | 1 - 5 files changed, 38 insertions(+), 67 deletions(-) diff --git a/yarn-project/simulator/src/client/client_execution_context.ts b/yarn-project/simulator/src/client/client_execution_context.ts index 7c25c2af929..cc12ace4b3d 100644 --- a/yarn-project/simulator/src/client/client_execution_context.ts +++ b/yarn-project/simulator/src/client/client_execution_context.ts @@ -2,7 +2,6 @@ import { type AuthWitness, type AztecNode, EncryptedL2Log, - EncryptedL2NoteLog, L1NotePayload, Note, type NoteStatus, @@ -29,7 +28,7 @@ import { type NoteData, toACVMWitness } from '../acvm/index.js'; import { type PackedValuesCache } from '../common/packed_values_cache.js'; import { type DBOracle } from './db_oracle.js'; import { type ExecutionNoteCache } from './execution_note_cache.js'; -import { CountedLog, type ExecutionResult, type NoteAndSlot } from './execution_result.js'; +import { CountedLog, type CountedNoteLog, type ExecutionResult, type NoteAndSlot } from './execution_result.js'; import { pickNotes } from './pick_notes.js'; import { executePrivateFunction } from './private_execution.js'; import { ViewDataOracle } from './view_data_oracle.js'; @@ -57,7 +56,7 @@ export class ClientExecutionContext extends ViewDataOracle { */ private noteHashLeafIndexMap: Map = new Map(); private nullifiedNoteHashCounters: Map = new Map(); - private noteEncryptedLogs: CountedLog[] = []; + private noteEncryptedLogs: CountedNoteLog[] = []; private encryptedLogs: CountedLog[] = []; private unencryptedLogs: CountedLog[] = []; private nestedExecutions: ExecutionResult[] = []; @@ -137,33 +136,6 @@ export class ClientExecutionContext extends ViewDataOracle { return this.noteEncryptedLogs; } - /** - * Sometimes notes can be chopped after a nested execution is complete. - * This means finished nested executions still hold transient logs. This method removes them. - * TODO(Miranda): is there a cleaner solution? - */ - public chopNoteEncryptedLogs() { - // Do not return logs that have been chopped in the cache - const allNoteLogs = this.noteCache.getLogs(); - this.noteEncryptedLogs = this.noteEncryptedLogs.filter(l => allNoteLogs.includes(l)); - const chop = (thing: any) => - thing.nestedExecutions.forEach((result: ExecutionResult) => { - if (!result.noteEncryptedLogs[0]?.isEmpty()) { - // The execution has note logs - result.noteEncryptedLogs = result.noteEncryptedLogs.filter(l => allNoteLogs.includes(l)); - } - chop(result); - }); - chop(this); - } - - /** - * Return the note encrypted logs emitted during this execution and nested executions. - */ - public getAllNoteEncryptedLogs() { - return this.noteCache.getLogs(); - } - /** * Return the encrypted logs emitted during this execution. */ @@ -382,9 +354,8 @@ export class ClientExecutionContext extends ViewDataOracle { * @param counter - The effects counter. */ public override emitEncryptedNoteLog(noteHash: Fr, encryptedNote: Buffer, counter: number) { - const encryptedLog = new CountedLog(new EncryptedL2NoteLog(encryptedNote), counter); + const encryptedLog = this.noteCache.addNewLog(encryptedNote, counter, noteHash); this.noteEncryptedLogs.push(encryptedLog); - this.noteCache.addNewLog(encryptedLog, noteHash); } /** diff --git a/yarn-project/simulator/src/client/execution_note_cache.ts b/yarn-project/simulator/src/client/execution_note_cache.ts index 43a54b32f48..499fd87b05f 100644 --- a/yarn-project/simulator/src/client/execution_note_cache.ts +++ b/yarn-project/simulator/src/client/execution_note_cache.ts @@ -1,10 +1,10 @@ -import { type EncryptedL2NoteLog } from '@aztec/circuit-types'; +import { EncryptedL2NoteLog } from '@aztec/circuit-types'; import { siloNullifier } from '@aztec/circuits.js/hash'; import { type AztecAddress } from '@aztec/foundation/aztec-address'; import { Fr } from '@aztec/foundation/fields'; import { type NoteData } from '../acvm/index.js'; -import { type CountedLog } from './execution_result.js'; +import { CountedNoteLog } from './execution_result.js'; export interface PendingNote { note: NoteData; @@ -29,13 +29,6 @@ export class ExecutionNoteCache { */ private nullifiers: Map> = new Map(); - /** - * The list of encrypted logs linked to note hashes created in this transaction. - * This mapping maps from inner note hash to log(s) emitted for that note hash. - * Note that their value (bigint representation) is used because Frs cannot be looked up in Sets. - */ - private logs: Map[]> = new Map(); - /** * Add a new note to cache. * @param note - New note created during execution. @@ -47,13 +40,20 @@ export class ExecutionNoteCache { } /** - * Add a new note to cache. - * @param note - New note created during execution. + * Create a new log linked to a note in this cache. + * @param encryptedNote - Encrypted new note created during execution. + * @param counter - Counter of the log. + * @param innerNoteHash - Hashed new note created during execution. */ - public addNewLog(log: CountedLog, innerNoteHash: Fr) { - const logs = this.logs.get(innerNoteHash.toBigInt()) ?? []; - logs.push(log); - this.logs.set(innerNoteHash.toBigInt(), logs); + public addNewLog(encryptedNote: Buffer, counter: number, innerNoteHash: Fr) { + const note = Array.from(this.newNotes.values()) + .flat() + .find(n => n.note.innerNoteHash.equals(innerNoteHash)); + if (!note) { + throw new Error('Attempt to add a note log for note that does not exist.'); + } + const log = new CountedNoteLog(new EncryptedL2NoteLog(encryptedNote), counter, note.counter); + return log; } /** @@ -81,8 +81,6 @@ export class ExecutionNoteCache { const note = notes.splice(noteIndexToRemove, 1)[0]; nullifiedNoteHashCounter = note.counter; this.newNotes.set(contractAddress.toBigInt(), notes); - // If a log linked to the note hash does not exist, this method just does nothing - this.logs.delete(innerNoteHash.toBigInt()); } return nullifiedNoteHashCounter; @@ -117,11 +115,4 @@ export class ExecutionNoteCache { public getNullifiers(contractAddress: AztecAddress): Set { return this.nullifiers.get(contractAddress.toBigInt()) ?? new Set(); } - - /** - * Return all note logs emitted from a contract. - */ - public getLogs(): CountedLog[] { - return Array.from(this.logs.values()).flat(); - } } diff --git a/yarn-project/simulator/src/client/execution_result.ts b/yarn-project/simulator/src/client/execution_result.ts index b5f5cdec5a3..0328e04ede3 100644 --- a/yarn-project/simulator/src/client/execution_result.ts +++ b/yarn-project/simulator/src/client/execution_result.ts @@ -32,6 +32,11 @@ export class CountedLog { + constructor(log: EncryptedL2NoteLog, counter: number, public noteHashCounter: number) { + super(log, counter); + } +} /** * The result of executing a private function. */ @@ -64,7 +69,7 @@ export interface ExecutionResult { * Encrypted note logs emitted during execution of this function call. * Note: These are preimages to `noteEncryptedLogsHashes`. */ - noteEncryptedLogs: CountedLog[]; + noteEncryptedLogs: CountedNoteLog[]; /** * Encrypted logs emitted during execution of this function call. * Note: These are preimages to `encryptedLogsHashes`. @@ -94,8 +99,14 @@ export function collectNullifiedNoteHashCounters(execResult: ExecutionResult, ac * @param execResult - The topmost execution result. * @returns All encrypted logs. */ -function collectNoteEncryptedLogs(execResult: ExecutionResult): CountedLog[] { - return [execResult.noteEncryptedLogs, ...execResult.nestedExecutions.flatMap(collectNoteEncryptedLogs)].flat(); +function collectNoteEncryptedLogs( + execResult: ExecutionResult, + nullifiedNoteHashCounters: Map, +): CountedLog[] { + return [ + execResult.noteEncryptedLogs.filter(noteLog => !nullifiedNoteHashCounters.has(noteLog.noteHashCounter)), + ...execResult.nestedExecutions.flatMap(res => collectNoteEncryptedLogs(res, nullifiedNoteHashCounters)), + ].flat(); } /** @@ -104,7 +115,8 @@ function collectNoteEncryptedLogs(execResult: ExecutionResult): CountedLog l.log)); } diff --git a/yarn-project/simulator/src/client/private_execution.test.ts b/yarn-project/simulator/src/client/private_execution.test.ts index 27c7962b14a..b6975dab658 100644 --- a/yarn-project/simulator/src/client/private_execution.test.ts +++ b/yarn-project/simulator/src/client/private_execution.test.ts @@ -970,9 +970,8 @@ describe('Private Execution test suite', () => { const [encryptedLog] = newEncryptedLogs; expect(encryptedLog.noteHashCounter).toEqual(newNoteHashes[0].counter); - // We expect the note log to be chopped in ts. - // (note logs are chopped in kernel tail, so will still exist in the call stack item) - expect(result.noteEncryptedLogs).toHaveLength(0); + expect(encryptedLog.noteHashCounter).toEqual(result.noteEncryptedLogs[0].noteHashCounter); + expect(encryptedLog.value).toEqual(Fr.fromBuffer(result.noteEncryptedLogs[0].log.hash())); // read request should match innerNoteHash for pending notes (there is no nonce, so can't compute "unique" hash) const readRequest = getNonEmptyItems(result.callStackItem.publicInputs.noteHashReadRequests)[0]; @@ -1049,9 +1048,8 @@ describe('Private Execution test suite', () => { const [encryptedLog] = newEncryptedLogs; expect(encryptedLog.noteHashCounter).toEqual(newNoteHashes[0].counter); - // We expect the note log to be chopped in ts. - // (note logs are chopped in kernel tail, so will still exist in the call stack item) - expect(execInsert.noteEncryptedLogs).toHaveLength(0); + expect(encryptedLog.noteHashCounter).toEqual(execInsert.noteEncryptedLogs[0].noteHashCounter); + expect(encryptedLog.value).toEqual(Fr.fromBuffer(execInsert.noteEncryptedLogs[0].log.hash())); // read request should match innerNoteHash for pending notes (there is no nonce, so can't compute "unique" hash) const readRequest = execGetThenNullify.callStackItem.publicInputs.noteHashReadRequests[0]; diff --git a/yarn-project/simulator/src/client/private_execution.ts b/yarn-project/simulator/src/client/private_execution.ts index 96e43869564..335fe84a31a 100644 --- a/yarn-project/simulator/src/client/private_execution.ts +++ b/yarn-project/simulator/src/client/private_execution.ts @@ -54,7 +54,6 @@ export async function executePrivateFunction( appCircuitName: functionName, } satisfies CircuitWitnessGenerationStats); - context.chopNoteEncryptedLogs(); const noteEncryptedLogs = context.getNoteEncryptedLogs(); const encryptedLogs = context.getEncryptedLogs(); const unencryptedLogs = context.getUnencryptedLogs(); From 3e786dea3a6ff1b76bf251d1fcd14b05a7204a85 Mon Sep 17 00:00:00 2001 From: Cody Gunton Date: Tue, 28 May 2024 09:56:41 -0400 Subject: [PATCH 2/4] chore: Delete redundant CircleCI GCC job (#6712) In our switch to Earthly we have introduced a job to build Barretenberg using GCC-13, the goal being to ensure that our code is not overly clang-specific. Given the goal, is it unnecessary to keep building with GCC-12 in our backup CircleCI setup. This CirclCI job has recently broken due to a difference in settings. It is not a goal of ours to support old versions of GCC, so we delete the job. --- .circleci/config.yml | 14 -------------- build_manifest.yml | 6 ------ 2 files changed, 20 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index e85ad8cca45..c90c37c371a 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -105,18 +105,6 @@ jobs: command: cond_spot_run_build barretenberg-wasm-linux-clang 128 aztec_manifest_key: barretenberg-wasm-linux-clang - barretenberg-x86_64-linux-gcc: - docker: - - image: aztecprotocol/alpine-build-image - resource_class: small - steps: - - *checkout - - *setup_env - - run: - name: "Build" - command: cond_spot_run_build barretenberg-x86_64-linux-gcc 128 - aztec_manifest_key: barretenberg-x86_64-linux-gcc - barretenberg-x86_64-linux-clang: docker: - image: aztecprotocol/alpine-build-image @@ -505,7 +493,6 @@ workflows: - avm-transpiler: *defaults # Barretenberg - - barretenberg-x86_64-linux-gcc: *defaults - barretenberg-x86_64-linux-clang: *defaults - barretenberg-x86_64-linux-clang-assert: *defaults # - barretenberg-x86_64-linux-clang-fuzzing: *defaults @@ -555,7 +542,6 @@ workflows: # Everything that must complete before deployment. - end: requires: - - barretenberg-x86_64-linux-gcc - barretenberg-x86_64-linux-clang - barretenberg-x86_64-linux-clang-assert # - barretenberg-x86_64-linux-clang-fuzzing diff --git a/build_manifest.yml b/build_manifest.yml index 4257cd66e11..234a09418a3 100644 --- a/build_manifest.yml +++ b/build_manifest.yml @@ -84,12 +84,6 @@ barretenberg-x86_64-linux-clang-fuzzing: dockerfile: dockerfiles/Dockerfile.x86_64-linux-clang-fuzzing rebuildPatterns: .rebuild_patterns -# Builds all of barretenberg with gcc. Ensures compiler compatibility. -barretenberg-x86_64-linux-gcc: - buildDir: barretenberg/cpp - dockerfile: dockerfiles/Dockerfile.x86_64-linux-gcc - rebuildPatterns: .rebuild_patterns - # Builds barretenberg.wasm (single and multithreaded builds). barretenberg-wasm-linux-clang: buildDir: barretenberg/cpp From 0f86674bb99db37b39bedeff2169f6a08fc2c507 Mon Sep 17 00:00:00 2001 From: Santiago Palladino Date: Tue, 28 May 2024 15:57:43 +0200 Subject: [PATCH 3/4] fix: Estimate gas cost of contract deployment (#6710) Fixes automatic gas estimation for contract deployments from aztecjs when the contract has a public initializer, as they would fail with an "unknown contract" during that simulation. --- .../src/contract/base_contract_interaction.ts | 2 +- .../aztec.js/src/contract/batch_call.ts | 2 +- .../contract/contract_function_interaction.ts | 2 +- .../aztec.js/src/contract/deploy_method.ts | 20 ++++++-- .../aztec.js/src/wallet/signerless_wallet.ts | 12 ++--- .../src/e2e_fees/gas_estimation.test.ts | 50 ++++++++++++++++++- .../foundation/src/serialize/serialize.ts | 2 + 7 files changed, 76 insertions(+), 14 deletions(-) diff --git a/yarn-project/aztec.js/src/contract/base_contract_interaction.ts b/yarn-project/aztec.js/src/contract/base_contract_interaction.ts index fc1dfdb44e8..20af8dce86d 100644 --- a/yarn-project/aztec.js/src/contract/base_contract_interaction.ts +++ b/yarn-project/aztec.js/src/contract/base_contract_interaction.ts @@ -95,7 +95,7 @@ export abstract class BaseContractInteraction { * @param request - Request to execute for this interaction. * @returns Fee options for the actual transaction. */ - protected async getFeeOptions(request: ExecutionRequestInit) { + protected async getFeeOptionsFromEstimatedGas(request: ExecutionRequestInit) { const fee = request.fee; if (fee) { const txRequest = await this.wallet.createTxExecutionRequest(request); diff --git a/yarn-project/aztec.js/src/contract/batch_call.ts b/yarn-project/aztec.js/src/contract/batch_call.ts index d2f1f6c9cd1..3a02a713901 100644 --- a/yarn-project/aztec.js/src/contract/batch_call.ts +++ b/yarn-project/aztec.js/src/contract/batch_call.ts @@ -20,7 +20,7 @@ export class BatchCall extends BaseContractInteraction { public async create(opts?: SendMethodOptions): Promise { if (!this.txRequest) { const calls = this.calls; - const fee = opts?.estimateGas ? await this.getFeeOptions({ calls, fee: opts?.fee }) : opts?.fee; + const fee = opts?.estimateGas ? await this.getFeeOptionsFromEstimatedGas({ calls, fee: opts?.fee }) : opts?.fee; this.txRequest = await this.wallet.createTxExecutionRequest({ calls, fee }); } return this.txRequest; 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 d74a282efff..7d31f4f74f4 100644 --- a/yarn-project/aztec.js/src/contract/contract_function_interaction.ts +++ b/yarn-project/aztec.js/src/contract/contract_function_interaction.ts @@ -54,7 +54,7 @@ export class ContractFunctionInteraction extends BaseContractInteraction { } if (!this.txRequest) { const calls = [this.request()]; - const fee = opts?.estimateGas ? await this.getFeeOptions({ calls, fee: opts?.fee }) : opts?.fee; + const fee = opts?.estimateGas ? await this.getFeeOptionsFromEstimatedGas({ calls, fee: opts?.fee }) : opts?.fee; this.txRequest = await this.wallet.createTxExecutionRequest({ calls, fee }); } return this.txRequest; diff --git a/yarn-project/aztec.js/src/contract/deploy_method.ts b/yarn-project/aztec.js/src/contract/deploy_method.ts index 5521901f943..d3f67b865b8 100644 --- a/yarn-project/aztec.js/src/contract/deploy_method.ts +++ b/yarn-project/aztec.js/src/contract/deploy_method.ts @@ -79,8 +79,6 @@ export class DeployMethod extends Bas public async create(options: DeployOptions = {}): Promise { if (!this.txRequest) { this.txRequest = await this.wallet.createTxExecutionRequest(await this.request(options)); - // TODO: Should we add the contracts to the DB here, or once the tx has been sent or mined? - await this.wallet.registerContract({ artifact: this.artifact, instance: this.instance! }); } return this.txRequest; } @@ -98,6 +96,14 @@ export class DeployMethod extends Bas */ public async request(options: DeployOptions = {}): Promise { if (!this.functionCalls) { + // TODO: Should we add the contracts to the DB here, or once the tx has been sent or mined? + // Note that we need to run this registerContract here so it's available when computeFeeOptionsFromEstimatedGas + // runs, since it needs the contract to have been registered in order to estimate gas for its initialization, + // in case the initializer is public. This hints at the need of having "transient" contracts scoped to a + // simulation, so we can run the simulation with a set of contracts, but only "commit" them to the wallet + // once this tx has gone through. + await this.wallet.registerContract({ artifact: this.artifact, instance: this.getInstance(options) }); + const deployment = await this.getDeploymentFunctionCalls(options); const bootstrap = await this.getInitializeFunctionCalls(options); @@ -113,7 +119,7 @@ export class DeployMethod extends Bas }; if (options.estimateGas) { - request.fee = await this.getFeeOptions(request); + request.fee = await this.getFeeOptionsFromEstimatedGas(request); } this.functionCalls = request; @@ -233,6 +239,14 @@ export class DeployMethod extends Bas return super.prove(options); } + /** + * Estimates gas cost for this deployment operation. + * @param options - Options. + */ + public override estimateGas(options?: Omit) { + return super.estimateGas(options); + } + /** Return this deployment address. */ public get address() { return this.instance?.address; diff --git a/yarn-project/aztec.js/src/wallet/signerless_wallet.ts b/yarn-project/aztec.js/src/wallet/signerless_wallet.ts index f265107e4f1..bba8c3ec66e 100644 --- a/yarn-project/aztec.js/src/wallet/signerless_wallet.ts +++ b/yarn-project/aztec.js/src/wallet/signerless_wallet.ts @@ -24,26 +24,26 @@ export class SignerlessWallet extends BaseWallet { } getChainId(): Fr { - throw new Error('Method not implemented.'); + throw new Error('SignerlessWallet: Method getChainId not implemented.'); } getVersion(): Fr { - throw new Error('Method not implemented.'); + throw new Error('SignerlessWallet: Method getVersion not implemented.'); } getPublicKeysHash(): Fr { - throw new Error('Method not implemented.'); + throw new Error('SignerlessWallet: Method getPublicKeysHash not implemented.'); } getCompleteAddress(): CompleteAddress { - throw new Error('Method not implemented.'); + throw new Error('SignerlessWallet: Method getCompleteAddress not implemented.'); } createAuthWit(_messageHash: Fr): Promise { - throw new Error('Method not implemented.'); + throw new Error('SignerlessWallet: Method createAuthWit not implemented.'); } rotateNullifierKeys(_newNskM: Fq): Promise { - throw new Error('Method not implemented.'); + throw new Error('SignerlessWallet: Method rotateNullifierKeys not implemented.'); } } 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 e9e926fcfaa..d718794a5ad 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 @@ -6,7 +6,10 @@ import { PublicFeePaymentMethod, } from '@aztec/aztec.js'; import { GasFees, type GasSettings } from '@aztec/circuits.js'; -import { type TokenContract as BananaCoin, type FPCContract } from '@aztec/noir-contracts.js'; +import { type Logger } from '@aztec/foundation/log'; +import { TokenContract as BananaCoin, type FPCContract } from '@aztec/noir-contracts.js'; + +import { inspect } from 'util'; import { FeesTest } from './fees_test.js'; @@ -18,6 +21,7 @@ describe('e2e_fees gas_estimation', () => { let bananaFPC: FPCContract; let gasSettings: GasSettings; let teardownFixedFee: bigint; + let logger: Logger; const t = new FeesTest('gas_estimation'); @@ -26,7 +30,7 @@ describe('e2e_fees gas_estimation', () => { await t.applyFPCSetupSnapshot(); await t.applyFundAliceWithBananas(); await t.applyFundAliceWithGasToken(); - ({ aliceWallet, aliceAddress, bobAddress, bananaCoin, bananaFPC, gasSettings } = await t.setup()); + ({ aliceWallet, aliceAddress, bobAddress, bananaCoin, bananaFPC, gasSettings, logger } = await t.setup()); teardownFixedFee = gasSettings.teardownGasLimits.computeFee(GasFees.default()).toBigInt(); }); @@ -51,9 +55,17 @@ describe('e2e_fees gas_estimation', () => { .add(estimatedGas.teardownGasLimits.computeFee(GasFees.default())) .toBigInt(); + const logGasEstimate = (estimatedGas: Pick) => + logger.info(`Estimated gas at`, { + gasLimits: inspect(estimatedGas.gasLimits), + teardownGasLimits: inspect(estimatedGas.teardownGasLimits), + }); + it('estimates gas with native fee payment method', async () => { const paymentMethod = new NativeFeePaymentMethod(aliceAddress); const estimatedGas = await makeTransferRequest().estimateGas({ fee: { gasSettings, paymentMethod } }); + logGasEstimate(estimatedGas); + const [withEstimate, withoutEstimate] = await sendTransfers(paymentMethod); const actualFee = withEstimate.transactionFee!; @@ -75,6 +87,8 @@ describe('e2e_fees gas_estimation', () => { it('estimates gas with public payment method', async () => { const paymentMethod = new PublicFeePaymentMethod(bananaCoin.address, bananaFPC.address, aliceWallet); const estimatedGas = await makeTransferRequest().estimateGas({ fee: { gasSettings, paymentMethod } }); + logGasEstimate(estimatedGas); + const [withEstimate, withoutEstimate] = await sendTransfers(paymentMethod); const actualFee = withEstimate.transactionFee!; @@ -93,4 +107,36 @@ describe('e2e_fees gas_estimation', () => { expect(feeFromEstimatedGas).toBeLessThan(actualFee * 3n); expect(feeFromEstimatedGas).toBeGreaterThanOrEqual(actualFee); }); + + it('estimates gas for public contract initialization with native fee payment method', async () => { + const paymentMethod = new NativeFeePaymentMethod(aliceAddress); + const deployMethod = () => BananaCoin.deploy(aliceWallet, aliceAddress, 'TKN', 'TKN', 8); + const deployOpts = { fee: { gasSettings, paymentMethod }, skipClassRegistration: true }; + const estimatedGas = await deployMethod().estimateGas(deployOpts); + logGasEstimate(estimatedGas); + + const [withEstimate, withoutEstimate] = await Promise.all([ + deployMethod() + .send({ ...deployOpts, estimateGas: true }) + .wait(), + deployMethod() + .send({ ...deployOpts, estimateGas: false }) + .wait(), + ]); + + // Estimation should yield that teardown has no cost, so should send the tx with zero for teardown + const actualFee = withEstimate.transactionFee!; + expect(actualFee + teardownFixedFee).toEqual(withoutEstimate.transactionFee!); + + // Check that estimated gas for teardown are zero + expect(estimatedGas.teardownGasLimits.l2Gas).toEqual(0); + expect(estimatedGas.teardownGasLimits.daGas).toEqual(0); + + // Check that the estimate was close to the actual gas used by recomputing the tx fee from it + const feeFromEstimatedGas = getFeeFromEstimatedGas(estimatedGas); + + // The actual fee should be under the estimate, but not too much + expect(feeFromEstimatedGas).toBeLessThan(actualFee * 2n); + expect(feeFromEstimatedGas).toBeGreaterThanOrEqual(actualFee); + }); }); diff --git a/yarn-project/foundation/src/serialize/serialize.ts b/yarn-project/foundation/src/serialize/serialize.ts index eaa81c27c7f..ae305070714 100644 --- a/yarn-project/foundation/src/serialize/serialize.ts +++ b/yarn-project/foundation/src/serialize/serialize.ts @@ -242,6 +242,8 @@ export function toFriendlyJSON(obj: object): string { ).toFriendlyJSON ) { return value.toFriendlyJSON(); + } else if (value && value.type && ['Fr', 'Fq', 'AztecAddress'].includes(value.type)) { + return value.value; } else { return value; } From 1d785fd1087d7387fc29213ca3be50b2fc9c4725 Mon Sep 17 00:00:00 2001 From: Santiago Palladino Date: Tue, 28 May 2024 15:57:55 +0200 Subject: [PATCH 4/4] fix: Tx receipt serialization to JSON (#6711) Thought we had finally got rid of all serialization issues? Never! --- yarn-project/circuit-types/src/tx/tx_hash.ts | 9 ++++++++ .../circuit-types/src/tx/tx_receipt.test.ts | 23 +++++++++++++++++++ .../circuit-types/src/tx/tx_receipt.ts | 3 ++- 3 files changed, 34 insertions(+), 1 deletion(-) create mode 100644 yarn-project/circuit-types/src/tx/tx_receipt.test.ts diff --git a/yarn-project/circuit-types/src/tx/tx_hash.ts b/yarn-project/circuit-types/src/tx/tx_hash.ts index 514097b3d6b..c92340c1928 100644 --- a/yarn-project/circuit-types/src/tx/tx_hash.ts +++ b/yarn-project/circuit-types/src/tx/tx_hash.ts @@ -1,3 +1,4 @@ +import { randomBytes } from '@aztec/foundation/crypto'; import { BufferReader, deserializeBigInt, serializeBigInt } from '@aztec/foundation/serialize'; /** @@ -105,4 +106,12 @@ export class TxHash { public static fromString(str: string): TxHash { return new TxHash(Buffer.from(str, 'hex')); } + + /** + * Generates a random TxHash. + * @returns A new TxHash object. + */ + public static random(): TxHash { + return new TxHash(Buffer.from(randomBytes(TxHash.SIZE))); + } } diff --git a/yarn-project/circuit-types/src/tx/tx_receipt.test.ts b/yarn-project/circuit-types/src/tx/tx_receipt.test.ts new file mode 100644 index 00000000000..2eac60ece2e --- /dev/null +++ b/yarn-project/circuit-types/src/tx/tx_receipt.test.ts @@ -0,0 +1,23 @@ +import { TxHash } from './tx_hash.js'; +import { TxReceipt, TxStatus } from './tx_receipt.js'; + +describe('TxReceipt', () => { + it('serializes and deserializes from json', () => { + const receipt = new TxReceipt( + TxHash.random(), + TxStatus.SUCCESS, + 'error', + BigInt(1), + Buffer.from('blockHash'), + undefined, + ); + + expect(TxReceipt.fromJSON(receipt.toJSON())).toEqual(receipt); + }); + + it('serializes and deserializes from json with undefined fields', () => { + const receipt = new TxReceipt(TxHash.random(), TxStatus.DROPPED, 'error', undefined, undefined, undefined); + + expect(TxReceipt.fromJSON(receipt.toJSON())).toEqual(receipt); + }); +}); diff --git a/yarn-project/circuit-types/src/tx/tx_receipt.ts b/yarn-project/circuit-types/src/tx/tx_receipt.ts index 39c9f29f540..fc32f1b077f 100644 --- a/yarn-project/circuit-types/src/tx/tx_receipt.ts +++ b/yarn-project/circuit-types/src/tx/tx_receipt.ts @@ -66,6 +66,7 @@ export class TxReceipt { error: this.error, blockHash: this.blockHash?.toString('hex'), blockNumber: this.blockNumber, + transactionFee: this.transactionFee?.toString(), }; } @@ -78,7 +79,7 @@ export class TxReceipt { const txHash = TxHash.fromString(obj.txHash); const status = obj.status as TxStatus; const error = obj.error; - const transactionFee = obj.transactionFee; + const transactionFee = obj.transactionFee ? BigInt(obj.transactionFee) : undefined; const blockHash = obj.blockHash ? Buffer.from(obj.blockHash, 'hex') : undefined; const blockNumber = obj.blockNumber ? Number(obj.blockNumber) : undefined; return new TxReceipt(txHash, status, error, transactionFee, blockHash, blockNumber);