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 6 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
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
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
@@ -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); // File stores the timestamp of last compilation
const folderToCheck = path.join(process.cwd(), CONTRACTS_DIR); // Directory to check if files & imports were changed after last compilation

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
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); // File stores the timestamp of last compilation
const folderToCheck = path.join(process.cwd(), CONTRACTS_DIR); // Directory to check if files & imports were changed after last compilation

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
160 changes: 160 additions & 0 deletions core/tests/ts-integration/scripts/utils.ts
@@ -0,0 +1,160 @@
import * as fs from 'fs';
import * as fsPr from 'fs/promises';
import path from 'path';
import { exec } from 'child_process';
import { glob } from 'glob';

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

// Function to check if a file exists
function fileExists(filePath: string): boolean {
try {
return fs.existsSync(filePath);
} catch {
return false;
}
}

/**
* Resolves the import path, handling relative imports and node module imports.
* @param currentDir The current directory from which the import originates.
* @param importPath The import path to resolve.
* @returns The resolved absolute path to the imported file.
*/
function resolveImportPath(currentDir: string, importPath: string): string {
if (importPath.startsWith('.') || importPath.startsWith('/')) {
// Relative or absolute path
return path.resolve(currentDir, importPath);
} else {
// Likely a node module
return require.resolve(importPath, { paths: [process.cwd()] });
}
}

/**
* Get the list of imported Solidity files recursively, considering both relative and node module imports.
* @param filePath The path of the Solidity file to analyze.
* @param seenFiles A set to track already seen files to avoid loops.
* @returns A set of unique file paths that are imported.
*/
function getImportedFiles(filePath: string, seenFiles: Set<string> = new Set()): Set<string> {
const imports = new Set<string>();

if (!fileExists(filePath)) {
console.error(`File not found: ${filePath}`);
return imports; // Return empty set if the file doesn't exist
}

const content = fs.readFileSync(filePath, 'utf-8');
const importRegex = /import ["'](.+?)["']/g;

let match: RegExpExecArray | null;
const currentDir = path.dirname(filePath);
while ((match = importRegex.exec(content)) !== null) {
const importedFile = resolveImportPath(currentDir, match[1]);

if (!seenFiles.has(importedFile)) {
seenFiles.add(importedFile);
imports.add(importedFile);

// Recursively find imports within this imported file
if (fileExists(importedFile)) {
getImportedFiles(importedFile, seenFiles).forEach((file) => imports.add(file));
} else {
console.error(`Imported file not found: ${importedFile}`);
}
}
}

return imports;
}

/**
* Get the latest modification time among all Solidity files and their imports.
* @param folder The root folder to start the search.
* @returns The latest modification time among all Solidity files.
*/
function getLatestModificationTime(folder: string): Date {
const files = glob.sync(`${folder}/**/*.sol`); // Find all Solidity files
let latestTime: Date | null = null;

files.forEach((filePath) => {
const allFiles = getImportedFiles(filePath);

allFiles.forEach((file) => {
if (!fileExists(file)) {
console.error(`File not found: ${file}`);
return; // Continue to the next file if this one doesn't exist
}

const stats = fs.statSync(file);
if (!latestTime || stats.mtime > latestTime) {
latestTime = stats.mtime;
}
});
});

if (latestTime) {
return latestTime;
} else {
throw new Error('No Solidity files found or no modification times detected.');
}
}

// 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
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
@@ -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); // File stores the timestamp of last compilation
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 this twice (as it already exists in the other build.rs) - maybe move it to a shared util 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.

That's right, I was thinking of it, but as these are different workspaces, I was not sure if a shared util is a good idea

const folderToCheck = path.join(process.cwd(), CONTRACTS_DIR); // Directory to check if files & imports were changed after last compilation

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();