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(scripts): add MACI key generation script #1869

Open
wants to merge 6 commits into
base: dev
Choose a base branch
from

Conversation

programmerraja
Copy link

@programmerraja programmerraja commented Oct 23, 2024

Description

add MACI key generation script
- Add generateMaciKeys.js script
- Update package.json with new script command
- Update .gitignore to exclude maci-keys directory

feat for #1861

Additional Notes

Related issue(s)

Confirmation

    - Add generateMaciKeys.js script
    - Update package.json with new script command
    - Update .gitignore to exclude maci-keys directory
Copy link

vercel bot commented Oct 23, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
maci-website ❌ Failed (Inspect) Oct 28, 2024 3:05pm

Copy link
Collaborator

@0xmad 0xmad left a comment

Choose a reason for hiding this comment

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

@programmerraja thanks, there are some changes requested

Copy link
Collaborator

Choose a reason for hiding this comment

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

Please use typescript

publicKey: pubKey.toString(),
};

const outputDir = path.join(__dirname, "..", "maci-keys");
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
const outputDir = path.join(__dirname, "..", "maci-keys");
const outputDir = path.resolve(__dirname, "..", "maci-keys");

fs.mkdirSync(outputDir, { recursive: true });
}

const outputFile = path.join(outputDir, "maci-keys.json");
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
const outputFile = path.join(outputDir, "maci-keys.json");
const outputFile = path.resolve(outputDir, "maci-keys.json");

Comment on lines 14 to 16
if (!fs.existsSync(outputDir)) {
fs.mkdirSync(outputDir, { recursive: true });
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use async version here

}

const outputFile = path.join(outputDir, "maci-keys.json");
fs.writeFileSync(outputFile, JSON.stringify(keys, null, 2));
Copy link
Collaborator

Choose a reason for hiding this comment

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

The same with async

@programmerraja
Copy link
Author

@0xmad can you check and let me know anything else i want to change

Comment on lines 3 to 4
import { promises as fs } from "fs";
import * as path from "path";
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
import { promises as fs } from "fs";
import * as path from "path";
import fs from "fs";
import path from "path";


const outputDir = path.resolve(__dirname, "..", "maci-keys");
try {
await fs.access(outputDir);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
await fs.access(outputDir);
await fs.promises.access(outputDir);

Copy link
Collaborator

Choose a reason for hiding this comment

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

And the same change for other fs calls

Copy link
Author

Choose a reason for hiding this comment

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

@0xmad done can you check

Copy link
Collaborator

@ctrlc03 ctrlc03 left a comment

Choose a reason for hiding this comment

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

I'm not sure writing coordinator keys to disk is something we should be encouraging. I'd probably only let the script print to console (basically just call the cli script using pnpm: node build/ts/index.js genMaciKeyPair)

@programmerraja
Copy link
Author

@0xmad is everything fine or did i need to do any changes?

@0xmad
Copy link
Collaborator

0xmad commented Nov 1, 2024

I'm not sure writing coordinator keys to disk is something we should be encouraging. I'd probably only let the script print to console (basically just call the cli script using pnpm: node build/ts/index.js genMaciKeyPair)

@programmerraja here is a question and also there are failed ci jobs.

@programmerraja
Copy link
Author

@0xmad can you give me guidance what next I need to do

@0xmad
Copy link
Collaborator

0xmad commented Nov 1, 2024

@0xmad can you give me guidance what next I need to do

You need to fix the errors from ci job so they are passed, squash all the commits into one and answer the question @ctrlc03 asked.

@crisgarner
Copy link
Collaborator

Hello @programmerraja thanks for working on this, anything else you need assistance with?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants