Skip to content

feat(api): add peer management to admin RPC - #7035

Open
317787106 wants to merge 29 commits into
tronprotocol:release_v4.8.3from
317787106:feature/peer_management
Open

317787106 wants to merge 29 commits into
tronprotocol:release_v4.8.3from
317787106:feature/peer_management

Conversation

@317787106

@317787106 317787106 commented Oct 15, 2026 •

Copy link
Copy Markdown
Collaborator

What does this PR do?

Adds runtime peer management to Admin JSON-RPC over HTTP and IPC, so operators can adjust active nodes, inspect connections, and block malicious IPs without restarting FullNode.

The API adds admin_addPeer, admin_removePeer, admin_disconnectPeer, admin_listActivePeers, admin_blockIp, admin_unblockIp, and admin_listBlockedIps. Mutations return a shared PeerOperationResult; active-peer queries reuse the connection, latency, and synchronization statistics used by PeerConnection.log(). The IPC client also supports optional text-table output for admin_listActivePeers.

The five mutating Admin methods emit INFO-level write-attempt logs. Each line records the method and endpoint/IP target; unsafe or overlong targets appear as omitted. The log indicates an attempted call, not a successful mutation.

Why are these changes required?

Operators need to disconnect or block identified malicious peers and add trusted endpoints promptly when investigating degraded connectivity or network isolation. Persistent manual IP blocking keeps those decisions effective across node restarts.

Implementation

This PR builds on the Admin HTTP/IPC framework introduced in upstream PR #6979 and targets release_v4.8.3. It adds peer-management APIs to that framework, together with the supporting behavior and tests in framework and :p2p.

  • framework owns Admin HTTP/IPC dispatch, operation coordination, peer statistics, and persistence of normalized IPs and blockedAtMillis timestamps in CommonStore["blocked-ips"].
  • p2p provides addActiveNode, removeActiveNode, disconnect, and replaceBlockedIps, plus TCP admission, dialing, candidate filtering, and disconnection enforcement.
  • ConnectionPolicy publishes an immutable IP set through a volatile reference and normalizes IPv4-mapped IPv6 addresses for matching. P2pConfig.blockedIps supplies the initial policy; runtime changes update the policy without writing back to startup configuration.
  • The independently runnable p2p-standalone.jar retains --blocked-ips / -b. Its argument coverage is consolidated in StartAppArgsTest, and the module README documents startup and runtime usage.

Behavior and compatibility

1.Active peers

  • admin_addPeer adds an endpoint to the active-node set without adding it to trustNodes or waiting for a successful connection. It rejects endpoints whose IP is manually blocked.
  • admin_removePeer removes an active endpoint and disconnects any currently registered connection to it. It does not cancel a dial or handshake already in progress, so that connection may complete after the call. admin_disconnectPeer disconnects the exact endpoint without removing it from the active-node set, so it may reconnect later.
  • admin_listActivePeers reports current connection, latency, and synchronization statistics. Mutations report success, changed, and disconnectedCount, distinguishing a successful no-op from a business failure.
  • When node.dynamicConfig.enable is enabled, add/remove operations return a structured failure directing operators to update node.active. The other peer-management operations remain available.

2.Manual IP blocklist

  • admin_blockIp and admin_unblockIp accept IPv4 or IPv6 literals. Repeated blocking is a successful no-op that preserves the original blockedAtMillis timestamp; unblocking an absent IP is also a successful no-op. admin_listBlockedIps returns normalized IPs and their timestamps.
  • Persisted blocks are loaded and installed before network initialization. Malformed stored data is deleted on a best-effort basis and the node starts with an empty manual blocklist.
  • A blocklist change first installs the p2p policy and closes matching connections, then persists the snapshot, then publishes the framework query snapshot. A persistence failure returns an internal error and leaves the query snapshot unchanged, although the runtime policy may already have changed; retrying the operation reapplies and persists the intended snapshot.

3.Network scope and compatibility

  • Manual blocking applies to TCP connections and dialing candidates. UDP discovery traffic remains outside its scope. FullNode gains no new peer-management configuration switch.
  • Connect.proto adds DisconnectReason.REQUESTED = 0x0F for explicit disconnects. The existing disconnect message structure, consensus rules, and chain-data formats remain unchanged.

4.JSON-RPC behavior

  • Public JSON-RPC and Admin JSON-RPC share request-envelope validation through JsonRpcRequestValidator. Admin follows the existing public endpoint: explicit id: null and empty method names are invalid; numeric IDs are not limited to signed 64-bit values; params: null passes envelope validation and reaches method binding.
  • Admin continues to reject batch requests. Its five mutating methods emit INFO-level write-attempt logs; a log entry records the requested operation and target, not whether the mutation succeeded.

Testing

Validation on macOS ARM64 / JDK 17 includes:

  • ./gradlew clean build -x test and ./gradlew test (full multi-module suite, completed during development)
  • ./gradlew :framework:checkstyleMain :framework:checkstyleTest :plugins:checkstyleMain (completed during development)
  • check_reference_conf.py, check_reference_comments.py, and check_math_usage.sh (completed during development)
  • ./gradlew :framework:test --tests org.tron.core.services.admin.AdminJsonRpcImplTest
  • ./gradlew :framework:test --tests org.tron.core.services.admin.ipc.server.AdminJsonRpcRequestTest --tests org.tron.core.services.jsonrpc.JsonRpcServletTest
  • ./gradlew :framework:checkstyleMain :framework:checkstyleTest (after the logging and validation changes)

Coverage includes startup policy installation, persistence and corrupt-data recovery, update ordering and retries, idempotency, concurrent mutations, dynamic-config restrictions, IP normalization, and TCP inbound/outbound rejection. HTTP/IPC dispatch, error codes, peer statistics, IPC text output, network lifecycle, and effective-connection compatibility have regression tests. The standalone p2p JAR build and --blocked-ips help invocation were also checked during development.

Deferred audit logging

Issue #6989 specifies audit logging in the shared Admin invocation path. This PR includes INFO-level write-attempt logs for mutating methods on HTTP and IPC, including notifications that reach the method, and service logs for completed changes. These operational logs do not capture transport, caller context, binding failures, or every final outcome. Full audit logging across the shared HTTP/IPC dispatch path is deferred to follow-up work because it requires broader changes and is not an immediate operational need for this feature.

Add HTTP and Unix-domain-socket transports for the shared Admin JSON-RPC API, with an attach console for trusted node operators.

Include transport configuration, socket permissions, request handling, bounded client lifecycle, and startup isolation regression tests.
Add peer management to Admin HTTP and IPC, including active-peer inspection,
persistent IP blocking, and optional IPC text output.

Implement the management APIs and TCP connection policy in the built-in p2p
module. Retain the standalone blocked-ips option and consolidate its tests
in StartAppArgsTest.

Base the change on feature/admin_rpc with java-tron 4.8.3.
# Conflicts:
#	common/src/main/resources/reference.conf
#	framework/build.gradle
#	framework/src/main/java/org/tron/core/config/args/Args.java
#	framework/src/main/java/org/tron/core/config/args/CommandLineArguments.java
#	framework/src/main/java/org/tron/core/services/admin/AdminJsonRpc.java
#	framework/src/main/java/org/tron/core/services/admin/AdminJsonRpcImpl.java
#	framework/src/main/java/org/tron/core/services/admin/AdminJsonRpcRequestHandler.java
#	framework/src/main/java/org/tron/core/services/admin/http/AdminRpcHttpService.java
#	framework/src/main/java/org/tron/core/services/admin/http/AdminRpcServlet.java
#	framework/src/main/java/org/tron/core/services/admin/http/VirtualHostValidator.java
#	framework/src/main/java/org/tron/core/services/admin/ipc/client/IpcClient.java
#	framework/src/main/java/org/tron/core/services/admin/ipc/client/IpcConsoleCommands.java
#	framework/src/main/java/org/tron/core/services/admin/ipc/client/IpcResponse.java
#	framework/src/main/java/org/tron/core/services/admin/ipc/server/IpcRequestHandler.java
#	framework/src/main/java/org/tron/core/services/admin/ipc/server/IpcSocketFiles.java
#	framework/src/main/java/org/tron/core/services/jsonrpc/JsonRpcServlet.java
#	framework/src/main/java/org/tron/program/FullNode.java
#	framework/src/test/java/org/tron/core/services/admin/http/AdminRpcServletTest.java
#	framework/src/test/java/org/tron/core/services/admin/ipc/client/IpcClientTest.java
#	framework/src/test/java/org/tron/core/services/admin/ipc/client/IpcConsoleCommandsTest.java
#	framework/src/test/java/org/tron/core/services/admin/ipc/client/IpcResponseTest.java
#	framework/src/test/java/org/tron/core/services/admin/ipc/server/AdminJsonRpcRequestTest.java
#	framework/src/test/java/org/tron/core/services/admin/ipc/server/IpcRequestHandlerTest.java
#	framework/src/test/java/org/tron/core/services/admin/ipc/server/IpcServiceTest.java
#	framework/src/test/java/org/tron/core/services/admin/ipc/server/IpcSocketFilesTest.java
#	framework/src/test/java/org/tron/program/FullNodeTest.java
#	gradle/verification-metadata.xml
# Conflicts:
#	p2p/src/main/java/org/tron/p2p/P2pService.java
#	p2p/src/main/java/org/tron/p2p/connection/business/pool/ConnPoolService.java
#	p2p/src/main/java/org/tron/p2p/utils/NetUtil.java
#	p2p/src/test/java/org/tron/p2p/connection/ChannelManagerTest.java
#	p2p/src/test/java/org/tron/p2p/connection/ConnPoolServiceTest.java

@lxcmyf lxcmyf left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Three blocking correctness and security issues are noted inline.

InetAddress address = parseIp(ip);
synchronized (managementLock) {
P2pService p2pService = requireP2pService();
if (blockedIps.containsKey(address)) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[MUST] This no-op assumes the framework snapshot matches the live P2P policy, but a persistence failure intentionally leaves them divergent. If admin_unblockIp applies the live unblock and the store write fails, a later admin_blockIp returns success here without restoring the block, so the query reports this IP as blocked while TCP connections are accepted. Reconcile the live policy on both no-op paths, or roll it back when persistence fails.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Thanks for raising this. The scenario assumes the node continues normal operation after CommonStore.put fails. For the underlying disk failure considered here, the node can no longer persist blocks and requires operator intervention; subsequent Admin mutations are not a supported recovery path. We intentionally apply the P2P policy first so a block takes effect immediately, and we surface and log any persistence failure. Given this failure model, we’ll keep the current ordering.

@Override
public PeerOperationResult addPeer(String endpoint)
throws JsonRpcInvalidParamsException, JsonRpcInternalException {
logger.info("Admin write attempt method=admin_addPeer target={}", safeTarget(endpoint));

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[MUST] Attempt-only method logging omits the transport, available caller context, and final outcome, so binding errors, rejected requests, and business failures (success: false) are not audited. Add auditing in the shared HTTP/IPC invocation path and record transport, method, caller context, and outcome for every mutating request, including notifications, without logging raw parameters.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Thanks for the review. I agree the current logs are operational logs, not a complete audit trail. Mutating calls that reach AdminJsonRpcImpl log an attempt, including notifications; successful state changes are also logged by PeerManagementService. Binding failures do not change state. These logs provide a useful initial record for incident analysis, though they do not capture transport, caller context, or every failure outcome.

Full auditing across HTTP and IPC would require changes to the shared dispatch path, where we do not yet have an audit facility. I propose documenting the current limits and tracking that work separately. This would defer the audit requirement in #6989, so I’d like maintainer agreement to make it a follow-up rather than a blocker for this PR.

}
logger.info("Connect to peer {}", address);
peerClientCache.put(address.getAddress(), System.currentTimeMillis());
if (peerClient.connectAsync(n, false) == null) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[MUST] Removing an active node does not cancel a dial or handshake already started here. Because the channel is registered only after the handshake, removePeer can remove the address and find no connected channel, then this pending connection completes and remains active. Track and cancel pending active-node dials, or tag and revalidate them at handshake admission.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Thanks for pointing this out. The race is real: a dial already in progress may complete after admin_removePeer returns. The intended behavior of removePeer is to remove the endpoint from the active-node set and disconnect currently registered peer connections; it does not guarantee that the endpoint cannot connect again. The scheduler already rechecks active-node membership before starting a dial, so removal prevents subsequent active-node attempts.

Preventing an endpoint from connecting through in-flight dials, discovery, or inbound connections would require a broader exclusion policy. For an IP that must not reconnect, operators can use admin_blockIp. I’ll clarify the pending-dial limitation in the PR description, but I don’t think cancelling in-flight dials is required for the specified removePeer behavior.

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.

2 participants