`admin_addPeer` can return `true` after the dial scheduler has been cancelled

[BenWhite713]

Summary

The documentation states that admin_addPeer:

returns a BOOL indicating whether the peer was accepted for tracking or some error occurred.

adminAPI.AddPeer calls Server.AddPeer and then unconditionally returns true. The call chain has no acknowledgement channel:

adminAPI.AddPeer
  -> Server.AddPeer
  -> dialScheduler.addStatic
  -> select { addStaticCh <- node; <-ctx.Done() }

If the scheduler context is already cancelled, addStatic silently returns without adding the node, but the RPC implementation still reports success.

Documentation: https://geth.ethereum.org/docs/interacting-with-geth/rpc/ns-admin#admin-addpeer

Affected revision and environment

• Forced lifecycle fixture: commit 81ab8b594ebe0f672450779d4b308f3f33191828
• Platform: macOS arm64, Go 1.26.3
• The forced lifecycle fixture reproduced again on current master at 7e520c43104fd0447acd7372a63ba1daa5c05929 (2026-08-08).

Steps to reproduce

Add the following test as node/admin_addpeer_cancelled_test.go:

package node

import (
	"testing"

	"github.com/ethereum/go-ethereum/crypto"
	"github.com/ethereum/go-ethereum/p2p"
	"github.com/ethereum/go-ethereum/p2p/enode"
)

func TestAdminAddPeerReturnsTrueAfterSchedulerStop(t *testing.T) {
	n, err := New(&Config{
		Name: "admin-addpeer-cancelled",
		P2P: p2p.Config{
			ListenAddr:  "127.0.0.1:0",
			NoDiscovery: true,
			MaxPeers:    1,
		},
	})
	if err != nil {
		t.Fatal(err)
	}
	defer n.Close()
	if err := n.Start(); err != nil {
		t.Fatal(err)
	}

	// Cancel dialScheduler.ctx while leaving the admin service object callable.
	n.server.Stop()

	key, err := crypto.GenerateKey()
	if err != nil {
		t.Fatal(err)
	}
	peerURL := enode.NewV4(&key.PublicKey, nil, 30399, 0).URLv4()

	result, err := (&adminAPI{node: n}).AddPeer(peerURL)
	if err != nil || !result {
		t.Fatalf("AddPeer result=%v err=%v", result, err)
	}
	t.Logf("AddPeer returned true after the dial scheduler was stopped: %s", peerURL)
}

Run:

go test ./node -run '^TestAdminAddPeerReturnsTrueAfterSchedulerStop$' -count=1 -v

Observed behavior

The method returns:

result=true
error=nil

At that point dialScheduler.ctx is cancelled, so addStatic can only take the <-d.ctx.Done() branch and the peer is not accepted for tracking.

Stock Node.Close stops RPC before stopping P2P; this fixture intentionally isolates the API/scheduler contract by forcing the cancelled-scheduler state while the admin service object remains callable.

Expected behavior

If the scheduler does not accept the peer for tracking, admin_addPeer should return false or an error. A true result should mean that the request crossed the scheduler boundary successfully.

Relevant code

• node/api.go:59-74: the admin API returns true after a void Server.AddPeer call.
• p2p/server.go:234-239: Server.AddPeer also has no result.
• p2p/dial.go:204-210: scheduler cancellation silently drops the request.

Impact

Control-plane code can treat a peer as accepted for tracking even though the scheduler discarded the request. This is especially relevant to embedded or custom-lifecycle uses of Node where the admin service can outlive P2P availability.

Suggested direction

Make dialScheduler.addStatic return an acknowledgement or cancellation error, propagate it through Server.AddPeer, and return false or an RPC error when the scheduler did not accept the node.

[SnowingFox]

Opened a PR to address this: #35513