Skip to content

Commit

Permalink
chore: Enforce bracing around blocks. Generally considered easier to …
Browse files Browse the repository at this point in the history
…read and less error prone. (#3349)

As title.

`yarn formatting:fix` now runs eslint with fix enabled first, allowing
auto fixing of these kinds of issues.

Calling out to ChatGPT as my curly supporting hype-man:

1. Readability: Curly braces make the structure of the code clearer,
especially in complex codebases. It's easier to identify the start and
end of control blocks.
2. Maintainability: When modifying code, clear block delimiters reduce
the chance of introducing errors. For example, if you need to add a new
statement to an if block that was initially written without braces, the
absence of braces might lead to logical errors.
3. Error Reduction: One of the famous bugs related to the absence of
curly braces is Apple's "goto fail;" bug. Such bugs occur when the
intended block scope is not properly defined, leading to unexpected
behavior.
4. Consistency: Enforcing a consistent style, like always using braces,
can make the code more uniform and easier to understand for all team
members.


![](https://github.com/AztecProtocol/aztec-packages/assets/5764343/d24b4133-2e57-4790-97df-fe0e88880011)
  • Loading branch information
charlielye committed Nov 18, 2023
1 parent 6307e12 commit ee11dec
Show file tree
Hide file tree
Showing 94 changed files with 550 additions and 203 deletions.
2 changes: 1 addition & 1 deletion yarn-project/acir-simulator/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
"build:dev": "tsc -b --watch",
"clean": "rm -rf ./dest .tsbuildinfo",
"formatting": "run -T prettier --check ./src && run -T eslint ./src",
"formatting:fix": "run -T prettier -w ./src",
"formatting:fix": "run -T eslint --fix ./src && run -T prettier -w ./src",
"test": "NODE_NO_WARNINGS=1 node --experimental-vm-modules $(yarn bin jest) --passWithNoTests"
},
"inherits": [
Expand Down
4 changes: 3 additions & 1 deletion yarn-project/acir-simulator/src/acvm/deserialize.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,9 @@ export class PublicInputsReader {
*/
public readField(): Fr {
const acvmField = this.publicInputs.shift();
if (!acvmField) throw new Error('Not enough public inputs');
if (!acvmField) {
throw new Error('Not enough public inputs');
}
return fromACVMField(acvmField);
}

Expand Down
4 changes: 3 additions & 1 deletion yarn-project/acir-simulator/src/acvm/oracle/oracle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,9 @@ export class Oracle {
async getAuthWitness([messageHash]: ACVMField[]): Promise<ACVMField[]> {
const messageHashField = fromACVMField(messageHash);
const witness = await this.typedOracle.getAuthWitness(messageHashField);
if (!witness) throw new Error(`Authorization not found for message hash ${messageHashField}`);
if (!witness) {
throw new Error(`Authorization not found for message hash ${messageHashField}`);
}
return witness.map(toACVMField);
}

Expand Down
8 changes: 6 additions & 2 deletions yarn-project/acir-simulator/src/client/pick_notes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,10 +78,14 @@ const selectNotes = <T extends ContainsNote>(noteDatas: T[], selects: Select[]):
noteDatas.filter(noteData => selects.every(({ index, value }) => noteData.note.items[index]?.equals(value)));

const sortNotes = (a: Fr[], b: Fr[], sorts: Sort[], level = 0): number => {
if (sorts[level] === undefined) return 0;
if (sorts[level] === undefined) {
return 0;
}

const { index, order } = sorts[level];
if (order === 0) return 0;
if (order === 0) {
return 0;
}

const dir = order === 1 ? [-1, 1] : [1, -1];
return a[index].value === b[index].value
Expand Down
24 changes: 18 additions & 6 deletions yarn-project/acir-simulator/src/client/private_execution.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -156,8 +156,12 @@ describe('Private Execution test suite', () => {
beforeEach(() => {
oracle = mock<DBOracle>();
oracle.getSecretKey.mockImplementation((contractAddress: AztecAddress, pubKey: PublicKey) => {
if (pubKey.equals(ownerCompleteAddress.publicKey)) return Promise.resolve(ownerPk);
if (pubKey.equals(recipientCompleteAddress.publicKey)) return Promise.resolve(recipientPk);
if (pubKey.equals(ownerCompleteAddress.publicKey)) {
return Promise.resolve(ownerPk);
}
if (pubKey.equals(recipientCompleteAddress.publicKey)) {
return Promise.resolve(recipientPk);
}
throw new Error(`Unknown address ${pubKey}`);
});
oracle.getHistoricBlockData.mockResolvedValue(blockData);
Expand Down Expand Up @@ -209,8 +213,12 @@ describe('Private Execution test suite', () => {

beforeEach(() => {
oracle.getCompleteAddress.mockImplementation((address: AztecAddress) => {
if (address.equals(owner)) return Promise.resolve(ownerCompleteAddress);
if (address.equals(recipient)) return Promise.resolve(recipientCompleteAddress);
if (address.equals(owner)) {
return Promise.resolve(ownerCompleteAddress);
}
if (address.equals(recipient)) {
return Promise.resolve(recipientCompleteAddress);
}
throw new Error(`Unknown address ${address}`);
});

Expand Down Expand Up @@ -420,7 +428,9 @@ describe('Private Execution test suite', () => {

beforeEach(() => {
oracle.getCompleteAddress.mockImplementation((address: AztecAddress) => {
if (address.equals(recipient)) return Promise.resolve(recipientCompleteAddress);
if (address.equals(recipient)) {
return Promise.resolve(recipientCompleteAddress);
}
throw new Error(`Unknown address ${address}`);
});
});
Expand Down Expand Up @@ -558,7 +568,9 @@ describe('Private Execution test suite', () => {
describe('pending commitments contract', () => {
beforeEach(() => {
oracle.getCompleteAddress.mockImplementation((address: AztecAddress) => {
if (address.equals(owner)) return Promise.resolve(ownerCompleteAddress);
if (address.equals(owner)) {
return Promise.resolve(ownerCompleteAddress);
}
throw new Error(`Unknown address ${address}`);
});
});
Expand Down
4 changes: 3 additions & 1 deletion yarn-project/acir-simulator/src/client/simulator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,9 @@ export class AcirSimulator {
* @returns ACVM WasmBlackBoxFunctionSolver
*/
public static getSolver(): Promise<WasmBlackBoxFunctionSolver> {
if (!this.solver) this.solver = createBlackBoxSolver();
if (!this.solver) {
this.solver = createBlackBoxSolver();
}
return this.solver;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,9 @@ describe('Unconstrained Execution test suite', () => {
owner = ownerCompleteAddress.address;

oracle.getCompleteAddress.mockImplementation((address: AztecAddress) => {
if (address.equals(owner)) return Promise.resolve(ownerCompleteAddress);
if (address.equals(owner)) {
return Promise.resolve(ownerCompleteAddress);
}
throw new Error(`Unknown address ${address}`);
});
});
Expand Down
4 changes: 3 additions & 1 deletion yarn-project/acir-simulator/src/public/executor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,9 @@ export class PublicExecutor {
public async simulate(execution: PublicExecution, globalVariables: GlobalVariables): Promise<PublicExecutionResult> {
const selector = execution.functionData.selector;
const acir = await this.contractsDb.getBytecode(execution.contractAddress, selector);
if (!acir) throw new Error(`Bytecode not found for ${execution.contractAddress}:${selector}`);
if (!acir) {
throw new Error(`Bytecode not found for ${execution.contractAddress}:${selector}`);
}

// Functions can request to pack arguments before calling other functions.
// We use this cache to hold the packed arguments.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,9 @@ export class PublicExecutionContext extends TypedOracle {
}

const acir = await this.contractsDb.getBytecode(targetContractAddress, functionSelector);
if (!acir) throw new Error(`Bytecode not found for ${targetContractAddress}:${functionSelector}`);
if (!acir) {
throw new Error(`Bytecode not found for ${targetContractAddress}:${functionSelector}`);
}

const functionData = new FunctionData(functionSelector, isInternal, false, false);

Expand Down
8 changes: 6 additions & 2 deletions yarn-project/acir-simulator/src/public/state_actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,13 @@ export class ContractStorageActionsCollector {
public async read(storageSlot: Fr, sideEffectCounter: number): Promise<Fr> {
const slot = storageSlot.value;
const updateRequest = this.contractStorageUpdateRequests.get(slot);
if (updateRequest) return updateRequest.newValue;
if (updateRequest) {
return updateRequest.newValue;
}
const read = this.contractStorageReads.get(slot);
if (read) return read.currentValue;
if (read) {
return read.currentValue;
}
const value = await this.db.storageRead(this.address, storageSlot);
this.contractStorageReads.set(slot, { currentValue: value, sideEffectCounter });
return value;
Expand Down
2 changes: 1 addition & 1 deletion yarn-project/archiver/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
"build:dev": "tsc -b --watch",
"clean": "rm -rf ./dest .tsbuildinfo",
"formatting": "run -T prettier --check ./src && run -T eslint ./src",
"formatting:fix": "run -T prettier -w ./src",
"formatting:fix": "run -T eslint --fix ./src && run -T prettier -w ./src",
"test": "NODE_NO_WARNINGS=1 node --experimental-vm-modules $(yarn bin jest) --passWithNoTests",
"start": "node ./dest",
"start:dev": "tsc-watch -p tsconfig.json --onSuccess 'yarn start'",
Expand Down
4 changes: 3 additions & 1 deletion yarn-project/archiver/src/archiver/archiver_store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -507,7 +507,9 @@ export class MemoryArchiverStore implements ArchiverDataStore {
* @returns The number of the latest L2 block processed.
*/
public getBlockNumber(): Promise<number> {
if (this.l2BlockContexts.length === 0) return Promise.resolve(INITIAL_L2_BLOCK_NUM - 1);
if (this.l2BlockContexts.length === 0) {
return Promise.resolve(INITIAL_L2_BLOCK_NUM - 1);
}
return Promise.resolve(this.l2BlockContexts[this.l2BlockContexts.length - 1].block.number);
}
}
4 changes: 3 additions & 1 deletion yarn-project/archiver/src/archiver/eth_log_handlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,9 @@ async function getBlockFromCallData(
abi: RollupAbi.filter(item => item.type.toString() !== 'constructor'),
data,
});
if (functionName !== 'process') throw new Error(`Unexpected method called ${functionName}`);
if (functionName !== 'process') {
throw new Error(`Unexpected method called ${functionName}`);
}
const [, l2BlockHex] = args! as [Hex, Hex];
const block = L2Block.fromBufferWithLogs(Buffer.from(hexToBytes(l2BlockHex)));
if (BigInt(block.number) !== l2BlockNum) {
Expand Down
4 changes: 3 additions & 1 deletion yarn-project/archiver/src/archiver/l1_to_l2_message_store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,9 @@ export class PendingL1ToL2MessageStore extends L1ToL2MessageStore {

removeMessage(messageKey: Fr) {
// ignore 0 - messageKey is a hash, so a 0 can probabilistically never occur. It is best to skip it.
if (messageKey.equals(Fr.ZERO)) return;
if (messageKey.equals(Fr.ZERO)) {
return;
}
const messageKeyBigInt = messageKey.value;
const msgAndCount = this.store.get(messageKeyBigInt);
if (!msgAndCount) {
Expand Down
2 changes: 1 addition & 1 deletion yarn-project/aztec-faucet/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
"build:dev": "tsc -b --watch",
"clean": "rm -rf ./dest .tsbuildinfo",
"formatting": "run -T prettier --check ./src && run -T eslint ./src",
"formatting:fix": "run -T prettier -w ./src",
"formatting:fix": "run -T eslint --fix ./src && run -T prettier -w ./src",
"test": "NODE_NO_WARNINGS=1 node --experimental-vm-modules $(yarn bin jest) --passWithNoTests"
},
"inherits": [
Expand Down
2 changes: 1 addition & 1 deletion yarn-project/aztec-node/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
"build:dev": "tsc -b --watch",
"clean": "rm -rf ./dest .tsbuildinfo",
"formatting": "run -T prettier --check ./src && run -T eslint ./src",
"formatting:fix": "run -T prettier -w ./src",
"formatting:fix": "run -T eslint --fix ./src && run -T prettier -w ./src",
"test": "NODE_NO_WARNINGS=1 node --experimental-vm-modules $(yarn bin jest) --passWithNoTests"
},
"inherits": [
Expand Down
2 changes: 1 addition & 1 deletion yarn-project/aztec-sandbox/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
"start": "node --no-warnings ./dest/bin",
"clean": "rm -rf ./dest .tsbuildinfo",
"formatting": "run -T prettier --check ./src && run -T eslint ./src",
"formatting:fix": "run -T prettier -w ./src",
"formatting:fix": "run -T eslint --fix ./src && run -T prettier -w ./src",
"build:dev": "tsc -b --watch",
"test": "NODE_NO_WARNINGS=1 node --experimental-vm-modules $(yarn bin jest) --passWithNoTests",
"run:example:token": "DEBUG='aztec:*' node ./dest/examples/token.js"
Expand Down
2 changes: 1 addition & 1 deletion yarn-project/aztec.js/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
"build:ts": "tsc -b",
"clean": "rm -rf ./dest .tsbuildinfo",
"formatting": "run -T prettier --check ./src && run -T eslint ./src",
"formatting:fix": "run -T prettier -w ./src",
"formatting:fix": "run -T eslint --fix ./src && run -T prettier -w ./src",
"test": "NODE_NO_WARNINGS=1 node --experimental-vm-modules $(yarn bin jest) --passWithNoTests"
},
"inherits": [
Expand Down
4 changes: 3 additions & 1 deletion yarn-project/aztec.js/src/account/manager/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,9 @@ export class AccountManager {
*/
public async getDeployMethod() {
if (!this.deployMethod) {
if (!this.salt) throw new Error(`Cannot deploy account contract without known salt.`);
if (!this.salt) {
throw new Error(`Cannot deploy account contract without known salt.`);
}
await this.#register();
const encryptionPublicKey = this.getEncryptionPublicKey();
const deployer = new ContractDeployer(this.accountContract.getContractArtifact(), this.pxe, encryptionPublicKey);
Expand Down
15 changes: 11 additions & 4 deletions yarn-project/aztec.js/src/contract/sent_tx.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,9 @@ export class SentTx {
throw new Error('Cannot set getNotes to true if waitForNotesSync is false');
}
const receipt = await this.waitForReceipt(opts);
if (receipt.status !== TxStatus.MINED)
if (receipt.status !== TxStatus.MINED) {
throw new Error(`Transaction ${await this.getTxHash()} was ${receipt.status}`);
}
if (opts?.debug) {
const txHash = await this.getTxHash();
const tx = (await this.pxe.getTx(txHash))!;
Expand Down Expand Up @@ -110,12 +111,18 @@ export class SentTx {
async () => {
const txReceipt = await this.pxe.getTxReceipt(txHash);
// If receipt is not yet available, try again
if (txReceipt.status === TxStatus.PENDING) return undefined;
if (txReceipt.status === TxStatus.PENDING) {
return undefined;
}
// If the tx was dropped, return it
if (txReceipt.status === TxStatus.DROPPED) return txReceipt;
if (txReceipt.status === TxStatus.DROPPED) {
return txReceipt;
}
// If we don't care about waiting for notes to be synced, return the receipt
const waitForNotesSync = opts?.waitForNotesSync ?? DefaultWaitOpts.waitForNotesSync;
if (!waitForNotesSync) return txReceipt;
if (!waitForNotesSync) {
return txReceipt;
}
// Check if all sync blocks on the PXE Service are greater or equal than the block in which the tx was mined
const { blocks, notes } = await this.pxe.getSyncStatus();
const targetBlock = txReceipt.blockNumber!;
Expand Down
4 changes: 3 additions & 1 deletion yarn-project/aztec.js/src/contract_deployer/deploy_method.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,9 @@ export class DeployMethod<TContract extends ContractBase = Contract> extends Bas
) {
super(pxe);
const constructorArtifact = artifact.functions.find(f => f.name === 'constructor');
if (!constructorArtifact) throw new Error('Cannot find constructor in the artifact.');
if (!constructorArtifact) {
throw new Error('Cannot find constructor in the artifact.');
}
this.constructorArtifact = constructorArtifact;
}

Expand Down
8 changes: 6 additions & 2 deletions yarn-project/aztec.js/src/contract_deployer/deploy_sent_tx.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,12 @@ export class DeploySentTx<TContract extends Contract = Contract> extends SentTx
protected getContractInstance(wallet?: Wallet, address?: AztecAddress): Promise<TContract> {
const isWallet = (pxe: PXE | Wallet): pxe is Wallet => !!(pxe as Wallet).createTxExecutionRequest;
const contractWallet = wallet ?? (isWallet(this.pxe) && this.pxe);
if (!contractWallet) throw new Error(`A wallet is required for creating a contract instance`);
if (!address) throw new Error(`Contract address is missing from transaction receipt`);
if (!contractWallet) {
throw new Error(`A wallet is required for creating a contract instance`);
}
if (!address) {
throw new Error(`Contract address is missing from transaction receipt`);
}
return Contract.at(address, this.artifact, contractWallet) as Promise<TContract>;
}
}
32 changes: 24 additions & 8 deletions yarn-project/aztec.js/src/utils/cheat_codes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,9 @@ export class EthCheatCodes {
*/
public async mine(numberOfBlocks = 1): Promise<void> {
const res = await this.rpcCall('hardhat_mine', [numberOfBlocks]);
if (res.error) throw new Error(`Error mining: ${res.error.message}`);
if (res.error) {
throw new Error(`Error mining: ${res.error.message}`);
}
this.logger(`Mined ${numberOfBlocks} blocks`);
}

Expand All @@ -97,7 +99,9 @@ export class EthCheatCodes {
*/
public async setNextBlockTimestamp(timestamp: number): Promise<void> {
const res = await this.rpcCall('evm_setNextBlockTimestamp', [timestamp]);
if (res.error) throw new Error(`Error setting next block timestamp: ${res.error.message}`);
if (res.error) {
throw new Error(`Error setting next block timestamp: ${res.error.message}`);
}
this.logger(`Set next block timestamp to ${timestamp}`);
}

Expand All @@ -107,7 +111,9 @@ export class EthCheatCodes {
*/
public async dumpChainState(fileName: string): Promise<void> {
const res = await this.rpcCall('hardhat_dumpState', []);
if (res.error) throw new Error(`Error dumping state: ${res.error.message}`);
if (res.error) {
throw new Error(`Error dumping state: ${res.error.message}`);
}
const jsonContent = JSON.stringify(res.result);
fs.writeFileSync(`${fileName}.json`, jsonContent, 'utf8');
this.logger(`Dumped state to ${fileName}`);
Expand All @@ -120,7 +126,9 @@ export class EthCheatCodes {
public async loadChainState(fileName: string): Promise<void> {
const data = JSON.parse(fs.readFileSync(`${fileName}.json`, 'utf8'));
const res = await this.rpcCall('hardhat_loadState', [data]);
if (res.error) throw new Error(`Error loading state: ${res.error.message}`);
if (res.error) {
throw new Error(`Error loading state: ${res.error.message}`);
}
this.logger(`Loaded state from ${fileName}`);
}

Expand All @@ -144,7 +152,9 @@ export class EthCheatCodes {
public async store(contract: EthAddress, slot: bigint, value: bigint): Promise<void> {
// for the rpc call, we need to change value to be a 32 byte hex string.
const res = await this.rpcCall('hardhat_setStorageAt', [contract.toString(), toHex(slot), toHex(value, true)]);
if (res.error) throw new Error(`Error setting storage for contract ${contract} at ${slot}: ${res.error.message}`);
if (res.error) {
throw new Error(`Error setting storage for contract ${contract} at ${slot}: ${res.error.message}`);
}
this.logger(`Set storage for contract ${contract} at ${slot} to ${value}`);
}

Expand All @@ -166,7 +176,9 @@ export class EthCheatCodes {
*/
public async startImpersonating(who: EthAddress): Promise<void> {
const res = await this.rpcCall('hardhat_impersonateAccount', [who.toString()]);
if (res.error) throw new Error(`Error impersonating ${who}: ${res.error.message}`);
if (res.error) {
throw new Error(`Error impersonating ${who}: ${res.error.message}`);
}
this.logger(`Impersonating ${who}`);
}

Expand All @@ -176,7 +188,9 @@ export class EthCheatCodes {
*/
public async stopImpersonating(who: EthAddress): Promise<void> {
const res = await this.rpcCall('hardhat_stopImpersonatingAccount', [who.toString()]);
if (res.error) throw new Error(`Error when stopping the impersonation of ${who}: ${res.error.message}`);
if (res.error) {
throw new Error(`Error when stopping the impersonation of ${who}: ${res.error.message}`);
}
this.logger(`Stopped impersonating ${who}`);
}

Expand All @@ -187,7 +201,9 @@ export class EthCheatCodes {
*/
public async etch(contract: EthAddress, bytecode: `0x${string}`): Promise<void> {
const res = await this.rpcCall('hardhat_setCode', [contract.toString(), bytecode]);
if (res.error) throw new Error(`Error setting bytecode for ${contract}: ${res.error.message}`);
if (res.error) {
throw new Error(`Error setting bytecode for ${contract}: ${res.error.message}`);
}
this.logger(`Set bytecode for ${contract} to ${bytecode}`);
}

Expand Down
Loading

0 comments on commit ee11dec

Please sign in to comment.