Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions packages/cli/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,6 @@
"@lodestar/utils": "^1.14.0",
"@lodestar/validator": "^1.14.0",
"@multiformats/multiaddr": "^12.1.3",
"@types/lockfile": "^1.0.2",
"bip39": "^3.1.0",
"deepmerge": "^4.3.1",
"ethers": "^6.7.0",
Expand All @@ -86,9 +85,9 @@
"got": "^11.8.6",
"inquirer": "^9.1.5",
"js-yaml": "^4.1.0",
"lockfile": "^1.0.4",
"lodash": "^4.17.21",
"prom-client": "^15.1.0",
"proper-lockfile": "^4.1.2",
"rimraf": "^4.4.1",
"source-map-support": "^0.5.21",
"uint8arrays": "^4.0.9",
Expand All @@ -102,6 +101,7 @@
"@types/got": "^9.6.12",
"@types/inquirer": "^9.0.3",
"@types/lodash": "^4.14.192",
"@types/proper-lockfile": "^4.1.4",
"@types/yargs": "^17.0.24"
}
}
54 changes: 31 additions & 23 deletions packages/cli/src/util/lockfile.ts
Original file line number Diff line number Diff line change
@@ -1,37 +1,45 @@
export type Lockfile = {
lockSync(path: string): void;
unlockSync(path: string): void;
};

const lockFile: Lockfile = (await import("lockfile")) as Lockfile;

function getLockFilepath(filepath: string): string {
return `${filepath}.lock`;

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

No longer required as proper-lockfile adds this extension by default, see lib/lockfile.js#L12

}

/**
* When lockfile is imported, it registers listeners to process
* Since it's only used by the validator client, require lazily to not pollute
* beacon_node client context
Comment on lines -13 to -15

@nflaig nflaig Jan 28, 2024

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This statement was no longer true since the migration to ESM in #3978. It was also already noted in #3978 (comment) but having async file locking causes a bunch of upstream issues.

*/
function getLockFile(): Lockfile {
return lockFile;
}
import {lockSync, unlockSync} from "proper-lockfile";

/**
* Creates a .lock file for `filepath`, argument passed must not be the lock path
* @param filepath File to lock, i.e. `keystore_0001.json`
*/
export function lockFilepath(filepath: string): void {
getLockFile().lockSync(getLockFilepath(filepath));
try {
lockSync(filepath, {
// Allows to lock files that do not exist
realpath: false,
});
} catch (e) {
if (isLockfileError(e) && e.code === "ELOCKED") {
e.message = `${filepath} is already in use by another process`;
}
throw e;
}
}

/**
* Deletes a .lock file for `filepath`, argument passed must not be the lock path
* @param filepath File to unlock, i.e. `keystore_0001.json`
*/
export function unlockFilepath(filepath: string): void {
// Does not throw if the lock file is already deleted
// https://github.com/npm/lockfile/blob/6590779867ee9bdc5dbebddc962640759892bb91/lockfile.js#L68
getLockFile().unlockSync(getLockFilepath(filepath));
try {
unlockSync(filepath, {
// Allows to unlock files that do not exist
realpath: false,
});
} catch (e) {
if (isLockfileError(e) && e.code === "ENOTACQUIRED") {
// Do not throw if the lock file is already deleted
return;
}
throw e;
}
}

// https://github.com/moxystudio/node-proper-lockfile/blob/9f8c303c91998e8404a911dc11c54029812bca69/lib/lockfile.js#L53
export type LockfileError = Error & {code: "ELOCKED" | "ENOTACQUIRED"};

function isLockfileError(e: unknown): e is LockfileError {
return e instanceof Error && (e as LockfileError).code !== undefined;
}
8 changes: 4 additions & 4 deletions packages/cli/test/e2e/importKeystoresFromApi.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,10 +95,10 @@ describe("import keystores from api", function () {
validator.on("exit", (code) => {
if (code !== null && code > 0) {
// process should exit with code > 0, and an error related to locks. Sample error:
// vc 351591: ✖ Error: EEXIST: file already exists, open '/tmp/tmp-351554-dMctEAj7sJIz/import-keystores-test/keystores/0x8be678633e927aa0435addad5dcd5283fef6110d91362519cd6d43e61f6c017d724fa579cc4b2972134e050b6ba120c0/voting-keystore.json.lock'
// at Object.openSync (node:fs:585:3)
// at Module.exports.lockSync (/home/lion/Code/eth2.0/lodestar/node_modules/lockfile/lockfile.js:277:17)
if (/EEXIST.*voting-keystore\.json\.lock/.test(vcProc2Stderr.read())) {
// vc 351591: ✖ Error: /tmp/tmp-5080-lwNxdM5Ok9ya/import-keystores-test/keystores/0x8be678633e927aa0435addad5dcd5283fef6110d91362519cd6d43e61f6c017d724fa579cc4b2972134e050b6ba120c0/voting-keystore.json is already in use by another process
// at /home/runner/actions-runner/_work/lodestar/lodestar/node_modules/proper-lockfile/lib/lockfile.js:68:47
// ... more stack trace
if (/Error.*voting-keystore\.json is already in use by another process/.test(vcProc2Stderr.read())) {
resolve();
} else {
reject(Error(`Second validator proc exited with unknown error. stderr:\n${vcProc2Stderr.read()}`));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {cachedSeckeysHex} from "../../utils/cachedKeys.js";
import {testFilesDir} from "../../utils.js";
import {decryptKeystoreDefinitions} from "../../../src/cmds/validator/keymanager/decryptKeystoreDefinitions.js";
import {LocalKeystoreDefinition} from "../../../src/cmds/validator/keymanager/interface.js";
import {LockfileError, unlockFilepath} from "../../../src/util/lockfile.js";

describe("decryptKeystoreDefinitions", () => {
vi.setConfig({testTimeout: 100_000});
Expand All @@ -22,6 +23,10 @@ describe("decryptKeystoreDefinitions", () => {
let definitions: LocalKeystoreDefinition[] = [];

beforeEach(async () => {
// remove lockfiles from proper-lockfile cache
for (const {keystorePath} of definitions) {
unlockFilepath(keystorePath);
}
rimraf.sync(dataDir);
rimraf.sync(importFromDir);

Expand All @@ -46,7 +51,9 @@ describe("decryptKeystoreDefinitions", () => {
expect(fs.existsSync(cacheFilePath)).toBe(true);

// remove lockfiles created during cache file preparation
rimraf.sync(path.join(importFromDir, "*.lock"), {glob: true});
for (const {keystorePath} of definitions) {
unlockFilepath(keystorePath);
}
});

testDecryptKeystoreDefinitions(cacheFilePath);
Expand Down Expand Up @@ -75,14 +82,14 @@ describe("decryptKeystoreDefinitions", () => {
await decryptKeystoreDefinitions(definitions, {logger: console, signal, cacheFilePath});
expect.fail("Second decrypt should fail due to failure to get lockfile");
} catch (e) {
expect((e as Error).message.startsWith("EEXIST: file already exists")).toBe(true);
expect((e as LockfileError).code).toBe<LockfileError["code"]>("ELOCKED");
}
});

it("decrypt keystores if lockfiles already exist if ignoreLockFile=true", async () => {
await decryptKeystoreDefinitions(definitions, {logger: console, signal, cacheFilePath});

// lockfiles should exist after the first run

await decryptKeystoreDefinitions(definitions, {logger: console, signal, cacheFilePath, ignoreLockFile: true});
});
}
Expand Down
2 changes: 1 addition & 1 deletion packages/db/src/controller/level.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ export class LevelDbController implements DatabaseController<Uint8Array, Uint8Ar
await db.open();
} catch (e) {
if ((e as LevelDbError).cause?.code === "LEVEL_LOCKED") {
throw new Error("Database is already in use by another Lodestar instance");
throw new Error("Database is already in use by another process");
}
throw e;
}
Expand Down
19 changes: 7 additions & 12 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -2955,11 +2955,6 @@
"@types/level-errors" "*"
"@types/node" "*"

"@types/lockfile@^1.0.2":
version "1.0.2"
resolved "https://registry.yarnpkg.com/@types/lockfile/-/lockfile-1.0.2.tgz#3f77e84171a2b7e3198bd5717c7547a54393baf8"
integrity sha512-jD5VbvhfMhaYN4M3qPJuhMVUg3Dfc4tvPvLEAXn6GXbs/ajDFtCQahX37GIE65ipTI3I+hEvNaXS3MYAn9Ce3Q==

"@types/lodash@^4.14.192":
version "4.14.192"
resolved "https://registry.yarnpkg.com/@types/lodash/-/lodash-4.14.192.tgz#5790406361a2852d332d41635d927f1600811285"
Expand Down Expand Up @@ -3043,6 +3038,13 @@
resolved "https://registry.yarnpkg.com/@types/normalize-package-data/-/normalize-package-data-2.4.1.tgz#d3357479a0fdfdd5907fe67e17e0a85c906e1301"
integrity sha512-Gj7cI7z+98M282Tqmp2K5EIsoouUEzbBJhQQzDE3jSIRk6r9gsz0oUokqIUR4u1R3dMHo0pDHM7sNOHyhulypw==

"@types/proper-lockfile@^4.1.4":
version "4.1.4"
resolved "https://registry.yarnpkg.com/@types/proper-lockfile/-/proper-lockfile-4.1.4.tgz#cd9fab92bdb04730c1ada542c356f03620f84008"
integrity sha512-uo2ABllncSqg9F1D4nugVl9v93RmjxF6LJzQLMLDdPaXCUIDPeOJ21Gbqi43xNKzBi/WQ0Q0dICqufzQbMjipQ==
dependencies:
"@types/retry" "*"

"@types/qs@^6.9.7":
version "6.9.7"
resolved "https://registry.yarnpkg.com/@types/qs/-/qs-6.9.7.tgz#63bb7d067db107cc1e457c303bc25d511febf6cb"
Expand Down Expand Up @@ -9045,13 +9047,6 @@ locate-path@^7.1.0:
dependencies:
p-locate "^6.0.0"

lockfile@^1.0.4:
version "1.0.4"
resolved "https://registry.npmjs.org/lockfile/-/lockfile-1.0.4.tgz"
integrity sha512-cvbTwETRfsFh4nHsL1eGWapU1XFi5Ot9E85sWAwia7Y7EgB7vfqcZhTKZ+l7hCGxSPoushMv5GKhT5PdLv03WA==
dependencies:
signal-exit "^3.0.2"

lodash._reinterpolate@^3.0.0:
version "3.0.0"
resolved "https://registry.yarnpkg.com/lodash._reinterpolate/-/lodash._reinterpolate-3.0.0.tgz#0ccf2d89166af03b3663c796538b75ac6e114d9d"
Expand Down