Conversation
|
WalkthroughThis update introduces new CLI commands and supporting classes for managing Ethereum Development Suite (EDS) repositories. It adds commands to deploy repositories, push new releases, and retrieve repository information. Two new client classes, Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant EDSClient
participant RepositoryClient
participant Blockchain
User->>CLI: Run 'deploy repository' command
CLI->>EDSClient: newRepository(owner, name, uri)
EDSClient->>Blockchain: Deploy repository contract
Blockchain-->>EDSClient: Contract address
EDSClient->>CLI: Return RepositoryClient instance
CLI->>User: Output repository address and details
User->>CLI: Run 'repository push' command
CLI->>RepositoryClient: newRelease(distHash, metadata, version)
RepositoryClient->>Blockchain: Submit new release transaction
Blockchain-->>RepositoryClient: Transaction receipt
RepositoryClient->>CLI: Release info
CLI->>User: Output release details
User->>CLI: Run 'repository info' command
CLI->>RepositoryClient: getName(), getUri(), getLatestRelease()
RepositoryClient->>Blockchain: Read contract data
Blockchain-->>RepositoryClient: Return data
RepositoryClient->>CLI: Repository info
CLI->>User: Output repository metadata and latest release
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (7)
src/cli/commands/eds/deploy/index.ts (1)
4-5: Add missing newline at EOF to prevent POSIX-tool warningsMany editors and linters warn when a text file doesn’t end with a terminating newline.
Adding it avoids spurious diffs in future commits.-export const deployCommand = new Command("deploy").description("Deploy EDS components").addCommand(repositoryCommand); +export const deployCommand = new Command("deploy") + .description("Deploy EDS components") + .addCommand(repositoryCommand); +src/cli/commands/eds/deploy/repository.ts (1)
28-44: Usespinner.succeed()instead of silentstop()for consistencyWe display either
failorsucceedfor the deploy spinner, but the initial “Initializing clients…” spinner is simply stopped, leaving no visual feedback on success.- spinner.stop(); + spinner.succeed(chalk.green("Clients initialised"));src/cli/commands/eds/deploy/README.md (1)
80-88: Add language identifier to fenced block (markdown-lint MD040)The example output block is missing a language spec, triggering MD040. Mark it as
console(ortext) to silence the linter.-``` +```consolesrc/cli/commands/eds/repository/README.md (2)
46-56: Specify language for fenced code block (MD040)The “Release Details” output block lacks a language identifier.
-``` +```console
81-93: Specify language for fenced code block (MD040)Same issue for the “Repository Information” output block.
-``` +```consolesrc/cli/commands/eds/repository/push.ts (1)
140-152: Type safety: avoid unchecked cast ofdistHash
The castdistHash as \0x${string}`suppresses compile-time safety. After theisHex` validation you can narrow the type instead of asserting, e.g.import { Hex } from "viem"; const distHashHex = distHash as Hex; // <= still unsafe // safer const distHashHex: Hex = distHash as Hex; // relies on runtime check aboveEven better: wrap the validation into a helper that returns
Hex, eliminating the cast completely.src/cli/commands/eds/repository/info.ts (1)
73-78: Hex → UTF-8 conversion for metadata
metadatais likely returned as hex. Printing it directly yields a raw 0x-string. Convert to UTF-8 (or JSON) for usability:import { hexToString } from "viem"; ... console.log(`${chalk.blue("Metadata:")} ${hexToString(metadata)}`);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
src/cli/commands/eds/deploy/README.md(1 hunks)src/cli/commands/eds/deploy/index.ts(1 hunks)src/cli/commands/eds/deploy/repository.ts(1 hunks)src/cli/commands/eds/repository/README.md(1 hunks)src/cli/commands/eds/repository/index.ts(1 hunks)src/cli/commands/eds/repository/info.ts(1 hunks)src/cli/commands/eds/repository/push.ts(1 hunks)src/eds/EDS.ts(1 hunks)src/eds/Repository.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/cli/commands/eds/deploy/index.ts (2)
src/cli/commands/eds/repository/index.ts (1)
repositoryCommand(5-8)src/cli/commands/eds/deploy/repository.ts (1)
repositoryCommand(11-130)
src/eds/EDS.ts (2)
src/utils/EnvioGraphQLClient.ts (1)
EnvioGraphQLClient(81-1151)src/eds/Repository.ts (1)
RepositoryClient(4-69)
🪛 LanguageTool
src/cli/commands/eds/deploy/README.md
[uncategorized] ~47-~47: Loose punctuation mark.
Context: ...hql ``` ### Parameters - --owner, -o: Owner address for the repository (requi...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~57-~57: Loose punctuation mark.
Context: ... ### Environment Variables - RPC_URL: Default RPC endpoint if not provided vi...
(UNLIKELY_OPENING_PUNCTUATION)
src/cli/commands/eds/repository/README.md
[uncategorized] ~34-~34: Loose punctuation mark.
Context: ... ``` #### Parameters - --address, -a: Repository contract address (required) ...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~74-~74: Loose punctuation mark.
Context: ... ``` #### Parameters - --address, -a: Repository contract address (required) ...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~97-~97: Loose punctuation mark.
Context: ... ## Environment Variables -RPC_URL`: Default RPC endpoint if not provided vi...
(UNLIKELY_OPENING_PUNCTUATION)
🪛 markdownlint-cli2 (0.17.2)
src/cli/commands/eds/deploy/README.md
80-80: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
src/cli/commands/eds/repository/README.md
46-46: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
81-81: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
🪛 GitHub Check: lint
src/cli/commands/eds/repository/info.ts
[failure] 82-82:
Property 'distributionHash' does not exist on type '{ version: { major: bigint; minor: bigint; patch: bigint; }; sourceId: 0x${string}; metadata: 0x${string}; }'.
[failure] 81-81:
Property 'patch' does not exist on type '{ version: { major: bigint; minor: bigint; patch: bigint; }; sourceId: 0x${string}; metadata: 0x${string}; }'.
[failure] 81-81:
Property 'minor' does not exist on type '{ version: { major: bigint; minor: bigint; patch: bigint; }; sourceId: 0x${string}; metadata: 0x${string}; }'.
[failure] 81-81:
Property 'major' does not exist on type '{ version: { major: bigint; minor: bigint; patch: bigint; }; sourceId: 0x${string}; metadata: 0x${string}; }'.
🪛 GitHub Actions: CI
src/cli/commands/eds/repository/info.ts
[error] 81-81: TypeScript error TS2339: Property 'major' does not exist on type '{ version: { major: bigint; minor: bigint; patch: bigint; }; sourceId: 0x${string}; metadata: 0x${string}; }'.
🔇 Additional comments (3)
src/cli/commands/eds/repository/index.ts (1)
5-8: Looks good – concise aggregation of sub-commands
No issues spotted with therepositoryparent command.src/cli/commands/eds/deploy/repository.ts (1)
104-110: Potential unhandled promise rejection insidenewRepository
edsClient.newRepositorymight throw asynchronously inside the contract transaction (e.g., if the TX is reverted after submission). Consider wrapping the awaited call in a try/catch insideEDSClientor surfacing a clearer error message here (gas price, revert reason).src/eds/Repository.ts (1)
23-44: Clarify metadata encoding
toHex(metadata)encodes the entire string; however, on-chain contracts often expect a fixed-sizebytes32or an IPFS CID rather than an arbitrary-length bytes array. Make the expectation explicit or add length validation to prevent oversized calldata and potential “out-of-gas” failures.if (metadata.length > 256) { throw new Error("Metadata string too long; store off-chain and pass a CID instead"); }
| "-e, --envio <url>", | ||
| "Envio GraphQL endpoint URL. If not provided, http://localhost:8080/v1/graphql will be used. Alternatively INDEXER_URL environment variable may be used", | ||
| "http://localhost:8080/v1/graphql" | ||
| ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
INDEXER_URL env-var advertised but not honoured
The help-string says the Envio endpoint can be pulled from INDEXER_URL, yet we always pass options.envio (whose default is hard-coded). This makes the env-var unusable and contradicts the README.
- .option(
- "-e, --envio <url>",
- "Envio GraphQL endpoint URL. If not provided, http://localhost:8080/v1/graphql will be used. Alternatively INDEXER_URL environment variable may be used",
- "http://localhost:8080/v1/graphql"
- )
+ .option(
+ "-e, --envio <url>",
+ "Envio GraphQL endpoint URL. If not provided, INDEXER_URL env var or http://localhost:8080/v1/graphql will be used"
+ )-const envioClient = new EnvioGraphQLClient({ endpoint: options.envio });
+const envioEndpoint =
+ options.envio ??
+ process.env.INDEXER_URL ??
+ "http://localhost:8080/v1/graphql";
+const envioClient = new EnvioGraphQLClient({ endpoint: envioEndpoint });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "-e, --envio <url>", | |
| "Envio GraphQL endpoint URL. If not provided, http://localhost:8080/v1/graphql will be used. Alternatively INDEXER_URL environment variable may be used", | |
| "http://localhost:8080/v1/graphql" | |
| ) | |
| .option( | |
| "-e, --envio <url>", | |
| "Envio GraphQL endpoint URL. If not provided, INDEXER_URL env var or http://localhost:8080/v1/graphql will be used" | |
| ) | |
| // … later in the file … | |
| // Resolve the endpoint from flag, then env var, then default | |
| const envioEndpoint = | |
| options.envio ?? | |
| process.env.INDEXER_URL ?? | |
| "http://localhost:8080/v1/graphql"; | |
| const envioClient = new EnvioGraphQLClient({ endpoint: envioEndpoint }); |
🤖 Prompt for AI Agents
In src/cli/commands/eds/deploy/repository.ts around lines 20 to 23, the help
text claims the Envio GraphQL endpoint can be set via the INDEXER_URL
environment variable, but the code always uses the options.envio value with a
hardcoded default, ignoring the env-var. To fix this, modify the code to check
if the INDEXER_URL environment variable is set and use it as the Envio endpoint
URL if present; otherwise, fall back to options.envio or the default URL. This
will align the behavior with the help text and README.
| const receipt = await this.publicClient.waitForTransactionReceipt({ hash }); | ||
| if (!receipt.contractAddress) { | ||
| throw new Error("Repository deployment failed"); | ||
| } |
There was a problem hiding this comment.
Check transaction status, not only address
A receipt can contain a contractAddress even when the transaction ultimately reverts (status = 0). Add an explicit status check before returning the client.
-if (!receipt.contractAddress) {
- throw new Error("Repository deployment failed");
-}
+if (!receipt.contractAddress || receipt.status !== "success") {
+ throw new Error("Repository deployment failed");
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const receipt = await this.publicClient.waitForTransactionReceipt({ hash }); | |
| if (!receipt.contractAddress) { | |
| throw new Error("Repository deployment failed"); | |
| } | |
| const receipt = await this.publicClient.waitForTransactionReceipt({ hash }); | |
| - if (!receipt.contractAddress) { | |
| - throw new Error("Repository deployment failed"); | |
| - } | |
| + if (!receipt.contractAddress || receipt.status !== "success") { | |
| + throw new Error("Repository deployment failed"); | |
| + } |
🤖 Prompt for AI Agents
In src/eds/EDS.ts around lines 36 to 39, the code only checks for the presence
of receipt.contractAddress but does not verify the transaction status. Update
the code to also check that receipt.status is 1 (indicating success) before
proceeding. If the status is 0 or undefined, throw an error indicating the
deployment failed. This ensures the transaction was successful, not just that an
address exists.
| async newRepository(owner: Address, name: string, uri: string): Promise<RepositoryClient> { | ||
| if (!this.walletClient.account) { | ||
| throw new Error("Wallet client not found"); | ||
| } | ||
| const hash = await this.walletClient.deployContract({ | ||
| abi: OwnableRepositoryAbi, | ||
| bytecode: bytecode as `0x${string}`, | ||
| args: [owner, stringToHex(name, { size: 32 }), uri], | ||
| account: this.walletClient.account, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Validate repository name length before encoding
stringToHex(name, { size: 32 }) silently truncates / pads the string to 32 bytes. If the caller passes a name > 32 bytes, the contract will receive a truncated value without notice. Consider validating the input length and throwing early to avoid hard-to-debug on-chain state.
+if (name.length > 32) {
+ throw new Error("Repository name must not exceed 32 bytes");
+}🤖 Prompt for AI Agents
In src/eds/EDS.ts around lines 25 to 33, the repository name passed to
stringToHex is not validated for length, which can cause silent truncation or
padding to 32 bytes. Add a check before encoding to verify that the name's byte
length does not exceed 32 bytes, and throw an error if it does. This prevents
silent data corruption and ensures the contract receives the intended full name.
| const [name, uri, latestRelease] = await Promise.all([ | ||
| repositoryClient.getName(), | ||
| repositoryClient.getUri(), | ||
| repositoryClient.getLatestRelease().catch(() => null), // May not have releases yet | ||
| ]); | ||
|
|
||
| infoSpinner.succeed(chalk.green("Repository information retrieved!")); | ||
|
|
||
| // Print the repository information | ||
| console.log(chalk.bold("\n📚 Repository Information:")); | ||
| console.log(`${chalk.blue("Address:")} ${repositoryAddress}`); | ||
| console.log(`${chalk.blue("Name:")} ${name}`); | ||
| console.log(`${chalk.blue("URI:")} ${uri}`); | ||
|
|
||
| if (latestRelease) { | ||
| console.log(chalk.bold("\n📦 Latest Release:")); | ||
| console.log(`${chalk.blue("Version:")} ${latestRelease.major}.${latestRelease.minor}.${latestRelease.patch}`); | ||
| console.log(`${chalk.blue("Distribution Hash:")} ${latestRelease.distributionHash}`); | ||
| console.log(`${chalk.blue("Metadata:")} ${latestRelease.metadata}`); | ||
| } else { | ||
| console.log(chalk.yellow("\n📦 No releases found")); | ||
| } | ||
| } catch (error) { |
There was a problem hiding this comment.
Fix: incorrect property access breaks build (TS2339)
getLatestRelease() returns an object shaped like
{ version: { major, minor, patch }, sourceId, metadata }.
The current code accesses non-existent top-level fields (latestRelease.major, distributionHash, …) causing the CI failure.
- console.log(`${chalk.blue("Version:")} ${latestRelease.major}.${latestRelease.minor}.${latestRelease.patch}`);
- console.log(`${chalk.blue("Distribution Hash:")} ${latestRelease.distributionHash}`);
- console.log(`${chalk.blue("Metadata:")} ${latestRelease.metadata}`);
+ const { version, sourceId, metadata } = latestRelease;
+ console.log(
+ `${chalk.blue("Version:")} ${version.major}.${version.minor}.${version.patch}`
+ );
+ console.log(`${chalk.blue("Distribution Hash:")} ${sourceId}`);
+ console.log(`${chalk.blue("Metadata:")} ${metadata}`);This change satisfies the type checker and surfaces the correct data.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const [name, uri, latestRelease] = await Promise.all([ | |
| repositoryClient.getName(), | |
| repositoryClient.getUri(), | |
| repositoryClient.getLatestRelease().catch(() => null), // May not have releases yet | |
| ]); | |
| infoSpinner.succeed(chalk.green("Repository information retrieved!")); | |
| // Print the repository information | |
| console.log(chalk.bold("\n📚 Repository Information:")); | |
| console.log(`${chalk.blue("Address:")} ${repositoryAddress}`); | |
| console.log(`${chalk.blue("Name:")} ${name}`); | |
| console.log(`${chalk.blue("URI:")} ${uri}`); | |
| if (latestRelease) { | |
| console.log(chalk.bold("\n📦 Latest Release:")); | |
| console.log(`${chalk.blue("Version:")} ${latestRelease.major}.${latestRelease.minor}.${latestRelease.patch}`); | |
| console.log(`${chalk.blue("Distribution Hash:")} ${latestRelease.distributionHash}`); | |
| console.log(`${chalk.blue("Metadata:")} ${latestRelease.metadata}`); | |
| } else { | |
| console.log(chalk.yellow("\n📦 No releases found")); | |
| } | |
| } catch (error) { | |
| if (latestRelease) { | |
| console.log(chalk.bold("\n📦 Latest Release:")); | |
| const { version, sourceId, metadata } = latestRelease; | |
| console.log( | |
| `${chalk.blue("Version:")} ${version.major}.${version.minor}.${version.patch}` | |
| ); | |
| console.log(`${chalk.blue("Distribution Hash:")} ${sourceId}`); | |
| console.log(`${chalk.blue("Metadata:")} ${metadata}`); | |
| } else { | |
| console.log(chalk.yellow("\n📦 No releases found")); | |
| } |
🧰 Tools
🪛 GitHub Check: lint
[failure] 82-82:
Property 'distributionHash' does not exist on type '{ version: { major: bigint; minor: bigint; patch: bigint; }; sourceId: 0x${string}; metadata: 0x${string}; }'.
[failure] 81-81:
Property 'patch' does not exist on type '{ version: { major: bigint; minor: bigint; patch: bigint; }; sourceId: 0x${string}; metadata: 0x${string}; }'.
[failure] 81-81:
Property 'minor' does not exist on type '{ version: { major: bigint; minor: bigint; patch: bigint; }; sourceId: 0x${string}; metadata: 0x${string}; }'.
[failure] 81-81:
Property 'major' does not exist on type '{ version: { major: bigint; minor: bigint; patch: bigint; }; sourceId: 0x${string}; metadata: 0x${string}; }'.
🪛 GitHub Actions: CI
[error] 81-81: TypeScript error TS2339: Property 'major' does not exist on type '{ version: { major: bigint; minor: bigint; patch: bigint; }; sourceId: 0x${string}; metadata: 0x${string}; }'.
🤖 Prompt for AI Agents
In src/cli/commands/eds/repository/info.ts lines 65 to 87, the code incorrectly
accesses properties of latestRelease directly (e.g., latestRelease.major)
instead of accessing them through the version object
(latestRelease.version.major). To fix this, update all property accesses to use
latestRelease.version.major, latestRelease.version.minor,
latestRelease.version.patch, and replace latestRelease.distributionHash with
latestRelease.sourceId as appropriate, ensuring the code matches the actual
shape of the object returned by getLatestRelease().
Summary by CodeRabbit
New Features
Documentation