chore: Accumulated backports to v5-next

yarn-project/aztec.js/src/contract/wait_opts.ts

@@ -4,14 +4,19 @@ import type { TxStatus } from '@aztec/stdlib/tx';
 export type WaitOpts = {
   /** The amount of time to ignore TxStatus.DROPPED receipts (in seconds) due to the presumption that it is being propagated by the p2p network. Defaults to 5. */
   ignoreDroppedReceiptsFor?: number;
-  /** The maximum time (in seconds) to wait for the transaction to be mined. Defaults to 60. */
+  /** The maximum time (in seconds) to wait for the transaction to be mined. Defaults to 300 (5 min). */
   timeout?: number;
   /** The time interval (in seconds) between retries to fetch the transaction receipt. Defaults to 1. */
   interval?: number;
   /** Whether to accept a revert as a status code for the tx when waiting for it. If false, will throw if the tx reverts. */
   dontThrowOnRevert?: boolean;
   /** The minimum inclusion status to wait for. If set, waits until the receipt reaches this status or higher. Defaults to CHECKPOINTED. */
   waitForStatus?: TxStatus;
+  /**
+   * The time (in seconds) to wait before the first receipt poll. Defaults to 0. Used to avoid checking for a receipt
+   * right after sending a tx, when we know it cannot have been mined yet. Counts against `timeout`.
+   */
+  initialDelay?: number;
 };
 
 export const DefaultWaitOpts: WaitOpts = {

yarn-project/aztec.js/src/utils/node.test.ts

@@ -78,6 +78,45 @@ describe('waitForTx', () => {
     });
   });
 
+  describe('initialDelay option', () => {
+    it('delays the first receipt poll', async () => {
+      const start = Date.now();
+      let firstPollAt: number | undefined;
+      node.getTxReceipt.mockImplementation(() => {
+        firstPollAt ??= Date.now();
+        return Promise.resolve(minedReceipt(TxStatus.CHECKPOINTED));
+      });
+
+      const receipt = await waitForTx(node, txHash, { timeout: 1, interval: 0.05, initialDelay: 0.2 });
+
+      expect(receipt.status).toBe(TxStatus.CHECKPOINTED);
+      expect(node.getTxReceipt).toHaveBeenCalledTimes(1);
+      expect(firstPollAt! - start).toBeGreaterThanOrEqual(150);
+    });
+
+    it('does not consume the dropped-receipt grace period', async () => {
+      node.getTxReceipt.mockResolvedValue(new DroppedTxReceipt(txHash, 'Tx dropped'));
+      const start = Date.now();
+
+      await expect(
+        waitForTx(node, txHash, { timeout: 2, interval: 0.05, initialDelay: 0.3, ignoreDroppedReceiptsFor: 0.2 }),
+      ).rejects.toThrow(/dropped/);
+
+      expect(Date.now() - start).toBeGreaterThanOrEqual(450);
+    });
+
+    it('counts against the timeout', async () => {
+      node.getTxReceipt.mockResolvedValue(new PendingTxReceipt(txHash, undefined));
+      const start = Date.now();
+
+      await expect(waitForTx(node, txHash, { timeout: 0.3, interval: 0.05, initialDelay: 1 })).rejects.toThrow(
+        /Timeout/,
+      );
+
+      expect(Date.now() - start).toBeLessThan(1000);
+    });
+  });
+
   describe('waitForStatus option', () => {
     it('returns immediately when receipt status matches requested status', async () => {
       node.getTxReceipt.mockResolvedValue(minedReceipt(TxStatus.CHECKPOINTED));

yarn-project/aztec.js/src/utils/node.ts

@@ -1,5 +1,6 @@
 import type { Logger } from '@aztec/foundation/log';
 import { retryUntil } from '@aztec/foundation/retry';
+import { sleep } from '@aztec/foundation/sleep';
 import type { AztecNode } from '@aztec/stdlib/interfaces/client';
 import type { TxHash, TxReceipt } from '@aztec/stdlib/tx';
 import { SortedTxStatuses, TxStatus } from '@aztec/stdlib/tx';
@@ -47,9 +48,20 @@ function hasReachedStatus(receipt: TxReceipt, desiredStatus: TxStatus): boolean
  * @throws If the transaction fails and dontThrowOnRevert is not set
  */
 export async function waitForTx(node: AztecNode, txHash: TxHash, opts?: WaitOpts): Promise<TxReceipt> {
-  const startTime = Date.now();
   const ignoreDroppedReceiptsFor = opts?.ignoreDroppedReceiptsFor ?? DefaultWaitOpts.ignoreDroppedReceiptsFor;
   const waitForStatus = opts?.waitForStatus ?? TxStatus.CHECKPOINTED;
+  const timeout = opts?.timeout ?? DefaultWaitOpts.timeout;
+  // The initial delay counts against the timeout: the deadline is fixed before sleeping, and the sleep never
+  // exceeds it.
+  const deadline = timeout ? { deadline: new Date(Date.now() + timeout * 1000) } : undefined;
+
+  if (opts?.initialDelay) {
+    const delay = Math.min(opts.initialDelay, timeout ?? Infinity);
+    await sleep(delay * 1000);
+  }
+
+  // The grace period for DROPPED receipts starts at the first poll, so the initial delay does not consume it.
+  const startTime = Date.now();
 
   const receipt = await retryUntil(
     async () => {
@@ -75,7 +87,7 @@ export async function waitForTx(node: AztecNode, txHash: TxHash, opts?: WaitOpts
       return txReceipt;
     },
     'isMined',
-    opts?.timeout ?? DefaultWaitOpts.timeout,
+    deadline,
     opts?.interval ?? DefaultWaitOpts.interval,
   );
 

yarn-project/aztec.js/src/wallet/wallet.ts

@@ -333,6 +333,7 @@ export const WaitOptsSchema = z.object({
   timeout: optional(z.number()),
   interval: optional(z.number()),
   dontThrowOnRevert: optional(z.boolean()),
+  initialDelay: optional(z.number()),
 });
 
 const FromSchema = z.union([schemas.AztecAddress, z.literal(NO_FROM)]);

yarn-project/end-to-end/src/automine/double_spend.test.ts

@@ -1,6 +1,7 @@
 import { AztecAddress } from '@aztec/aztec.js/addresses';
 import { Fr } from '@aztec/aztec.js/fields';
 import type { Logger } from '@aztec/aztec.js/log';
+import type { AztecNode } from '@aztec/aztec.js/node';
 import { TxExecutionResult } from '@aztec/aztec.js/tx';
 import type { Wallet } from '@aztec/aztec.js/wallet';
 import { TestContract } from '@aztec/noir-test-contracts.js/Test';
@@ -11,6 +12,7 @@ import { AutomineTestContext } from './automine_test_context.js';
 // Uses setup(1, AUTOMINE_E2E_OPTS) with one node, automine sequencer, one funded account.
 describe('automine/double_spend', () => {
   let wallet: Wallet;
+  let aztecNode: AztecNode;
   let defaultAccountAddress: AztecAddress;
 
   let logger: Logger;
@@ -20,12 +22,14 @@ describe('automine/double_spend', () => {
 
   beforeAll(async () => {
     // Setup environment
+    const ctx = await AutomineTestContext.setup({ numberOfAccounts: 1 });
     ({
       teardown,
       wallet,
       accounts: [defaultAccountAddress],
       logger,
-    } = (await AutomineTestContext.setup({ numberOfAccounts: 1 })).context);
+    } = ctx.context);
+    aztecNode = ctx.aztecNode;
 
     ({ contract } = await TestContract.deploy(wallet).send({ from: defaultAccountAddress }));
 
@@ -53,5 +57,16 @@ describe('automine/double_spend', () => {
         contract.methods.emit_nullifier_public(nullifier).send({ from: defaultAccountAddress }),
       ).rejects.toThrow(TxExecutionResult.REVERTED);
     });
+
+    it('rejects re-sending a tx that was already mined', async () => {
+      const { receipt } = await contract.methods.emit_nullifier_public(new Fr(2)).send({
+        from: defaultAccountAddress,
+      });
+
+      const minedTx = await aztecNode.getTxByHash(receipt.txHash, { includeProof: true });
+      expect(minedTx).toBeDefined();
+
+      await expect(aztecNode.sendTx(minedTx!)).rejects.toThrow(/Existing nullifier/);
+    });
   });
 });

yarn-project/pxe/src/block_synchronizer/block_synchronizer.test.ts

@@ -24,8 +24,8 @@ import { TxHash } from '@aztec/stdlib/tx';
 import { type MockProxy, mock } from 'jest-mock-extended';
 
 import type { BlockSynchronizerConfig } from '../config/index.js';
-import type { ContractClassService } from '../contract/contract_class_service.js';
 import type { ContractSyncService } from '../contract/contract_sync_service.js';
+import { type CachingAztecNode, withCache } from '../node/caching_aztec_node.js';
 import { AnchorBlockStore } from '../storage/anchor_block_store/anchor_block_store.js';
 import { FactStore } from '../storage/fact_store/fact_store.js';
 import { FactCollectionKey, FactCollectionTypeKey } from '../storage/fact_store/fact_store_keys.js';
@@ -51,7 +51,7 @@ describe('BlockSynchronizer', () => {
   let getBlock: NodeGetBlockMock;
   let blockStream: MockProxy<L2BlockStream>;
   let contractSyncService: MockProxy<ContractSyncService>;
-  let contractClassService: MockProxy<ContractClassService>;
+  let cachedNode: CachingAztecNode;
 
   const TestSynchronizer = class extends BlockSynchronizer {
     protected override createBlockStream(): L2BlockStream {
@@ -61,15 +61,14 @@ describe('BlockSynchronizer', () => {
 
   const createSynchronizer = (config: Partial<BlockSynchronizerConfig> = {}) => {
     return new TestSynchronizer(
-      aztecNode,
+      cachedNode,
       store,
       anchorBlockStore,
       noteStore,
       privateEventStore,
       factStore,
       tipsStore,
       contractSyncService,
-      contractClassService,
       config,
     );
   };
@@ -130,7 +129,7 @@ describe('BlockSynchronizer', () => {
     privateEventStore = new PrivateEventStore(store);
     factStore = new FactStore(store);
     contractSyncService = mock<ContractSyncService>();
-    contractClassService = mock<ContractClassService>();
+    cachedNode = withCache(aztecNode);
     synchronizer = createSynchronizer();
   });
 
@@ -169,13 +168,23 @@ describe('BlockSynchronizer', () => {
     expect(obtainedHeader.equals(block.header)).toBe(true);
   });
 
-  it('wipes the contract sync and contract class caches when the anchor block changes', async () => {
+  it('wipes the contract sync and node read caches when the anchor block changes', async () => {
     const block = await L2Block.random(BlockNumber(1));
     await serveBlockDataByHash(block);
+    const referenceBlock = BlockHash.random();
+    const contractAddress = await AztecAddress.random();
+    const storageSlot = Fr.random();
+    aztecNode.getPublicStorageAt.mockResolvedValue(new Fr(1));
+    await cachedNode.getPublicStorageAt(referenceBlock, contractAddress, storageSlot);
+    await cachedNode.getPublicStorageAt(referenceBlock, contractAddress, storageSlot);
+    expect(aztecNode.getPublicStorageAt).toHaveBeenCalledTimes(1);
+
     await synchronizer.handleBlockStreamEvent(await proposedEvent(block));
 
     expect(contractSyncService.wipe).toHaveBeenCalled();
-    expect(contractClassService.wipe).toHaveBeenCalled();
+    // The anchor update wiped the node read cache: the same read reaches the node again.
+    await cachedNode.getPublicStorageAt(referenceBlock, contractAddress, storageSlot);
+    expect(aztecNode.getPublicStorageAt).toHaveBeenCalledTimes(2);
   });
 
   it('updates anchor block on a reorg', async () => {
@@ -801,15 +810,14 @@ describe('BlockSynchronizer', () => {
       );
 
       realSynchronizer = new BlockSynchronizer(
-        aztecNode,
+        withCache(aztecNode),
         store,
         anchorBlockStore,
         noteStore,
         privateEventStore,
         factStore,
         tipsStore,
         contractSyncService,
-        contractClassService,
         { syncChainTip: 'proposed' },
       );
     });

yarn-project/pxe/src/block_synchronizer/block_synchronizer.ts

@@ -4,12 +4,11 @@ import { SerialQueue } from '@aztec/foundation/queue';
 import type { AztecAsyncKVStore } from '@aztec/kv-store';
 import type { L2TipsKVStore } from '@aztec/kv-store/stores';
 import { BlockHash, L2BlockStream, type L2BlockStreamEvent, type L2BlockStreamEventHandler } from '@aztec/stdlib/block';
-import type { AztecNode } from '@aztec/stdlib/interfaces/client';
 import type { BlockHeader } from '@aztec/stdlib/tx';
 
 import type { BlockSynchronizerConfig } from '../config/index.js';
-import type { ContractClassService } from '../contract/contract_class_service.js';
 import type { ContractSyncService } from '../contract/contract_sync_service.js';
+import type { CachingAztecNode } from '../node/caching_aztec_node.js';
 import type { AnchorBlockStore } from '../storage/anchor_block_store/index.js';
 import type { FactStore } from '../storage/fact_store/fact_store.js';
 import type { NoteStore } from '../storage/note_store/index.js';
@@ -28,15 +27,14 @@ export class BlockSynchronizer implements L2BlockStreamEventHandler {
   protected readonly blockStream: L2BlockStream;
 
   constructor(
-    private readonly node: AztecNode,
+    private readonly node: CachingAztecNode,
     private readonly store: AztecAsyncKVStore,
     private readonly anchorBlockStore: AnchorBlockStore,
     private readonly noteStore: NoteStore,
     private readonly privateEventStore: PrivateEventStore,
     private readonly factStore: FactStore,
     private readonly l2TipsStore: L2TipsKVStore,
     private readonly contractSyncService: ContractSyncService,
-    private readonly contractClassService: ContractClassService,
     private readonly config: Partial<BlockSynchronizerConfig> = {},
     bindings?: LoggerBindings,
   ) {
@@ -180,10 +178,9 @@ export class BlockSynchronizer implements L2BlockStreamEventHandler {
     // execution.
     this.contractSyncService.wipe();
 
-    // The contract class service keeps a per-block cache - since updating our anchor means it is very unlikely we'd
-    // ever re-simulate at past anchors, we wipe its cache to prevent runaway memory growth on very long-lived PXE
-    // instances.
-    this.contractClassService.wipe();
+    // Cached node reads stay correct indefinitely, so this wipe is only about bounding memory growth. An anchor
+    // update is the recurring trigger PXE already has.
+    this.node.wipeCache();
 
     this.log.verbose(`Updated pxe last block to ${blockHeader.getBlockNumber()}`, blockHeader.toInspect());
     await this.anchorBlockStore.setHeader(blockHeader);

yarn-project/pxe/src/contract/contract_class_service.test.ts

@@ -68,27 +68,6 @@ describe('ContractClassService', () => {
     expect(await service.getCurrentClassId(address, anchorWithHash(hashB))).toEqual(classAtB);
   });
 
-  it('caches per (address, anchor) so the node is queried once per anchor', async () => {
-    const hash = new BlockHash(new Fr(1n));
-    node.getContract.mockResolvedValue({ currentContractClassId: new Fr(7n) } as ContractInstanceWithAddress);
-
-    await service.getCurrentClassId(address, anchorWithHash(hash));
-    await service.getCurrentClassId(address, anchorWithHash(hash));
-
-    expect(node.getContract).toHaveBeenCalledTimes(1);
-  });
-
-  it('re-queries after wipe', async () => {
-    const hash = new BlockHash(new Fr(1n));
-    node.getContract.mockResolvedValue({ currentContractClassId: new Fr(7n) } as ContractInstanceWithAddress);
-
-    await service.getCurrentClassId(address, anchorWithHash(hash));
-    service.wipe();
-    await service.getCurrentClassId(address, anchorWithHash(hash));
-
-    expect(node.getContract).toHaveBeenCalledTimes(2);
-  });
-
   it('falls back to the original class when the node does not know the instance', async () => {
     node.getContract.mockResolvedValue(undefined);
 

yarn-project/pxe/src/contract/contract_class_service.ts

@@ -14,14 +14,6 @@ import type { ContractStore } from '../storage/contract_store/contract_store.js'
  * falling back to a local instance if no upgrades were scheduled.
  */
 export class ContractClassService {
-  /**
-   * Class ids are cached per `(address, anchorHash)`. This avoid unnecessary network roundtrips in scenarios where
-   * multiple executions are done on the same anchor block (e.g. simulation followed by witgen), or when the same
-   * contract is invoked multiple times in an execution (e.g. authwit checks).
-   * It also means the callers don't need to worry about caching this service's return values, simplifying callsites.
-   */
-  #cache: Map<string, Promise<Fr | undefined>> = new Map();
-
   constructor(
     private node: AztecNode,
     private contractStore: ContractStore,
@@ -40,44 +32,14 @@ export class ContractClassService {
       return instance?.originalContractClassId;
     }
 
-    const key = `${address.toString()}:${(await anchorBlockHeader.hash()).toString()}`;
-    let promise = this.#cache.get(key);
-    if (!promise) {
-      promise = (async () => {
-        // The node resolves the current class from the same scheduled value change the AVM enforces against the public
-        // data tree. If the contract was upgraded the node returns a non-undefined instance; an undefined result means
-        // no upgrade happened (or the node has no record of it, e.g. it was never publicly deployed), so the original
-        // class is current.
-
-        const nodeInstance = await this.node.getContract(address, await anchorBlockHeader.hash());
-
-        if (nodeInstance) {
-          return nodeInstance.currentContractClassId;
-        } else {
-          return (await this.contractStore.getContractInstance(address))?.originalContractClassId;
-        }
-      })().catch(err => {
-        this.#cache.delete(key);
-        throw err;
-      });
-      this.#cache.set(key, promise);
-    }
-
-    const classId = await promise;
-    if (classId === undefined) {
-      // Don't memoize a missing instance: it may be registered later in this PXE (e.g. via contract sync
-      // mid-execution), and a cached miss would then hide it. Only successful resolutions stay cached.
-      this.#cache.delete(key);
+    // The node resolves the current class from the same scheduled value change the AVM enforces against the public
+    // data tree. If the contract was upgraded the node returns a non-undefined instance; an undefined result means no
+    // upgrade happened (or the node has no record of it, e.g. it was never publicly deployed), so the original class
+    // is current.
+    const nodeInstance = await this.node.getContract(address, await anchorBlockHeader.hash());
+    if (nodeInstance) {
+      return nodeInstance.currentContractClassId;
     }
-    return classId;
-  }
-
-  /**
-   * Clears the cache.
-   *
-   * This is not required for correctness, only to limit how much memory the cache uses. The cache is resilient against
-   * reorgs etc. as it is based on block hashes, not block numbers. */
-  wipe(): void {
-    this.#cache.clear();
+    return (await this.contractStore.getContractInstance(address))?.originalContractClassId;
   }
 }