feat(nodectl): v0.3.0 migration#29
Conversation
There was a problem hiding this comment.
Pull request overview
Release-oriented migration to nodectl v0.3.0, expanding the node-control service and CLI with runtime-configurable REST API auth, richer elections/validators visibility, and more user-friendly CLI output for operational workflows.
Changes:
- Add/extend REST API capabilities (JWT auth, OpenAPI bearer scheme, expanded elections/validators payloads, participation lifecycle tracking).
- Update elections runner/provider logic (next validator set p36 support, new snapshot fields, base64 pubkeys, stake submission history).
- CLI/UX improvements (table/json output formats across
config ... ls, enriched--version, better warnings, filtering, and stake-related commands).
Reviewed changes
Copilot reviewed 38 out of 39 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/node-control/ton-http-api-client/src/v2/client_json_rpc.rs | Improves error context for JSON-RPC calls and endpoint-failover messaging. |
| src/node-control/ton-http-api-client/Cargo.toml | Bumps crate version to 0.3.0; removes unused dependency. |
| src/node-control/service/src/http/http_server_task.rs | Makes auth middleware always present (runtime-config gated), adds elections query + OpenAPI bearer scheme, extends responses/tests. |
| src/node-control/service/src/http/auth_tests.rs | Updates auth tests for non-optional JwtAuth and TTL-aware token generation. |
| src/node-control/service/src/contracts/contracts_task.rs | Adjusts master deploy threshold constant. |
| src/node-control/service/src/auth/middleware.rs | Switches auth enablement check to live runtime config (hot reload). |
| src/node-control/service/src/auth/jwt.rs | Refactors JWT creation to accept secret directly; moves TTL decision to caller (live config). |
| src/node-control/service/Cargo.toml | Bumps version to 0.3.0; removes unused dependency. |
| src/node-control/nodectl/src/app_cli_args.rs | Extends --version output to include build metadata from build script env. |
| src/node-control/nodectl/Cargo.toml | Bumps version to 0.3.0; wires build script and trims dependencies. |
| src/node-control/elections/src/runner_tests.rs | Adds tests for participation lifecycle, participant stake min/max, base64 pubkeys, p36 handling. |
| src/node-control/elections/src/runner.rs | Adds p36 “next vset”, stake submission history, our-participants snapshot, richer validators snapshot. |
| src/node-control/elections/src/providers/traits.rs | Extends provider trait with get_next_vset() for p36. |
| src/node-control/elections/src/providers/default.rs | Implements get_next_vset() via config param 36 parsing. |
| src/node-control/elections/Cargo.toml | Bumps version to 0.3.0; removes unused deps. |
| src/node-control/docs/nodectl-setup.md | Reorders setup steps; documents secrets creation earlier + REST auth setup. |
| src/node-control/docs/nodectl-security.md | Expands quick overview to reflect default-off auth and hot reload behavior. |
| src/node-control/control-client/src/config_params.rs | Adds p36 parsing via shared validator-set parser. |
| src/node-control/control-client/Cargo.toml | Bumps version to 0.3.0; trims dependencies. |
| src/node-control/contracts/src/elector/elector_impl.rs | Minor import ordering tweak. |
| src/node-control/contracts/Cargo.toml | Bumps version to 0.3.0; trims dependencies. |
| src/node-control/common/src/ton_utils.rs | Adds display helpers for TON values and tests. |
| src/node-control/common/src/snapshot.rs | Extends snapshot schema: our participants, submission history, validator metadata, elections view helper. |
| src/node-control/common/Cargo.toml | Bumps version to 0.3.0; trims dependencies. |
| src/node-control/commands/src/commands/nodectl/utils.rs | Standardizes warnings; splits wallet address vs wallet info helper; exposes TON API connection check. |
| src/node-control/commands/src/commands/nodectl/service_api_cmd.rs | Adds table/json output, node filtering, elections participants toggle, improved request plumbing. |
| src/node-control/commands/src/commands/nodectl/output_format.rs | Introduces shared OutputFormat enum. |
| src/node-control/commands/src/commands/nodectl/mod.rs | Registers new output_format module. |
| src/node-control/commands/src/commands/nodectl/master_wallet_cmd.rs | Adds json/table output for master wallet info, uses new TON display formatting. |
| src/node-control/commands/src/commands/nodectl/config_wallet_cmd.rs | Adds json/table wallet listing, improved TON API warnings, and manual pool stake command. |
| src/node-control/commands/src/commands/nodectl/config_pool_cmd.rs | Adds json/table pool listing and improved warning path when TON API/vault unavailable. |
| src/node-control/commands/src/commands/nodectl/config_node_cmd.rs | Adds json/table node listing with status checking and JSON view. |
| src/node-control/commands/src/commands/nodectl/config_log_cmd.rs | Switches to shared OutputFormat. |
| src/node-control/commands/src/commands/nodectl/config_elections_cmd.rs | Switches to shared OutputFormat. |
| src/node-control/commands/src/commands/nodectl/config_bind_cmd.rs | Adds json/table bind listing with JSON view. |
| src/node-control/commands/Cargo.toml | Bumps version to 0.3.0; trims dependencies. |
| src/node-control/README.md | Updates docs for auth, API usage, security, new flags and endpoints. |
| src/node-control/CHANGELOG.md | Adds structured Keep-a-Changelog entry for 0.3.0. |
| src/Cargo.lock | Updates lockfile for version bumps and dependency pruning. |
| nodectl/CHANGELOG.md | Minor formatting adjustment. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let (validator_entry, is_next_validator) = find_validator_entries( | ||
| node, | ||
| self.snapshot_cache.last_validator_set.as_ref(), | ||
| self.snapshot_cache.last_next_validator_set.as_ref(), | ||
| ) | ||
| .await | ||
| .map_err(|e| { | ||
| let error = | ||
| anyhow::anyhow!("node [{}] find validator entry error: {:#}", node_id, e); | ||
| tracing::error!("{:#}", error); | ||
| node.last_error = Some(format!("{:#}", error)) | ||
| }) | ||
| .unwrap_or((None, false)); |
There was a problem hiding this comment.
build_validators_snapshot calls find_validator_entries unconditionally. When both last_validator_set and last_next_validator_set are None, this still exports public keys for up to 3 recent election keys per node (network/CPU work) even though the result cannot change. Consider short-circuiting (returning (None, false)) when both vsets are absent, or moving the export_public_key calls behind if current_vset.is_some() || next_vset.is_some() to avoid a per-tick performance regression during API/vset outages.
| let (validator_entry, is_next_validator) = find_validator_entries( | |
| node, | |
| self.snapshot_cache.last_validator_set.as_ref(), | |
| self.snapshot_cache.last_next_validator_set.as_ref(), | |
| ) | |
| .await | |
| .map_err(|e| { | |
| let error = | |
| anyhow::anyhow!("node [{}] find validator entry error: {:#}", node_id, e); | |
| tracing::error!("{:#}", error); | |
| node.last_error = Some(format!("{:#}", error)) | |
| }) | |
| .unwrap_or((None, false)); | |
| let (validator_entry, is_next_validator) = | |
| if self.snapshot_cache.last_validator_set.is_none() | |
| && self.snapshot_cache.last_next_validator_set.is_none() | |
| { | |
| // When both current and next validator sets are absent, the node | |
| // cannot be recognized as a validator; avoid unnecessary work. | |
| (None, false) | |
| } else { | |
| find_validator_entries( | |
| node, | |
| self.snapshot_cache.last_validator_set.as_ref(), | |
| self.snapshot_cache.last_next_validator_set.as_ref(), | |
| ) | |
| .await | |
| .map_err(|e| { | |
| let error = anyhow::anyhow!( | |
| "node [{}] find validator entry error: {:#}", | |
| node_id, | |
| e | |
| ); | |
| tracing::error!("{:#}", error); | |
| node.last_error = Some(format!("{:#}", error)) | |
| }) | |
| .unwrap_or((None, false)) | |
| }; |
| let mut sign_data = 0x654C5074u32.to_be_bytes().to_vec(); | ||
| sign_data.extend_from_slice(&(election_id as u32).to_be_bytes()); | ||
| sign_data.extend_from_slice(&max_factor_raw.to_be_bytes()); | ||
| sign_data.extend_from_slice(&pool_addr_bytes); | ||
| sign_data.extend_from_slice(&adnl_addr); | ||
|
|
||
| let signature = provider.sign(key_id, sign_data).await.context("sign election bid")?; | ||
|
|
||
| // Build NEW_STAKE payload for nominator pool | ||
| let payload = nominator::new_stake(&nominator::NewStakeParams { | ||
| query_id: time_format::now(), | ||
| stake_amount: stake_nanotons, | ||
| validator_pubkey: &pub_key, | ||
| stake_at: election_id as u32, | ||
| max_factor: max_factor_raw, |
There was a problem hiding this comment.
This code truncates election_id from u64 to u32 via as in two places. If election_id ever exceeds u32::MAX, the signed payload and stake_at field will be wrong with no error. Prefer a checked conversion (e.g., u32::try_from(election_id) with a clear error) and reuse the converted value for both sign_data and NewStakeParams.
|
|
||
| /// Minimal required balance for the master wallet before it can be deployed. | ||
| const DEPLOY_AMOUNT: u64 = 1_000_000_000; // 1 TON | ||
| const DEPLOY_AMOUNT: u64 = 1_100_000_000; // 1 TON |
There was a problem hiding this comment.
DEPLOY_AMOUNT was changed to 1_100_000_000 but the comment still says // 1 TON. Please either adjust the constant back to 1 TON or update the comment (and ideally add a brief rationale for the extra 0.1 TON so future readers know it’s intentional, e.g. to cover fees).
| match self.client.get_config_param(36).await { | ||
| Ok(bytes) => Ok(Some(parse_config_param_36(&bytes)?)), | ||
| Err(e) => { | ||
| tracing::trace!("get_next_vset: config param 36 not available: {e:?}"); |
There was a problem hiding this comment.
get_next_vset treats any get_config_param(36) error as “not available” and returns Ok(None). This can hide real failures (timeouts, auth, etc.) behind a misleading message. Consider either changing the log wording to “failed to fetch config param 36” or only converting to None for the specific “missing param” error (and propagating other errors).
| tracing::trace!("get_next_vset: config param 36 not available: {e:?}"); | |
| tracing::trace!("get_next_vset: failed to fetch config param 36: {e:?}"); |
Changes for a nodectl v0.3.0 release.