feat!: refactor contract interaction pt.1 (#8938)

Part 1 of a refactor to `ContractFunctionInteraction`

## PXE APIs better reflect the lifecycle of a TX:

`execute private -> simulate kernels -> simulate public (estimate gas)
-> prove -> send`

`prove` potentially goes back to `execute private` (so we can swap to a
`FakeAccountContract` for example), but that's a story for another time.

## Removal of weird `SimulatedTx` class, now we have: 

- `PrivateExecutionResult` (old `ExecutionResult`)
- `PrivateSimulationResult`: output of private simulation + kernel
simulation: includes the former and
`PrivateKernelTailCircuitPublicInputs`
- `TxSimulationResult`: output of private + kernel + public simulation:
includes the former + public outputs
- `TxProvingResult`: output of private + proving: includes everything in
`PrivateSimulationResult` + clientIvcProof

## Removal of mutability in `ContractFunctionInteraction`

no more `this.txRequest = undefined` nonsense

Now you can `.prove()` a `BaseContractInteraction`, which returns a
`ProvenTx`. This doesn't mutate the original object, and the returned
value can later be `.send()`.

```typescript
// Example from block_building e2e, used to generate a block with multiple txs in it:
const provenTxs = [];
for (let i = 0; i < TX_COUNT; i++) {
  provenTxs.push(
    await methods[i].prove({
      contractAddressSalt: new Fr(BigInt(i + 1)),
      skipClassRegistration: true,
      skipPublicDeployment: true,
    }),
  );
}

// Send them simultaneously to be picked up by the sequencer
const txs = await Promise.all(provenTxs.map(tx => tx.send()));
logger.info(`Txs sent with hashes: `);
for (const tx of txs) {
  logger.info(` ${await tx.getTxHash()}`);
}
```

Author: Thunkar

docs/docs/migration_notes.md

@@ -8,6 +8,29 @@ Aztec is in full-speed development. Literally every version breaks compatibility
 
 ## TBD
 
+### Changes to PXE API and `ContractFunctionInteraction``
+
+PXE APIs have been refactored to better reflext the lifecycle of a Tx (`execute private -> simulate kernels -> simulate public (estimate gas) -> prove -> send`)
+
+* `.simulateTx`: Now returns a `TxSimulationResult`, containing the output of private execution, kernel simulation and public simulation (optional).
+* `.proveTx`: Now accepts the result of executing the private part of a transaction, so simulation doesn't have to happen again.
+
+Thanks to this refactor, `ContractFunctionInteraction` has been updated to remove its internal cache and avoid bugs due to its mutable nature. As a result our type-safe interfaces now have to be used as follows:
+
+```diff
+-const action = MyContract.at(address).method(args);
+-await action.prove();
+-await action.send().wait();
++const action = MyContract.at(address).method(args);
++const provenTx = await action.prove();
++await provenTx.send().wait();
+```
+
+It's still possible to use `.send()` as before, which will perform proving under the hood.
+
+More changes are coming to these APIs to better support gas estimation mechanisms and advanced features.
+
+
 ### Changes to public calling convention
 
 Contracts that include public functions (that is, marked with `#[public]`), are required to have a function `public_dispatch(selector: Field)` which acts as an entry point. This will be soon the only public function registered/deployed in contracts. The calling convention is updated so that external calls are made to this function.

yarn-project/accounts/src/testing/configuration.ts

@@ -1,10 +1,12 @@
 import { generatePublicKey } from '@aztec/aztec.js';
-import { type AccountWalletWithSecretKey } from '@aztec/aztec.js/wallet';
+import { registerContractClass } from '@aztec/aztec.js/deployment';
+import { DefaultMultiCallEntrypoint } from '@aztec/aztec.js/entrypoint';
+import { type AccountWalletWithSecretKey, SignerlessWallet } from '@aztec/aztec.js/wallet';
 import { type PXE } from '@aztec/circuit-types';
 import { deriveMasterIncomingViewingSecretKey, deriveSigningKey } from '@aztec/circuits.js/keys';
 import { Fr } from '@aztec/foundation/fields';
 
-import { getSchnorrAccount } from '../schnorr/index.js';
+import { SchnorrAccountContractArtifact, getSchnorrAccount } from '../schnorr/index.js';
 
 export const INITIAL_TEST_SECRET_KEYS = [
   Fr.fromString('2153536ff6628eee01cf4024889ff977a18d9fa61d0e414422f7681cf085c281'),
@@ -69,28 +71,28 @@ export async function deployInitialTestAccounts(pxe: PXE) {
       secretKey,
     };
   });
+  // Register contract class to avoid duplicate nullifier errors
+  const { l1ChainId: chainId, protocolVersion } = await pxe.getNodeInfo();
+  const deployWallet = new SignerlessWallet(pxe, new DefaultMultiCallEntrypoint(chainId, protocolVersion));
+  await (await registerContractClass(deployWallet, SchnorrAccountContractArtifact)).send().wait();
   // Attempt to get as much parallelism as possible
-  const deployMethods = await Promise.all(
+  const deployTxs = await Promise.all(
     accounts.map(async x => {
       const deployMethod = await x.account.getDeployMethod();
-      await deployMethod.create({
+      const tx = await deployMethod.prove({
         contractAddressSalt: x.account.salt,
-        skipClassRegistration: true,
-        skipPublicDeployment: true,
         universalDeploy: true,
       });
-      await deployMethod.prove({});
-      return deployMethod;
+      return tx;
     }),
   );
   // Send tx together to try and get them in the same rollup
-  const sentTxs = deployMethods.map(dm => {
-    return dm.send();
+  const sentTxs = deployTxs.map(tx => {
+    return tx.send();
   });
   await Promise.all(
-    sentTxs.map(async (tx, i) => {
-      const wallet = await accounts[i].account.getWallet();
-      return tx.wait({ wallet });
+    sentTxs.map(tx => {
+      return tx.wait();
     }),
   );
   return accounts;

yarn-project/accounts/src/testing/create_account.ts

@@ -30,34 +30,29 @@ export async function createAccounts(
   secrets: Fr[] = [],
   waitOpts: WaitOpts = { interval: 0.1 },
 ): Promise<AccountWalletWithSecretKey[]> {
-  const accounts = [];
-
   if (secrets.length == 0) {
     secrets = Array.from({ length: numberOfAccounts }, () => Fr.random());
   } else if (secrets.length > 0 && secrets.length !== numberOfAccounts) {
     throw new Error('Secrets array must be empty or have the same length as the number of accounts');
   }
 
   // Prepare deployments
-  for (const secret of secrets) {
-    const signingKey = deriveSigningKey(secret);
-    const account = getSchnorrAccount(pxe, secret, signingKey);
-    // Unfortunately the function below is not stateless and we call it here because it takes a long time to run and
-    // the results get stored within the account object. By calling it here we increase the probability of all the
-    // accounts being deployed in the same block because it makes the deploy() method basically instant.
-    await account.getDeployMethod().then(d =>
-      d.prove({
+  const accountsAndDeployments = await Promise.all(
+    secrets.map(async secret => {
+      const signingKey = deriveSigningKey(secret);
+      const account = getSchnorrAccount(pxe, secret, signingKey);
+      const deployMethod = await account.getDeployMethod();
+      const provenTx = await deployMethod.prove({
         contractAddressSalt: account.salt,
         skipClassRegistration: true,
         skipPublicDeployment: true,
         universalDeploy: true,
-      }),
-    );
-    accounts.push(account);
-  }
+      });
+      return { account, provenTx };
+    }),
+  );
 
   // Send them and await them to be mined
-  const txs = await Promise.all(accounts.map(account => account.deploy()));
-  await Promise.all(txs.map(tx => tx.wait(waitOpts)));
-  return Promise.all(accounts.map(account => account.getWallet()));
+  await Promise.all(accountsAndDeployments.map(({ provenTx }) => provenTx.send().wait(waitOpts)));
+  return Promise.all(accountsAndDeployments.map(({ account }) => account.getWallet()));
 }

yarn-project/aztec.js/src/account_manager/deploy_account_sent_tx.ts

@@ -2,7 +2,7 @@ import { type PXE, type TxHash, type TxReceipt } from '@aztec/circuit-types';
 import { type FieldsOf } from '@aztec/foundation/types';
 
 import { type Wallet } from '../account/index.js';
-import { DefaultWaitOpts, SentTx, type WaitOpts } from '../contract/index.js';
+import { DefaultWaitOpts, SentTx, type WaitOpts } from '../contract/sent_tx.js';
 import { waitForAccountSynch } from '../utils/account.js';
 
 /** Extends a transaction receipt with a wallet instance for the newly deployed contract. */

yarn-project/aztec.js/src/account_manager/index.ts

@@ -34,7 +34,6 @@ export class AccountManager {
   private completeAddress?: CompleteAddress;
   private instance?: ContractInstanceWithAddress;
   private publicKeysHash?: Fr;
-  private deployMethod?: DeployAccountMethod;
 
   constructor(private pxe: PXE, private secretKey: Fr, private accountContract: AccountContract, salt?: Salt) {
     this.salt = salt !== undefined ? new Fr(salt) : Fr.random();
@@ -131,33 +130,30 @@ export class AccountManager {
    * @returns A DeployMethod instance that deploys this account contract.
    */
   public async getDeployMethod() {
-    if (!this.deployMethod) {
-      if (!this.isDeployable()) {
-        throw new Error(
-          `Account contract ${this.accountContract.getContractArtifact().name} does not require deployment.`,
-        );
-      }
-
-      await this.pxe.registerAccount(this.secretKey, this.getCompleteAddress().partialAddress);
-
-      const { l1ChainId: chainId, protocolVersion } = await this.pxe.getNodeInfo();
-      const deployWallet = new SignerlessWallet(this.pxe, new DefaultMultiCallEntrypoint(chainId, protocolVersion));
-
-      // We use a signerless wallet with the multi call entrypoint in order to make multiple calls in one go
-      // If we used getWallet, the deployment would get routed via the account contract entrypoint
-      // and it can't be used unless the contract is initialized
-      const args = this.accountContract.getDeploymentArgs() ?? [];
-      this.deployMethod = new DeployAccountMethod(
-        this.accountContract.getAuthWitnessProvider(this.getCompleteAddress()),
-        this.getPublicKeysHash(),
-        deployWallet,
-        this.accountContract.getContractArtifact(),
-        args,
-        'constructor',
-        'entrypoint',
+    if (!this.isDeployable()) {
+      throw new Error(
+        `Account contract ${this.accountContract.getContractArtifact().name} does not require deployment.`,
       );
     }
-    return this.deployMethod;
+
+    await this.pxe.registerAccount(this.secretKey, this.getCompleteAddress().partialAddress);
+
+    const { l1ChainId: chainId, protocolVersion } = await this.pxe.getNodeInfo();
+    const deployWallet = new SignerlessWallet(this.pxe, new DefaultMultiCallEntrypoint(chainId, protocolVersion));
+
+    // We use a signerless wallet with the multi call entrypoint in order to make multiple calls in one go
+    // If we used getWallet, the deployment would get routed via the account contract entrypoint
+    // and it can't be used unless the contract is initialized
+    const args = this.accountContract.getDeploymentArgs() ?? [];
+    return new DeployAccountMethod(
+      this.accountContract.getAuthWitnessProvider(this.getCompleteAddress()),
+      this.getPublicKeysHash(),
+      deployWallet,
+      this.accountContract.getContractArtifact(),
+      args,
+      'constructor',
+      'entrypoint',
+    );
   }
 
   /**

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

@@ -1,10 +1,11 @@
-import { type Tx, type TxExecutionRequest } from '@aztec/circuit-types';
+import { type TxExecutionRequest, type TxProvingResult } from '@aztec/circuit-types';
 import { type Fr, GasSettings } from '@aztec/circuits.js';
 import { createDebugLogger } from '@aztec/foundation/log';
 
 import { type Wallet } from '../account/wallet.js';
 import { type ExecutionRequestInit, type FeeOptions } from '../entrypoint/entrypoint.js';
 import { getGasLimits } from './get_gas_limits.js';
+import { ProvenTx } from './proven_tx.js';
 import { SentTx } from './sent_tx.js';
 
 /**
@@ -29,13 +30,6 @@ export type SendMethodOptions = {
  * Implements the sequence create/simulate/send.
  */
 export abstract class BaseContractInteraction {
-  /**
-   * The transaction execution result. Set by prove().
-   * Made public for simple mocking.
-   */
-  public tx?: Tx;
-  protected txRequest?: TxExecutionRequest;
-
   protected log = createDebugLogger('aztec:js:contract_interaction');
 
   constructor(protected wallet: Wallet) {}
@@ -47,15 +41,27 @@ export abstract class BaseContractInteraction {
    */
   public abstract create(options?: SendMethodOptions): Promise<TxExecutionRequest>;
 
+  /**
+   * Creates a transaction execution request, simulates and proves it. Differs from .prove in
+   * that its result does not include the wallet nor the composed tx object, but only the proving result.
+   * This object can then be used to either create a ProvenTx ready to be sent, or directly send the transaction.
+   * @param options - optional arguments to be used in the creation of the transaction
+   * @returns The proving result.
+   */
+  protected async proveInternal(options: SendMethodOptions = {}): Promise<TxProvingResult> {
+    const txRequest = await this.create(options);
+    const txSimulationResult = await this.wallet.simulateTx(txRequest, !options.skipPublicSimulation, undefined, true);
+    return await this.wallet.proveTx(txRequest, txSimulationResult.privateExecutionResult);
+  }
+
   /**
    * Proves a transaction execution request and returns a tx object ready to be sent.
    * @param options - optional arguments to be used in the creation of the transaction
    * @returns The resulting transaction
    */
-  public async prove(options: SendMethodOptions = {}): Promise<Tx> {
-    const txRequest = this.txRequest ?? (await this.create(options));
-    this.tx = await this.wallet.proveTx(txRequest, !options.skipPublicSimulation);
-    return this.tx;
+  public async prove(options: SendMethodOptions = {}): Promise<ProvenTx> {
+    const txProvingResult = await this.proveInternal(options);
+    return new ProvenTx(this.wallet, txProvingResult.toTx());
   }
 
   /**
@@ -67,12 +73,11 @@ export abstract class BaseContractInteraction {
    * the AztecAddress of the sender. If not provided, the default address is used.
    * @returns A SentTx instance for tracking the transaction status and information.
    */
-  public send(options: SendMethodOptions = {}) {
+  public send(options: SendMethodOptions = {}): SentTx {
     const promise = (async () => {
-      const tx = this.tx ?? (await this.prove(options));
-      return this.wallet.sendTx(tx);
+      const txProvingResult = await this.proveInternal(options);
+      return this.wallet.sendTx(txProvingResult.toTx());
     })();
-
     return new SentTx(this.wallet, promise);
   }
 
@@ -84,15 +89,7 @@ export abstract class BaseContractInteraction {
   public async estimateGas(
     opts?: Omit<SendMethodOptions, 'estimateGas' | 'skipPublicSimulation'>,
   ): Promise<Pick<GasSettings, 'gasLimits' | 'teardownGasLimits'>> {
-    // REFACTOR: both `this.txRequest = undefined` below are horrible, we should not be caching stuff that doesn't need to be.
-    // This also hints at a weird interface for create/request/estimate/send etc.
-
-    // Ensure we don't accidentally use a version of tx request that has estimateGas set to true, leading to an infinite loop.
-    this.txRequest = undefined;
     const txRequest = await this.create({ ...opts, estimateGas: false });
-    // Ensure we don't accidentally cache a version of tx request that has estimateGas forcefully set to false.
-    this.txRequest = undefined;
-
     const simulationResult = await this.wallet.simulateTx(txRequest, true);
     const { totalGas: gasLimits, teardownGas: teardownGasLimits } = getGasLimits(
       simulationResult,

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

@@ -18,12 +18,9 @@ export class BatchCall extends BaseContractInteraction {
    * @returns A Promise that resolves to a transaction instance.
    */
   public async create(opts?: SendMethodOptions): Promise<TxExecutionRequest> {
-    if (!this.txRequest) {
-      const calls = this.calls;
-      const fee = opts?.estimateGas ? await this.getFeeOptionsFromEstimatedGas({ calls, fee: opts?.fee }) : opts?.fee;
-      this.txRequest = await this.wallet.createTxExecutionRequest({ calls, fee });
-    }
-    return this.txRequest;
+    const calls = this.calls;
+    const fee = opts?.estimateGas ? await this.getFeeOptionsFromEstimatedGas({ calls, fee: opts?.fee }) : opts?.fee;
+    return await this.wallet.createTxExecutionRequest({ calls, fee });
   }
 
   /**
@@ -92,8 +89,8 @@ export class BatchCall extends BaseContractInteraction {
       // For public functions we retrieve the first values directly from the public output.
       const rawReturnValues =
         call.type == FunctionType.PRIVATE
-          ? simulatedTx.privateReturnValues?.nested?.[resultIndex].values
-          : simulatedTx.publicOutput?.publicReturnValues?.[resultIndex].values;
+          ? simulatedTx.getPrivateReturnValues()?.nested?.[resultIndex].values
+          : simulatedTx.getPublicReturnValues()?.[resultIndex].values;
 
       results[callIndex] = rawReturnValues ? decodeFromAbi(call.returnTypes, rawReturnValues) : [];
     });

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

@@ -1,13 +1,20 @@
-import { type Tx, type TxExecutionRequest, type TxHash, type TxReceipt } from '@aztec/circuit-types';
+import {
+  type Tx,
+  type TxExecutionRequest,
+  type TxHash,
+  type TxProvingResult,
+  type TxReceipt,
+  type TxSimulationResult,
+} from '@aztec/circuit-types';
 import { AztecAddress, CompleteAddress, EthAddress } from '@aztec/circuits.js';
 import { type L1ContractAddresses } from '@aztec/ethereum';
 import { type AbiDecoded, type ContractArtifact, FunctionType } from '@aztec/foundation/abi';
+import { type ContractInstanceWithAddress } from '@aztec/types/contracts';
 import { type NodeInfo } from '@aztec/types/interfaces';
 
 import { type MockProxy, mock } from 'jest-mock-extended';
 
-import { type ContractInstanceWithAddress } from '../index.js';
-import { type Wallet } from '../wallet/index.js';
+import { type Wallet } from '../account/wallet.js';
 import { Contract } from './contract.js';
 
 describe('Contract Class', () => {
@@ -17,9 +24,11 @@ describe('Contract Class', () => {
   let contractInstance: ContractInstanceWithAddress;
 
   const mockTx = { type: 'Tx' } as any as Tx;
+  const mockTxProvingResult = { type: 'TxProvingResult', toTx: () => mockTx } as any as TxProvingResult;
   const mockTxRequest = { type: 'TxRequest' } as any as TxExecutionRequest;
   const mockTxHash = { type: 'TxHash' } as any as TxHash;
   const mockTxReceipt = { type: 'TxReceipt' } as any as TxReceipt;
+  const mockTxSimulationResult = { type: 'TxSimulationResult' } as any as TxSimulationResult;
   const mockUnconstrainedResultValue = 1;
   const l1Addresses: L1ContractAddresses = {
     rollupAddress: EthAddress.random(),
@@ -124,13 +133,14 @@ describe('Contract Class', () => {
     contractInstance = { address: contractAddress } as ContractInstanceWithAddress;
 
     wallet = mock<Wallet>();
+    wallet.simulateTx.mockResolvedValue(mockTxSimulationResult);
     wallet.createTxExecutionRequest.mockResolvedValue(mockTxRequest);
     wallet.getContractInstance.mockResolvedValue(contractInstance);
     wallet.sendTx.mockResolvedValue(mockTxHash);
     wallet.simulateUnconstrained.mockResolvedValue(mockUnconstrainedResultValue as any as AbiDecoded);
     wallet.getTxReceipt.mockResolvedValue(mockTxReceipt);
     wallet.getNodeInfo.mockResolvedValue(mockNodeInfo);
-    wallet.proveTx.mockResolvedValue(mockTx);
+    wallet.proveTx.mockResolvedValue(mockTxProvingResult);
     wallet.getRegisteredAccounts.mockResolvedValue([account]);
   });