Call miner_start with no parameters for the cores and resume controls

The node's miner_start takes no core count and its miner API offers no call to set one, so passing the count made it answer "Invalid number of input parameters" and both controls failed. Apply now saves the count to <node-dir>/miner-threads and starts the miner with no parameters; resume does the same. The page, the messages and the README say a new core count applies at the next node restart. The test node now refuses miner_start with any parameter, as the real node does.

Author: ghostcoin-org

README.md

@@ -41,8 +41,8 @@ The dashboard finds your blocks by reading the chain over IPC and keeps what it
 
 The Mining panel has a cores slider, from 1 to the computer's logical core count, and a pause or resume button.
 
-- **Apply** asks the node to mine on that many cores at once (`miner_start`) and saves the number in `<node-dir>/miner-threads`, the file the installer and updater read, so restarts and updates keep it. The file is replaced in one step (a temporary file renamed over it) and keeps its permissions. While mining is paused the number is only saved, and resuming uses it.
-- **Pause** stops mining (`miner_stop`) and writes `<node-dir>/mining-paused`; the node keeps syncing. **Resume** removes that file and starts mining on the saved core count. Until the node installer reads `mining-paused`, a node restart resumes mining.
+- **Apply** saves the number in `<node-dir>/miner-threads`, the file the installer and updater read, and makes sure the node is mining (`miner_start`, with no parameters). The node's `miner_start` takes no core count and its miner API has no call to set one, so the new count applies at the next node restart, and restarts and updates keep it. The file is replaced in one step (a temporary file renamed over it) and keeps its permissions. While mining is paused the number is only saved.
+- **Pause** stops mining (`miner_stop`) and writes `<node-dir>/mining-paused`; the node keeps syncing. **Resume** removes that file and starts mining (`miner_start`, with no parameters) on the core count the node started with; a saved count applies at the next node restart. Until the node installer reads `mining-paused`, a node restart resumes mining.
 
 After each change the dashboard reads the node back (`eth_mining`) and says in plain words what happened, or what went wrong. The controls are `POST /api/mining/cores` (body `{"cores": n}`, a whole number in range, else 400), `POST /api/mining/pause` and `POST /api/mining/resume`, behind the dashboard key and the Origin check.
 

internal/ipc/ipctest/fake.go

@@ -129,6 +129,20 @@ func (s *Server) serve(conn net.Conn) {
 	}
 }
 
+// NoParams wraps h so that a call with any parameter gets the error the real
+// node sends for a method that takes none (its miner_start does).
+func NoParams(h Handler) Handler {
+	return func(params json.RawMessage) (any, *Error) {
+		var list []json.RawMessage
+		if len(params) > 0 && string(params) != "null" {
+			if err := json.Unmarshal(params, &list); err != nil || len(list) > 0 {
+				return nil, &Error{Code: -32602, Message: "Invalid number of input parameters to RPC method"}
+			}
+		}
+		return h(params)
+	}
+}
+
 // Value is a handler that always returns v.
 func Value(v any) Handler {
 	return func(json.RawMessage) (any, *Error) { return v, nil }

internal/server/controls_test.go

@@ -1,7 +1,9 @@
 package server
 
 import (
+	"context"
 	"encoding/json"
+	"errors"
 	"net/http"
 	"net/http/httptest"
 	"os"
@@ -12,14 +14,16 @@ import (
 	"sync"
 	"testing"
 
+	"github.com/ghostcoin-core/ghost-node-dashboard/internal/ipc"
 	"github.com/ghostcoin-core/ghost-node-dashboard/internal/ipc/ipctest"
 	"github.com/ghostcoin-core/ghost-node-dashboard/internal/status"
 )
 
 const goodOrigin = "http://127.0.0.1:8580"
 
 // fakeMiner is a node whose miner_start and miner_stop change what
-// eth_mining reports, recording the thread counts miner_start was given.
+// eth_mining reports, recording the params of each miner_start it took. Like
+// the real node, its miner_start refuses any parameter.
 type fakeMiner struct {
 	mu      sync.Mutex
 	mining  bool
@@ -28,13 +32,13 @@ type fakeMiner struct {
 
 func (m *fakeMiner) handlers() map[string]ipctest.Handler {
 	return map[string]ipctest.Handler{
-		"miner_start": func(params json.RawMessage) (any, *ipctest.Error) {
+		"miner_start": ipctest.NoParams(func(params json.RawMessage) (any, *ipctest.Error) {
 			m.mu.Lock()
 			defer m.mu.Unlock()
 			m.mining = true
 			m.started = append(m.started, params)
 			return nil, nil
-		},
+		}),
 		"miner_stop": func(json.RawMessage) (any, *ipctest.Error) {
 			m.mu.Lock()
 			defer m.mu.Unlock()
@@ -119,15 +123,15 @@ func TestSetCores(t *testing.T) {
 	if !res.OK || res.Error != "" {
 		t.Errorf("result %+v, want ok", res)
 	}
-	if !strings.Contains(res.Message, "3 cores") {
-		t.Errorf("message %q does not name 3 cores", res.Message)
+	if !strings.Contains(res.Message, "3 cores") || !strings.Contains(res.Message, "next node restart") {
+		t.Errorf("message %q does not say 3 cores apply at the next node restart", res.Message)
 	}
 	if res.Mining == nil || !*res.Mining || res.CoresUsed != 3 {
 		t.Errorf("read back mining %v, cores %d", res.Mining, res.CoresUsed)
 	}
 
-	if got := miner.starts(); !slices.Equal(got, []string{"[3]"}) {
-		t.Errorf("miner_start params %v, want [[3]]", got)
+	if got := miner.starts(); !slices.Equal(got, []string{"[]"}) {
+		t.Errorf("miner_start params %v, want one call with []", got)
 	}
 	calls := fake.Calls()
 	if i := slices.Index(calls, "miner_start"); i < 0 || !slices.Contains(calls[i:], "eth_mining") {
@@ -298,8 +302,14 @@ func TestPauseResume(t *testing.T) {
 	if !res.OK || res.Paused || res.Mining == nil || !*res.Mining {
 		t.Errorf("resume result %+v, want ok, not paused and mining", res)
 	}
-	if got := miner.starts(); !slices.Equal(got, []string{"[6]"}) {
-		t.Errorf("resume miner_start params %v, want the saved [6]", got)
+	if got := miner.starts(); !slices.Equal(got, []string{"[]"}) {
+		t.Errorf("resume miner_start params %v, want one call with []", got)
+	}
+	if !strings.Contains(res.Message, "6 cores") || !strings.Contains(res.Message, "next node restart") {
+		t.Errorf("resume message %q does not say the saved 6 cores apply at the next node restart", res.Message)
+	}
+	if got := readFile(t, filepath.Join(dir, status.MinerThreadsFile)); got != "6\n" {
+		t.Errorf("miner-threads = %q, want 6", got)
 	}
 	if _, err := os.Stat(paused); !os.IsNotExist(err) {
 		t.Errorf("mining-paused still there after resume (%v)", err)
@@ -309,6 +319,57 @@ func TestPauseResume(t *testing.T) {
 	}
 }
 
+// TestMinerStartNoParams checks that the fake miner refuses miner_start with a
+// thread count, as the real node does, and that the cores control and resume
+// still start mining on it.
+func TestMinerStartNoParams(t *testing.T) {
+	miner := &fakeMiner{}
+	_, node := startFake(t, miner.handlers())
+
+	var rpcErr *ipc.RPCError
+	err := node.Call(context.Background(), nil, "miner_start", 2)
+	if !errors.As(err, &rpcErr) || !strings.Contains(rpcErr.Message, "Invalid number of input parameters") {
+		t.Fatalf("miner_start(2) on the fake: %v, want the node's invalid parameters error", err)
+	}
+	if err := node.Call(context.Background(), nil, "miner_start"); err != nil {
+		t.Fatalf("miner_start() on the fake: %v", err)
+	}
+	if err := node.Call(context.Background(), nil, "miner_stop"); err != nil {
+		t.Fatal(err)
+	}
+
+	dir := t.TempDir()
+	h := panelHandler(t, node, panelOpts{nodeDir: dir})
+
+	res := decodeResult(t, post(t, h, "/api/mining/cores", `{"cores": 2}`, signedPost))
+	if !res.OK || res.Error != "" || res.Mining == nil || !*res.Mining {
+		t.Errorf("cores result %+v, want ok and mining", res)
+	}
+	if !strings.Contains(res.Message, "next node restart") {
+		t.Errorf("cores message %q does not say the count applies at the next node restart", res.Message)
+	}
+	if got := readFile(t, filepath.Join(dir, status.MinerThreadsFile)); got != "2\n" {
+		t.Errorf("miner-threads = %q, want 2", got)
+	}
+
+	if res := decodeResult(t, post(t, h, "/api/mining/pause", "", signedPost)); !res.OK {
+		t.Fatalf("pause result %+v", res)
+	}
+	res = decodeResult(t, post(t, h, "/api/mining/resume", "", signedPost))
+	if !res.OK || res.Error != "" || res.Mining == nil || !*res.Mining {
+		t.Errorf("resume result %+v, want ok and mining", res)
+	}
+
+	for _, p := range miner.starts() {
+		if p != "[]" && p != "" && p != "null" {
+			t.Errorf("miner_start took params %s, want none", p)
+		}
+	}
+	if got := len(miner.starts()); got != 3 {
+		t.Errorf("miner_start ran %d times, want 3", got)
+	}
+}
+
 func TestPauseNodeStillMining(t *testing.T) {
 	handlers := (&fakeMiner{}).handlers()
 	handlers["eth_mining"] = ipctest.Value(true)

internal/status/control.go

@@ -55,8 +55,12 @@ func ValidCores(n, logical int) bool {
 	return n >= 1 && n <= logical
 }
 
-// SetCores saves n to <node-dir>/miner-threads and, unless mining is paused,
-// starts the miner on n cores at once.
+// The node's miner_start takes no parameters and its miner API has no way to
+// set the thread count, so a new core count only reaches the node through
+// <node-dir>/miner-threads when the node next starts.
+
+// SetCores saves n to <node-dir>/miner-threads, which applies at the next node
+// restart, and, unless mining is paused, makes sure the miner is running.
 func SetCores(ctx context.Context, node Caller, env MiningEnv, n int) ControlResult {
 	if !ValidCores(n, env.LogicalCores) {
 		return ControlResult{
@@ -73,17 +77,17 @@ func SetCores(ctx context.Context, node Caller, env MiningEnv, n int) ControlRes
 	res.CoresUsed = n
 	if res.Paused {
 		res.OK = true
-		res.Message = fmt.Sprintf("Saved %d cores. Mining is paused; resuming will use them.", n)
+		res.Message = fmt.Sprintf("Saved %d cores. They apply at the next node restart. Mining is paused.", n)
 		res.Mining = readMining(ctx, node)
 		return res
 	}
-	if err := node.Call(ctx, nil, "miner_start", n); err != nil {
-		res.Error = fmt.Sprintf("Saved %d cores for the next start, but the node did not take them: %s.", n, nodeProblem(err))
+	if err := node.Call(ctx, nil, "miner_start"); err != nil {
+		res.Error = fmt.Sprintf("Saved %d cores for the next node restart, but the node did not start mining: %s.", n, nodeProblem(err))
 		res.Mining = readMining(ctx, node)
 		return res
 	}
 	res.Mining = readMining(ctx, node)
-	res.OK, res.Message = miningOutcome(res.Mining, true, fmt.Sprintf("Mining on %d cores. Saved for restarts and updates.", n))
+	res.OK, res.Message = miningOutcome(res.Mining, true, fmt.Sprintf("Saved %d cores. They apply at the next node restart; until then the node keeps its current core count.", n))
 	if !res.OK {
 		res.Error, res.Message = res.Message, ""
 	}
@@ -112,8 +116,8 @@ func Pause(ctx context.Context, node Caller, env MiningEnv) ControlResult {
 	return res
 }
 
-// Resume removes <node-dir>/mining-paused and starts the miner on the saved
-// core count.
+// Resume removes <node-dir>/mining-paused and starts the miner. The node mines
+// on the core count it started with; a saved count applies at its next restart.
 func Resume(ctx context.Context, node Caller, env MiningEnv) ControlResult {
 	cores := SavedCores(env)
 	res := ControlResult{CoresUsed: cores, Paused: MiningPaused(env.NodeDir)}
@@ -125,13 +129,13 @@ func Resume(ctx context.Context, node Caller, env MiningEnv) ControlResult {
 		}
 	}
 	res.Paused = false
-	if err := node.Call(ctx, nil, "miner_start", cores); err != nil {
+	if err := node.Call(ctx, nil, "miner_start"); err != nil {
 		res.Error = "Cleared the pause, but the node did not start mining: " + nodeProblem(err) + "."
 		res.Mining = readMining(ctx, node)
 		return res
 	}
 	res.Mining = readMining(ctx, node)
-	res.OK, res.Message = miningOutcome(res.Mining, true, fmt.Sprintf("Mining resumed on %d cores.", cores))
+	res.OK, res.Message = miningOutcome(res.Mining, true, fmt.Sprintf("Mining resumed. The saved %d cores apply at the next node restart.", cores))
 	if !res.OK {
 		res.Error, res.Message = res.Message, ""
 	}

web/index.html

@@ -84,7 +84,7 @@ <h3>Controls</h3>
     </form>
     <p class="message" id="control-result" role="status" hidden></p>
     <p class="note">A new payout address applies at once and is saved in the node's <code>config.toml</code>, so restarts keep it. Only the address is needed: never enter a private key or recovery words here.</p>
-    <p class="note">The core count is saved in the node folder, so restarts and updates keep it. While mining is paused the node keeps syncing. For now a node restart resumes mining, until the node installer learns to read the <code>mining-paused</code> file.</p>
+    <p class="note">The core count is saved in the node folder and applies at the next node restart: the node cannot change it while running. Restarts and updates keep it. While mining is paused the node keeps syncing. For now a node restart resumes mining, until the node installer learns to read the <code>mining-paused</code> file.</p>
     <p class="updated">Updated <span data-updated>never</span></p>
   </section>