feat: add artifacts flag#101
Conversation
Add ability to selectively generate artifacts without genesis and private keys. - Add ArtifactFilter type supporting: genesis, keys, abis, subgraph, allocations - Add --artifacts CLI flag accepting comma-separated artifact types (default: all) - Update OutputPayload to include artifactFilter for selective output - Filter screen output to only print selected artifacts - Filter file output to only write selected artifact files - Filter kubernetes output to only create ConfigMaps/Secrets for selected artifacts - Add comprehensive validation for artifact types - Add tests for artifact filter parsing and selective generation - All output modes (screen, file, kubernetes) respect artifact selection This enables secure deployments where private keys can be omitted and allows generating only needed configuration (subgraph, allocations, ABIs) without requiring full genesis configuration.
Add comprehensive documentation for the new --artifacts CLI flag feature. Includes: - Complete user guide and examples - All artifact types explained - Kubernetes/Helm integration guide - Security considerations - Real-world use cases - Implementation details
Summary of ChangesHello @insider89, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request implements a significant enhancement to the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
To view in Slack, search for: 1762847275.587189 |
There was a problem hiding this comment.
Hey there - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `src/cli/commands/bootstrap/bootstrap.output.ts:159` </location>
<code_context>
};
const outputToScreen = (payload: OutputPayload): void => {
+ const { artifactFilter } = payload;
const genesisJson = JSON.stringify(payload.genesis, null, 2);
process.stdout.write("\n\n");
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Consider defaulting artifactFilter to a safe value if not present.
Default artifactFilter to DEFAULT_ARTIFACT_FILTER or validate its presence to prevent runtime errors if omitted.
Suggested implementation:
```typescript
import { DEFAULT_ARTIFACT_FILTER } from "./bootstrap.constants";
const outputToScreen = (payload: OutputPayload): void => {
const artifactFilter = payload.artifactFilter ?? DEFAULT_ARTIFACT_FILTER;
const genesisJson = JSON.stringify(payload.genesis, null, 2);
process.stdout.write("\n\n");
```
If `DEFAULT_ARTIFACT_FILTER` is not defined or imported from `"./bootstrap.constants"`, you will need to define it or adjust the import path accordingly.
</issue_to_address>
### Comment 2
<location> `src/cli/commands/bootstrap/bootstrap.command.ts:685-691` </location>
<code_context>
trimmed.length === 0 ? undefined : trimmed;
}
+ if (sanitizedOptions.artifacts) {
+ const trimmed = sanitizedOptions.artifacts.trim();
+ if (trimmed.length > 0) {
+ try {
+ parseArtifactList(trimmed);
+ } catch (error) {
+ throw new InvalidArgumentError(
+ error instanceof Error ? error.message : String(error)
+ );
+ }
+ }
+ sanitizedOptions.artifacts = trimmed.length === 0 ? undefined : trimmed;
+ }
+
</code_context>
<issue_to_address>
**suggestion:** Artifacts option is validated and sanitized, but error handling could be more descriptive.
Prefix the error message with the CLI option name to clarify which argument caused the failure.
```suggestion
try {
parseArtifactList(trimmed);
} catch (error) {
throw new InvalidArgumentError(
`--artifacts: ${error instanceof Error ? error.message : String(error)}`
);
}
```
</issue_to_address>
### Comment 3
<location> `src/cli/commands/bootstrap/bootstrap.output.ts:192` </location>
<code_context>
- payload.faucet,
- artifactNames.faucetPrefix
- );
+ const { artifactNames, abiArtifacts, artifactFilter } = payload;
+ const validatorSpecs = artifactFilter.keys
+ ? createValidatorSpecs(payload.validators, artifactNames.validatorPrefix)
</code_context>
<issue_to_address>
**issue (complexity):** Consider refactoring repeated conditional logic into a map of artifact types to generator functions for cleaner, more maintainable code.
You have three places where you’re doing the same thing over and over: “if this flag then build these entries, else skip.” You can collapse all of that into a single map from artifact‐kind → generator and then just iterate it.
For example, here’s how you could DRY up `outputToFile`:
```ts
type FileEntry = { path: string; description: string; contents: string };
type FileGen = (p: OutputPayload, dir: string) => FileEntry[];
const fileGenerators: Record<keyof ArtifactFilter, FileGen> = {
genesis: (p, dir) =>
p.artifactFilter.genesis
? [{
path: join(dir, `${p.artifactNames.genesisConfigMapName}.json`),
description: `${p.artifactNames.genesisConfigMapName}.json`,
contents: JSON.stringify(p.genesis, null, 2) + "\n"
}]
: [],
keys: (p, dir) =>
p.artifactFilter.keys
? [
...createValidatorSpecs(p.validators, p.artifactNames.validatorPrefix),
...createFaucetConfigSpecs(p.faucet, p.artifactNames.faucetPrefix),
...createFaucetSecretSpecs(p.faucet, p.artifactNames.faucetPrefix),
{
name: `${p.artifactNames.faucetPrefix}-enode`,
key: "enode",
value: p.faucet.enode
}
].map(spec => ({
path: join(dir, spec.name),
description: spec.name,
contents: JSON.stringify({ [spec.key]: spec.value }, null, 2) + "\n"
}))
: [],
abis: (p, dir) =>
p.artifactFilter.abis
? p.abiArtifacts.map(a => ({
path: join(dir, `${a.configMapName}.json`),
description: `${a.configMapName}.json`,
contents: a.contents
}))
: [],
staticNodes: (p, dir) => [
{
path: join(dir, `${p.artifactNames.staticNodesConfigMapName}.json`),
description: `${p.artifactNames.staticNodesConfigMapName}.json`,
contents: JSON.stringify(p.staticNodes, null, 2) + "\n"
}
],
subgraph: (p, dir) =>
p.artifactFilter.subgraph && p.subgraphHash
? [{
path: join(dir, `${p.artifactNames.subgraphConfigMapName}.json`),
description: `${p.artifactNames.subgraphConfigMapName}.json`,
contents:
JSON.stringify(
{ [SUBGRAPH_HASH_KEY]: formatSubgraphHash(p.subgraphHash) },
null,
2
) + "\n"
}]
: []
};
const outputToFile = async (p: OutputPayload): Promise<string> => {
// …setup directory…
const dir = join(OUTPUT_DIR, formatTimestampForDirectory(new Date()));
await mkdir(dir, { recursive: true });
// flatten all gens
const fileEntries = Object
.values(fileGenerators)
.flatMap(gen => gen(p, dir));
await Promise.all(fileEntries.map(e => Bun.write(e.path, e.contents)));
return dir;
};
```
You can do exactly the same in `outputToScreen` (map each key to a `void`-returning printer) and in `outputToKubernetes` (map each key to a pair of configMap/secret specs). This removes all of the repeated `if (…)` blocks and makes it trivial to add/remove artifact kinds.
</issue_to_address>
### Comment 4
<location> `src/cli/commands/bootstrap/bootstrap.artifacts-filter.ts:27` </location>
<code_context>
+ allocations: true,
+};
+
+const parseArtifactList = (input: string): ArtifactFilter => {
+ const filter = { ...DEFAULT_ARTIFACT_FILTER };
+
</code_context>
<issue_to_address>
**suggestion (review_instructions):** parseArtifactList throws a generic Error for invalid artifact kinds; consider using a more specific error type for CLI argument errors.
Using a custom error type (e.g., InvalidArgumentError) would improve clarity and integration with CLI error handling, making it easier to catch and display user-friendly messages.
<details>
<summary>Review instructions:</summary>
**Path patterns:** `**/*.ts,**/*.tsx,**/*.sol`
**Instructions:**
Proper error handling and edge case coverage must be implemented for all user input parsing functions.
</details>
</issue_to_address>
### Comment 5
<location> `src/cli/commands/bootstrap/bootstrap.artifacts-filter.ts:27` </location>
<code_context>
+ allocations: true,
+};
+
+const parseArtifactList = (input: string): ArtifactFilter => {
+ const filter = { ...DEFAULT_ARTIFACT_FILTER };
+
</code_context>
<issue_to_address>
**suggestion (review_instructions):** parseArtifactList mutates a copy of DEFAULT_ARTIFACT_FILTER before immediately overwriting it; this could be simplified for clarity.
You can initialize selectedFilter directly and remove the unused filter variable to make the function more readable and maintainable.
<details>
<summary>Review instructions:</summary>
**Path patterns:** `**/*.ts,**/*.tsx,**/*.sol`
**Instructions:**
Code readability, maintainability, and clarity must be ensured, especially for exported utility functions.
</details>
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| }; | ||
|
|
||
| const outputToScreen = (payload: OutputPayload): void => { | ||
| const { artifactFilter } = payload; |
There was a problem hiding this comment.
suggestion (bug_risk): Consider defaulting artifactFilter to a safe value if not present.
Default artifactFilter to DEFAULT_ARTIFACT_FILTER or validate its presence to prevent runtime errors if omitted.
Suggested implementation:
import { DEFAULT_ARTIFACT_FILTER } from "./bootstrap.constants";
const outputToScreen = (payload: OutputPayload): void => {
const artifactFilter = payload.artifactFilter ?? DEFAULT_ARTIFACT_FILTER;
const genesisJson = JSON.stringify(payload.genesis, null, 2);
process.stdout.write("\n\n");If DEFAULT_ARTIFACT_FILTER is not defined or imported from "./bootstrap.constants", you will need to define it or adjust the import path accordingly.
| try { | ||
| parseArtifactList(trimmed); | ||
| } catch (error) { | ||
| throw new InvalidArgumentError( | ||
| error instanceof Error ? error.message : String(error) | ||
| ); | ||
| } |
There was a problem hiding this comment.
suggestion: Artifacts option is validated and sanitized, but error handling could be more descriptive.
Prefix the error message with the CLI option name to clarify which argument caused the failure.
| try { | |
| parseArtifactList(trimmed); | |
| } catch (error) { | |
| throw new InvalidArgumentError( | |
| error instanceof Error ? error.message : String(error) | |
| ); | |
| } | |
| try { | |
| parseArtifactList(trimmed); | |
| } catch (error) { | |
| throw new InvalidArgumentError( | |
| `--artifacts: ${error instanceof Error ? error.message : String(error)}` | |
| ); | |
| } |
| allocations: true, | ||
| }; | ||
|
|
||
| const parseArtifactList = (input: string): ArtifactFilter => { |
There was a problem hiding this comment.
suggestion (review_instructions): parseArtifactList throws a generic Error for invalid artifact kinds; consider using a more specific error type for CLI argument errors.
Using a custom error type (e.g., InvalidArgumentError) would improve clarity and integration with CLI error handling, making it easier to catch and display user-friendly messages.
Review instructions:
Path patterns: **/*.ts,**/*.tsx,**/*.sol
Instructions:
Proper error handling and edge case coverage must be implemented for all user input parsing functions.
| allocations: true, | ||
| }; | ||
|
|
||
| const parseArtifactList = (input: string): ArtifactFilter => { |
There was a problem hiding this comment.
suggestion (review_instructions): parseArtifactList mutates a copy of DEFAULT_ARTIFACT_FILTER before immediately overwriting it; this could be simplified for clarity.
You can initialize selectedFilter directly and remove the unused filter variable to make the function more readable and maintainable.
Review instructions:
Path patterns: **/*.ts,**/*.tsx,**/*.sol
Instructions:
Code readability, maintainability, and clarity must be ensured, especially for exported utility functions.
There was a problem hiding this comment.
Code Review
This pull request introduces a new --artifacts flag, allowing users to selectively generate bootstrap artifacts. This is an excellent feature that enhances flexibility and security. The implementation is well-structured, with clear logic for parsing the new flag and integrating it into the screen, file, and Kubernetes output modes. The accompanying documentation is comprehensive, and the feature is well-tested. I have a couple of minor suggestions to improve code clarity and remove a small redundancy in the tests. Overall, this is a high-quality contribution.
| afterEach(() => { | ||
| process.stdout.write = originalWrite; | ||
| }); |
| const parseArtifactList = (input: string): ArtifactFilter => { | ||
| const filter = { ...DEFAULT_ARTIFACT_FILTER }; | ||
|
|
||
| if (!input || input.trim().length === 0) { | ||
| return filter; | ||
| } | ||
|
|
||
| const items = input | ||
| .split(",") | ||
| .map((item) => item.trim().toLowerCase()) | ||
| .filter((item) => item.length > 0); | ||
|
|
||
| // If items are provided, start with all false and enable only selected ones | ||
| const selectedFilter: ArtifactFilter = { | ||
| genesis: false, | ||
| keys: false, | ||
| abis: false, | ||
| subgraph: false, | ||
| allocations: false, | ||
| }; | ||
|
|
||
| for (const item of items) { | ||
| if (!ALL_ARTIFACT_KINDS.includes(item as ArtifactKind)) { | ||
| throw new Error( | ||
| `Invalid artifact kind: "${item}". Must be one of: ${ALL_ARTIFACT_KINDS.join(", ")}` | ||
| ); | ||
| } | ||
| selectedFilter[item as ArtifactKind] = true; | ||
| } | ||
|
|
||
| return selectedFilter; | ||
| }; |
There was a problem hiding this comment.
The parseArtifactList function can be slightly refactored for better readability. The filter variable is declared at the beginning but is only used within the initial if block, which can be a bit confusing for future readers. Returning directly from the if block and removing the initial variable declaration would make the control flow clearer.
const parseArtifactList = (input: string): ArtifactFilter => {
if (!input || input.trim().length === 0) {
return { ...DEFAULT_ARTIFACT_FILTER };
}
const items = input
.split(",")
.map((item) => item.trim().toLowerCase())
.filter((item) => item.length > 0);
// If items are provided, start with all false and enable only selected ones
const selectedFilter: ArtifactFilter = {
genesis: false,
keys: false,
abis: false,
subgraph: false,
allocations: false,
};
for (const item of items) {
if (!ALL_ARTIFACT_KINDS.includes(item as ArtifactKind)) {
throw new Error(
`Invalid artifact kind: "${item}". Must be one of: ${ALL_ARTIFACT_KINDS.join(", There was a problem hiding this comment.
1 issue found across 8 files
Prompt for AI agents (all 1 issues)
Understand the root cause of the following 1 issues and fix them.
<file name="src/cli/commands/bootstrap/bootstrap.selective-artifacts.test.ts">
<violation number="1" location="src/cli/commands/bootstrap/bootstrap.selective-artifacts.test.ts:21">
This duplicate afterEach block restores process.stdout.write a second time with no additional effect. Please remove the extra teardown to keep the tests tidy.</violation>
</file>
React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.
| }) as typeof process.stdout.write; | ||
| }); | ||
|
|
||
| afterEach(() => { |
There was a problem hiding this comment.
This duplicate afterEach block restores process.stdout.write a second time with no additional effect. Please remove the extra teardown to keep the tests tidy.
Prompt for AI agents
Address the following comment on src/cli/commands/bootstrap/bootstrap.selective-artifacts.test.ts at line 21:
<comment>This duplicate afterEach block restores process.stdout.write a second time with no additional effect. Please remove the extra teardown to keep the tests tidy.</comment>
<file context>
@@ -0,0 +1,293 @@
+ }) as typeof process.stdout.write;
+});
+
+afterEach(() => {
+ process.stdout.write = originalWrite;
+});
</file context>
Summary by Sourcery
Allow users to selectively generate specific bootstrap artifacts via a new
--artifactsflag, updating output logic across all modes and providing comprehensive documentation and tests for the featureNew Features:
--artifactsCLI flag to specify which artifacts (genesis, keys, abis, subgraph, allocations) to generateparseArtifactListfunction to parse and validate comma-separated artifact listsEnhancements:
Documentation:
--artifactsflag and selective artifact generation feature in README.md and add a dedicated SELECTIVE_ARTIFACTS.md guideTests: