Repository navigation
feat: rename studio validators commands - #249
Conversation
WalkthroughIntroduces a new top-level CLI namespace "localnet" and nests the existing "validators" subcommand under it. Adjusts imports to reference the new module path, updates tests and README to use the "localnet" command hierarchy, and applies a minor formatting tweak in validators.ts. No functional changes to ValidatorsAction or command signatures. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as CLI User
participant P as Program (CLI)
participant L as Localnet Command
participant V as ValidatorsAction
U->>P: run "genlayer localnet validators [args]"
P->>L: dispatch to localnet namespace
L->>V: invoke validators subcommand handler
V-->>U: return output / status
note right of L: New nesting: validators now under localnet
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
✨ Finishing Touches🧪 Generate unit tests
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. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (8)
src/commands/localnet/validators.ts (2)
108-121: Normalize stake type in updateValidator (align with createValidator).createValidator sends a numeric stake; updateValidator validates numerically but forwards a string when provided. Standardize to number to avoid RPC/type drift.
- const parsedStake = options.stake - ? parseInt(options.stake, 10) - : currentValidator.result.stake; + const parsedStake = options.stake != null + ? parseInt(options.stake, 10) + : currentValidator.result.stake; if (isNaN(parsedStake) || parsedStake < 0) { return this.failSpinner("Invalid stake value. Stake must be a positive integer."); } const updatedValidator = { address: options.address, - stake: options.stake || currentValidator.result.stake, + stake: parsedStake, provider: options.provider || currentValidator.result.provider, model: options.model || currentValidator.result.model, config: options.config ? JSON.parse(options.config) : currentValidator.result.config, };If the simulator expects strings here, keep current behavior and update createValidator to also send a string for consistency instead. Please confirm the expected RPC types.
Also applies to: 128-137
156-159: Avoid magic numbers in createRandomValidators.Expose min/max stake as flags or constants to make behavior explicit and configurable.
+const DEFAULT_MIN_STAKE = 1; +const DEFAULT_MAX_STAKE = 10; ... - params: [count, 1, 10, options.providers, options.models], + params: [count, DEFAULT_MIN_STAKE, DEFAULT_MAX_STAKE, options.providers, options.models],src/index.ts (1)
9-9: Good rewire to localnet initializer; consider a temporary deprecation alias.To smooth the breaking change, add a root-level "validators" alias that prints a deprecation notice guiding users to
localnet validators ....Example (in your command wiring, not necessarily this file):
program .command("validators") .allowUnknownOption() .action(() => { console.error("The 'validators' commands moved under 'localnet'. Use: genlayer localnet validators <subcommand>"); process.exitCode = 1; });tests/commands/localnet.test.ts (1)
3-129: Wiring tests updated to cover the new 'localnet validators' path — LGTM.End-to-end command argument plumbing remains validated via Commander parsing and method spy assertions. If you add the deprecation alias, consider a small test asserting the notice for root-level
validators.src/commands/localnet/index.ts (4)
7-14: Add a deprecation shim for root-levelvalidatorsto reduce breakageProvide a transitional alias that surfaces a clear migration message if users invoke the old entrypoint.
const validatorsCommand = localnetCommand .command("validators") .description("Manage localnet validators operations"); + + // Back-compat shim for renamed validators commands (remove in next major) + program + .command("validators") + .description("This command moved under 'localnet'.") + .allowUnknownOption(true) + .action(() => { + program.error("Use: genlayer localnet validators <subcommand>"); + });
24-31: Guard destructive delete: require explicit--allwhen no--addressPrevents accidental mass-deletion while preserving current behavior when
--allis supplied.validatorsCommand .command("delete") .description("Delete a specific validator or all validators") .option("--address <validatorAddress>", "The address of the validator to delete (omit to delete all validators)") - .action(async (options) => { - await validatorsAction.deleteValidator({ address: options.address }); - }); + .option("--all", "Delete ALL validators") + .action(async (options, cmd) => { + if (!options.address && !options.all) { + cmd.error("Refusing to delete all validators without --all. Provide --address or --all."); + } + await validatorsAction.deleteValidator({ address: options.address }); + });
82-85: Validate--configas JSON at parse time; improve shell-agnostic exampleEarly validation gives better UX; the example uses escaped double quotes to work across shells.
- .option( - "--config <config>", - `Optional JSON configuration for the validator (e.g., '{"max_tokens": 500, "temperature": 0.75}')` - ) + .addOption( + new Option("--config <config>", "Optional JSON configuration for the validator (e.g., {\"max_tokens\": 500, \"temperature\": 0.75})") + .argParser((s) => { + try { JSON.parse(s); return s; } catch { throw new InvalidArgumentError("Invalid JSON for --config"); } + }) + )Apply import update outside this hunk if not already done:
import { Command, Option, InvalidArgumentError } from "commander";
59-76: Validate and parse--countas a positive integer at the CLI boundaryShifting the
countparsing into Commander’sargParserwill turnoptions.countinto anumberinstead of astring, which is a worthwhile improvement—but it also means downstream code and tests must be updated to expect a numericcount.• In src/commands/localnet/index.ts
- Replace the
.option("--count <count>", …)call with thenew Option()+.argParser()+.default(1)form.- Update imports to include
OptionandInvalidArgumentError.• In src/commands/localnet/validators.ts
- Change the
CreateRandomValidatorsOptions.counttype fromstringtonumber.- Remove the in-method
parseInt(options.count, 10)and its NaN/<1guards (Commander will have already enforced validity).• In tests/actions/validators.test.ts
- Update all invocations of
createRandomValidators({ count: "…" })to pass a number literal, e.g.{ count: 5 }.- Adjust the “invalid count” test to assert that Commander rejects non-numeric values (e.g. invoking the CLI with
--count invalidthrows anInvalidArgumentError).Example diff for the CLI change in src/commands/localnet/index.ts:
-import { Command } from "commander"; +import { Command, Option, InvalidArgumentError } from "commander"; command - .option("--count <count>", "Number of validators to create", "1") + .addOption( + new Option("--count <count>", "Number of validators to create") + .argParser((v: string) => { + const n = parseInt(v, 10); + if (!Number.isFinite(n) || n < 1) { + throw new InvalidArgumentError("--count must be a positive integer"); + } + return n; + }) + .default(1) + )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
src/commands/localnet/index.ts(3 hunks)src/commands/localnet/validators.ts(1 hunks)src/index.ts(1 hunks)tests/actions/validators.test.ts(1 hunks)tests/commands/localnet.test.ts(7 hunks)tests/index.test.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
tests/commands/localnet.test.ts (1)
src/commands/localnet/validators.ts (1)
ValidatorsAction(30-267)
🔇 Additional comments (5)
src/commands/localnet/validators.ts (1)
268-269: No-op formatting change.Safe whitespace-only change.
tests/actions/validators.test.ts (1)
2-2: Import path update looks correct.Tests now target the relocated ValidatorsAction under localnet.
tests/index.test.ts (1)
28-30: Mocks updated to new module — OK.Keeps the CLI init test green after the namespace move.
src/commands/localnet/index.ts (2)
11-14: LGTM: nestingvalidatorsunderlocalnetThe namespacing is clear and aligns with the upcoming testnet split. No functional concerns here.
39-55: No additional stake validation needed in CLIThe validatorsAction.updateValidator method already parses and validates the incoming stake string, rejecting non-numeric or non-positive values and surface errors via failSpinner (covered by the “should log an error for invalid stake value” test in tests/actions/validators.test.ts) . Since the CLI simply forwards options.stake as a string to that layer, no extra parsing or validation is required in index.ts.
2 participants
Rename Validators Commands to Localnet Namespace
Summary
Moves existing Studio validators CLI commands under a new localnet namespace to avoid confusion with upcoming testnet validators. Commands are now accessed via
genlayer localnet validators ....Changes
🚀 Command Namespace Update
genlayer localnet validators <subcommand>get,delete,count,update,create,create-random🔧 Implementation Details
src/commands/localnetwith validators command group and actionssrc/commands/validatorsfolder📁 File Structure
🏗️ Command Architecture
localnetvalidatorsget --address <address>delete [--address <address>]countupdate <address> [--stake <n>] [--provider <name>] [--model <name>] [--config <json>]create [--stake <n>] [--provider <name>] [--model <name>] [--config <json>]create-random --count <n> [--providers <...>] [--models <...>]🧪 Testing
tests/commands/localnet.test.tstests/actions/validators.test.ts(import path)localnetinitializer🔧 Usage
✨ Code Quality
BaseAction)🔗 Dependencies
🛡️ Backward Compatibility
validatorscommands replaced bylocalnet validatorsTesting: ✅ All tests pass
Type Safety: ✅ TypeScript maintained
UX: ✅ Clear namespace separation for localnet vs testnet
Summary by CodeRabbit
New Features
Refactor
Tests
Documentation