Repository navigation
Conversation
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.
…ken count restrict
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
…ating request for two jsonrpc services
# 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
left a comment
There was a problem hiding this comment.
Three blocking correctness and security issues are noted inline.
| InetAddress address = parseIp(ip); | ||
| synchronized (managementLock) { | ||
| P2pService p2pService = requireP2pService(); | ||
| if (blockedIps.containsKey(address)) { |
There was a problem hiding this comment.
[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.
There was a problem hiding this comment.
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)); |
There was a problem hiding this comment.
[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.
There was a problem hiding this comment.
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) { |
There was a problem hiding this comment.
[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.
There was a problem hiding this comment.
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.
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, andadmin_listBlockedIps. Mutations return a sharedPeerOperationResult; active-peer queries reuse the connection, latency, and synchronization statistics used byPeerConnection.log(). The IPC client also supports optional text-table output foradmin_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 inframeworkand:p2p.frameworkowns Admin HTTP/IPC dispatch, operation coordination, peer statistics, and persistence of normalized IPs andblockedAtMillistimestamps inCommonStore["blocked-ips"].p2pprovidesaddActiveNode,removeActiveNode,disconnect, andreplaceBlockedIps, plus TCP admission, dialing, candidate filtering, and disconnection enforcement.ConnectionPolicypublishes an immutable IP set through a volatile reference and normalizes IPv4-mapped IPv6 addresses for matching.P2pConfig.blockedIpssupplies the initial policy; runtime changes update the policy without writing back to startup configuration.p2p-standalone.jarretains--blocked-ips/-b. Its argument coverage is consolidated inStartAppArgsTest, and the module README documents startup and runtime usage.Behavior and compatibility
1.Active peers
admin_addPeeradds an endpoint to the active-node set without adding it totrustNodesor waiting for a successful connection. It rejects endpoints whose IP is manually blocked.admin_removePeerremoves 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_disconnectPeerdisconnects the exact endpoint without removing it from the active-node set, so it may reconnect later.admin_listActivePeersreports current connection, latency, and synchronization statistics. Mutations reportsuccess,changed, anddisconnectedCount, distinguishing a successful no-op from a business failure.node.dynamicConfig.enableis enabled, add/remove operations return a structured failure directing operators to updatenode.active. The other peer-management operations remain available.2.Manual IP blocklist
admin_blockIpandadmin_unblockIpaccept IPv4 or IPv6 literals. Repeated blocking is a successful no-op that preserves the originalblockedAtMillistimestamp; unblocking an absent IP is also a successful no-op.admin_listBlockedIpsreturns normalized IPs and their timestamps.3.Network scope and compatibility
Connect.protoaddsDisconnectReason.REQUESTED = 0x0Ffor explicit disconnects. The existing disconnect message structure, consensus rules, and chain-data formats remain unchanged.4.JSON-RPC behavior
JsonRpcRequestValidator. Admin follows the existing public endpoint: explicitid: nulland empty method names are invalid; numeric IDs are not limited to signed 64-bit values;params: nullpasses envelope validation and reaches method binding.Testing
Validation on macOS ARM64 / JDK 17 includes:
./gradlew clean build -x testand./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, andcheck_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-ipshelp 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.