Skip to content

Commit

Permalink
fix: bad contract txs publishing contract data
Browse files Browse the repository at this point in the history
  • Loading branch information
alexghr committed Oct 10, 2023
1 parent 3a54678 commit 1fb1ae9
Show file tree
Hide file tree
Showing 8 changed files with 59 additions and 9 deletions.
1 change: 1 addition & 0 deletions yarn-project/end-to-end/src/cli_docs_sandbox.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ SchnorrAccountContractAbi
SchnorrHardcodedAccountContractAbi
SchnorrSingleKeyAccountContractAbi
StatefulTestContractAbi
TestAssertContractAbi
TestContractAbi
TokenBridgeContractAbi
TokenContractAbi
Expand Down
25 changes: 24 additions & 1 deletion yarn-project/end-to-end/src/e2e_deploy_contract.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
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 { 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';
Expand Down Expand Up @@ -128,4 +128,27 @@ describe('e2e_deploy_contract', () => {
portalContract.toString(),
);
});

it('it should not deploy a contract which failed the public part of the execution', async () => {
// 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(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();
});
});
1 change: 1 addition & 0 deletions yarn-project/noir-contracts/Nargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,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",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
[package]
name = "test_assert_contract"
authors = [""]
compiler_version = "0.1"
type = "contract"

[dependencies]
aztec = { path = "../../../../aztec-nr/aztec" }
Original file line number Diff line number Diff line change
@@ -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);
}
}
6 changes: 4 additions & 2 deletions yarn-project/sequencer-client/src/sequencer/processed_tx.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand Down Expand Up @@ -78,6 +78,7 @@ export async function makeProcessedTx(
proof: proof ?? tx.proof,
encryptedLogs: tx.encryptedLogs,
unencryptedLogs: tx.unencryptedLogs,
newContracts: tx.newContracts,
isEmpty: false,
};
}
Expand All @@ -104,6 +105,7 @@ export function makeEmptyProcessedTx(
unencryptedLogs: new TxL2Logs([]),
data: emptyKernelOutput,
proof: emptyProof,
newContracts: [ExtendedContractData.random()],
isEmpty: true,
});
}
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ describe('public_processor', () => {
proof: tx.proof,
encryptedLogs: tx.encryptedLogs,
unencryptedLogs: tx.unencryptedLogs,
newContracts: tx.newContracts,
},
]);
expect(failed).toEqual([]);
Expand Down
9 changes: 3 additions & 6 deletions yarn-project/sequencer-client/src/sequencer/sequencer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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`);
Expand All @@ -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<typeof cd, undefined> => cd !== undefined);

Expand Down

0 comments on commit 1fb1ae9

Please sign in to comment.