Skip to content

Commit

Permalink
chore: add tests for gov proposer (#11633)
Browse files Browse the repository at this point in the history
Key changes:

- Add an e2e and spartan test for proposing a new payload to the
governance proposer, and executing it (i.e. passing it up to
governance); follow-on work is needed to test execution by governance.
- Use `forge` to generate an artifact for the rollup contract's storage,
which we copy into the l1-artifacts project; use this artifact to when
overriding the slot for blob checks.
- The typescript helper for the forwarder contract now explicitly takes
a ref to the rollup in use to aid in error extraction.
- Improvement made in extracting nested error codes from forward
functions

Fix #11681
  • Loading branch information
just-mitch authored Feb 3, 2025
1 parent 43fdbb1 commit 5c6a48a
Show file tree
Hide file tree
Showing 33 changed files with 953 additions and 267 deletions.
3 changes: 3 additions & 0 deletions l1-contracts/bootstrap.sh
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ function build {
# Step 1: Build everything in src.
forge build $(find src test -name '*.sol')

# Step 1.5: Output storage information for the rollup contract.
forge inspect src/core/Rollup.sol:Rollup storage > ./out/Rollup.sol/storage.json

# Step 2: Build the the generated verifier contract with optimization.
forge build $(find generated -name '*.sol') \
--optimize \
Expand Down
2 changes: 1 addition & 1 deletion l1-contracts/src/core/Rollup.sol
Original file line number Diff line number Diff line change
Expand Up @@ -434,7 +434,7 @@ contract Rollup is EIP712("Aztec Rollup", "1"), Ownable, ValidatorSelection, IRo
}

/**
* @notice Validate blob transactions against given inputs.
* @notice Validate blob transactions against given inputs
* @dev Only exists here for gas estimation.
*/
function validateBlobs(bytes calldata _blobsInput)
Expand Down
2 changes: 1 addition & 1 deletion spartan/aztec-network/templates/eth/eth-execution.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ spec:
- name: entrypoint-scripts
mountPath: /entrypoints
resources:
{{- toYaml .Values.ethereum.resources | nindent 12 }}
{{- toYaml .Values.ethereum.execution.resources | nindent 12 }}
volumes:
- name: shared-volume
persistentVolumeClaim:
Expand Down
6 changes: 1 addition & 5 deletions spartan/aztec-network/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -270,11 +270,7 @@ ethereum:
timeoutSeconds: 5
successThreshold: 1
failureThreshold: 3
resources:
requests:
memory: "4Gi"
cpu: "1.5"
storageSize: "80Gi"
deployL1ContractsPrivateKey:

proverAgent:
service:
Expand Down
14 changes: 14 additions & 0 deletions spartan/aztec-network/values/3-validators-with-metrics.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,17 @@ validator:
bootNode:
validator:
disabled: true

ethereum:
execution:
resources:
requests:
memory: "1Gi"
beacon:
resources:
requests:
memory: "1Gi"
validator:
resources:
requests:
memory: "1Gi"
27 changes: 16 additions & 11 deletions spartan/scripts/test_kind.sh
Original file line number Diff line number Diff line change
Expand Up @@ -82,13 +82,14 @@ if [ "$fresh_install" != "no-deploy" ]; then
./deploy_kind.sh $namespace $values_file
fi

# Find 3 free ports between 9000 and 10000
free_ports=$(find_ports 3)
# Find 4 free ports between 9000 and 10000
free_ports=$(find_ports 4)

# Extract the free ports from the list
pxe_port=$(echo $free_ports | awk '{print $1}')
anvil_port=$(echo $free_ports | awk '{print $2}')
metrics_port=$(echo $free_ports | awk '{print $3}')
forwarded_pxe_port=$(echo $free_ports | awk '{print $1}')
forwarded_anvil_port=$(echo $free_ports | awk '{print $2}')
forwarded_metrics_port=$(echo $free_ports | awk '{print $3}')
forwarded_node_port=$(echo $free_ports | awk '{print $4}')

if [ "$install_metrics" = "true" ]; then
grafana_password=$(kubectl get secrets -n metrics metrics-grafana -o jsonpath='{.data.admin-password}' | base64 --decode)
Expand All @@ -114,11 +115,13 @@ if [ "$use_docker" = "true" ]; then
-e INSTANCE_NAME="spartan" \
-e SPARTAN_DIR="/usr/src/spartan" \
-e NAMESPACE="$namespace" \
-e HOST_PXE_PORT=$pxe_port \
-e HOST_PXE_PORT=$forwarded_pxe_port \
-e CONTAINER_PXE_PORT=8081 \
-e HOST_ETHEREUM_PORT=$anvil_port \
-e HOST_ETHEREUM_PORT=$forwarded_anvil_port \
-e CONTAINER_ETHEREUM_PORT=8545 \
-e HOST_METRICS_PORT=$metrics_port \
-e HOST_NODE_PORT=$forwarded_node_port \
-e CONTAINER_NODE_PORT=8080 \
-e HOST_METRICS_PORT=$forwarded_metrics_port \
-e CONTAINER_METRICS_PORT=80 \
-e GRAFANA_PASSWORD=$grafana_password \
-e DEBUG=${DEBUG:-""} \
Expand All @@ -136,11 +139,13 @@ else
export INSTANCE_NAME="spartan"
export SPARTAN_DIR="$(pwd)/.."
export NAMESPACE="$namespace"
export HOST_PXE_PORT="$pxe_port"
export HOST_PXE_PORT="$forwarded_pxe_port"
export CONTAINER_PXE_PORT="8081"
export HOST_ETHEREUM_PORT="$anvil_port"
export HOST_ETHEREUM_PORT="$forwarded_anvil_port"
export CONTAINER_ETHEREUM_PORT="8545"
export HOST_METRICS_PORT="$metrics_port"
export HOST_NODE_PORT="$forwarded_node_port"
export CONTAINER_NODE_PORT="8080"
export HOST_METRICS_PORT="$forwarded_metrics_port"
export CONTAINER_METRICS_PORT="80"
export GRAFANA_PASSWORD="$grafana_password"
export DEBUG="${DEBUG:-""}"
Expand Down
4 changes: 2 additions & 2 deletions yarn-project/aztec-node/src/aztec-node/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -863,7 +863,7 @@ export class AztecNodeService implements AztecNode, Traceable {
);
const fork = await this.worldStateSynchronizer.fork();

this.log.verbose(`Simulating public calls for tx ${tx.getTxHash()}`, {
this.log.verbose(`Simulating public calls for tx ${txHash}`, {
globalVariables: newGlobalVariables.toInspect(),
txHash,
blockNumber,
Expand All @@ -876,7 +876,7 @@ export class AztecNodeService implements AztecNode, Traceable {
const [processedTxs, failedTxs, returns] = await processor.process([tx]);
// REFACTOR: Consider returning the error rather than throwing
if (failedTxs.length) {
this.log.warn(`Simulated tx ${tx.getTxHash()} fails: ${failedTxs[0].error}`, { txHash });
this.log.warn(`Simulated tx ${txHash} fails: ${failedTxs[0].error}`, { txHash });
throw failedTxs[0].error;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ SCRIPT_NAME=$(basename "$0" .sh)
exec > >(tee -a "$(dirname $0)/logs/${SCRIPT_NAME}.log") 2> >(tee -a "$(dirname $0)/logs/${SCRIPT_NAME}.log" >&2)

export BOOTNODE_URL=${BOOTNODE_URL:-http://127.0.0.1:8080}
export NODE_URL=${NODE_URL:-${BOOTNODE_URL:-http://127.0.0.1:8080}}
export PXE_URL=${PXE_URL:-http://127.0.0.1:8079}
export ETHEREUM_HOST=${ETHEREUM_HOST:-http://127.0.0.1:8545}
export K8S=${K8S:-false}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,11 @@ import { BlockBlobPublicInputs } from '@aztec/circuits.js/blobs';
import { fr } from '@aztec/circuits.js/testing';
import { EpochCache } from '@aztec/epoch-cache';
import {
GovernanceProposerContract,
type L1ContractAddresses,
L1TxUtilsWithBlobs,
RollupContract,
SlashingProposerContract,
createEthereumChain,
createL1Clients,
} from '@aztec/ethereum';
Expand Down Expand Up @@ -189,7 +191,20 @@ describe('L1Publisher integration', () => {
);
const l1TxUtils = new L1TxUtilsWithBlobs(sequencerPublicClient, sequencerWalletClient, logger, config);
const rollupContract = new RollupContract(sequencerPublicClient, l1ContractAddresses.rollupAddress.toString());
const forwarderContract = await createForwarderContract(config, sequencerPK);
const forwarderContract = await createForwarderContract(
config,
sequencerPK,
l1ContractAddresses.rollupAddress.toString(),
);
const slashingProposerAddress = await rollupContract.getSlashingProposerAddress();
const slashingProposerContract = new SlashingProposerContract(
sequencerPublicClient,
slashingProposerAddress.toString(),
);
const governanceProposerContract = new GovernanceProposerContract(
sequencerPublicClient,
l1ContractAddresses.governanceProposerAddress.toString(),
);
const epochCache = await EpochCache.create(l1ContractAddresses.rollupAddress, config, {
dateProvider: new TestDateProvider(),
});
Expand All @@ -211,6 +226,8 @@ describe('L1Publisher integration', () => {
rollupContract,
forwarderContract,
epochCache,
governanceProposerContract,
slashingProposerContract,
},
);

Expand Down Expand Up @@ -566,26 +583,44 @@ describe('L1Publisher integration', () => {
await expect(publisher.enqueueProposeL2Block(block)).resolves.toEqual(true);

await expect(publisher.sendRequests()).resolves.toMatchObject({
errorMsg: expect.stringContaining('Rollup__InvalidBlobHash'),
errorMsg: expect.stringContaining('Rollup__InvalidInHash'),
});

// Test for both calls
// NOTE: First error is from the simulate fn, which isn't supported by anvil
expect(loggerErrorSpy).toHaveBeenCalledTimes(2);

expect(loggerErrorSpy).toHaveBeenNthCalledWith(1, 'Bundled [propose] transaction [failed]');
expect(loggerErrorSpy).toHaveBeenCalledTimes(3);

expect(loggerErrorSpy).toHaveBeenNthCalledWith(
1,
'Forwarder transaction failed',
undefined,
expect.objectContaining({
receipt: expect.objectContaining({
type: 'eip4844',
blockHash: expect.any(String),
blockNumber: expect.any(BigInt),
transactionHash: expect.any(String),
}),
}),
);
expect(loggerErrorSpy).toHaveBeenNthCalledWith(
2,
expect.stringContaining('Bundled [propose] transaction [failed]'),
);

expect(loggerErrorSpy).toHaveBeenNthCalledWith(
3,
expect.stringMatching(
/^Rollup process tx reverted\. The contract function "forward" reverted\. Error: Rollup__InvalidBlobHash/i,
/^Rollup process tx reverted\. The contract function "forward" reverted\. Error: Rollup__InvalidInHash/i,
),
undefined,
expect.objectContaining({
blockHash: expect.any(String),
blockHash: expect.any(Fr),
blockNumber: expect.any(Number),
slotNumber: expect.any(BigInt),
txHash: expect.any(String),
txCount: expect.any(Number),
blockTimestamp: expect.any(Number),
}),
);
});
Expand Down
40 changes: 3 additions & 37 deletions yarn-project/end-to-end/src/e2e_l1_with_wall_time.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { getSchnorrAccount } from '@aztec/accounts/schnorr';
import { Fr, GrumpkinScalar, type Logger, type PXE, TxStatus } from '@aztec/aztec.js';
import { type Logger, type PXE } from '@aztec/aztec.js';
import { EthAddress } from '@aztec/circuits.js';
import { getL1ContractsConfigEnvVars } from '@aztec/ethereum';
import { type PXEService } from '@aztec/pxe';
Expand All @@ -8,6 +7,7 @@ import { jest } from '@jest/globals';
import { privateKeyToAccount } from 'viem/accounts';

import { getPrivateKeyFromIndex, setup } from './fixtures/utils.js';
import { submitTxsTo } from './shared/submit-transactions.js';

jest.setTimeout(1000 * 60 * 10);

Expand All @@ -32,7 +32,7 @@ describe('e2e_l1_with_wall_time', () => {

it('should produce blocks with a bunch of transactions', async () => {
for (let i = 0; i < 4; i++) {
const txs = await submitTxsTo(pxe as PXEService, 8);
const txs = await submitTxsTo(pxe as PXEService, 8, logger);
await Promise.all(
txs.map(async (tx, j) => {
logger.info(`Waiting for tx ${i}-${j}: ${await tx.getTxHash()} to be mined`);
Expand All @@ -41,38 +41,4 @@ describe('e2e_l1_with_wall_time', () => {
);
}
});

// submits a set of transactions to the provided Private eXecution Environment (PXE)
const submitTxsTo = async (pxe: PXEService, numTxs: number) => {
const provenTxs = [];
for (let i = 0; i < numTxs; i++) {
const accountManager = await getSchnorrAccount(pxe, Fr.random(), GrumpkinScalar.random(), Fr.random());
const deployMethod = await accountManager.getDeployMethod();
const tx = await deployMethod.prove({
contractAddressSalt: new Fr(accountManager.salt),
skipClassRegistration: true,
skipPublicDeployment: true,
universalDeploy: true,
});
provenTxs.push(tx);
}
const sentTxs = await Promise.all(
provenTxs.map(async provenTx => {
const tx = provenTx.send();
const txHash = await tx.getTxHash();

logger.info(`Tx sent with hash ${txHash}`);
const receipt = await tx.getReceipt();
expect(receipt).toEqual(
expect.objectContaining({
status: TxStatus.PENDING,
error: '',
}),
);
logger.info(`Receipt received for ${txHash}`);
return tx;
}),
);
return sentTxs;
};
});
45 changes: 5 additions & 40 deletions yarn-project/end-to-end/src/e2e_p2p/shared.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
import { getSchnorrAccount } from '@aztec/accounts/schnorr';
import { type AztecNodeService } from '@aztec/aztec-node';
import { type Logger, type SentTx } from '@aztec/aztec.js';
import { CompleteAddress, TxStatus } from '@aztec/aztec.js';
import { Fr, GrumpkinScalar } from '@aztec/foundation/fields';
import { CompleteAddress, type Logger, type SentTx, TxStatus } from '@aztec/aztec.js';
import { Fr } from '@aztec/foundation/fields';
import { type SpamContract } from '@aztec/noir-contracts.js/Spam';
import { type PXEService, createPXEService, getPXEServiceConfig as getRpcConfig } from '@aztec/pxe';
import { createPXEService, getPXEServiceConfig as getRpcConfig } from '@aztec/pxe';

import { type NodeContext } from '../fixtures/setup_p2p_test.js';
import { submitTxsTo } from '../shared/submit-transactions.js';

// submits a set of transactions to the provided Private eXecution Environment (PXE)
export const submitComplexTxsTo = async (
Expand Down Expand Up @@ -52,45 +51,11 @@ export const createPXEServiceAndSubmitTransactions = async (
const completeAddress = await CompleteAddress.fromSecretKeyAndPartialAddress(secretKey, Fr.random());
await pxeService.registerAccount(secretKey, completeAddress.partialAddress);

const txs = await submitTxsTo(logger, pxeService, numTxs);
const txs = await submitTxsTo(pxeService, numTxs, logger);
return {
txs,
account: completeAddress.address,
pxeService,
node,
};
};

// submits a set of transactions to the provided Private eXecution Environment (PXE)
const submitTxsTo = async (logger: Logger, pxe: PXEService, numTxs: number) => {
const provenTxs = [];
for (let i = 0; i < numTxs; i++) {
const accountManager = await getSchnorrAccount(pxe, Fr.random(), GrumpkinScalar.random(), Fr.random());
const deployMethod = await accountManager.getDeployMethod();
const tx = await deployMethod.prove({
contractAddressSalt: new Fr(accountManager.salt),
skipClassRegistration: true,
skipPublicDeployment: true,
universalDeploy: true,
});
provenTxs.push(tx);
}
const sentTxs = await Promise.all(
provenTxs.map(async provenTx => {
const tx = provenTx.send();
const txHash = await tx.getTxHash();

logger.info(`Tx sent with hash ${txHash}`);
const receipt = await tx.getReceipt();
expect(receipt).toEqual(
expect.objectContaining({
status: TxStatus.PENDING,
error: '',
}),
);
logger.info(`Receipt received for ${txHash}`);
return tx;
}),
);
return sentTxs;
};
Original file line number Diff line number Diff line change
Expand Up @@ -191,9 +191,9 @@ describe('e2e_p2p_governance_proposer', () => {
const proposal = await governance.read.getProposal([0n]);

const timeToActive = proposal.creation + proposal.config.votingDelay;
t.logger.info(`Warpping to ${timeToActive + 1n}`);
t.logger.info(`Warping to ${timeToActive + 1n}`);
await t.ctx.cheatCodes.eth.warp(Number(timeToActive + 1n));
t.logger.info(`Warpped to ${timeToActive + 1n}`);
t.logger.info(`Warped to ${timeToActive + 1n}`);
await waitL1Block();

t.logger.info(`Voting`);
Expand All @@ -202,9 +202,9 @@ describe('e2e_p2p_governance_proposer', () => {
t.logger.info(`Voted`);

const timeToExecutable = timeToActive + proposal.config.votingDuration + proposal.config.executionDelay + 1n;
t.logger.info(`Warpping to ${timeToExecutable}`);
t.logger.info(`Warping to ${timeToExecutable}`);
await t.ctx.cheatCodes.eth.warp(Number(timeToExecutable));
t.logger.info(`Warpped to ${timeToExecutable}`);
t.logger.info(`Warped to ${timeToExecutable}`);
await waitL1Block();

t.logger.info(`Checking governance proposer`);
Expand Down
Loading

0 comments on commit 5c6a48a

Please sign in to comment.