feat: add quickstart command for node setup#510
Merged
Conversation
Add the quickstart Cobra command (cmd/initiad/quickstart/cmd.go) with QuickstartConfig struct, flag definitions, input validation, and stub functions for genesis download, config application, and sync setup. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…state sync) Implement downloadGenesis, downloadAddrbook, and setupStateSync in network.go with real HTTP downloads from Polkachu endpoints, and remove the corresponding stubs from cmd.go. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add config.go: config.toml/app.toml modification using native CometBFT and Cosmos SDK config structs (preserves comments and formatting) - Add interactive.go: guided interactive setup with prompts - Add snapshot.go: snapshot download and lz4 extraction - Rewrite network.go: atomic file writes, HTTP timeouts, response validation, empty hash/peer checks, body size limits, native config structs for statesync - Fix flag defaults: remove incorrect defaults, rename memiavl -> memiavl-enable to avoid collision with app.toml section - Register quickstart command in root.go Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…for non-custom pruning Fixes initiad start failing with "invalid argument for --pruning-interval" when pruning mode is not custom. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Show descriptive labels for tx indexing options: - disable (instead of null) - default with IBC keys description - custom with explanation Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… fallback - Initialize appConfig with initiad's default values before viper unmarshal, so fields not modified by quickstart retain their defaults (min-gas-prices, etc.) - Add addrbook.json RPC fallback: when Polkachu download fails, rebuild addrbook from /net_info peers and /status node identity Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…structs viper's default Unmarshal doesn't handle mapstructure:",squash" tags on embedded structs (like serverconfig.BaseConfig), causing fields like minimum-gas-prices and query-gas-limit to be zeroed out. Fix by passing Squash:true DecoderConfigOption. Remove defaultAppConfig workaround — viper now correctly loads all existing values from app.toml. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Same viper unmarshal fix as app.toml — ensure CometBFT config embedded structs are properly decoded. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…nfig Move InitiaAppConfig struct, AppConfigTemplate(), InitAppConfig(), and InitTendermintConfig() from main package to cmd/initiad/appconfig/ so both main and quickstart can import them without duplication. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- snapshot.go: fix command injection by using exec.Command with separate args instead of bash -c interpolation; validate URL scheme; check all required tools (curl, lz4, tar) - config.go: only set TxIndex.RetainHeight when indexing is enabled - network.go: add 500MB download size limit; use CometBFT pex.AddrBook API for addrbook fallback instead of manually crafting JSON - network.go: move loadCometConfig to config.go with other config loaders Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Use provider-agnostic naming for the state sync peer variable and related functions. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Move Polkachu-specific URLs, API calls, and file download logic to cmd/initiad/quickstart/providers/polkachu.go. network.go now only contains provider-agnostic orchestration and RPC fallback logic. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- cmd_test.go: 10 tests for readFromFlags validation (valid configs, error cases, default tx indexing keys) - config_test.go: 4 tests for applyConfigToml and applyAppToml - interactive_test.go: 8 tests for prompt helpers (choice, string, yes/no, choice with labels) - network_test.go: 2 tests for buildAddrbookFromRPC and parsePortFromListenAddr - snapshot_test.go: 3 tests for URL validation and splitAndTrim - providers/polkachu_test.go: 5 tests for fetch functions and file download Total: 28 tests, all using mock HTTP servers (no real network requests). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add assertions in TestApplyConfigToml and TestApplyAppToml to confirm that fields not modified by quickstart (min-gas-prices, mempool.max-txs, query-gas-limit, grpc settings, etc.) retain their original values. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
MemIAVL is not compatible with snapshot sync. In interactive mode, skip the MemIAVL question when snapshot is selected. In flag mode, reject --memiavl-enable with --sync-method=snapshot. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Covers interactive mode, flag mode examples (mainnet/testnet, statesync/ snapshot, custom pruning/indexing), flags reference, and what the command does under the hood. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a new interactive and flag-driven Changes
Sequence DiagramsequenceDiagram
actor User
participant Quickstart as "QuickstartCmd\n(command)"
participant Polkachu as "Polkachu Provider\n(HTTP)"
participant RPC as "RPC Node\n(net_info/status)"
participant Config as "ConfigManager\n(appconfig + viper)"
participant Sync as "SyncManager\n(statesync/snapshot)"
User->>Quickstart: run quickstart (flags or interactive)
Quickstart->>Polkachu: downloadGenesis()
Polkachu-->>Quickstart: genesis.json
Quickstart->>Polkachu: downloadAddrbook()
Polkachu-->>Quickstart: addrbook.json or fail
alt addrbook download failed
Quickstart->>RPC: query /net_info & /status
RPC-->>Quickstart: peers + node info
Quickstart->>Quickstart: build addrbook.json
end
Quickstart->>Config: applyConfigToml(cfg)
Config-->>Quickstart: config.toml updated
Quickstart->>Config: applyAppToml(cfg)
Config-->>Quickstart: app.toml updated
alt state-sync
Quickstart->>Polkachu: FetchLatestHeight & FetchBlockHash
Polkachu-->>Quickstart: trustHeight & trustHash
Quickstart->>Config: setupStateSync(trustHeight, trustHash, peer)
else snapshot
Quickstart->>Sync: downloadAndExtractSnapshot(url)
Sync-->>Quickstart: snapshot extracted
end
Quickstart-->>User: Quickstart setup complete
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (8)
cmd/initiad/appconfig/appconfig.go (2)
21-28: Addmapstructure:",squash"tag to embedded struct for explicit flattening.The embedded
serverconfig.Configlacks amapstructure:",squash"tag. WhileloadAppConfiginquickstart/config.gosetsdc.Squash = trueduring unmarshal, adding the tag to the struct definition makes the flattening behavior explicit and ensures correct unmarshaling even when consumers don't configure Squash manually.type InitiaAppConfig struct { - serverconfig.Config + serverconfig.Config `mapstructure:",squash"` ABCIPP abcipp.AppConfig `mapstructure:"abcipp"`🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/initiad/appconfig/appconfig.go` around lines 21 - 28, The embedded serverconfig.Config in the InitiaAppConfig struct should explicitly declare the mapstructure squash tag to ensure fields are flattened during unmarshalling; update the InitiaAppConfig definition to add the struct tag `mapstructure:",squash"` to the embedded serverconfig.Config so that unmarshalling (e.g., in loadAppConfig) works even if Squash isn't set on the decoder.
66-66: Typo: "turning" → "tuning".- // performance turning configs + // performance tuning configs🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/initiad/appconfig/appconfig.go` at line 66, Typo in the comment "performance turning configs" in appconfig.go: change the comment text to "performance tuning configs" (or "performance tuning configuration") to correct the spelling; locate the comment string in cmd/initiad/appconfig/appconfig.go and update it accordingly.docs/quickstart.md (1)
133-141: Minor terminology inconsistency: "disable" vs "null".Line 135 uses "disable" but the flags reference (Line 113) and command examples use
null. Consider using consistent terminology:-- **disable** (`null`): no transaction indexing +- **null**: disables transaction indexing🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/quickstart.md` around lines 133 - 141, The documentation uses inconsistent terminology for the no-indexing preset under the "TX Indexing Presets" section: change the bullet label "disable" to match the flags and examples by using `null` (or explicitly document both by saying "`null` (disable)" if you want to support both names); update the first bullet to read `null` and ensure the description still states "no transaction indexing" so references to the preset across the docs (e.g., command examples and flags) are consistent with the `null` token.cmd/initiad/quickstart/providers/polkachu_test.go (1)
13-28: Consider adding test for emptylast_block_heightresponse.The current test only covers the happy path. Consider adding a test case for when the RPC returns an empty
last_block_heightstring to ensure proper error handling (especially relevant if the fix suggested forFetchLatestHeightis applied).func TestFetchLatestHeight_EmptyHeight(t *testing.T) { srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusOK) _, _ = w.Write([]byte(`{"result":{"response":{"last_block_height":""}}}`)) })) defer srv.Close() origClient := HTTPClient HTTPClient = srv.Client() defer func() { HTTPClient = origClient }() _, err := FetchLatestHeight(srv.URL) require.Error(t, err) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/initiad/quickstart/providers/polkachu_test.go` around lines 13 - 28, Add a unit test that verifies FetchLatestHeight returns an error when the RPC response contains an empty last_block_height string: create a new test function (e.g., TestFetchLatestHeight_EmptyHeight) that spins up an httptest.Server returning {"result":{"response":{"last_block_height":""}}}, swaps in srv.Client() for the package-level HTTPClient and defers restoring it, calls FetchLatestHeight(srv.URL), and asserts that an error is returned; reference the existing TestFetchLatestHeight pattern to match setup/teardown and HTTPClient usage.cmd/initiad/quickstart/network_test.go (1)
45-85: Consider adding IPv6 test case.The table tests cover common cases well. Consider adding an IPv6 test case if IPv6 listen addresses are expected to be supported:
{ name: "tcp scheme with IPv6", input: "tcp://[::1]:26656", expected: 26656, },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/initiad/quickstart/network_test.go` around lines 45 - 85, Add an IPv6 table-driven test case to TestParsePortFromListenAddr so parsePortFromListenAddr is validated for IPv6 bracketed addresses; specifically, update the tests slice inside TestParsePortFromListenAddr to include a case named like "tcp scheme with IPv6" with input "tcp://[::1]:26656" and expected 26656 (expectErr false) to ensure the function correctly parses bracketed IPv6 listen addresses.cmd/initiad/quickstart/network.go (1)
82-84: Consider using a different secondary RPC server for redundancy.State sync requires two RPC servers for redundancy, but currently the same URL is duplicated. If the primary Polkachu RPC is down, state sync will fail. Consider using a fallback RPC (e.g., from
rpcEndpoints).+ // Use Polkachu RPC as primary, public RPC as secondary for redundancy + secondaryRPC := rpcEndpoints[network] // assuming network is accessible or passed as param cfg.StateSync.RPCServers = []string{rpc, rpc}This would require passing the network parameter to
applyStateSync. Alternatively, document why the same RPC is used twice if this is intentional.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/initiad/quickstart/network.go` around lines 82 - 84, The StateSync RPC list is populated with the same URL twice (cfg.StateSync.RPCServers = []string{rpc, rpc}), which removes redundancy; update applyStateSync to accept the network (or rpcEndpoints) so you can select a different secondary RPC and set cfg.StateSync.RPCServers = []string{primaryRPC, fallbackRPC} (e.g., pick a different entry from rpcEndpoints) or, if you intentionally want duplicates, add a comment documenting that choice; modify the applyStateSync signature and its callers to pass the network (or provide a fallbackRPC) and use that to populate cfg.StateSync.RPCServers.cmd/initiad/quickstart/config.go (1)
78-82: Consider preserving existing API settings when--api-addressis not provided.Currently, if
APIAddressis empty, existingAPI.EnableandAPI.Addressvalues are preserved (since only theifblock modifies them). This behavior is correct, but the comment could clarify that the API is only modified when explicitly configured.// REST API + // Only modify API settings when explicitly configured via --api-address if cfg.APIAddress != "" { appCfg.API.Enable = true appCfg.API.Address = cfg.APIAddress }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/initiad/quickstart/config.go` around lines 78 - 82, Clarify in a comment near the REST API handling (the block that checks cfg.APIAddress and sets appCfg.API.Enable and appCfg.API.Address) that the API configuration is only modified when --api-address (cfg.APIAddress) is provided and otherwise existing appCfg.API.Enable and appCfg.API.Address are intentionally preserved; update the inline comment above the if (cfg.APIAddress != "") { ... } block to state this explicit preservation behavior and that the flag is the only trigger for changes.cmd/initiad/quickstart/snapshot.go (1)
26-64: Consider adding a timeout or context for long-running downloads.The snapshot download pipeline has no timeout mechanism. If the remote server becomes unresponsive,
curl(and therefore the entire quickstart command) will hang indefinitely. Since snapshots can be large and downloads can take a while, a very generous timeout (or context propagation with cancellation) would improve the user experience.The security measures (URL scheme validation, tool existence checks, avoiding shell injection) are well implemented.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/initiad/quickstart/snapshot.go` around lines 26 - 64, Add a cancellable context with a generous timeout and use exec.CommandContext instead of exec.Command for the pipeline processes so the download won’t hang indefinitely; create ctx, cancel := context.WithTimeout(context.Background(), <duration>) (or accept a passed-in context/configurable timeout), defer cancel(), then replace curl := exec.Command(...), lz4 := exec.Command(...), tar := exec.Command(...) with curl := exec.CommandContext(ctx, ...), lz4 := exec.CommandContext(ctx, ...), tar := exec.CommandContext(ctx, ...); ensure you check ctx.Err() or return a clear timeout error if the context times out/cancels after waiting on the processes (references: the curl, lz4, tar command variables and the curl.Start/Wait, lz4.Start/Wait, tar.Start/Wait flow).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmd/initiad/quickstart/cmd.go`:
- Around line 117-118: The flags flagPruningKeepRecent and flagPruningInterval
default to "0", so readFromFlags receives "0" even when the user didn't set them
and validation in readFromFlags (pruning validation) will incorrectly accept a
custom pruning config; change behavior by either making the flag defaults empty
strings (set default to "" when declaring flags flagPruningKeepRecent and
flagPruningInterval) and update usage docs to mark them required for
pruning=custom, or keep defaults but update readFromFlags to check
cmd.Flags().Changed(flagPruningKeepRecent) and
cmd.Flags().Changed(flagPruningInterval) when pruning == "custom" (and return an
error if either was not explicitly set) to ensure missing values are detected.
In `@cmd/initiad/quickstart/interactive.go`:
- Around line 91-100: When cfg.TxIndexing == TxIndexCustom in the interactive
flow, validate that the entered keys string is non-empty after
splitting/trimming and return an error (same behavior as readFromFlags) instead
of assigning an empty slice; update the branch handling promptString(...) and
splitAndTrim(...) to check that splitAndTrim(keysStr) yields at least one key
and return a clear error if not, referencing cfg.TxIndexing, TxIndexCustom,
promptString, splitAndTrim and mirroring the flag-path validation in
readFromFlags.
In `@cmd/initiad/quickstart/network.go`:
- Around line 161-167: parsePortFromListenAddr block passes
net.ParseIP(peer.RemoteIP) directly to p2p.NewNetAddressIPPort which can receive
nil for invalid IP strings; validate the parsed IP first by calling parsedIP :=
net.ParseIP(peer.RemoteIP) and if parsedIP == nil skip or handle the peer (e.g.,
continue and/or log the invalid peer.RemoteIP), then call
p2p.NewNetAddressIPPort(parsedIP, port) and set addr.ID =
p2p.ID(peer.NodeInfo.ID) before appending to addrs.
In `@cmd/initiad/quickstart/providers/polkachu.go`:
- Around line 87-89: The code currently uses fmt.Sscanf on
result.Result.Response.LastBlockHeight which silently yields height=0 and nil
for an empty string; update the function (the block that reads LastBlockHeight)
to explicitly check if result.Result.Response.LastBlockHeight is empty before
calling fmt.Sscanf and return a descriptive error if empty, otherwise parse into
height as now; ensure the returned error propagates so callers in network.go
(which compute max(latestHeight-2000, 1)) can detect RPC/response issues rather
than treating 0 as a valid height.
---
Nitpick comments:
In `@cmd/initiad/appconfig/appconfig.go`:
- Around line 21-28: The embedded serverconfig.Config in the InitiaAppConfig
struct should explicitly declare the mapstructure squash tag to ensure fields
are flattened during unmarshalling; update the InitiaAppConfig definition to add
the struct tag `mapstructure:",squash"` to the embedded serverconfig.Config so
that unmarshalling (e.g., in loadAppConfig) works even if Squash isn't set on
the decoder.
- Line 66: Typo in the comment "performance turning configs" in appconfig.go:
change the comment text to "performance tuning configs" (or "performance tuning
configuration") to correct the spelling; locate the comment string in
cmd/initiad/appconfig/appconfig.go and update it accordingly.
In `@cmd/initiad/quickstart/config.go`:
- Around line 78-82: Clarify in a comment near the REST API handling (the block
that checks cfg.APIAddress and sets appCfg.API.Enable and appCfg.API.Address)
that the API configuration is only modified when --api-address (cfg.APIAddress)
is provided and otherwise existing appCfg.API.Enable and appCfg.API.Address are
intentionally preserved; update the inline comment above the if (cfg.APIAddress
!= "") { ... } block to state this explicit preservation behavior and that the
flag is the only trigger for changes.
In `@cmd/initiad/quickstart/network_test.go`:
- Around line 45-85: Add an IPv6 table-driven test case to
TestParsePortFromListenAddr so parsePortFromListenAddr is validated for IPv6
bracketed addresses; specifically, update the tests slice inside
TestParsePortFromListenAddr to include a case named like "tcp scheme with IPv6"
with input "tcp://[::1]:26656" and expected 26656 (expectErr false) to ensure
the function correctly parses bracketed IPv6 listen addresses.
In `@cmd/initiad/quickstart/network.go`:
- Around line 82-84: The StateSync RPC list is populated with the same URL twice
(cfg.StateSync.RPCServers = []string{rpc, rpc}), which removes redundancy;
update applyStateSync to accept the network (or rpcEndpoints) so you can select
a different secondary RPC and set cfg.StateSync.RPCServers =
[]string{primaryRPC, fallbackRPC} (e.g., pick a different entry from
rpcEndpoints) or, if you intentionally want duplicates, add a comment
documenting that choice; modify the applyStateSync signature and its callers to
pass the network (or provide a fallbackRPC) and use that to populate
cfg.StateSync.RPCServers.
In `@cmd/initiad/quickstart/providers/polkachu_test.go`:
- Around line 13-28: Add a unit test that verifies FetchLatestHeight returns an
error when the RPC response contains an empty last_block_height string: create a
new test function (e.g., TestFetchLatestHeight_EmptyHeight) that spins up an
httptest.Server returning {"result":{"response":{"last_block_height":""}}},
swaps in srv.Client() for the package-level HTTPClient and defers restoring it,
calls FetchLatestHeight(srv.URL), and asserts that an error is returned;
reference the existing TestFetchLatestHeight pattern to match setup/teardown and
HTTPClient usage.
In `@cmd/initiad/quickstart/snapshot.go`:
- Around line 26-64: Add a cancellable context with a generous timeout and use
exec.CommandContext instead of exec.Command for the pipeline processes so the
download won’t hang indefinitely; create ctx, cancel :=
context.WithTimeout(context.Background(), <duration>) (or accept a passed-in
context/configurable timeout), defer cancel(), then replace curl :=
exec.Command(...), lz4 := exec.Command(...), tar := exec.Command(...) with curl
:= exec.CommandContext(ctx, ...), lz4 := exec.CommandContext(ctx, ...), tar :=
exec.CommandContext(ctx, ...); ensure you check ctx.Err() or return a clear
timeout error if the context times out/cancels after waiting on the processes
(references: the curl, lz4, tar command variables and the curl.Start/Wait,
lz4.Start/Wait, tar.Start/Wait flow).
In `@docs/quickstart.md`:
- Around line 133-141: The documentation uses inconsistent terminology for the
no-indexing preset under the "TX Indexing Presets" section: change the bullet
label "disable" to match the flags and examples by using `null` (or explicitly
document both by saying "`null` (disable)" if you want to support both names);
update the first bullet to read `null` and ensure the description still states
"no transaction indexing" so references to the preset across the docs (e.g.,
command examples and flags) are consistent with the `null` token.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: bc5fb812-9a22-4f44-ab05-84d5b81a0506
📒 Files selected for processing (16)
cmd/initiad/appconfig/appconfig.gocmd/initiad/config.gocmd/initiad/quickstart/cmd.gocmd/initiad/quickstart/cmd_test.gocmd/initiad/quickstart/config.gocmd/initiad/quickstart/config_test.gocmd/initiad/quickstart/interactive.gocmd/initiad/quickstart/interactive_test.gocmd/initiad/quickstart/network.gocmd/initiad/quickstart/network_test.gocmd/initiad/quickstart/providers/polkachu.gocmd/initiad/quickstart/providers/polkachu_test.gocmd/initiad/quickstart/snapshot.gocmd/initiad/quickstart/snapshot_test.gocmd/initiad/root.godocs/quickstart.md
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Wrap bare URL in angle brackets. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Bound uint64 -> int64 conversion with min(value, math.MaxInt64). Also run go mod tidy to mark mapstructure as direct dependency. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/quickstart.md`:
- Around line 125-126: The wording currently implies both "genesis.json" and
"addrbook.json" use an RPC fallback, but only "addrbook.json" has an explicit
RPC `/net_info` fallback; update the two bullet points so they accurately
reflect behavior—for example, keep "Downloads genesis.json from Polkachu"
(remove the RPC fallback clause unless a genesis RPC fallback exists) and change
the second bullet to "Downloads addrbook.json from Polkachu (with RPC
`/net_info` fallback)"; ensure the terms "genesis.json", "addrbook.json", and
"/net_info" are used so readers can map this text to the implemented behavior.
- Around line 37-46: The example command exposes RPC publicly via the
--rpc-address=tcp://0.0.0.0:26657; update the quickstart example (the initiad
quickstart command) to use a safe default such as
--rpc-address=tcp://127.0.0.1:26657 (or document a clear, prominent warning and
firewall/hardening steps if you intend to show a public binding), and optionally
mirror that guidance for --api-address if applicable so the example does not
recommend exposing services on 0.0.0.0 by default.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
There was a problem hiding this comment.
🧹 Nitpick comments (2)
cmd/initiad/quickstart/config.go (2)
80-83: Make API assignment explicit in the empty-address case for deterministic reruns.When
cfg.APIAddressis empty, existingapp.tomlAPI state is preserved. Re-running quickstart can keep API enabled from a prior config unexpectedly.Proposed deterministic behavior
// REST API if cfg.APIAddress != "" { appCfg.API.Enable = true appCfg.API.Address = cfg.APIAddress + } else { + appCfg.API.Enable = false + appCfg.API.Address = "" }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/initiad/quickstart/config.go` around lines 80 - 83, The current logic only sets appCfg.API.Enable and appCfg.API.Address when cfg.APIAddress != "", which preserves prior API state when cfg.APIAddress is empty; change it so the API fields are explicitly set in both branches: if cfg.APIAddress != "" set appCfg.API.Enable = true and appCfg.API.Address = cfg.APIAddress, otherwise set appCfg.API.Enable = false and clear appCfg.API.Address (e.g., empty string) so rerunning quickstart produces deterministic API configuration; update the branch around cfg.APIAddress handling in config.go to apply these explicit assignments.
31-43: Add a defensivedefaultbranch forcfg.TxIndexingto prevent silent partial writes.Upstream validation exists, but this function can still be called with unexpected values. Right now, an invalid value can leave
TxIndex.Indexerunchanged while still changing retain-height.Proposed hardening
// TX indexing + indexingEnabled := false switch cfg.TxIndexing { case TxIndexNull: cmtCfg.TxIndex.Indexer = "null" case TxIndexDefault, TxIndexCustom: cmtCfg.TxIndex.Indexer = "kv" + indexingEnabled = true + default: + return fmt.Errorf("invalid tx indexing mode %q", cfg.TxIndexing) } // TX index retain-height follows min-retain-blocks (only when indexing is enabled) - if cfg.TxIndexing != TxIndexNull { + if indexingEnabled { cmtCfg.TxIndex.RetainHeight = int64(min(cfg.MinRetainBlocks, uint64(math.MaxInt64))) //nolint:gosec // bounded by min() } else { cmtCfg.TxIndex.RetainHeight = 0 }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/initiad/quickstart/config.go` around lines 31 - 43, The switch on cfg.TxIndexing needs a defensive default to avoid leaving cmtCfg.TxIndex.Indexer unchanged for unknown values and causing partial writes: add a default case in the switch that sets cmtCfg.TxIndex.Indexer = "null" (and optionally log a warning), and change the subsequent retain-height check to only set cmtCfg.TxIndex.RetainHeight when cfg.TxIndexing is one of the explicit enabled values (e.g., cfg.TxIndexing == TxIndexDefault || cfg.TxIndexing == TxIndexCustom) instead of cfg.TxIndexing != TxIndexNull so unknown values are treated as disabled; reference cfg.TxIndexing, TxIndexNull, TxIndexDefault, TxIndexCustom, cmtCfg.TxIndex.Indexer, and cmtCfg.TxIndex.RetainHeight.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@cmd/initiad/quickstart/config.go`:
- Around line 80-83: The current logic only sets appCfg.API.Enable and
appCfg.API.Address when cfg.APIAddress != "", which preserves prior API state
when cfg.APIAddress is empty; change it so the API fields are explicitly set in
both branches: if cfg.APIAddress != "" set appCfg.API.Enable = true and
appCfg.API.Address = cfg.APIAddress, otherwise set appCfg.API.Enable = false and
clear appCfg.API.Address (e.g., empty string) so rerunning quickstart produces
deterministic API configuration; update the branch around cfg.APIAddress
handling in config.go to apply these explicit assignments.
- Around line 31-43: The switch on cfg.TxIndexing needs a defensive default to
avoid leaving cmtCfg.TxIndex.Indexer unchanged for unknown values and causing
partial writes: add a default case in the switch that sets
cmtCfg.TxIndex.Indexer = "null" (and optionally log a warning), and change the
subsequent retain-height check to only set cmtCfg.TxIndex.RetainHeight when
cfg.TxIndexing is one of the explicit enabled values (e.g., cfg.TxIndexing ==
TxIndexDefault || cfg.TxIndexing == TxIndexCustom) instead of cfg.TxIndexing !=
TxIndexNull so unknown values are treated as disabled; reference cfg.TxIndexing,
TxIndexNull, TxIndexDefault, TxIndexCustom, cmtCfg.TxIndex.Indexer, and
cmtCfg.TxIndex.RetainHeight.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 08bcaaed-9d0f-4cb0-850a-6f630378cbd8
⛔ Files ignored due to path filters (1)
go.modis excluded by!**/*.mod
📒 Files selected for processing (1)
cmd/initiad/quickstart/config.go
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #510 +/- ##
==========================================
- Coverage 38.09% 38.09% -0.01%
==========================================
Files 325 325
Lines 30819 30817 -2
==========================================
- Hits 11742 11740 -2
Misses 17184 17184
Partials 1893 1893
🚀 New features to boost your workflow:
|
Add nolint:gosec directives for safe integer conversions in ledger, cluster, connector. Apply gofmt and markdownlint auto-fixes. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- cmd.go: use cmd.Flags().Changed() for pruning custom validation to distinguish default "0" from user not providing the flag - interactive.go: validate custom tx indexing keys are non-empty - network.go: validate net.ParseIP result before use - providers/polkachu.go: check empty LastBlockHeight from RPC Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
cmd/initiad/quickstart/network.go (1)
214-224: Consider validating IP after DNS lookup.While DNS resolvers typically return valid IPs,
net.ParseIP(ips[0])at line 220 could theoretically returnnil. For consistency with the validation pattern used infetchNetInfoPeerAddrs, consider adding a nil check.Suggested fix
ip := net.ParseIP(host) if ip == nil { ips, err := net.LookupHost(host) if err != nil || len(ips) == 0 { return nil, fmt.Errorf("failed to resolve RPC hostname %q: %w", host, err) } ip = net.ParseIP(ips[0]) + if ip == nil { + return nil, fmt.Errorf("failed to parse resolved IP %q for host %q", ips[0], host) + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/initiad/quickstart/network.go` around lines 214 - 224, After DNS lookup, net.ParseIP(ips[0]) can still return nil; update the code that computes ip (the block using net.ParseIP(host), net.LookupHost, and net.ParseIP(ips[0])) to check whether the parsed IP is nil after the lookup and return an error if so. Specifically, validate ip after calling net.ParseIP(ips[0]) (same pattern used in fetchNetInfoPeerAddrs) before creating the p2p.NewNetAddressIPPort and assigning addr.ID, and return a clear fmt.Errorf when the parsed IP is nil.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmd/initiad/quickstart/network.go`:
- Around line 124-125: The call to book.Save() ignores its returned error;
change it to capture the error (e.g., err := book.Save()) and if non-nil return
or propagate that error instead of unconditionally returning nil so failures
(disk/perm issues) surface; update the surrounding function to return the error
and add/import fmt or errors for wrapping if needed.
In `@docs/quickstart.md`:
- Around line 125-126: The docs claim that genesis.json has an RPC `/genesis`
fallback but the code in downloadGenesis (network.go) only calls
providers.DownloadFile and has no RPC fallback (only downloadAddrbook uses
buildAddrbookFromRPC), so either implement the RPC fallback in downloadGenesis
or update docs/quickstart.md to remove/adjust the "with RPC `/genesis` fallback"
text; specifically, either add the RPC fallback logic mirroring
buildAddrbookFromRPC inside downloadGenesis or change the line in
docs/quickstart.md to state that genesis.json is downloaded from Polkachu
without RPC fallback to match the current behavior.
---
Nitpick comments:
In `@cmd/initiad/quickstart/network.go`:
- Around line 214-224: After DNS lookup, net.ParseIP(ips[0]) can still return
nil; update the code that computes ip (the block using net.ParseIP(host),
net.LookupHost, and net.ParseIP(ips[0])) to check whether the parsed IP is nil
after the lookup and return an error if so. Specifically, validate ip after
calling net.ParseIP(ips[0]) (same pattern used in fetchNetInfoPeerAddrs) before
creating the p2p.NewNetAddressIPPort and assigning addr.ID, and return a clear
fmt.Errorf when the parsed IP is nil.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 2db4c619-1483-426f-95d6-2e8751e34b69
⛔ Files ignored due to path filters (1)
.github/workflows/lint.ymlis excluded by!**/*.yml
📒 Files selected for processing (23)
abcipp/proposals.gocmd/initiad/appconfig/appconfig.gocmd/initiad/quickstart/cmd.gocmd/initiad/quickstart/config.gocmd/initiad/quickstart/interactive.gocmd/initiad/quickstart/network.gocmd/initiad/quickstart/providers/polkachu.gocrypto/ledger/usbwallet/ledger.godocs/quickstart.mdintegration-tests/e2e/benchmark/load.gointegration-tests/e2e/cluster/cluster.gointegration-tests/e2e/cluster/toml.gox/distribution/types/dec_pool.gox/distribution/types/pool.gox/distribution/types/validator.gox/dynamic-fee/keeper/keeper.gox/gov/keeper/proposal.gox/gov/types/proposal.gox/ibc/testing/chain.gox/ibc/testing/solomachine.gox/move/types/connector.gox/move/types/env.gox/mstaking/types/delegation.go
✅ Files skipped from review due to trivial changes (17)
- x/ibc/testing/solomachine.go
- x/ibc/testing/chain.go
- integration-tests/e2e/cluster/toml.go
- abcipp/proposals.go
- x/distribution/types/dec_pool.go
- x/distribution/types/validator.go
- x/distribution/types/pool.go
- integration-tests/e2e/cluster/cluster.go
- x/dynamic-fee/keeper/keeper.go
- x/gov/types/proposal.go
- x/move/types/env.go
- x/gov/keeper/proposal.go
- x/mstaking/types/delegation.go
- integration-tests/e2e/benchmark/load.go
- x/move/types/connector.go
- crypto/ledger/usbwallet/ledger.go
- cmd/initiad/quickstart/cmd.go
🚧 Files skipped from review as they are similar to previous changes (3)
- cmd/initiad/appconfig/appconfig.go
- cmd/initiad/quickstart/interactive.go
- cmd/initiad/quickstart/config.go
When Polkachu genesis download fails, fall back to fetching from the public RPC /genesis endpoint. Extracts .result.genesis from the RPC response wrapper. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… between integer types' Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmd/initiad/quickstart/network.go`:
- Around line 91-97: The code currently checks for an existing peer using
strings.Contains on cfg.P2P.PersistentPeers which can yield false positives;
instead split cfg.P2P.PersistentPeers on ',' into entries, trim spaces of each
entry and compare each entry exactly to stateSyncPeer (use equality), and only
append stateSyncPeer if no exact match is found; update the block around
cfg.P2P.PersistentPeers and stateSyncPeer to perform this normalized exact-match
check before appending.
- Around line 152-159: The current loop calling book.AddAddress(peerAddr,
srcAddr) ignores rejections and always calls book.Save() and returns nil; change
it to track whether any AddAddress call succeeded (e.g., acceptedCount or
accepted bool) by treating err==nil as accepted, call book.Save() only if at
least one address was accepted, and if none were accepted return a non-nil error
indicating fallback addrbook creation failed so Quickstart does not proceed with
an unusable addrbook.
- Around line 86-87: The current state-sync config sets cfg.StateSync.RPCServers
to the same RPC twice ([]string{rpc, rpc}); update setupStateSync (and the
callsite that provides pc.StateSyncRPC) to supply two distinct RPC endpoints:
either gather multiple unique RPC URLs from providers (deduplicate
pc.StateSyncRPC values) or fall back to a curated list of independent public RPC
endpoints, then set cfg.StateSync.RPCServers = []string{primaryRPC,
secondaryRPC} only when primaryRPC != secondaryRPC and enable
cfg.StateSync.Enable accordingly so CometBFT gets two distinct servers for
light-client verification.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: c67ca781-f739-45cb-93d4-ea9c1ca46feb
⛔ Files ignored due to path filters (1)
go.modis excluded by!**/*.mod
📒 Files selected for processing (2)
cmd/initiad/quickstart/config.gocmd/initiad/quickstart/network.go
✅ Files skipped from review due to trivial changes (1)
- cmd/initiad/quickstart/config.go
Instead of duplicating the Polkachu RPC, use the official Initia RPC as the second rpc_server for state sync redundancy. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
pex.AddAddress rejects short node IDs and non-routable IPs. Use 40-char hex IDs and real-world IP addresses in mock responses. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…akiness Bump idleWindow from 2ms to 50ms in drainEvents/collectEvents. The 2ms window was too tight for CI runners, causing TestQueuedSameNonceReplacement to miss events and fail intermittently. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
traviolus
requested changes
Apr 17, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
initiad quickstart(aliases:qstart,qs) command that configures a node afterinitiad init--interactive) with guided prompts and flag mode for scriptingcmd/initiad/appconfig/packageNew files
Test plan
go test ./cmd/initiad/quickstart/... -count=1all passinitiad init test && initiad quickstart --interactive🤖 Generated with Claude Code