From 56afacc800ca3939ea1ac8fcf2c157a69730abac Mon Sep 17 00:00:00 2001 From: Alex Gherghisan Date: Wed, 4 Oct 2023 09:27:11 +0100 Subject: [PATCH] fix: bad contract txs publishing contract data --- .../src/e2e_deploy_contract.test.ts | 23 ++++++++++++++++++- yarn-project/noir-contracts/Nargo.toml | 1 + .../contracts/test_assert_contract/Nargo.toml | 8 +++++++ .../test_assert_contract/src/main.nr | 17 ++++++++++++++ .../src/sequencer/processed_tx.ts | 6 +++-- .../src/sequencer/public_processor.test.ts | 1 + .../src/sequencer/sequencer.ts | 9 +++----- 7 files changed, 56 insertions(+), 9 deletions(-) create mode 100644 yarn-project/noir-contracts/src/contracts/test_assert_contract/Nargo.toml create mode 100644 yarn-project/noir-contracts/src/contracts/test_assert_contract/src/main.nr diff --git a/yarn-project/end-to-end/src/e2e_deploy_contract.test.ts b/yarn-project/end-to-end/src/e2e_deploy_contract.test.ts index b1b4196fbe8d..a4926d74ee96 100644 --- a/yarn-project/end-to-end/src/e2e_deploy_contract.test.ts +++ b/yarn-project/end-to-end/src/e2e_deploy_contract.test.ts @@ -1,7 +1,7 @@ import { AztecAddress, Contract, ContractDeployer, Fr, Wallet, isContractDeployed } from '@aztec/aztec.js'; import { CompleteAddress, getContractDeploymentInfo } from '@aztec/circuits.js'; import { DebugLogger } from '@aztec/foundation/log'; -import { TestContractAbi } from '@aztec/noir-contracts/artifacts'; +import { TestAssertContractAbi, TestContractAbi } from '@aztec/noir-contracts/artifacts'; import { PXE, TxStatus } from '@aztec/types'; import { setup } from './fixtures/utils.js'; @@ -111,4 +111,25 @@ describe('e2e_deploy_contract', () => { ); } }, 30_000); + + it('it should not deploy a bad contract', async () => { + const goodDeploy = new ContractDeployer(TestAssertContractAbi, wallet).deploy(0); + const badDeploy = new ContractDeployer(TestAssertContractAbi, wallet).deploy(1); + + 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'); + + 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(); + }); }); diff --git a/yarn-project/noir-contracts/Nargo.toml b/yarn-project/noir-contracts/Nargo.toml index fc837a45a5ab..639a82b19e11 100644 --- a/yarn-project/noir-contracts/Nargo.toml +++ b/yarn-project/noir-contracts/Nargo.toml @@ -22,6 +22,7 @@ members = [ "src/contracts/schnorr_single_key_account_contract", "src/contracts/stateful_test_contract", "src/contracts/test_contract", + "src/contracts/test_assert_contract", "src/contracts/token_contract", "src/contracts/token_bridge_contract", "src/contracts/uniswap_contract", diff --git a/yarn-project/noir-contracts/src/contracts/test_assert_contract/Nargo.toml b/yarn-project/noir-contracts/src/contracts/test_assert_contract/Nargo.toml new file mode 100644 index 000000000000..3ddb06f75efb --- /dev/null +++ b/yarn-project/noir-contracts/src/contracts/test_assert_contract/Nargo.toml @@ -0,0 +1,8 @@ +[package] +name = "test_assert_contract" +authors = [""] +compiler_version = "0.1" +type = "contract" + +[dependencies] +aztec = { path = "../../../../aztec-nr/aztec" } diff --git a/yarn-project/noir-contracts/src/contracts/test_assert_contract/src/main.nr b/yarn-project/noir-contracts/src/contracts/test_assert_contract/src/main.nr new file mode 100644 index 000000000000..9e6a3d13ad1e --- /dev/null +++ b/yarn-project/noir-contracts/src/contracts/test_assert_contract/src/main.nr @@ -0,0 +1,17 @@ +// A contract used in end to end tests +contract TestAssert { + use dep::aztec::{ + oracle::compute_selector::compute_selector, + }; + + #[aztec(private)] + fn constructor(value: Field) { + let selector = compute_selector("assert_is_zero(Field)"); + context.call_public_function(context.this_address(), selector, [value]); + } + + #[aztec(public)] + fn assert_is_zero(value: Field) { + assert(0 == value); + } +} diff --git a/yarn-project/sequencer-client/src/sequencer/processed_tx.ts b/yarn-project/sequencer-client/src/sequencer/processed_tx.ts index c66eb8adb3e5..f9a5c31f5946 100644 --- a/yarn-project/sequencer-client/src/sequencer/processed_tx.ts +++ b/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 & { +export type ProcessedTx = Pick & { /** * 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.random()], isEmpty: true, }); } diff --git a/yarn-project/sequencer-client/src/sequencer/public_processor.test.ts b/yarn-project/sequencer-client/src/sequencer/public_processor.test.ts index 3e6686b3e617..e0761465d09f 100644 --- a/yarn-project/sequencer-client/src/sequencer/public_processor.test.ts +++ b/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([]); diff --git a/yarn-project/sequencer-client/src/sequencer/sequencer.ts b/yarn-project/sequencer-client/src/sequencer/sequencer.ts index 4398545fc30c..97e5332ffa51 100644 --- a/yarn-project/sequencer-client/src/sequencer/sequencer.ts +++ b/yarn-project/sequencer-client/src/sequencer/sequencer.ts @@ -169,7 +169,7 @@ export class Sequencer { const block = await this.buildBlock(processedValidTxs, l1ToL2Messages, emptyTx, newGlobalVariables); this.log(`Assembled block ${block.number}`); - await this.publishExtendedContractData(validTxs, block); + await this.publishExtendedContractData(processedTxs, block); await this.publishL2Block(block); this.log.info(`Submitted rollup block ${block.number} with ${processedValidTxs.length} transactions`); @@ -184,16 +184,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 => cd !== undefined);