fix: bad contract txs publishing contract data

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

@@ -1,7 +1,8 @@
 import { AztecAddress, Contract, ContractDeployer, EthAddress, Fr, Wallet, isContractDeployed } from '@aztec/aztec.js';
 import { CompleteAddress, getContractDeploymentInfo } from '@aztec/circuits.js';
 import { DebugLogger } from '@aztec/foundation/log';
-import { TestContractArtifact } from '@aztec/noir-contracts/artifacts';
+import { TestContractArtifact, TokenContractArtifact } from '@aztec/noir-contracts/artifacts';
+import { SequencerClient } from '@aztec/sequencer-client';
 import { PXE, TxStatus } from '@aztec/types';
 
 import { setup } from './fixtures/utils.js';
@@ -11,10 +12,11 @@ describe('e2e_deploy_contract', () => {
   let accounts: CompleteAddress[];
   let logger: DebugLogger;
   let wallet: Wallet;
+  let sequencer: SequencerClient | undefined;
   let teardown: () => Promise<void>;
 
   beforeEach(async () => {
-    ({ teardown, pxe, accounts, logger, wallet } = await setup());
+    ({ teardown, pxe, accounts, logger, wallet, sequencer } = await setup());
   }, 100_000);
 
   afterEach(() => teardown());
@@ -128,4 +130,47 @@ describe('e2e_deploy_contract', () => {
       portalContract.toString(),
     );
   });
+
+  it('it should not deploy a contract which failed the public part of the execution', async () => {
+    sequencer?.updateSequencerConfig({
+      minTxsPerBlock: 2,
+    });
+
+    try {
+      // This test requires at least another good transaction to go through in the same block as the bad one.
+      // I deployed the same contract again but it could really be any valid transaction here.
+      const goodDeploy = new ContractDeployer(TokenContractArtifact, wallet).deploy(AztecAddress.random());
+      const badDeploy = new ContractDeployer(TokenContractArtifact, wallet).deploy(AztecAddress.ZERO);
+
+      await Promise.all([
+        goodDeploy.simulate({ skipPublicSimulation: true }),
+        badDeploy.simulate({ skipPublicSimulation: true }),
+      ]);
+
+      const [goodTx, badTx] = [
+        goodDeploy.send({ skipPublicSimulation: true }),
+        badDeploy.send({ skipPublicSimulation: true }),
+      ];
+
+      const [goodTxPromiseResult, badTxReceiptResult] = await Promise.allSettled([goodTx.wait(), badTx.wait()]);
+
+      expect(goodTxPromiseResult.status).toBe('fulfilled');
+      expect(badTxReceiptResult.status).toBe('rejected');
+
+      const [goodTxReceipt, badTxReceipt] = await Promise.all([goodTx.getReceipt(), badTx.getReceipt()]);
+
+      expect(goodTxReceipt.blockNumber).toEqual(expect.any(Number));
+      expect(badTxReceipt.blockNumber).toBeUndefined();
+
+      await expect(pxe.getExtendedContractData(goodDeploy.completeAddress!.address)).resolves.toBeDefined();
+      await expect(pxe.getExtendedContractData(goodDeploy.completeAddress!.address)).resolves.toBeDefined();
+
+      await expect(pxe.getContractData(badDeploy.completeAddress!.address)).resolves.toBeUndefined();
+      await expect(pxe.getExtendedContractData(badDeploy.completeAddress!.address)).resolves.toBeUndefined();
+    } finally {
+      sequencer?.updateSequencerConfig({
+        minTxsPerBlock: 1,
+      });
+    }
+  });
 });

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

@@ -40,6 +40,7 @@ import {
 } from '@aztec/l1-artifacts';
 import { TokenBridgeContract, TokenContract } from '@aztec/noir-contracts/types';
 import { PXEService, createPXEService, getPXEServiceConfig } from '@aztec/pxe';
+import { SequencerClient } from '@aztec/sequencer-client';
 import { AztecNode, L2BlockL2Logs, LogType, PXE, TxStatus, createAztecNodeRpcClient } from '@aztec/types';
 
 import * as path from 'path';
@@ -206,6 +207,7 @@ async function setupWithSandbox(account: Account, config: AztecNodeConfig, logge
   const teardown = () => Promise.resolve();
   return {
     aztecNode,
+    sequencer: undefined,
     pxe: pxeClient,
     deployL1ContractsValues,
     accounts: await pxeClient!.getRegisteredAccounts(),
@@ -225,6 +227,8 @@ type SetupOptions = { /** State load */ stateLoad?: string } & Partial<AztecNode
 export type EndToEndContext = {
   /** The Aztec Node service or client a connected to it. */
   aztecNode: AztecNode | undefined;
+  /** A client to the sequencer service */
+  sequencer: SequencerClient | undefined;
   /** The Private eXecution Environment (PXE). */
   pxe: PXE;
   /** Return values from deployL1Contracts function. */
@@ -286,6 +290,7 @@ export async function setup(numberOfAccounts = 1, opts: SetupOptions = {}): Prom
 
   logger('Creating and synching an aztec node...');
   const aztecNode = await AztecNodeService.createAndSync(config);
+  const sequencer = aztecNode.getSequencer();
 
   const { pxe, accounts, wallets } = await setupPXEService(numberOfAccounts, aztecNode!, logger);
 
@@ -306,6 +311,7 @@ export async function setup(numberOfAccounts = 1, opts: SetupOptions = {}): Prom
     wallets,
     logger,
     cheatCodes,
+    sequencer,
     teardown,
   };
 }

yarn-project/noir-contracts/Nargo.toml

@@ -1,8 +1,8 @@
 [workspace]
 members = [
-    "src/contracts/benchmarking_contract", 
-    "src/contracts/card_game_contract", 
-    "src/contracts/child_contract", 
+    "src/contracts/benchmarking_contract",
+    "src/contracts/card_game_contract",
+    "src/contracts/child_contract",
     "src/contracts/docs_example_contract",
     "src/contracts/easy_private_token_contract",
     "src/contracts/ecdsa_account_contract",

yarn-project/noir-contracts/src/contracts/token_contract/src/main.nr

@@ -36,8 +36,8 @@ contract Token {
     // docs:start:import_authwit
     use dep::authwit::{
         auth::{
-            assert_current_call_valid_authwit, 
-            assert_current_call_valid_authwit_public, 
+            assert_current_call_valid_authwit,
+            assert_current_call_valid_authwit_public,
         },
     };
     // docs:end:import_authwit
@@ -363,6 +363,7 @@ contract Token {
     internal fn _initialize(
         new_admin: AztecAddress,
     ) {
+        assert(new_admin.address != 0, "invalid admin");
         storage.admin.write(new_admin);
         storage.minters.at(new_admin.address).write(true);
     }

yarn-project/sequencer-client/src/sequencer/processed_tx.ts

@@ -6,13 +6,13 @@ import {
   PublicKernelPublicInputs,
   makeEmptyProof,
 } from '@aztec/circuits.js';
-import { Tx, TxHash, TxL2Logs } from '@aztec/types';
+import { ExtendedContractData, Tx, TxHash, TxL2Logs } from '@aztec/types';
 
 /**
  * Represents a tx that has been processed by the sequencer public processor,
  * so its kernel circuit public inputs are filled in.
  */
-export type ProcessedTx = Pick<Tx, 'proof' | 'encryptedLogs' | 'unencryptedLogs'> & {
+export type ProcessedTx = Pick<Tx, 'proof' | 'encryptedLogs' | 'unencryptedLogs' | 'newContracts'> & {
   /**
    * Output of the public kernel circuit for this tx.
    */
@@ -78,6 +78,7 @@ export async function makeProcessedTx(
     proof: proof ?? tx.proof,
     encryptedLogs: tx.encryptedLogs,
     unencryptedLogs: tx.unencryptedLogs,
+    newContracts: tx.newContracts,
     isEmpty: false,
   };
 }
@@ -104,6 +105,7 @@ export function makeEmptyProcessedTx(
     unencryptedLogs: new TxL2Logs([]),
     data: emptyKernelOutput,
     proof: emptyProof,
+    newContracts: [ExtendedContractData.empty()],
     isEmpty: true,
   });
 }

yarn-project/sequencer-client/src/sequencer/public_processor.test.ts

@@ -97,6 +97,7 @@ describe('public_processor', () => {
           proof: tx.proof,
           encryptedLogs: tx.encryptedLogs,
           unencryptedLogs: tx.unencryptedLogs,
+          newContracts: tx.newContracts,
         },
       ]);
       expect(failed).toEqual([]);

yarn-project/sequencer-client/src/sequencer/sequencer.ts

@@ -185,7 +185,7 @@ export class Sequencer {
         ...block.getStats(),
       } satisfies L2BlockBuiltStats);
 
-      await this.publishExtendedContractData(validTxs, block);
+      await this.publishExtendedContractData(processedValidTxs, block);
 
       await this.publishL2Block(block);
       this.log.info(`Submitted rollup block ${block.number} with ${processedValidTxs.length} transactions`);
@@ -200,16 +200,13 @@ export class Sequencer {
    * @param validTxs - The set of real transactions being published as part of the block.
    * @param block - The L2Block to be published.
    */
-  protected async publishExtendedContractData(validTxs: Tx[], block: L2Block) {
+  protected async publishExtendedContractData(validTxs: ProcessedTx[], block: L2Block) {
     // Publishes contract data for txs to the network and awaits the tx to be mined
     this.state = SequencerState.PUBLISHING_CONTRACT_DATA;
     const newContractData = validTxs
       .map(tx => {
         // Currently can only have 1 new contract per tx
-        const newContract = tx.data?.end.newContracts[0];
-        if (newContract) {
-          return tx.newContracts[0];
-        }
+        return tx.newContracts[0];
       })
       .filter((cd): cd is Exclude<typeof cd, undefined> => cd !== undefined);