Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: improve zk init perfromance #1774

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ artifacts-zk/
cache-zk/
zksolc
verified_sources
*.timestamp

# Hyperchain related
hyperchain-*.yml
Expand Down
4 changes: 2 additions & 2 deletions core/tests/ts-integration/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@
"fee-test": "RUN_FEE_TEST=1 zk f jest -- fees.test.ts",
"api-test": "zk f jest -- api/web3.test.ts api/debug.test.ts",
"contract-verification-test": "zk f jest -- api/contract-verification.test.ts",
"build": "hardhat compile",
"build-yul": "hardhat run scripts/compile-yul.ts"
"build": "yarn ts-node scripts/build.ts",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: do you actually have to say 'yarn' here? I thought that "ts-node XX" should be enough.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

both will work, however 'yarn ts-node' is a safer choice in monorepo environment as it ensure execution in the context of yarn workspace.

"build-yul": "yarn ts-node scripts/compile-yul.ts"
},
"devDependencies": {
"@matterlabs/hardhat-zksync-deploy": "^0.6.5",
Expand Down
32 changes: 32 additions & 0 deletions core/tests/ts-integration/scripts/build.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import path from 'path';
import { exec } from 'child_process';
import {
needsRecompilation,
setCompilationTime,
isFolderEmpty,
CONTRACTS_DIR,
OUTPUT_DIR,
TIMESTAMP_FILE
} from './utils';

async function main() {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please add comment - describing what we're doing here (using some .timestamp file to record the last time that files were changed)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems that it was not done.
I'm looking for a 2-line comment, where reader can quickly see what's special about this 'build' script, without having to read the details of the code.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure it's the right move.
We could solve in two ways:

  1. Move l2 contracts compilation to foundry zksync
  2. Use the prebuild contracts from the repo. we do pulbish built contracts

const timestampFilePath = path.join(process.cwd(), TIMESTAMP_FILE);
const folderToCheck = path.join(process.cwd(), CONTRACTS_DIR);

if ((await isFolderEmpty(OUTPUT_DIR)) || needsRecompilation(folderToCheck, timestampFilePath)) {
console.log('Compilation needed.');
exec(`hardhat compile`, (error) => {
if (error) {
throw error;
} else {
console.log('Compilation successful.');
}
});
setCompilationTime(timestampFilePath);
} else {
console.log('Compilation not needed.');
return;
}
}

main();
16 changes: 13 additions & 3 deletions core/tests/ts-integration/scripts/compile-yul.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@ import * as hre from 'hardhat';
import * as fs from 'fs';
// eslint-disable-next-line @typescript-eslint/no-unused-vars
import { exec as _exec, spawn as _spawn } from 'child_process';

import { getZksolcUrl, saltFromUrl } from '@matterlabs/hardhat-zksync-solc';
import { getCompilersDir } from 'hardhat/internal/util/global-dir';
import path from 'path';
import { needsRecompilation, setCompilationTime, CONTRACTS_DIR, TIMESTAMP_FILE_YUL } from './utils';

const COMPILER_VERSION = '1.3.21';
const IS_COMPILER_PRE_RELEASE = false;
Expand Down Expand Up @@ -85,8 +85,18 @@ class CompilerPaths {
}

async function main() {
await compileFolder('contracts/yul', 'yul');
await compileFolder('contracts/zkasm', 'zkasm');
const timestampFilePath = path.join(process.cwd(), TIMESTAMP_FILE_YUL);
const folderToCheck = path.join(process.cwd(), CONTRACTS_DIR);

if (needsRecompilation(folderToCheck, timestampFilePath)) {
console.log('Compilation needed.');
await compileFolder('contracts/yul', 'yul');
await compileFolder('contracts/zkasm', 'zkasm');
setCompilationTime(timestampFilePath);
} else {
console.log('Compilation not needed.');
return;
}
}

main()
Expand Down
87 changes: 87 additions & 0 deletions core/tests/ts-integration/scripts/utils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
import * as fs from 'fs';
import * as fsPr from 'fs/promises';
import path from 'path';
import { exec } from 'child_process';

const CONTRACTS_DIR = 'contracts';
const OUTPUT_DIR = 'artifacts-zk';
const TIMESTAMP_FILE = 'last_compilation.timestamp'; // File to store the last compilation time
const TIMESTAMP_FILE_YUL = 'last_compilation_yul.timestamp'; // File to store the last compilation time

// Get the latest file modification time in the watched folder
function getLatestModificationTime(folder: string): Date | null {
const files = fs.readdirSync(folder);
let latestTime: Date | null = null;

files.forEach((file) => {
const filePath = path.join(folder, file);
const stats = fs.statSync(filePath);
if (stats.isDirectory()) {
const dirLatestTime = getLatestModificationTime(filePath);
if (dirLatestTime && (!latestTime || dirLatestTime > latestTime)) {
latestTime = dirLatestTime;
}
} else if (stats.isFile()) {
if (!latestTime || stats.mtime > latestTime) {
latestTime = stats.mtime;
}
}
});

return latestTime;
}

// Read the last compilation timestamp from the file
function getLastCompilationTime(timestampFile: string): Date | null {
try {
if (fs.existsSync(timestampFile)) {
const timestamp = fs.readFileSync(timestampFile, 'utf-8');
return new Date(parseInt(timestamp, 10));
}
} catch (error) {
const err = error as Error; // Cast `error` to `Error`
console.error(`Error reading timestamp: ${err.message}`);
}
return null;
}

// Write the current time to the timestamp file
export function setCompilationTime(timestampFile: string) {
fs.writeFileSync(timestampFile, Date.now().toString());
}

// Determine if recompilation is needed
export function needsRecompilation(folder: string, timestampFile: string): boolean {
const lastCompilationTime = getLastCompilationTime(timestampFile);
const latestModificationTime = getLatestModificationTime(folder);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do these sources have any dependencies on outside dirs?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


if (!latestModificationTime || !lastCompilationTime) {
return true; // If there's no history, always recompile
}

return latestModificationTime > lastCompilationTime;
}

export function deleteDir(path: string): Promise<void> {
return new Promise((resolve, reject) => {
exec(`rm -rf ${path}`, (error) => {
if (error) {
reject(error); // If an error occurs, reject the promise
} else {
resolve(); // If successful, resolve the promise
}
});
});
}

export async function isFolderEmpty(folderPath: string): Promise<boolean> {
try {
const files = await fsPr.readdir(folderPath); // Get a list of files in the folder
return files.length === 0; // If there are no files, the folder is empty
} catch (error) {
console.error('No target folder with artifacts.');
return true; // Return true if an error, as folder doesn't exist.
}
}

export { CONTRACTS_DIR, OUTPUT_DIR, TIMESTAMP_FILE, TIMESTAMP_FILE_YUL };
2 changes: 1 addition & 1 deletion etc/contracts-test-data/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
"@matterlabs/hardhat-zksync-solc": "^0.3.15"
},
"scripts": {
"build": "hardhat compile",
"build": "yarn ts-node ./scripts/build",
"clean": "hardhat clean"
}
}
33 changes: 33 additions & 0 deletions etc/contracts-test-data/scripts/build.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import path from 'path';
import {
needsRecompilation,
setCompilationTime,
isFolderEmpty,
CONTRACTS_DIR,
OUTPUT_DIR,
TIMESTAMP_FILE
} from './utils';
import { exec } from 'child_process';

async function main() {
const timestampFilePath = path.join(process.cwd(), TIMESTAMP_FILE);
const folderToCheck = path.join(process.cwd(), CONTRACTS_DIR);

if ((await isFolderEmpty(OUTPUT_DIR)) || needsRecompilation(folderToCheck, timestampFilePath)) {
console.log('Compilation needed.');
// Perform recompilation (e.g., run hardhat, truffle, etc.)
exec(`hardhat compile`, (error) => {
if (error) {
throw error; // If an error occurs, reject the promise
} else {
console.log('Compilation successful.');
}
});
setCompilationTime(timestampFilePath); // Update the timestamp after recompilation
} else {
console.log('Compilation not needed.');
return;
}
}

main();
86 changes: 86 additions & 0 deletions etc/contracts-test-data/scripts/utils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
import * as fs from 'fs';
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you are repeating the same file twice, that's not good, as it will be harder to manage and update in the future.

Could it be some shared library instead?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

switch to sync call, so it's 1 library

import * as fsPr from 'fs/promises';
import path from 'path';
import { exec } from 'child_process';

const CONTRACTS_DIR = 'contracts';
const OUTPUT_DIR = 'artifacts-zk';
const TIMESTAMP_FILE = 'last_compilation.timestamp'; // File to store the last compilation time

// Get the latest file modification time in the watched folder
function getLatestModificationTime(folder: string): Date | null {
const files = fs.readdirSync(folder);
let latestTime: Date | null = null;

files.forEach((file) => {
const filePath = path.join(folder, file);
const stats = fs.statSync(filePath);
if (stats.isDirectory()) {
const dirLatestTime = getLatestModificationTime(filePath);
if (dirLatestTime && (!latestTime || dirLatestTime > latestTime)) {
latestTime = dirLatestTime;
}
} else if (stats.isFile()) {
if (!latestTime || stats.mtime > latestTime) {
latestTime = stats.mtime;
}
}
});

return latestTime;
}

// Read the last compilation timestamp from the file
function getLastCompilationTime(timestampFile: string): Date | null {
try {
if (fs.existsSync(timestampFile)) {
const timestamp = fs.readFileSync(timestampFile, 'utf-8');
return new Date(parseInt(timestamp, 10));
}
} catch (error) {
const err = error as Error; // Cast `error` to `Error`
console.error(`Error reading timestamp: ${err.message}`);
}
return null;
}

// Write the current time to the timestamp file
export function setCompilationTime(timestampFile: string) {
fs.writeFileSync(timestampFile, Date.now().toString());
}

// Determine if recompilation is needed
export function needsRecompilation(folder: string, timestampFile: string): boolean {
const lastCompilationTime = getLastCompilationTime(timestampFile);
const latestModificationTime = getLatestModificationTime(folder);

if (!latestModificationTime || !lastCompilationTime) {
return true; // If there's no history, always recompile
}

return latestModificationTime > lastCompilationTime;
}

export function deleteDir(path: string): Promise<void> {
return new Promise((resolve, reject) => {
exec(`rm -rf ${path}`, (error) => {
if (error) {
reject(error); // If an error occurs, reject the promise
} else {
resolve(); // If successful, resolve the promise
}
});
});
}

export async function isFolderEmpty(folderPath: string): Promise<boolean> {
try {
const files = await fsPr.readdir(folderPath); // Get a list of files in the folder
return files.length === 0; // If there are no files, the folder is empty
} catch (error) {
console.error('No target folder with artifacts.');
return true; // Return true if an error, as folder doesn't exist.
}
}

export { CONTRACTS_DIR, OUTPUT_DIR, TIMESTAMP_FILE };
16 changes: 1 addition & 15 deletions infrastructure/zk/src/server.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { Command } from 'commander';
import * as utils from './utils';
import { clean } from './clean';
import fs from 'fs';
import * as path from 'path';
import * as db from './database';
import * as env from './env';
Expand Down Expand Up @@ -46,20 +45,7 @@ export async function externalNode(reinit: boolean = false, args: string[]) {

async function create_genesis(cmd: string) {
await utils.confirmAction();
await utils.spawn(`${cmd} | tee genesis.log`);

const date = new Date();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we not keep this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There was a discussion where it seems that we're not using these logs anymore

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mm-zk is it fine if we remove these log from here?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAIK yes. @Deniallugo - FYI.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i don't think we need this function at all

const [year, month, day, hour, minute, second] = [
date.getFullYear(),
date.getMonth(),
date.getDate(),
date.getHours(),
date.getMinutes(),
date.getSeconds()
];
const label = `${process.env.ZKSYNC_ENV}-Genesis_gen-${year}-${month}-${day}-${hour}${minute}${second}`;
fs.mkdirSync(`logs/${label}`, { recursive: true });
fs.copyFileSync('genesis.log', `logs/${label}/genesis.log`);
await utils.spawn(`${cmd}`);
}

export async function genesisFromSources() {
Expand Down