refactor: un-exposing PXE from e2e tests by benesjan · Pull Request #17163 · AztecProtocol/aztec-packages

benesjan · 2025-09-19T13:32:18Z

e2e tests should interact only with a wallet (and maybe a node) because PXE is becoming only a lib used by a wallet. This PR is a step in that direction.

Followup work

Drop PXE JSON RPC server,
have Wallet instantiate PXE instead of having it passed in as an arg to a constructor,
revisit the PXE methods exposed on the TestWallet.

benesjan · 2025-09-19T13:32:34Z

refactor: un-exposing PXE from e2e tests #17163 👈 (View in Graphite)
next

This stack of pull requests is managed by Graphite. Learn more about stacking.

benesjan · 2025-09-22T07:38:43Z

yarn-project/aztec/src/cli/cmds/start_pxe.ts

+export type { CliPXEOptions, PXEServiceConfig };

-export async function startPXE(
+export async function startPXEServicesGetWallet(


We now always just used the pxe to instantiate a test wallet so I just return a wallet from this function. This will get nuked in a followup PR when PXE will stop being a service.

benesjan · 2025-09-22T07:41:54Z

yarn-project/bot/src/factory.ts

-    dependencies: { pxe?: PXE; nodeAdmin?: AztecNodeAdmin; node?: AztecNode },
-  ) {
-    if (config.flushSetupTransactions && !dependencies.nodeAdmin) {
-      throw new Error(
-        `Either a node admin client or node admin url must be provided if transaction flushing is requested`,
-      );
-    }
-    if (config.senderPrivateKey && config.senderPrivateKey.getValue() && !dependencies.node) {
-      throw new Error(
-        `Either a node client or node url must be provided for bridging L1 fee juice to deploy an account with private key`,
-      );
-    }
-    if (!dependencies.pxe && !config.pxeUrl) {
-      throw new Error(`Either a PXE client or a PXE URL must be provided`);
-    }
-
-    this.nodeAdmin = dependencies.nodeAdmin;
-
-    if (dependencies.pxe) {
-      this.log.info(`Using local PXE`);
-      this.pxe = dependencies.pxe;
-    } else {
-      this.log.info(`Using remote PXE at ${config.pxeUrl!}`);
-      this.pxe = createPXEClient(config.pxeUrl!, getVersions(), makeTracedFetch([1, 2, 3], false));
-    }
-
-    if (dependencies.node) {
-      this.node = dependencies.node;
-    } else {
-      this.node = createAztecNodeClient(config.nodeUrl!, getVersions(), makeTracedFetch([1, 2, 3], false));
-    }
-
-    this.wallet = new TestWallet(this.pxe, this.node);
-  }


This was all very messy and the functionality seems to have been completely unused as we always fed in the real instances.

benesjan · 2025-09-22T07:51:47Z

yarn-project/bot/src/runner.ts

-    this.tracer = dependencies.telemetry.getTracer('Bot');
-    if (!dependencies.node && !config.nodeUrl) {
-      throw new Error(`Missing node URL in config or dependencies`);
-    }
-    const versions = getVersions();
-    const fetch = makeTracedFetch([1, 2, 3], true);
-    this.node = dependencies.node ?? createAztecNodeClient(config.nodeUrl!, versions, fetch);
-    this.nodeAdmin =
-      dependencies.nodeAdmin ??
-      (config.nodeAdminUrl ? createAztecNodeAdminClient(config.nodeAdminUrl, versions, fetch) : undefined);


Same here. This seems to have been unused.

I would ask @alexghr to make sure this is not used with certain configs in production

By dropping the ugly unreadable dependencies arg and having real types there, the type checker figured out that nowhere in the code we pass in those values as undefined.

So this shouldn't be a problem.

But agree that would be good to get Alex's feedback as it's strange that it was unused.

This code path used to run before v2 but in this #16962 I changed it to create a node client as part of start_node so I guess it's no longer used. This is ok to remove!

benesjan · 2025-09-22T07:53:01Z

yarn-project/end-to-end/src/composed/e2e_pxe.test.ts

-  return pxe;
-};
-
-pxeTestSuite('e2e_pxe', setupEnv);


Since PXE is becoming library, this became pointless.

benesjan · 2025-09-22T07:54:03Z

yarn-project/end-to-end/src/e2e_account_contracts.test.ts

+      wallet = new TestWalletInternals(wallet.getPxe(), aztecNode);

-      const accountManager = await AccountManager.create(wallet, pxe, secret, accountContract, salt);
+      const accountManager = await AccountManager.create(wallet, wallet.getPxe(), secret, accountContract, salt);


The .getPxe() getter is a temporary workaround that will get resolved once Wallet instantiates the PXE.

benesjan · 2025-09-22T09:07:36Z

yarn-project/end-to-end/src/bench/utils.ts

- * @param startingBlock - First l2 block to process.
- * @returns The new PXE.
- */
-export async function createNewPXE(node: AztecNode, contract: BenchmarkingContract): Promise<PXEService> {


This was unused

benesjan · 2025-09-22T09:22:14Z

yarn-project/aztec/src/testing/aztec_cheat_codes.ts

+     * The test wallet or pxe to use for getting notes
     */
-    public pxe: PXE,
+    public testWalletOrPxe: { getNotes(filter: NotesFilter): Promise<UniqueNote[]> },


Using TestWallet here would make this unusable for externals as it's likely their wallet will not expose the getNotes method. Just having the method defined in the type makes this PR a non-breaking change. Everything else should not be facing externals.

If this is unused, I would drop the loadPrivate method. That is a (testing) wallet utility

I am not sure if this is unused as this can be used by externals. Or do you expect externals to use our testing wallet?

Or their own. But this is false advertisement, I would deprecate it even if they're using it (because it uses the wrong mental model)

I would say this being on a cheatcode class makes it ok to have this method. It might also be useful when debugging typescript tests. We should just instruct devs to not use it in production (implement a utility function for that if needed). WDYT?

benesjan · 2025-09-22T10:01:31Z

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

 */
-export async function getDeployedTestAccounts(pxe: PXE): Promise<InitialAccountData[]> {
+export async function getDeployedTestAccounts(testWalletOrPxe: { getPxe(): PXE } | PXE): Promise<InitialAccountData[]> {
+  const pxe = 'getPxe' in testWalletOrPxe ? testWalletOrPxe.getPxe() : testWalletOrPxe;


Needed to do this ugly hack because currently TestWallet doesn't correctly load the accounts that were registered in PXE before the Wallet was instantiated. This will be easy to tackle once Wallets instantiate PXE instead of having it passed in as an arg.

This utility should just die. InitialAccountData contains everything theTestWallt needs to create the accounts. If they're not deployed, well, the consumer should do it.

I think it makes sense to address this in step 2 of followup work as then only wallet will have access to pxe making it a necessity that this is dropped.

Thunkar · 2025-09-22T12:36:11Z

yarn-project/aztec/src/cli/cmds/start_pxe.ts

-export type { PXEServiceConfig, CliPXEOptions };
+export type { CliPXEOptions, PXEServiceConfig };

-export async function startPXE(


I would drop "pxe" from the name here and just go for createWallet

I plan on just nuking this utility in step 1 or 2 of followup work. All these utils will change once PXE is not a standalone service. Now it's actually starting a PXE service and createWallet would be quite misleading name for this reason:

Thunkar · 2025-09-22T12:40:58Z

yarn-project/end-to-end/src/e2e_crowdfunding_and_claim.test.ts

    );
    const crowdfundingInstance = await crowdfundingDeployment.getInstance();
-    await pxe.registerAccount(crowdfundingSecretKey, await computePartialAddress(crowdfundingInstance));
+    await wallet.registerAccount(crowdfundingSecretKey, await computePartialAddress(crowdfundingInstance));


I think it would be better to extend registerContract so it accepts keys

That's beyond the scope of this PR. The point of this PR is to just un-expose PXE from tests. The test wallet functions will be revisited in a followup work:

Thunkar · 2025-09-22T12:47:38Z

yarn-project/end-to-end/src/e2e_deploy_contract/deploy_test.ts


  async registerRandomAccount(): Promise<AztecAddress> {
-    const completeAddress = await this.pxe.registerAccount(Fr.random(), Fr.random());
+    const completeAddress = await this.wallet.registerAccount(Fr.random(), Fr.random());


I dislike that the testwallet is getting so many methods. I'd try to refactor this into wallet-only tests

Beyond the scope of this PR. You agreed last week on the process of:

Moving the functions onto TestWallet,

unexposing pxe from tests,

then revisiting the methods.

I think this is the only reasonable approach how to make this big pxe refactor reviewable.

Thunkar · 2025-09-22T12:48:36Z

yarn-project/end-to-end/src/fixtures/utils.ts

- * @returns Private eXecution Environment (PXE), logger and teardown function.
+ * @returns A test wallet, logger and teardown function.
 */
-export async function setupPXEService(


I'd go with createWallet, the fact that it starts a PXE is internal

It's starting a pxe service which is an exposed service so would say that name would be misleading. Anyway makes sense to address in step 1 of followup work section.

Thunkar · 2025-09-22T12:50:16Z

yarn-project/test-wallet/src/wallet/test_wallet.ts

      return this.pxe.simulateTx(txRequest, true /* simulatePublic */, true, true, { contracts: contractOverrides });
    }
  }
+


These are way too much. Idk if now, but they need to go

See step 3:

e2e tests should interact only with a wallet (and maybe a node) because PXE is becoming only a lib used by a wallet. This PR is a step in that direction. # Followup work 1. Drop PXE JSON RPC server, 2. have Wallet instantiate PXE instead of having it passed in as an arg to a constructor, 3. revisit the PXE methods exposed on the TestWallet.

PXE is becoming just a lib used by the wallet and for this reason I am dropping the corresponding JSON RPC Server. This leads to a large refactor of basically all the setups that dealt with PXE and wallets. This is a followup work of [this PR](#17163) in which I had this tasks planned: 1. Drop PXE JSON RPC server, 2. have Wallet instantiate PXE instead of having it passed in as an arg to a constructor, 3. revisit the PXE methods exposed on the TestWallet. In this PR I tackled points 1 and 2.

benesjan marked this pull request as ready for review September 19, 2025 14:18

benesjan marked this pull request as draft September 22, 2025 07:35

benesjan commented Sep 22, 2025

View reviewed changes

benesjan force-pushed the 09-19-refactor_un-exposing_pxe_from_e2e_tests branch from 8a22b91 to 66e9d87 Compare September 22, 2025 08:32

benesjan commented Sep 22, 2025

View reviewed changes

benesjan marked this pull request as ready for review September 22, 2025 09:22

benesjan commented Sep 22, 2025

View reviewed changes

benesjan requested a review from Thunkar September 22, 2025 11:16

Thunkar reviewed Sep 22, 2025

View reviewed changes

benesjan requested a review from Thunkar September 22, 2025 13:02

Thunkar approved these changes Sep 22, 2025

View reviewed changes

benesjan added the ci-squash-and-merge label Sep 22, 2025

benesjan enabled auto-merge September 22, 2025 13:50

AztecBot force-pushed the 09-19-refactor_un-exposing_pxe_from_e2e_tests branch from a6f92f4 to 96e86f7 Compare September 22, 2025 13:52

AztecBot removed the ci-squash-and-merge label Sep 22, 2025

AztecBot force-pushed the 09-19-refactor_un-exposing_pxe_from_e2e_tests branch from 96e86f7 to 6456144 Compare September 22, 2025 13:56

benesjan added this pull request to the merge queue Sep 22, 2025

Merged via the queue into next with commit 280c5f5 Sep 22, 2025
15 checks passed

benesjan deleted the 09-19-refactor_un-exposing_pxe_from_e2e_tests branch September 22, 2025 14:44

benesjan mentioned this pull request Sep 24, 2025

refactor!: dropping PXE JSON RPC #17228

Merged

ludamad pushed a commit that referenced this pull request Dec 16, 2025

refactor: un-exposing PXE from e2e tests (#17163)

79c66f3

Conversation

benesjan commented Sep 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Followup work

Uh oh!

benesjan commented Sep 19, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

benesjan Sep 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

benesjan commented Sep 19, 2025 •

edited

Loading

benesjan Sep 22, 2025 •

edited

Loading