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.

Sign up for GitHub

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

Jump to bottom

fix(validator): lock keystores when loading from cache file #5474

Merged
merged 2 commits into from
May 14, 2023
Merged

fix(validator): lock keystores when loading from cache file #5474

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dc12406
Lock keystores when loading from cache file
nflaig May 9, 2023
c0e9629
Update tests to run with and without existing keystore cache
nflaig May 9, 2023
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
33 changes: 22 additions & 11 deletions packages/cli/src/cmds/validator/keymanager/decryptKeystoreDefinitions/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,17 @@ export async function decryptKeystoreDefinitions(
if (opts.cacheFilePath) {
try {
const signers = await loadKeystoreCache(opts.cacheFilePath, keystoreDefinitions);

for (const {keystorePath} of keystoreDefinitions) {
lockKeystore(keystorePath, opts);
}

if (opts?.onDecrypt) {
opts?.onDecrypt(signers.length - 1);
}

opts.logger.debug("Loaded keystores via keystore cache");

return signers;
} catch {
// Some error loading the cache, ignore and invalidate cache
Expand All @@ -41,17 +48,7 @@ export async function decryptKeystoreDefinitions(
defaultPoolSize
);
for (const [index, definition] of keystoreDefinitions.entries()) {
try {
lockFilepath(definition.keystorePath);
} catch (e) {
if (opts.ignoreLockFile) {
opts.logger.warn("Keystore forcefully loaded even though lockfile exists", {
path: definition.keystorePath,
});
} else {
throw e;
}
}
lockKeystore(definition.keystorePath, opts);

const task = pool.queue((thread) => thread.decryptKeystoreDefinition(definition));
tasks.push(task);
Expand Down Expand Up @@ -102,3 +99,17 @@ export async function decryptKeystoreDefinitions(

return signers;
}

function lockKeystore(keystorePath: string, opts: KeystoreDecryptOptions): void {
try {
lockFilepath(keystorePath);
} catch (e) {
if (opts.ignoreLockFile) {
opts.logger.warn("Keystore forcefully loaded even though lockfile exists", {
path: keystorePath,
});
} else {
throw e;
}
}
}
65 changes: 43 additions & 22 deletions packages/cli/test/unit/validator/decryptKeystoreDefinitions.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,31 +38,52 @@ describe("decryptKeystoreDefinitions", function () {
}
});

it("decrypt keystores", async () => {
const signers = await decryptKeystoreDefinitions(definitions, {logger: console});
expect(signers.length).to.equal(secretKeys.length);
for (const signer of signers) {
const hexSecret = signer.secretKey.toHex();
expect(secretKeys.includes(hexSecret), `secretKeys doesn't include ${hexSecret}`).to.be.true;
}
});
context("with keystore cache", () => {
const cacheFilePath = path.join(dataDir, "cache", "keystores.cache");

it("fail to decrypt keystores if lockfiles already exist", async () => {
await decryptKeystoreDefinitions(definitions, {logger: console});
// lockfiles should exist after the first run
beforeEach(async () => {
// create cache file to ensure keystores are loaded from cache during tests
await decryptKeystoreDefinitions(definitions, {logger: console, cacheFilePath});
expect(fs.existsSync(cacheFilePath)).to.be.true;

try {
await decryptKeystoreDefinitions(definitions, {logger: console});
expect.fail("Second decrypt should fail due to failure to get lockfile");
} catch (e) {
expect((e as Error).message.startsWith("EEXIST: file already exists"), "Wrong error is thrown").to.be.true;
}
});
// remove lockfiles created during cache file preparation
rimraf.sync(path.join(importFromDir, "*.lock"), {glob: true});
});

it("decrypt keystores if lockfiles already exist if ignoreLockFile=true", async () => {
await decryptKeystoreDefinitions(definitions, {logger: console});
// lockfiles should exist after the first run
testDecryptKeystoreDefinitions(cacheFilePath);
});

await decryptKeystoreDefinitions(definitions, {logger: console, ignoreLockFile: true});
context("without keystore cache", () => {
testDecryptKeystoreDefinitions();
});

function testDecryptKeystoreDefinitions(cacheFilePath?: string): void {
it("decrypt keystores", async () => {
const signers = await decryptKeystoreDefinitions(definitions, {logger: console, cacheFilePath});
expect(signers.length).to.equal(secretKeys.length);
for (const signer of signers) {
const hexSecret = signer.secretKey.toHex();
expect(secretKeys.includes(hexSecret), `secretKeys doesn't include ${hexSecret}`).to.be.true;
}
});

it("fail to decrypt keystores if lockfiles already exist", async () => {
await decryptKeystoreDefinitions(definitions, {logger: console, cacheFilePath});
// lockfiles should exist after the first run

try {
await decryptKeystoreDefinitions(definitions, {logger: console, 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"), "Wrong error is thrown").to.be.true;
}
});

it("decrypt keystores if lockfiles already exist if ignoreLockFile=true", async () => {
await decryptKeystoreDefinitions(definitions, {logger: console, cacheFilePath});
// lockfiles should exist after the first run

await decryptKeystoreDefinitions(definitions, {logger: console, cacheFilePath, ignoreLockFile: true});
});
}
});