fix: potential e2e-p2p fix (#10094)

yarn-project/end-to-end/scripts/e2e_test_config.yml

@@ -85,8 +85,8 @@ tests:
   e2e_p2p_upgrade_governance_proposer:
     test_path: 'e2e_p2p/upgrade_governance_proposer.test.ts'
   # https://github.com/AztecProtocol/aztec-packages/issues/9843
-  # e2e_p2p_rediscovery:
-  #   test_path: 'e2e_p2p/rediscovery.test.ts'
+  e2e_p2p_rediscovery:
+    test_path: 'e2e_p2p/rediscovery.test.ts'
   e2e_p2p_reqresp:
     test_path: 'e2e_p2p/reqresp.test.ts'
   flakey_e2e_tests:

yarn-project/end-to-end/src/e2e_p2p/gossip_network.test.ts

@@ -3,7 +3,7 @@ import { sleep } from '@aztec/aztec.js';
 
 import fs from 'fs';
 
-// import { METRICS_PORT } from '../fixtures/fixtures.js';
+import { shouldCollectMetrics } from '../fixtures/fixtures.js';
 import { type NodeContext, createNodes } from '../fixtures/setup_p2p_test.js';
 import { P2PNetworkTest, WAIT_FOR_TX_TIMEOUT } from './p2p_network.js';
 import { createPXEServiceAndSubmitTransactions } from './shared.js';
@@ -24,8 +24,8 @@ describe('e2e_p2p_network', () => {
       testName: 'e2e_p2p_network',
       numberOfNodes: NUM_NODES,
       basePort: BOOT_NODE_UDP_PORT,
-      // Uncomment to collect metrics - run in aztec-packages `docker compose --profile metrics up`
-      // metricsPort: METRICS_PORT,
+      // To collect metrics - run in aztec-packages `docker compose --profile metrics up` and set COLLECT_METRICS=true
+      metricsPort: shouldCollectMetrics(),
     });
     await t.applyBaseSnapshots();
     await t.setup();
@@ -44,6 +44,7 @@ describe('e2e_p2p_network', () => {
     if (!t.bootstrapNodeEnr) {
       throw new Error('Bootstrap node ENR is not available');
     }
+
     // create our network of nodes and submit txs into each of them
     // the number of txs per node and the number of txs per rollup
     // should be set so that the only way for rollups to be built
@@ -57,8 +58,8 @@ describe('e2e_p2p_network', () => {
       NUM_NODES,
       BOOT_NODE_UDP_PORT,
       DATA_DIR,
-      // Uncomment to collect metrics - run in aztec-packages `docker compose --profile metrics up`
-      // METRICS_PORT,
+      // To collect metrics - run in aztec-packages `docker compose --profile metrics up` and set COLLECT_METRICS=true
+      shouldCollectMetrics(),
     );
 
     // wait a bit for peers to discover each other

yarn-project/end-to-end/src/e2e_p2p/p2p_network.ts

@@ -12,6 +12,7 @@ import { getContract } from 'viem';
 import { privateKeyToAccount } from 'viem/accounts';
 
 import {
+  PRIVATE_KEYS_START_INDEX,
   createValidatorConfig,
   generateNodePrivateKeys,
   generatePeerIdPrivateKeys,
@@ -33,6 +34,7 @@ export class P2PNetworkTest {
 
   public ctx!: SubsystemsContext;
   public nodePrivateKeys: `0x${string}`[] = [];
+  public nodePublicKeys: string[] = [];
   public peerIdPrivateKeys: string[] = [];
 
   public bootstrapNodeEnr: string = '';
@@ -51,7 +53,8 @@ export class P2PNetworkTest {
 
     // Set up the base account and node private keys for the initial network deployment
     this.baseAccount = privateKeyToAccount(`0x${getPrivateKeyFromIndex(0)!.toString('hex')}`);
-    this.nodePrivateKeys = generateNodePrivateKeys(1, numberOfNodes);
+    this.nodePrivateKeys = generateNodePrivateKeys(PRIVATE_KEYS_START_INDEX, numberOfNodes);
+    this.nodePublicKeys = this.nodePrivateKeys.map(privateKey => privateKeyToAccount(privateKey).address);
     this.peerIdPrivateKeys = generatePeerIdPrivateKeys(numberOfNodes);
 
     this.bootstrapNodeEnr = bootstrapNode.getENR().encodeTxt();
@@ -108,16 +111,11 @@ export class P2PNetworkTest {
       const txHashes: `0x${string}`[] = [];
       for (let i = 0; i < this.numberOfNodes; i++) {
         const account = privateKeyToAccount(this.nodePrivateKeys[i]!);
+        this.logger.debug(`Adding ${account.address} as validator`);
         const txHash = await rollup.write.addValidator([account.address]);
         txHashes.push(txHash);
-        this.logger.debug(`Adding ${account.address} as validator`);
       }
 
-      // Remove the setup validator
-      const initialValidatorAddress = privateKeyToAccount(`0x${getPrivateKeyFromIndex(0)!.toString('hex')}`).address;
-      const txHash = await rollup.write.removeValidator([initialValidatorAddress]);
-      txHashes.push(txHash);
-
       // Wait for all the transactions adding validators to be mined
       await Promise.all(
         txHashes.map(txHash =>
@@ -150,12 +148,52 @@ export class P2PNetworkTest {
     });
   }
 
+  async removeInitialNode() {
+    await this.snapshotManager.snapshot(
+      'remove-inital-validator',
+      async ({ deployL1ContractsValues, aztecNodeConfig }) => {
+        const rollup = getContract({
+          address: deployL1ContractsValues.l1ContractAddresses.rollupAddress.toString(),
+          abi: RollupAbi,
+          client: deployL1ContractsValues.walletClient,
+        });
+
+        // Remove the setup validator
+        const initialValidatorAddress = privateKeyToAccount(`0x${getPrivateKeyFromIndex(0)!.toString('hex')}`).address;
+        const txHash = await rollup.write.removeValidator([initialValidatorAddress]);
+
+        await deployL1ContractsValues.publicClient.waitForTransactionReceipt({
+          hash: txHash,
+        });
+
+        //@note   Now we jump ahead to the next epoch such that the validator committee is picked
+        //        INTERVAL MINING: If we are using anvil interval mining this will NOT progress the time!
+        //        Which means that the validator set will still be empty! So anyone can propose.
+        const slotsInEpoch = await rollup.read.EPOCH_DURATION();
+        const timestamp = await rollup.read.getTimestampForSlot([slotsInEpoch]);
+        const cheatCodes = new EthCheatCodes(aztecNodeConfig.l1RpcUrl);
+        try {
+          await cheatCodes.warp(Number(timestamp));
+        } catch (err) {
+          this.logger.debug('Warp failed, time already satisfied');
+        }
+
+        // Send and await a tx to make sure we mine a block for the warp to correctly progress.
+        await deployL1ContractsValues.publicClient.waitForTransactionReceipt({
+          hash: await deployL1ContractsValues.walletClient.sendTransaction({
+            to: this.baseAccount.address,
+            value: 1n,
+            account: this.baseAccount,
+          }),
+        });
+
+        await this.ctx.aztecNode.stop();
+      },
+    );
+  }
+
   async setup() {
     this.ctx = await this.snapshotManager.setup();
-
-    // TODO(md): make it such that the test can set these up
-    this.ctx.aztecNodeConfig.minTxsPerBlock = 4;
-    this.ctx.aztecNodeConfig.maxTxsPerBlock = 4;
   }
 
   async stopNodes(nodes: AztecNodeService[]) {

yarn-project/end-to-end/src/e2e_p2p/rediscovery.test.ts

@@ -26,6 +26,9 @@ describe('e2e_p2p_rediscovery', () => {
     });
     await t.applyBaseSnapshots();
     await t.setup();
+
+    // We remove the initial node such that it will no longer attempt to build blocks / be in the sequencing set
+    await t.removeInitialNode();
   });
 
   afterEach(async () => {
@@ -61,7 +64,7 @@ describe('e2e_p2p_rediscovery', () => {
       const node = nodes[i];
       await node.stop();
       t.logger.info(`Node ${i} stopped`);
-      await sleep(1200);
+      await sleep(2500);
 
       const newNode = await createNode(
         t.ctx.aztecNodeConfig,

yarn-project/end-to-end/src/e2e_p2p/reqresp.test.ts

@@ -1,15 +1,17 @@
 import { type AztecNodeService } from '@aztec/aztec-node';
 import { sleep } from '@aztec/aztec.js';
+import { RollupAbi } from '@aztec/l1-artifacts';
 
 import { jest } from '@jest/globals';
 import fs from 'fs';
+import { getContract } from 'viem';
 
 import { type NodeContext, createNodes } from '../fixtures/setup_p2p_test.js';
 import { P2PNetworkTest, WAIT_FOR_TX_TIMEOUT } from './p2p_network.js';
 import { createPXEServiceAndSubmitTransactions } from './shared.js';
 
 // Don't set this to a higher value than 9 because each node will use a different L1 publisher account and anvil seeds
-const NUM_NODES = 4;
+const NUM_NODES = 6;
 const NUM_TXS_PER_NODE = 2;
 const BOOT_NODE_UDP_PORT = 40800;
 
@@ -27,6 +29,7 @@ describe('e2e_p2p_reqresp_tx', () => {
     });
     await t.applyBaseSnapshots();
     await t.setup();
+    await t.removeInitialNode();
   });
 
   afterEach(async () => {
@@ -37,10 +40,6 @@ describe('e2e_p2p_reqresp_tx', () => {
     }
   });
 
-  // NOTE: If this test fails in a PR where the shuffling algorithm is changed, then it is failing as the node with
-  // the mocked p2p layer is being picked as the sequencer, and it does not have any transactions in it's mempool.
-  // If this is the case, then we should update the test to switch off the mempool of a different node.
-  // adjust `nodeToTurnOffTxGossip` in the test below.
   it('should produce an attestation by requesting tx data over the p2p network', async () => {
     /**
      * Birds eye overview of the test
@@ -73,12 +72,15 @@ describe('e2e_p2p_reqresp_tx', () => {
     // wait a bit for peers to discover each other
     await sleep(4000);
 
-    t.logger.info('Turning off tx gossip');
+    const { proposerIndexes, nodesToTurnOffTxGossip } = await getProposerIndexes();
+
+    t.logger.info(`Turning off tx gossip for nodes: ${nodesToTurnOffTxGossip}`);
+    t.logger.info(`Sending txs to proposer nodes: ${proposerIndexes}`);
+
     // Replace the p2p node implementation of some of the nodes with a spy such that it does not store transactions that are gossiped to it
     // Original implementation of `processTxFromPeer` will store received transactions in the tx pool.
-    // We have chosen nodes 0,3 as they do not get chosen to be the sequencer in this test.
-    const nodeToTurnOffTxGossip = [0, 3];
-    for (const nodeIndex of nodeToTurnOffTxGossip) {
+    // We chose the first 2 nodes that will be the proposers for the next few slots
+    for (const nodeIndex of nodesToTurnOffTxGossip) {
       jest
         .spyOn((nodes[nodeIndex] as any).p2pClient.p2pService, 'processTxFromPeer')
         .mockImplementation((): Promise<void> => {
@@ -87,10 +89,9 @@ describe('e2e_p2p_reqresp_tx', () => {
     }
 
     t.logger.info('Submitting transactions');
-    // Only submit transactions to the first two nodes, so that we avoid our sequencer with a mocked p2p layer being picked to produce a block.
-    // If the shuffling algorithm changes, then this will need to be updated.
-    for (let i = 1; i < 3; i++) {
-      const context = await createPXEServiceAndSubmitTransactions(t.logger, nodes[i], NUM_TXS_PER_NODE);
+
+    for (const nodeIndex of proposerIndexes.slice(0, 2)) {
+      const context = await createPXEServiceAndSubmitTransactions(t.logger, nodes[nodeIndex], NUM_TXS_PER_NODE);
       contexts.push(context);
     }
 
@@ -107,4 +108,34 @@ describe('e2e_p2p_reqresp_tx', () => {
     );
     t.logger.info('All transactions mined');
   });
+
+  /**
+   * Get the indexes in the nodes array that will produce the next few blocks
+   */
+  async function getProposerIndexes() {
+    // Get the nodes for the next set of slots
+    const rollupContract = getContract({
+      address: t.ctx.deployL1ContractsValues.l1ContractAddresses.rollupAddress.toString(),
+      abi: RollupAbi,
+      client: t.ctx.deployL1ContractsValues.publicClient,
+    });
+
+    const currentTime = await t.ctx.cheatCodes.eth.timestamp();
+    const slotDuration = await rollupContract.read.SLOT_DURATION();
+
+    const proposers = [];
+
+    for (let i = 0; i < 3; i++) {
+      const nextSlot = BigInt(currentTime) + BigInt(i) * BigInt(slotDuration);
+      const proposer = await rollupContract.read.getProposerAt([nextSlot]);
+      proposers.push(proposer);
+    }
+
+    // Get the indexes of the nodes that are responsible for the next two slots
+    const proposerIndexes = proposers.map(proposer => t.nodePublicKeys.indexOf(proposer));
+    const nodesToTurnOffTxGossip = Array.from({ length: NUM_NODES }, (_, i) => i).filter(
+      i => !proposerIndexes.includes(i),
+    );
+    return { proposerIndexes, nodesToTurnOffTxGossip };
+  }
 });

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

@@ -1,5 +1,12 @@
 export const METRICS_PORT = 4318;
 
+export const shouldCollectMetrics = () => {
+  if (process.env.COLLECT_METRICS) {
+    return METRICS_PORT;
+  }
+  return undefined;
+};
+
 export const MNEMONIC = 'test test test test test test test test test test test junk';
 export const privateKey = Buffer.from('ac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80', 'hex');
 export const privateKey2 = Buffer.from('59c6995e998f97a5a0044966f0945389dc9e86dae88c7a8412f4603b6b78690d', 'hex');

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

@@ -12,6 +12,10 @@ import { generatePrivateKey } from 'viem/accounts';
 import { getPrivateKeyFromIndex } from './utils.js';
 import { getEndToEndTestTelemetryClient } from './with_telemetry_utils.js';
 
+// Setup snapshots will create a node with index 0, so all of our loops here
+// need to start from 1 to avoid running validators with the same key
+export const PRIVATE_KEYS_START_INDEX = 1;
+
 export interface NodeContext {
   node: AztecNodeService;
   pxeService: PXEService;
@@ -52,11 +56,19 @@ export function createNodes(
 ): Promise<AztecNodeService[]> {
   const nodePromises = [];
   for (let i = 0; i < numNodes; i++) {
-    // We run on ports from the bootnode upwards if a port if provided, otherwise we get a random port
+    // We run on ports from the bootnode upwards
     const port = bootNodePort + i + 1;
 
     const dataDir = dataDirectory ? `${dataDirectory}-${i}` : undefined;
-    const nodePromise = createNode(config, peerIdPrivateKeys[i], port, bootstrapNodeEnr, i, dataDir, metricsPort);
+    const nodePromise = createNode(
+      config,
+      peerIdPrivateKeys[i],
+      port,
+      bootstrapNodeEnr,
+      i + PRIVATE_KEYS_START_INDEX,
+      dataDir,
+      metricsPort,
+    );
     nodePromises.push(nodePromise);
   }
   return Promise.all(nodePromises);
@@ -95,7 +107,7 @@ export async function createValidatorConfig(
   bootstrapNodeEnr?: string,
   port?: number,
   peerIdPrivateKey?: string,
-  accountIndex: number = 0,
+  accountIndex: number = 1,
   dataDirectory?: string,
 ) {
   peerIdPrivateKey = peerIdPrivateKey ?? generatePeerIdPrivateKey();
@@ -114,8 +126,6 @@ export async function createValidatorConfig(
     tcpListenAddress: `0.0.0.0:${port}`,
     tcpAnnounceAddress: `127.0.0.1:${port}`,
     udpAnnounceAddress: `127.0.0.1:${port}`,
-    minTxsPerBlock: config.minTxsPerBlock,
-    maxTxsPerBlock: config.maxTxsPerBlock,
     p2pEnabled: true,
     blockCheckIntervalMS: 1000,
     transactionProtocol: '',

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

@@ -325,8 +325,8 @@ async function setupFromFresh(
   const publisherPrivKeyRaw = hdAccount.getHdKey().privateKey;
   const publisherPrivKey = publisherPrivKeyRaw === null ? null : Buffer.from(publisherPrivKeyRaw);
 
-  const validatorPrivKey = getPrivateKeyFromIndex(1);
-  const proverNodePrivateKey = getPrivateKeyFromIndex(2);
+  const validatorPrivKey = getPrivateKeyFromIndex(0);
+  const proverNodePrivateKey = getPrivateKeyFromIndex(0);
 
   aztecNodeConfig.publisherPrivateKey = `0x${publisherPrivKey!.toString('hex')}`;
   aztecNodeConfig.validatorPrivateKey = `0x${validatorPrivKey!.toString('hex')}`;