Skip to content

Feature/sma 85 migrate vote commands to rest api#132

Merged
mrnkslv merged 11 commits into
release/nodectl/v0.5.0from
feature/sma-85-migrate-vote-commands-to-rest-api
May 14, 2026
Merged

Feature/sma 85 migrate vote commands to rest api#132
mrnkslv merged 11 commits into
release/nodectl/v0.5.0from
feature/sma-85-migrate-vote-commands-to-rest-api

Conversation

@mrnkslv
Copy link
Copy Markdown
Contributor

@mrnkslv mrnkslv commented May 4, 2026

Summary

Migrates config-proposal voting to the nodectl control HTTP API: read endpoints for nominators (and operators), operator-only mutations for the tracked proposal list, persistence via update_and_save, and config_changed notifications. The nodectl vote CLI is now a thin REST client (shared URL, token, and config resolution with other REST commands).

Changes

  • HTTP: GET /v1/voting/config, GET /v1/voting/proposals, GET /v1/voting/proposals/{hash}, POST /v1/voting/proposals, DELETE /v1/voting/proposals/{hash} with role-appropriate auth middleware.
  • Service: voting DTOs and handlers in config_handlers.rs; route registration and OpenAPI updates in http_server_task.rs; unit tests in config_handlers_tests.rs and auth coverage in auth_tests.rs.
  • CLI: vote_cmd.rs uses the service API instead of loading/saving config locally for list, inspect, add, and remove flows.
  • Docs: README updates for voting, REST endpoint table, and env flags; CHANGELOG.md entry under Unreleased.
  • E2E: run_singlehost_nodectl.py smoke-checks voting REST and nodectl vote ls; optional VOTING_REST_VALIDATE bypass; test_run_net_py/readme.md note.

mrnkslv added 3 commits May 1, 2026 15:34
Co-authored-by: Cursor <cursoragent@cursor.com>

# Conflicts:
#	src/node-control/service/src/http/config_handlers.rs
Copilot AI review requested due to automatic review settings May 4, 2026 18:44
@linear
Copy link
Copy Markdown

linear Bot commented May 4, 2026

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR migrates config-proposal voting functionality from local CLI config reads/writes into the nodectl service control-plane REST API, making the CLI a thin REST client and adding E2E coverage for the new endpoints.

Changes:

  • Added voting REST endpoints (/v1/voting/*) with role-appropriate auth and OpenAPI exposure.
  • Implemented voting DTOs + handlers backed by runtime config persistence (update_and_save) and on-chain Config contract reads.
  • Updated nodectl vote CLI to call the service API, plus docs/tests/E2E smoke checks for the new flows.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/node/tests/test_run_net_py/run_singlehost_nodectl.py Adds phase-13 voting REST + CLI smoke checks and REST URL/token helpers.
src/node/tests/test_run_net_py/readme.md Documents the single-host nodectl E2E script and the voting REST smoke checks.
src/node-control/service/src/http/http_server_task.rs Registers voting routes (GET under nominator auth; POST/DELETE operator-only) and adds OpenAPI wiring/tests.
src/node-control/service/src/http/config_handlers.rs Introduces voting DTOs and /v1/voting/* handlers (config snapshot, list/inspect proposals, tracked-list mutations).
src/node-control/service/src/http/config_handlers_tests.rs Adds unit tests for voting config snapshot + tracked-list add/rm behavior (auth disabled).
src/node-control/service/src/http/auth_tests.rs Adds auth coverage for voting endpoints (401/200/403 cases).
src/node-control/README.md Updates CLI/docs to reflect nodectl vote as a REST client and documents the new endpoints and flags.
src/node-control/commands/src/commands/nodectl/vote_cmd.rs Refactors vote subcommands to use shared REST client helpers and service URL/token resolution.

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

Comment thread src/node/tests/test_run_net_py/run_singlehost_nodectl.py
Comment thread src/node/tests/test_run_net_py/run_singlehost_nodectl.py Outdated
mrnkslv and others added 4 commits May 4, 2026 21:51
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@mrnkslv mrnkslv requested review from Keshoid May 11, 2026 09:12
Comment thread src/node/tests/test_run_net_py/run_singlehost_nodectl.py
Copy link
Copy Markdown
Contributor

@Keshoid Keshoid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Solid PR overall — REST handlers mirror the existing CRUD shape, auth split is correct, and the smoke test covers the read paths. A handful of small simplifications and a couple of test gaps below; none blocking.

A single follow-up that addresses items 1, 2, 3, 4, 7, 9 (plus the operator-allowed auth test) should shave ~30–40 lines off the diff with no behavior change.

Comment thread src/node-control/service/src/http/config_handlers.rs Outdated
Comment thread src/node-control/service/src/http/config_handlers.rs Outdated
Comment thread src/node-control/service/src/http/config_handlers.rs Outdated
Comment thread src/node-control/service/src/http/config_handlers.rs Outdated
Comment thread src/node-control/service/src/http/config_handlers.rs Outdated
Comment thread src/node-control/service/src/http/config_handlers.rs Outdated
Comment thread src/node-control/commands/src/commands/nodectl/vote_cmd.rs Outdated
Comment thread src/node-control/commands/src/commands/nodectl/vote_cmd.rs
Comment thread src/node-control/commands/src/commands/nodectl/vote_cmd.rs Outdated
Comment thread src/node-control/service/src/http/auth_tests.rs
@mrnkslv mrnkslv merged commit 8e11f39 into release/nodectl/v0.5.0 May 14, 2026
6 of 8 checks passed
@mrnkslv mrnkslv deleted the feature/sma-85-migrate-vote-commands-to-rest-api branch May 14, 2026 09:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants