feat(nodectl): entity CRUD via REST API by Keshoid · Pull Request #88 · RSquad/ton-rust-node

Keshoid · 2026-04-13T21:25:41Z

Summary

Part 2 of centralizing config management. Moves the 8 entity mutation commands (config {node,wallet,pool,bind} {add,rm}) from local config-file writes to REST endpoints on the daemon.

POST /v1/{nodes,wallets,pools,bindings} and DELETE /v1/{nodes,wallets,pools,bindings}/{name}, all behind require_operator.
Validation moved server-side; daemon mutates via RuntimeConfigStore::update_with and persists with save_to_file.
CLI add/rm become thin REST clients; --url / --token / NODECTL_API_TOKEN work the same as in SMA-19. Best-effort vault check kept in CLI for node add / wallet add until the key REST API lands.
normalize_ton_address moved from commands to common::ton_utils.

Notes

Stacked on top of feat(nodectl): centralize config management through REST API (SMA-19) #86. Diff includes those commits until feat(nodectl): centralize config management through REST API (SMA-19) #86 merges.

Closes SMA-69

…config-management-through-rest-api

linear · 2026-04-13T21:25:45Z

SMA-69 Centralize config management: entity CRUD via REST API

Copilot

Pull request overview

This PR continues the “centralized config management” effort (SMA-69) by moving entity CRUD (nodes, wallets, pools, bindings) from direct config-file mutation in nodectl into daemon-backed REST API endpoints, and refactors the CLI + integration script accordingly.

Changes:

Add new REST handlers for listing and CRUD mutations of nodes/wallets/pools/bindings, plus “elections settings”, “log”, and “master wallet” read endpoints.
Refactor multiple nodectl config ... subcommands to become thin REST clients (URL/token resolution, JSON DTO parsing, etc.).
Update the singlehost network bootstrap script to generate vault secrets earlier, set up auth/JWT, and adjust phase flow/validation.

Reviewed changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
src/node/tests/test_run_net_py/run_singlehost_nodectl.py	Updates CI/bootstrap flow for auth + REST-based config and stake validation.
src/node-control/service/src/runtime_config.rs	Adds concurrent ADNL config resolution helper; exposes `open_wallet` for internal use.
src/node-control/service/src/http/mod.rs	Exposes the new `config_handlers` module.
src/node-control/service/src/http/http_server_task.rs	Wires new REST routes + OpenAPI types; widens `AppError` constructors for reuse.
src/node-control/service/src/http/config_handlers.rs	Introduces config REST endpoints (list + CRUD mutations) and related DTOs.
src/node-control/common/src/ton_utils.rs	Moves/introduces `normalize_ton_address` and adds unit tests.
src/node-control/common/src/app_config.rs	Adds OpenAPI schema derives for log-related enums behind `openapi` feature.
src/node-control/commands/src/commands/nodectl/utils.rs	Adds shared service-API helpers (URL resolution, GET/POST/DELETE wrappers).
src/node-control/commands/src/commands/nodectl/service_api_cmd.rs	Improves base URL normalization and adds tests.
src/node-control/commands/src/commands/nodectl/master_wallet_cmd.rs	Switches master-wallet info command to call service API.
src/node-control/commands/src/commands/nodectl/config_wallet_cmd.rs	Switches wallet add/ls/rm to service API (send/stake still require local config).
src/node-control/commands/src/commands/nodectl/config_pool_cmd.rs	Switches pool add/ls/rm to service API and simplifies table rendering.
src/node-control/commands/src/commands/nodectl/config_node_cmd.rs	Switches node add/ls/rm to service API; keeps best-effort vault warning.
src/node-control/commands/src/commands/nodectl/config_log_cmd.rs	Switches log ls to service API (log set remains local-config based).
src/node-control/commands/src/commands/nodectl/config_elections_cmd.rs	Switches elections “show” to service API and updates table formatting.
src/node-control/commands/src/commands/nodectl/config_cmd.rs	Makes `--config` optional and adds global `--url/--token` passthrough for REST-backed subcommands.
src/node-control/commands/src/commands/nodectl/config_bind_cmd.rs	Switches binding add/ls/rm to service API.

Comments suppressed due to low confidence (1)

src/node/tests/test_run_net_py/run_singlehost_nodectl.py:677

elections can still be None after the polling loop (e.g., if _fetch_nodectl_elections() keeps failing). In that case self._compare_stakes(elections, elector_map) will raise because _compare_stakes calls elections.get(...). Add an explicit if elections is None: ... handling (fail or return) after the loop before calling _compare_stakes.

        elector_map = self._fetch_elector_stake_map()
        if elector_map is None:
            return

        self._compare_stakes(elections, elector_map)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-13T21:32:40Z

+        )
+        os.environ["NODECTL_API_TOKEN"] = json.loads(result.stdout)["token"]
+        self.log.info("  Logged in and exported NODECTL_API_TOKEN")
+        self.log.info("  NODECTL_API_TOKEN=" + os.environ["NODECTL_API_TOKEN"])


The test logs the full NODECTL_API_TOKEN value (self.log.info("NODECTL_API_TOKEN=" + ...)). This exposes a bearer token in CI logs and makes it easy to accidentally leak credentials when rerunning locally. Prefer not logging the token at all (or log only a short prefix/suffix).

Suggested change

self.log.info(" NODECTL_API_TOKEN=" + os.environ["NODECTL_API_TOKEN"])

Copilot · 2026-04-13T21:32:40Z

+    let mut base = url.to_string();
+    if base.starts_with("0.0.0.0") {
+        base = base.replacen("0.0.0.0", "127.0.0.1", 1);
+    }
+    if !base.starts_with("http://") && !base.starts_with("https://") {
+        base = format!("http://{}", base);
+    }
+    base


normalize_base_url only replaces 0.0.0.0 when the string literally starts with 0.0.0.0. If the user passes --url http://0.0.0.0:8080/https://..., this won’t be normalized and the client will try to connect to 0.0.0.0. Consider trimming the scheme before the starts_with("0.0.0.0") check (same approach as in service_api_cmd.rs) or centralizing URL normalization in one helper.

Suggested change

let mut base = url.to_string();

if base.starts_with("0.0.0.0") {

base = base.replacen("0.0.0.0", "127.0.0.1", 1);

}

if !base.starts_with("http://") && !base.starts_with("https://") {

base = format!("http://{}", base);

}

base

let (scheme, rest) = if let Some(stripped) = url.strip_prefix("http://") {

("http://", stripped)

} else if let Some(stripped) = url.strip_prefix("https://") {

("https://", stripped)

} else {

("http://", url)

};

let normalized_rest = if rest.starts_with("0.0.0.0") {

rest.replacen("0.0.0.0", "127.0.0.1", 1)

} else {

rest.to_string()

};

format!("{}{}", scheme, normalized_rest)

Copilot · 2026-04-13T21:32:41Z

+                println!("{}", serde_json::to_string_pretty(result)?);
            }
            OutputFormat::Table => {
+                let log = serde_json::from_value::<LogConfig>(result.clone()).unwrap_or_default();


In table output mode, LogLsCmd silently falls back to LogConfig::default() when the API response can’t be parsed. That can hide server-side/schema bugs and print misleading configuration. Prefer returning an error (or at least surfacing the parse error) instead of defaulting.

Suggested change

let log = serde_json::from_value::<LogConfig>(result.clone()).unwrap_or_default();

let log = serde_json::from_value::<LogConfig>(result.clone()).map_err(|err| {

anyhow::anyhow!(

"failed to parse /v1/log response result as LogConfig: {err}; payload: {}",

result

)

})?;

Copilot · 2026-04-13T21:32:41Z

+            cfg.nodes.insert(name, adnl_config);
+        })
+        .map_err(|e| AppError::internal(e.to_string()))?;
+    state.runtime_cfg.save_to_file();


All mutation handlers call runtime_cfg.save_to_file(), but that method only logs write/serialization errors and doesn’t report failure back to the caller. This means the API can return success even when persistence failed (changes would be lost on restart). Consider making persistence fallible (return Result) and mapping failures to AppError::internal.

Suggested change

state.runtime_cfg.save_to_file();

state

.runtime_cfg

.save_to_file()

.map_err(|e| AppError::internal(e.to_string()))?;

Copilot · 2026-04-13T21:32:41Z

+        .runtime_cfg
+        .update_with(|cfg| {
+            cfg.nodes.insert(name, adnl_config);
+        })
+        .map_err(|e| AppError::internal(e.to_string()))?;


After update_with mutations (adding/removing nodes/wallets/pools/bindings), the service’s derived runtime state (opened wallets/pools, etc.) is not rebuilt. RuntimeConfigStore::update_with swaps only the AppConfig snapshot while keeping cached wallets/pools in RuntimeState, and there is no task restart here (unlike elections include/exclude). This can leave the daemon running with stale caches and tasks that never observe the new entities; consider triggering a reload/rebuild or restarting affected tasks after a successful mutation.

I will fix hot reload in the next PR.

Copilot · 2026-04-13T21:32:42Z

+    if !state.runtime_cfg.get().nodes.contains_key(&name) {
+        return Err(AppError::not_found(format!("node '{name}' not found")));
+    }
+


v1_nodes_rm_handler removes a node without checking whether a binding exists for that node name. Since v1_bindings_add_handler enforces that a binding’s node must exist, allowing node deletion can create an inconsistent config state (binding refers to a non-existent node). Consider rejecting deletion when a binding exists or removing the binding atomically as part of node deletion.

Copilot · 2026-04-13T21:32:42Z

+// Each handler validates input against the live config, applies the change via
+// `RuntimeConfigStore::update_with` (atomic Arc swap), then persists with
+// `save_to_file`. Validation errors map to 400, missing entities to 404,
+// I/O failures to 500. All routes are mounted behind `require_operator`.
+// ---------------------------------------------------------------------------


New CRUD mutation endpoints are introduced here (POST/DELETE for nodes/wallets/pools/bindings), but there don’t appear to be accompanying handler-level tests similar to the existing HTTP handler tests in http_server_task.rs. Adding coverage for success + key validation failures (400/404) + require_operator enforcement would help prevent regressions.

Created a separate task for tests.

…e errors, check bindings on node rm

…ailure

…ture/sma-69-centralize-config-management-entity-crud-via-rest-api

Lapo4kaKek · 2026-04-14T23:23:39Z

+    control_server_endpoint: &'a str,
+    control_server_pubkey: &'a str,
+    control_client_secret: &'a str,
+}


are you sure lifetimes are really needed here? i think there are almost no alloc here and it could be done more simply with String

I use refs to avoid string clones.

Lapo4kaKek · 2026-04-14T23:31:14Z

@@ -465,7 +460,7 @@ impl RuntimeConfig for RuntimeConfigStore {
        self.update_with(f)


check if any old wallet, pools, rpc_client data remains after change config

Created atomic function to 1)save config to disk and 2) swap runtime state.

Lapo4kaKek · 2026-04-14T23:36:20Z

+    let name = req.name.clone();
+    state
+        .runtime_cfg
+        .update_with(|cfg| {


config is mutated in memory before persistence. If save_to_file() fails, we return error 500, but leave the live runtime state changed and out of sync. I think worth thinking about it's a little

Fixed. Joint save_to_file and update_with into atomic operation which returns error on failure.

Lapo4kaKek · 2026-04-14T23:40:47Z

+
+/// Sends a POST request with a JSON body and returns the response body.
+pub async fn api_post<B: serde::Serialize>(
+    base_url: &str,


we usually write generics via where T

…_and_save for atomic config updates

Keshoid added 12 commits April 10, 2026 19:34

feat(service): add rest api handlers for config ls commands

b57e6a4

refactor(nodectl): update config commands to call REST API

7dd60a3

Merge branch 'release/nodectl/v0.4.0' into feature/sma-19-centralize-…

bfc5440

…config-management-through-rest-api

feat(service): return balance as u64 from REST API

6380606

feat(commands): use fallback url and default config path

12e332d

test: update ci test script to work with REST API commands

c3b9b42

fix: after review

1876217

fmt: make fmt

e534084

feat(commands): thin client commandsconfig ... add|rm

b49efe1

feat(service): add REST API handlers for add|rm commands

ba65c57

fix(service): error adding binding if it already exists

7692655

fix(commands): hide NODECTL_API_TOKEN from help print

aa7585a

Copilot AI review requested due to automatic review settings April 13, 2026 21:25

Copilot started reviewing on behalf of Keshoid April 13, 2026 21:26 View session

Keshoid changed the title ~~feat(nodectl): entity CRUD via REST API (SMA-69)~~ feat(nodectl): entity CRUD via REST API Apr 13, 2026

Keshoid requested review from ITBear, Lapo4kaKek and mrnkslv April 13, 2026 21:29

Copilot AI reviewed Apr 13, 2026

View reviewed changes

Keshoid mentioned this pull request Apr 14, 2026

feat(nodectl): centralize config management through REST API (SMA-19) #86

Merged

Keshoid added 2 commits April 14, 2026 09:57

fix(commands,service): consolidate normalize_base_url, propagate pars…

be3d374

…e errors, check bindings on node rm

fix(service): make save_to_file fallible, return 500 on persistence f…

e98566f

…ailure

Keshoid mentioned this pull request Apr 14, 2026

feat(nodectl): settings mutations via REST API #89

Closed

Merge remote-tracking branch 'origin/release/nodectl/v0.4.0' into fea…

242851f

…ture/sma-69-centralize-config-management-entity-crud-via-rest-api

Lapo4kaKek reviewed Apr 14, 2026

View reviewed changes

ITBear reviewed Apr 15, 2026

View reviewed changes

Comment thread src/node-control/service/src/http/config_handlers.rs

refactor(service): replace update_config and save_to_file with update…

ab1d555

…_and_save for atomic config updates

ITBear approved these changes Apr 15, 2026

View reviewed changes

Lapo4kaKek approved these changes Apr 15, 2026

View reviewed changes

Keshoid merged commit 6f3fd72 into release/nodectl/v0.4.0 Apr 15, 2026
10 of 11 checks passed

Keshoid deleted the feature/sma-69-centralize-config-management-entity-crud-via-rest-api branch April 15, 2026 11:15

-    let mut base = url.to_string();
-    if base.starts_with("0.0.0.0") {
-        base = base.replacen("0.0.0.0", "127.0.0.1", 1);
-    }
-    if !base.starts_with("http://") && !base.starts_with("https://") {
-        base = format!("http://{}", base);
-    }
-    base
+    let (scheme, rest) = if let Some(stripped) = url.strip_prefix("http://") {
+        ("http://", stripped)
+    } else if let Some(stripped) = url.strip_prefix("https://") {
+        ("https://", stripped)
+    } else {
+        ("http://", url)
+    };
+    let normalized_rest = if rest.starts_with("0.0.0.0") {
+        rest.replacen("0.0.0.0", "127.0.0.1", 1)
+    } else {
+        rest.to_string()
+    };
+    format!("{}{}", scheme, normalized_rest)

-                let log = serde_json::from_value::<LogConfig>(result.clone()).unwrap_or_default();
+                let log = serde_json::from_value::<LogConfig>(result.clone()).map_err(|err| {
+                    anyhow::anyhow!(
+                        "failed to parse /v1/log response result as LogConfig: {err}; payload: {}",
+                        result
+                    )
+                })?;

		@@ -465,7 +460,7 @@ impl RuntimeConfig for RuntimeConfigStore {
		self.update_with(f)

Conversation

Keshoid commented Apr 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Notes

Uh oh!

linear Bot commented Apr 13, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Keshoid Apr 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Keshoid commented Apr 13, 2026 •

edited

Loading

Keshoid Apr 15, 2026 •

edited

Loading