chore(p2p): internalize libp2p as local module#13
Open
barbatos2011 wants to merge 6 commits intodevelopfrom
Open
chore(p2p): internalize libp2p as local module#13barbatos2011 wants to merge 6 commits intodevelopfrom
barbatos2011 wants to merge 6 commits intodevelopfrom
Conversation
added 5 commits
April 6, 2026 02:05
Copy source code from tronprotocol/libp2p v2.2.7 into a new p2p/ gradle submodule. This commit introduces the original source without modifications, preserving the org.tron.p2p package structure. - Add p2p/build.gradle with protobuf plugin and dependency declarations - Add p2p/lombok.config to override root lombok field name (log vs logger) - Move example code (StartApp) to src/test/java - Proto generated code is not committed (built by protobuf plugin) - Update gradle/verification-metadata.xml for new dependencies
Replace all @Slf4j-generated 'log' references with 'logger' to match java-tron's lombok.config (lombok.log.fieldName=logger). Remove the temporary p2p/lombok.config override added in the previous commit. Checkstyle is not yet enabled for the p2p module due to ~1900 violations inherited from the original libp2p codebase. This will be addressed in a follow-up PR.
Replace the Maven dependency on io.github.tronprotocol:libp2p:2.2.7 with a project dependency on the local :p2p module. This eliminates 11 exclude rules that were needed for the external artifact and removes the libp2p entry from dependency verification metadata. java-tron no longer depends on any external libp2p artifact.
Apply google-java-format for bulk formatting (indentation, imports, whitespace), then fix remaining violations manually: - Lambda indentation adjustments for checkstyle 8.7 compatibility - Line length wrapping for long strings and license headers - Empty catch blocks: add // expected comments - Star import replacement, multiple variable declaration splits Enable checkstyle plugin in p2p/build.gradle with proto-generated code excluded. Both checkstyleMain and checkstyleTest now pass.
- Replace BasicThreadFactory.builder() with new BasicThreadFactory.Builder() to use commons-lang3 3.4-compatible API, preventing implicit global version upgrade from 3.4 to 3.18.0 - Remove unused grpc-core dependency (p2p protos define only messages, no gRPC services); retain grpc-netty as Netty transport provider - Add p2p/.gitignore to exclude proto-generated code for all developers - Update verification-metadata.xml for changed dependency tree
❌ Math Usage Detection ResultsFound forbidden usage of Please review if this usage is intended. Caution Note: You should use |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
40 issues found across 124 files
Note: This PR contains a large number of files. cubic only reviews up to 75 files per PR, so some files may not have been reviewed.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="p2p/src/main/java/org/tron/p2p/discover/Node.java">
<violation number="1" location="p2p/src/main/java/org/tron/p2p/discover/Node.java:111">
P1: `getPreferInetSocketAddress()` can return `null` (see its else branch), but `getHostKey()` dereferences the result without a null check, leading to a `NullPointerException`.</violation>
<violation number="2" location="p2p/src/main/java/org/tron/p2p/discover/Node.java:155">
P1: `hashCode()` is based on `{hostV4, hostV6, port}` (via `format()`) but `equals()` is based on `id` (via `getIdString()`). This violates the Java contract that equal objects must have equal hash codes, causing silent data loss or lookup failures in any hash-based collection (`HashMap`, `HashSet`, etc.). Both methods should use the same fields.</violation>
</file>
<file name="p2p/src/main/java/org/tron/p2p/dns/tree/Tree.java">
<violation number="1" location="p2p/src/main/java/org/tron/p2p/dns/tree/Tree.java:151">
P1: `StringUtils.isNoneEmpty` does not exist in commons-lang3 3.4 — it was added in 3.6. This will throw `NoSuchMethodError` at runtime. Use `isNotEmpty` (which is available in 3.4 and is already used elsewhere in this file at line 96).</violation>
</file>
<file name="p2p/src/main/java/org/tron/p2p/connection/socket/PeerServer.java">
<violation number="1" location="p2p/src/main/java/org/tron/p2p/connection/socket/PeerServer.java:18">
P1: `channelFuture` and `listening` are written on the `PeerServer` thread and read from the thread calling `close()`, but neither is `volatile`. This means `close()` can see stale defaults and silently skip channel shutdown.</violation>
</file>
<file name="p2p/src/main/java/org/tron/p2p/dns/DnsNode.java">
<violation number="1" location="p2p/src/main/java/org/tron/p2p/dns/DnsNode.java:29">
P1: The `id` parameter is accepted but silently discarded — `null` is passed to `super()` instead. The `decompress()` method extracts the node ID from protobuf and passes it here, so deserialized `DnsNode` objects will always have a null ID.</violation>
<violation number="2" location="p2p/src/main/java/org/tron/p2p/dns/DnsNode.java:91">
P2: `equals()` is overridden to compare `v4Hex`/`v6Hex`/`port`, but `hashCode()` is not overridden. The inherited `Node.hashCode()` uses `hostV4`/`hostV6`/`port` (different fields). This violates the `equals`/`hashCode` contract and will cause incorrect behavior in hash-based collections.</violation>
</file>
<file name="p2p/src/main/java/org/tron/p2p/discover/protocol/kad/NodeHandler.java">
<violation number="1" location="p2p/src/main/java/org/tron/p2p/discover/protocol/kad/NodeHandler.java:111">
P1: Potential NPE: `state` can be `null` (constructor skips `changeState` when `getPreferInetSocketAddress()` returns null), and `.equals()` on a null receiver throws. Use `==` for enum comparison, which is null-safe and consistent with the rest of this file.</violation>
</file>
<file name="p2p/src/main/java/org/tron/p2p/connection/business/detect/NodeDetectService.java">
<violation number="1" location="p2p/src/main/java/org/tron/p2p/connection/business/detect/NodeDetectService.java:170">
P1: Missing `return` after removing a bad node causes fall-through that marks the node as successfully detected. When `cost > NODE_DETECT_TIMEOUT` or `remainConnections == 0`, the node is correctly removed from `nodeStatMap` and added to `badNodesCache`, but then `setLastSuccessDetectTime` and `setStatusMessage` are still called on it. This contradicts the intent: if the node later re-enters the map, it could carry stale "success" state. Add a `return` (or wrap the subsequent lines in an `else`) so bad nodes skip the success-path updates.</violation>
</file>
<file name="p2p/src/main/java/org/tron/p2p/dns/sync/RandomIterator.java">
<violation number="1" location="p2p/src/main/java/org/tron/p2p/dns/sync/RandomIterator.java:91">
P1: `random.nextInt(size)` throws `IllegalArgumentException` when `clientTrees` is empty (size == 0). Add an empty check and return `null`, consistent with the null check above.</violation>
</file>
<file name="p2p/src/main/java/org/tron/p2p/connection/ChannelManager.java">
<violation number="1" location="p2p/src/main/java/org/tron/p2p/connection/ChannelManager.java:112">
P1: Same TOCTOU race as in `banNode`: the second `getIfPresent` can return `null` after the cache evicts the entry between calls, causing an NPE on unboxing. Use a local variable.</violation>
<violation number="2" location="p2p/src/main/java/org/tron/p2p/connection/ChannelManager.java:180">
P1: TOCTOU race on the Guava cache: the entry can be evicted between the two `getIfPresent` calls, causing an NPE when the second call returns `null` and it is unboxed to `long`. Store the result in a local variable.</violation>
</file>
<file name="p2p/src/main/java/org/tron/p2p/dns/tree/Algorithm.java">
<violation number="1" location="p2p/src/main/java/org/tron/p2p/dns/tree/Algorithm.java:29">
P1: `BigInteger.toString(16)` strips leading zeros, so if the x-coordinate has a leading zero nibble, `pubKeyHex` will be shorter than 128 chars and `substring(0, 64)` will mix x and y coordinate bytes. Pad the hex string to 128 characters before extracting the x-coordinate.</violation>
<violation number="2" location="p2p/src/main/java/org/tron/p2p/dns/tree/Algorithm.java:71">
P2: `msg.getBytes()` uses the platform-default charset, which is non-deterministic across JVMs. This file already imports and uses `StandardCharsets.UTF_8` in `encode64`/`encode32`; the same should be applied here and in the two other call sites (`sigData` and `encode32AndTruncate`) to ensure signatures and hashes are portable.</violation>
</file>
<file name="p2p/src/main/java/org/tron/p2p/discover/socket/MessageHandler.java">
<violation number="1" location="p2p/src/main/java/org/tron/p2p/discover/socket/MessageHandler.java:65">
P2: Exception stack trace is lost because `cause.getMessage()` is interpolated as a plain string. Pass the `Throwable` as an extra argument after all format-string placeholders so SLF4J appends the full stack trace.</violation>
</file>
<file name="p2p/src/main/java/org/tron/p2p/dns/lookup/LookUpTxt.java">
<violation number="1" location="p2p/src/main/java/org/tron/p2p/dns/lookup/LookUpTxt.java:55">
P2: Duplicate AdGuard entries: `"2a00:5a60::ad1:0ff"` and `"2a00:5a60::ad2:0ff"` appear twice in `publicDnsV6`, giving them 2× selection weight over other providers. Remove one pair.</violation>
<violation number="2" location="p2p/src/main/java/org/tron/p2p/dns/lookup/LookUpTxt.java:104">
P2: The comment says "only get first Record" but this loop overwrites `txt` on every iteration, so it actually keeps the **last** record. Replace the loop with a direct array access to match the stated intent.</violation>
</file>
<file name="p2p/src/main/java/org/tron/p2p/discover/protocol/kad/table/NodeBucket.java">
<violation number="1" location="p2p/src/main/java/org/tron/p2p/discover/protocol/kad/table/NodeBucket.java:32">
P2: `sorted` is an alias for `nodes`, not a copy. `Collections.sort(sorted, ...)` mutates the bucket's internal list order as a side effect every time the bucket is full. Create an actual copy before sorting.</violation>
</file>
<file name="p2p/src/main/java/org/tron/p2p/connection/Channel.java">
<violation number="1" location="p2p/src/main/java/org/tron/p2p/connection/Channel.java:182">
P2: `hashCode()` will throw `NullPointerException` if `inetSocketAddress` is null, while `equals()` handles the null case via `Objects.equals`. Use `Objects.hashCode` for consistency.</violation>
</file>
<file name="p2p/src/main/java/org/tron/p2p/dns/update/AwsClient.java">
<violation number="1" location="p2p/src/main/java/org/tron/p2p/dns/update/AwsClient.java:303">
P2: Swallowed `InterruptedException` — restore the thread's interrupt status so that callers (e.g., during node shutdown) can detect the interruption.</violation>
</file>
<file name="p2p/src/main/java/org/tron/p2p/discover/socket/DiscoverServer.java">
<violation number="1" location="p2p/src/main/java/org/tron/p2p/discover/socket/DiscoverServer.java:19">
P2: `channel` is written on the `DiscoverServer` thread and read in `close()` from another thread, but is not `volatile`. The `close()` caller may never see the assigned channel and skip the cleanup. Mark it `volatile` like `shutdown`.</violation>
</file>
<file name="p2p/src/main/java/org/tron/p2p/P2pService.java">
<violation number="1" location="p2p/src/main/java/org/tron/p2p/P2pService.java:25">
P2: Race condition in `close()`: the `volatile boolean` check-then-set is not atomic, so two concurrent calls (e.g., explicit close + shutdown hook) can both pass the guard and double-close the managers. Use `AtomicBoolean.compareAndSet` instead.</violation>
</file>
<file name="p2p/src/main/java/org/tron/p2p/connection/message/Message.java">
<violation number="1" location="p2p/src/main/java/org/tron/p2p/connection/message/Message.java:40">
P2: Access to `encode[0]` is outside the try-catch block, so a null or empty `encode` array throws an unchecked exception instead of a `P2pException`. Move the access inside the try block and declare `type` before it with a default, or add an explicit guard.</violation>
</file>
<file name="p2p/src/main/java/org/tron/p2p/dns/tree/RootEntry.java">
<violation number="1" location="p2p/src/main/java/org/tron/p2p/dns/tree/RootEntry.java:23">
P2: `new String(byte[])` without an explicit charset uses the platform default, which may not be UTF-8 on all systems. In a P2P networking context, this can cause cross-platform inconsistency. Use `new String(..., StandardCharsets.UTF_8)` here and in the corresponding `getBytes()` calls (lines 27, 44, 50, 57, 58, 76).</violation>
</file>
<file name="p2p/src/main/java/org/tron/p2p/connection/socket/PeerClient.java">
<violation number="1" location="p2p/src/main/java/org/tron/p2p/connection/socket/PeerClient.java:81">
P2: `future.cause()` can be `null` when the future is cancelled (not failed), since `isSuccess()` returns `false` for both cancellation and failure. This will throw a `NullPointerException`. Null-check `cause()` before dereferencing.</violation>
</file>
<file name="p2p/src/main/java/org/tron/p2p/dns/update/AliClient.java">
<violation number="1" location="p2p/src/main/java/org/tron/p2p/dns/update/AliClient.java:38">
P2: `retryWaitTime = 30` is passed to `Thread.sleep()`, which takes milliseconds. A 30 ms retry wait for DNS API calls provides no meaningful backoff. This likely should be `30_000` (30 seconds) or `30 * 1000L`.</violation>
<violation number="2" location="p2p/src/main/java/org/tron/p2p/dns/update/AliClient.java:39">
P2: Instance field `treeNodeTTL` (1 day) shadows the `Publish` interface constant of the same name (7 days). If the 1-day value is intentional for Alibaba DNS, this should at least have a comment explaining the divergence. If not, remove the field and let the interface constant apply.</violation>
</file>
<file name="p2p/src/main/java/org/tron/p2p/discover/protocol/kad/KadService.java">
<violation number="1" location="p2p/src/main/java/org/tron/p2p/discover/protocol/kad/KadService.java:218">
P2: Removing from `nodeHandlerMap.values()` inside `forEach` on the same view can skip entries during traversal. Use `removeIf` instead, which is both correct and more efficient for `ConcurrentHashMap` (it locks buckets properly during bulk removal and avoids the O(n) per-value scan).</violation>
</file>
<file name="p2p/src/main/java/org/tron/p2p/discover/socket/P2pPacketDecoder.java">
<violation number="1" location="p2p/src/main/java/org/tron/p2p/discover/socket/P2pPacketDecoder.java:25">
P2: Use `packet.sender()` instead of `ctx.channel().remoteAddress()`. On an unconnected `DatagramChannel`, `remoteAddress()` returns `null`, so this log line will always print `null` for the address. Every other log statement in this method already uses `packet.sender()`.</violation>
</file>
<file name="p2p/src/main/java/org/tron/p2p/base/Constant.java">
<violation number="1" location="p2p/src/main/java/org/tron/p2p/base/Constant.java:10">
P2: Two IP-discovery endpoints use plain HTTP, making external-IP resolution vulnerable to MITM. An attacker could inject a spoofed IP that the node then advertises to peers. Use HTTPS where the service supports it — `checkip.amazonaws.com` is available over HTTPS.</violation>
</file>
<file name="p2p/src/main/java/org/tron/p2p/dns/tree/LinkEntry.java">
<violation number="1" location="p2p/src/main/java/org/tron/p2p/dns/tree/LinkEntry.java:39">
P2: The original `RuntimeException` cause is lost when re-throwing as `DnsException`. Chain it using the three-argument constructor so the root cause appears in logs and stack traces.</violation>
</file>
<file name="p2p/src/main/java/org/tron/p2p/discover/message/Message.java">
<violation number="1" location="p2p/src/main/java/org/tron/p2p/discover/message/Message.java:65">
P2: Overriding `equals` without overriding `hashCode` violates the Java `equals`/`hashCode` contract. Moreover, this override just calls `super.equals(obj)`, so it adds no behavior and can be removed entirely. If `equals` semantics are intentionally identity-based, simply remove the override; if value-based equality is needed in the future, override both `equals` and `hashCode` together.</violation>
</file>
<file name="p2p/src/main/java/org/tron/p2p/discover/message/kad/NeighborsMessage.java">
<violation number="1" location="p2p/src/main/java/org/tron/p2p/discover/message/kad/NeighborsMessage.java:68">
P2: `getNodes()` is called three times in `valid()`, each time allocating a new list and re-converting every neighbour. Cache the result in a local variable.</violation>
</file>
<file name="p2p/src/main/java/org/tron/p2p/dns/sync/LinkCache.java">
<violation number="1" location="p2p/src/main/java/org/tron/p2p/dns/sync/LinkCache.java:18">
P2: `backrefs` is missing the `private` access modifier, leaving it package-private. The adjacent `changed` field is `private`; this appears to be an oversight.</violation>
<violation number="2" location="p2p/src/main/java/org/tron/p2p/dns/sync/LinkCache.java:59">
P2: Use `stk.remove(size - 1)` instead of reassigning `stk` to a `subList` view. `subList` returns a view backed by the original list, so repeated calls create increasingly nested view wrappers — wasteful and fragile for a simple stack-pop operation.</violation>
</file>
<file name="p2p/src/main/java/org/tron/p2p/discover/NodeManager.java">
<violation number="1" location="p2p/src/main/java/org/tron/p2p/discover/NodeManager.java:31">
P2: `discoverService` is not null-checked in getter methods, inconsistent with the defensive pattern used in `close()`. If `getConnectableNodes()` (or the other getters) is called before `init()` or during shutdown, it will throw a `NullPointerException`. Consider adding a null guard consistent with `close()`, returning an empty list (or null for `getHomeNode`) when the service is unavailable.</violation>
</file>
<file name="p2p/src/main/java/org/tron/p2p/base/Parameter.java">
<violation number="1" location="p2p/src/main/java/org/tron/p2p/base/Parameter.java:44">
P2: Race condition: `volatile` on `HashMap`/`ArrayList` does not make the collections thread-safe. `addP2pEventHandle` performs unsynchronized check-then-act on `handlerMap` and unsynchronized `add` on `handlerList`. Concurrent calls can corrupt internal state or silently lose registrations. Consider either synchronizing this method or using `ConcurrentHashMap` / `CopyOnWriteArrayList`.</violation>
</file>
<file name="p2p/src/main/java/org/tron/p2p/connection/socket/P2pChannelInitializer.java">
<violation number="1" location="p2p/src/main/java/org/tron/p2p/connection/socket/P2pChannelInitializer.java:37">
P2: `SO_BACKLOG` is a server-socket option (controls the TCP listen backlog); setting it on a client `NioSocketChannel` is a silent no-op. This line can be removed to avoid misleading future maintainers.</violation>
</file>
<file name="p2p/src/main/java/org/tron/p2p/discover/protocol/kad/table/NodeTable.java">
<violation number="1" location="p2p/src/main/java/org/tron/p2p/discover/protocol/kad/table/NodeTable.java:71">
P2: `getBucketsCount()` is missing `synchronized`, breaking the thread-safety pattern established by every other method that accesses the shared `buckets`/`nodes` state. Concurrent calls to `addNode`/`dropNode` could produce an inconsistent count.</violation>
</file>
<file name="p2p/src/main/java/org/tron/p2p/discover/protocol/kad/DiscoverTask.java">
<violation number="1" location="p2p/src/main/java/org/tron/p2p/discover/protocol/kad/DiscoverTask.java:74">
P2: After catching `InterruptedException` and restoring the interrupt flag, the method should `return` to allow prompt shutdown. Without this, `shutdownNow()` in `close()` will cause up to `MAX_STEPS` (8) recursive iterations that each immediately re-throw from `Thread.sleep`, logging spurious warnings before the task finally terminates.</violation>
</file>
<file name="p2p/src/main/java/org/tron/p2p/connection/business/pool/ConnPoolService.java">
<violation number="1" location="p2p/src/main/java/org/tron/p2p/connection/business/pool/ConnPoolService.java:286">
P3: Misleading comment: `// if len(peers) >= 0` is always true for a collection. Should be `> 0` to match the `!peers.isEmpty()` guard below it.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.
| } | ||
|
|
||
| @Override | ||
| public int hashCode() { |
There was a problem hiding this comment.
P1: hashCode() is based on {hostV4, hostV6, port} (via format()) but equals() is based on id (via getIdString()). This violates the Java contract that equal objects must have equal hash codes, causing silent data loss or lookup failures in any hash-based collection (HashMap, HashSet, etc.). Both methods should use the same fields.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At p2p/src/main/java/org/tron/p2p/discover/Node.java, line 155:
<comment>`hashCode()` is based on `{hostV4, hostV6, port}` (via `format()`) but `equals()` is based on `id` (via `getIdString()`). This violates the Java contract that equal objects must have equal hash codes, causing silent data loss or lookup failures in any hash-based collection (`HashMap`, `HashSet`, etc.). Both methods should use the same fields.</comment>
<file context>
@@ -0,0 +1,197 @@
+ }
+
+ @Override
+ public int hashCode() {
+ return this.format().hashCode();
+ }
</file context>
| // hash => lower(hash).domain | ||
| public Map<String, String> toTXT(String rootDomain) { | ||
| Map<String, String> dnsRecords = new HashMap<>(); | ||
| if (StringUtils.isNoneEmpty(rootDomain)) { |
There was a problem hiding this comment.
P1: StringUtils.isNoneEmpty does not exist in commons-lang3 3.4 — it was added in 3.6. This will throw NoSuchMethodError at runtime. Use isNotEmpty (which is available in 3.4 and is already used elsewhere in this file at line 96).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At p2p/src/main/java/org/tron/p2p/dns/tree/Tree.java, line 151:
<comment>`StringUtils.isNoneEmpty` does not exist in commons-lang3 3.4 — it was added in 3.6. This will throw `NoSuchMethodError` at runtime. Use `isNotEmpty` (which is available in 3.4 and is already used elsewhere in this file at line 96).</comment>
<file context>
@@ -0,0 +1,242 @@
+ // hash => lower(hash).domain
+ public Map<String, String> toTXT(String rootDomain) {
+ Map<String, String> dnsRecords = new HashMap<>();
+ if (StringUtils.isNoneEmpty(rootDomain)) {
+ dnsRecords.put(rootDomain, rootEntry.toFormat());
+ } else {
</file context>
Suggested change
| if (StringUtils.isNoneEmpty(rootDomain)) { | |
| if (StringUtils.isNotEmpty(rootDomain)) { |
Comment on lines
+18
to
+19
| private ChannelFuture channelFuture; | ||
| private boolean listening; |
There was a problem hiding this comment.
P1: channelFuture and listening are written on the PeerServer thread and read from the thread calling close(), but neither is volatile. This means close() can see stale defaults and silently skip channel shutdown.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At p2p/src/main/java/org/tron/p2p/connection/socket/PeerServer.java, line 18:
<comment>`channelFuture` and `listening` are written on the `PeerServer` thread and read from the thread calling `close()`, but neither is `volatile`. This means `close()` can see stale defaults and silently skip channel shutdown.</comment>
<file context>
@@ -0,0 +1,81 @@
+@Slf4j(topic = "net")
+public class PeerServer {
+
+ private ChannelFuture channelFuture;
+ private boolean listening;
+
</file context>
Suggested change
| private ChannelFuture channelFuture; | |
| private boolean listening; | |
| private volatile ChannelFuture channelFuture; | |
| private volatile boolean listening; |
| private String v6Hex = Constant.ipV6Hex; | ||
|
|
||
| public DnsNode(byte[] id, String hostV4, String hostV6, int port) throws UnknownHostException { | ||
| super(null, hostV4, hostV6, port); |
There was a problem hiding this comment.
P1: The id parameter is accepted but silently discarded — null is passed to super() instead. The decompress() method extracts the node ID from protobuf and passes it here, so deserialized DnsNode objects will always have a null ID.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At p2p/src/main/java/org/tron/p2p/dns/DnsNode.java, line 29:
<comment>The `id` parameter is accepted but silently discarded — `null` is passed to `super()` instead. The `decompress()` method extracts the node ID from protobuf and passes it here, so deserialized `DnsNode` objects will always have a null ID.</comment>
<file context>
@@ -0,0 +1,98 @@
+ private String v6Hex = Constant.ipV6Hex;
+
+ public DnsNode(byte[] id, String hostV4, String hostV6, int port) throws UnknownHostException {
+ super(null, hostV4, hostV6, port);
+ if (StringUtils.isNotEmpty(hostV4)) {
+ this.v4Hex = ipToString(hostV4);
</file context>
Suggested change
| super(null, hostV4, hostV6, port); | |
| super(id, hostV4, hostV6, port); |
| node.setP2pVersion(msg.getNetworkId()); | ||
| if (!node.isConnectible(Parameter.p2pConfig.getNetworkId())) { | ||
| changeState(State.DEAD); | ||
| } else if (state.equals(State.DEAD)) { |
There was a problem hiding this comment.
P1: Potential NPE: state can be null (constructor skips changeState when getPreferInetSocketAddress() returns null), and .equals() on a null receiver throws. Use == for enum comparison, which is null-safe and consistent with the rest of this file.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At p2p/src/main/java/org/tron/p2p/discover/protocol/kad/NodeHandler.java, line 111:
<comment>Potential NPE: `state` can be `null` (constructor skips `changeState` when `getPreferInetSocketAddress()` returns null), and `.equals()` on a null receiver throws. Use `==` for enum comparison, which is null-safe and consistent with the rest of this file.</comment>
<file context>
@@ -0,0 +1,247 @@
+ node.setP2pVersion(msg.getNetworkId());
+ if (!node.isConnectible(Parameter.p2pConfig.getNetworkId())) {
+ changeState(State.DEAD);
+ } else if (state.equals(State.DEAD)) {
+ changeState(State.DISCOVERED);
+ }
</file context>
| // limit the size of receiving buffer to 1024 | ||
| ch.config().setRecvByteBufAllocator(new FixedRecvByteBufAllocator(256 * 1024)); | ||
| ch.config().setOption(ChannelOption.SO_RCVBUF, 256 * 1024); | ||
| ch.config().setOption(ChannelOption.SO_BACKLOG, 1024); |
There was a problem hiding this comment.
P2: SO_BACKLOG is a server-socket option (controls the TCP listen backlog); setting it on a client NioSocketChannel is a silent no-op. This line can be removed to avoid misleading future maintainers.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At p2p/src/main/java/org/tron/p2p/connection/socket/P2pChannelInitializer.java, line 37:
<comment>`SO_BACKLOG` is a server-socket option (controls the TCP listen backlog); setting it on a client `NioSocketChannel` is a silent no-op. This line can be removed to avoid misleading future maintainers.</comment>
<file context>
@@ -0,0 +1,64 @@
+ // limit the size of receiving buffer to 1024
+ ch.config().setRecvByteBufAllocator(new FixedRecvByteBufAllocator(256 * 1024));
+ ch.config().setOption(ChannelOption.SO_RCVBUF, 256 * 1024);
+ ch.config().setOption(ChannelOption.SO_BACKLOG, 1024);
+
+ // be aware of channel closing
</file context>
| } | ||
| } | ||
|
|
||
| public int getBucketsCount() { |
There was a problem hiding this comment.
P2: getBucketsCount() is missing synchronized, breaking the thread-safety pattern established by every other method that accesses the shared buckets/nodes state. Concurrent calls to addNode/dropNode could produce an inconsistent count.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At p2p/src/main/java/org/tron/p2p/discover/protocol/kad/table/NodeTable.java, line 71:
<comment>`getBucketsCount()` is missing `synchronized`, breaking the thread-safety pattern established by every other method that accesses the shared `buckets`/`nodes` state. Concurrent calls to `addNode`/`dropNode` could produce an inconsistent count.</comment>
<file context>
@@ -0,0 +1,114 @@
+ }
+ }
+
+ public int getBucketsCount() {
+ int i = 0;
+ for (NodeBucket b : buckets) {
</file context>
| @Slf4j(topic = "net") | ||
| public class LinkCache { | ||
|
|
||
| @Getter Map<String, Set<String>> backrefs; |
There was a problem hiding this comment.
P2: backrefs is missing the private access modifier, leaving it package-private. The adjacent changed field is private; this appears to be an oversight.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At p2p/src/main/java/org/tron/p2p/dns/sync/LinkCache.java, line 18:
<comment>`backrefs` is missing the `private` access modifier, leaving it package-private. The adjacent `changed` field is `private`; this appears to be an oversight.</comment>
<file context>
@@ -0,0 +1,78 @@
+@Slf4j(topic = "net")
+public class LinkCache {
+
+ @Getter Map<String, Set<String>> backrefs;
+ @Getter @Setter private boolean changed; // if data in backrefs changes, we need to rebuild trees
+
</file context>
| try { | ||
| Thread.sleep(KademliaOptions.WAIT_TIME); | ||
| } catch (InterruptedException e) { | ||
| logger.warn("Discover task interrupted"); |
There was a problem hiding this comment.
P2: After catching InterruptedException and restoring the interrupt flag, the method should return to allow prompt shutdown. Without this, shutdownNow() in close() will cause up to MAX_STEPS (8) recursive iterations that each immediately re-throw from Thread.sleep, logging spurious warnings before the task finally terminates.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At p2p/src/main/java/org/tron/p2p/discover/protocol/kad/DiscoverTask.java, line 74:
<comment>After catching `InterruptedException` and restoring the interrupt flag, the method should `return` to allow prompt shutdown. Without this, `shutdownNow()` in `close()` will cause up to `MAX_STEPS` (8) recursive iterations that each immediately re-throw from `Thread.sleep`, logging spurious warnings before the task finally terminates.</comment>
<file context>
@@ -0,0 +1,92 @@
+ try {
+ Thread.sleep(KademliaOptions.WAIT_TIME);
+ } catch (InterruptedException e) {
+ logger.warn("Discover task interrupted");
+ Thread.currentThread().interrupt();
+ }
</file context>
| .filter(peer -> !peer.isActive()) | ||
| .collect(Collectors.toList()); | ||
|
|
||
| // if len(peers) >= 0, disconnect randomly |
There was a problem hiding this comment.
P3: Misleading comment: // if len(peers) >= 0 is always true for a collection. Should be > 0 to match the !peers.isEmpty() guard below it.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At p2p/src/main/java/org/tron/p2p/connection/business/pool/ConnPoolService.java, line 286:
<comment>Misleading comment: `// if len(peers) >= 0` is always true for a collection. Should be `> 0` to match the `!peers.isEmpty()` guard below it.</comment>
<file context>
@@ -0,0 +1,378 @@
+ .filter(peer -> !peer.isActive())
+ .collect(Collectors.toList());
+
+ // if len(peers) >= 0, disconnect randomly
+ if (!peers.isEmpty()) {
+ List<Channel> list = new ArrayList<>(peers);
</file context>
❌ Math Usage Detection ResultsFound forbidden usage of Please review if this usage is intended. Caution Note: You should use |
- Replace Math.min/max with StrictMath.min/max in NodeEntry and NodeTable to satisfy CI check-math rule (integer operations, results are identical) - Rewrite NetUtilTest.testGetIP with mocked URLConnection instead of calling external IP services (checkip.amazonaws.com, ifconfig.me), covering: valid IP, connection failure, invalid IP, empty response - Remove testExternalIp (covered by mock tests) - Fix testGetLanIP to not depend on www.baidu.com connectivity
barbatos2011
force-pushed
the
chore/internalize-libp2p
branch
from
75dab6b to
c54a7dc
Compare
Owner
Author
|
@barbatos2011 I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
12 issues found across 124 files
Note: This PR contains a large number of files. cubic only reviews up to 75 files per PR, so some files may not have been reviewed.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="p2p/src/main/java/org/tron/p2p/connection/Channel.java">
<violation number="1" location="p2p/src/main/java/org/tron/p2p/connection/Channel.java:182">
P2: `hashCode()` can throw a `NullPointerException` because `inetSocketAddress` may still be null before channel context initialization.</violation>
</file>
<file name="p2p/src/main/java/org/tron/p2p/discover/Node.java">
<violation number="1" location="p2p/src/main/java/org/tron/p2p/discover/Node.java:170">
P1: `equals` compares binary node IDs via `new String(id)`, which can produce false equality for different byte arrays. Compare raw bytes directly.</violation>
</file>
<file name="p2p/src/main/java/org/tron/p2p/connection/business/detect/NodeStat.java">
<violation number="1" location="p2p/src/main/java/org/tron/p2p/connection/business/detect/NodeStat.java:11">
P2: Make detect timestamp fields `volatile` to avoid stale cross-thread reads that can falsely mark nodes as failed.</violation>
</file>
<file name="p2p/src/main/java/org/tron/p2p/connection/socket/P2pChannelInitializer.java">
<violation number="1" location="p2p/src/main/java/org/tron/p2p/connection/socket/P2pChannelInitializer.java:61">
P2: Close the socket when channel initialization fails; logging alone can leave a partially initialized connection open.</violation>
</file>
<file name="p2p/src/main/java/org/tron/p2p/connection/message/Message.java">
<violation number="1" location="p2p/src/main/java/org/tron/p2p/connection/message/Message.java:40">
P2: Validate `encode` before indexing `encode[0]`; otherwise null/empty inputs throw unchecked exceptions outside the method’s `P2pException` handling.</violation>
</file>
<file name="p2p/src/main/java/org/tron/p2p/discover/socket/DiscoverServer.java">
<violation number="1" location="p2p/src/main/java/org/tron/p2p/discover/socket/DiscoverServer.java:33">
P2: Handle `InterruptedException` separately in `close()` and re-interrupt the thread instead of swallowing it via a generic `Exception` catch.</violation>
</file>
<file name="p2p/src/main/java/org/tron/p2p/connection/socket/PeerClient.java">
<violation number="1" location="p2p/src/main/java/org/tron/p2p/connection/socket/PeerClient.java:41">
P2: sync() can throw InterruptedException; catching Exception here clears the interrupt flag. Restore the interrupt status so callers can honor cancellation.</violation>
</file>
<file name="p2p/src/main/java/org/tron/p2p/discover/protocol/kad/NodeHandler.java">
<violation number="1" location="p2p/src/main/java/org/tron/p2p/discover/protocol/kad/NodeHandler.java:111">
P1: Use a null-safe state comparison in `handlePing` to avoid a possible `NullPointerException` when `state` has not been initialized.</violation>
</file>
<file name="p2p/src/main/java/org/tron/p2p/dns/tree/Algorithm.java">
<violation number="1" location="p2p/src/main/java/org/tron/p2p/dns/tree/Algorithm.java:28">
P1: `compressPubKey` slices X from a non-padded BigInteger hex string, which can produce an invalid compressed key when the public key has leading zeros.</violation>
</file>
<file name="p2p/src/main/java/org/tron/p2p/dns/update/AliClient.java">
<violation number="1" location="p2p/src/main/java/org/tron/p2p/dns/update/AliClient.java:203">
P2: Check the result of `deleteRecord` in `submitChanges`; failed deletes are currently ignored, so publish can report success with stale records still present.</violation>
</file>
<file name="p2p/src/main/java/org/tron/p2p/discover/protocol/kad/table/NodeTable.java">
<violation number="1" location="p2p/src/main/java/org/tron/p2p/discover/protocol/kad/table/NodeTable.java:33">
P2: Guard preferred socket availability before calling `getHostKey()` to avoid `NullPointerException` in peer insertion.</violation>
</file>
<file name="p2p/src/main/java/org/tron/p2p/dns/DnsNode.java">
<violation number="1" location="p2p/src/main/java/org/tron/p2p/dns/DnsNode.java:29">
P1: The constructor discards the provided node ID by calling `super(null, ...)`, so DNS node IDs are lost during publish/serialize flow.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.
| } | ||
|
|
||
| if (o.getClass() == getClass()) { | ||
| return StringUtils.equals(getIdString(), ((Node) o).getIdString()); |
There was a problem hiding this comment.
P1: equals compares binary node IDs via new String(id), which can produce false equality for different byte arrays. Compare raw bytes directly.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At p2p/src/main/java/org/tron/p2p/discover/Node.java, line 170:
<comment>`equals` compares binary node IDs via `new String(id)`, which can produce false equality for different byte arrays. Compare raw bytes directly.</comment>
<file context>
@@ -0,0 +1,197 @@
+ }
+
+ if (o.getClass() == getClass()) {
+ return StringUtils.equals(getIdString(), ((Node) o).getIdString());
+ }
+
</file context>
| node.setP2pVersion(msg.getNetworkId()); | ||
| if (!node.isConnectible(Parameter.p2pConfig.getNetworkId())) { | ||
| changeState(State.DEAD); | ||
| } else if (state.equals(State.DEAD)) { |
There was a problem hiding this comment.
P1: Use a null-safe state comparison in handlePing to avoid a possible NullPointerException when state has not been initialized.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At p2p/src/main/java/org/tron/p2p/discover/protocol/kad/NodeHandler.java, line 111:
<comment>Use a null-safe state comparison in `handlePing` to avoid a possible `NullPointerException` when `state` has not been initialized.</comment>
<file context>
@@ -0,0 +1,247 @@
+ node.setP2pVersion(msg.getNetworkId());
+ if (!node.isConnectible(Parameter.p2pConfig.getNetworkId())) {
+ changeState(State.DEAD);
+ } else if (state.equals(State.DEAD)) {
+ changeState(State.DISCOVERED);
+ }
</file context>
| /** return compress public key with hex */ | ||
| public static String compressPubKey(BigInteger pubKey) { | ||
| String pubKeyYPrefix = pubKey.testBit(0) ? "03" : "02"; | ||
| String pubKeyHex = pubKey.toString(16); |
There was a problem hiding this comment.
P1: compressPubKey slices X from a non-padded BigInteger hex string, which can produce an invalid compressed key when the public key has leading zeros.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At p2p/src/main/java/org/tron/p2p/dns/tree/Algorithm.java, line 28:
<comment>`compressPubKey` slices X from a non-padded BigInteger hex string, which can produce an invalid compressed key when the public key has leading zeros.</comment>
<file context>
@@ -0,0 +1,142 @@
+ /** return compress public key with hex */
+ public static String compressPubKey(BigInteger pubKey) {
+ String pubKeyYPrefix = pubKey.testBit(0) ? "03" : "02";
+ String pubKeyHex = pubKey.toString(16);
+ String pubKeyX = pubKeyHex.substring(0, 64);
+ String hexPub = pubKeyYPrefix + pubKeyX;
</file context>
| private String v6Hex = Constant.ipV6Hex; | ||
|
|
||
| public DnsNode(byte[] id, String hostV4, String hostV6, int port) throws UnknownHostException { | ||
| super(null, hostV4, hostV6, port); |
There was a problem hiding this comment.
P1: The constructor discards the provided node ID by calling super(null, ...), so DNS node IDs are lost during publish/serialize flow.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At p2p/src/main/java/org/tron/p2p/dns/DnsNode.java, line 29:
<comment>The constructor discards the provided node ID by calling `super(null, ...)`, so DNS node IDs are lost during publish/serialize flow.</comment>
<file context>
@@ -0,0 +1,98 @@
+ private String v6Hex = Constant.ipV6Hex;
+
+ public DnsNode(byte[] id, String hostV4, String hostV6, int port) throws UnknownHostException {
+ super(null, hostV4, hostV6, port);
+ if (StringUtils.isNotEmpty(hostV4)) {
+ this.v4Hex = ipToString(hostV4);
</file context>
Suggested change
| super(null, hostV4, hostV6, port); | |
| super(id, hostV4, hostV6, port); |
|
|
||
| @Override | ||
| public int hashCode() { | ||
| return inetSocketAddress.hashCode(); |
There was a problem hiding this comment.
P2: hashCode() can throw a NullPointerException because inetSocketAddress may still be null before channel context initialization.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At p2p/src/main/java/org/tron/p2p/connection/Channel.java, line 182:
<comment>`hashCode()` can throw a `NullPointerException` because `inetSocketAddress` may still be null before channel context initialization.</comment>
<file context>
@@ -0,0 +1,190 @@
+
+ @Override
+ public int hashCode() {
+ return inetSocketAddress.hashCode();
+ }
+
</file context>
Suggested change
| return inetSocketAddress.hashCode(); | |
| return inetSocketAddress != null ? inetSocketAddress.hashCode() : 0; |
| } | ||
|
|
||
| public static Message parse(byte[] encode) throws P2pException { | ||
| byte type = encode[0]; |
There was a problem hiding this comment.
P2: Validate encode before indexing encode[0]; otherwise null/empty inputs throw unchecked exceptions outside the method’s P2pException handling.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At p2p/src/main/java/org/tron/p2p/connection/message/Message.java, line 40:
<comment>Validate `encode` before indexing `encode[0]`; otherwise null/empty inputs throw unchecked exceptions outside the method’s `P2pException` handling.</comment>
<file context>
@@ -0,0 +1,78 @@
+ }
+
+ public static Message parse(byte[] encode) throws P2pException {
+ byte type = encode[0];
+ try {
+ byte[] data = ArrayUtils.subarray(encode, 1, encode.length);
</file context>
Suggested change
| byte type = encode[0]; | |
| if (encode == null || encode.length == 0) { | |
| throw new P2pException(P2pException.TypeEnum.BAD_MESSAGE, "empty message"); | |
| } | |
| byte type = encode[0]; |
| () -> { | ||
| try { | ||
| start(); | ||
| } catch (Exception e) { |
There was a problem hiding this comment.
P2: Handle InterruptedException separately in close() and re-interrupt the thread instead of swallowing it via a generic Exception catch.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At p2p/src/main/java/org/tron/p2p/discover/socket/DiscoverServer.java, line 33:
<comment>Handle `InterruptedException` separately in `close()` and re-interrupt the thread instead of swallowing it via a generic `Exception` catch.</comment>
<file context>
@@ -0,0 +1,98 @@
+ () -> {
+ try {
+ start();
+ } catch (Exception e) {
+ logger.error("Discovery server start failed", e);
+ }
</file context>
Comment on lines
+41
to
+43
| } catch (Exception e) { | ||
| logger.warn("PeerClient can't connect to {}:{} ({})", host, port, e.getMessage()); | ||
| } |
There was a problem hiding this comment.
P2: sync() can throw InterruptedException; catching Exception here clears the interrupt flag. Restore the interrupt status so callers can honor cancellation.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At p2p/src/main/java/org/tron/p2p/connection/socket/PeerClient.java, line 41:
<comment>sync() can throw InterruptedException; catching Exception here clears the interrupt flag. Restore the interrupt status so callers can honor cancellation.</comment>
<file context>
@@ -0,0 +1,111 @@
+ if (f != null) {
+ f.sync().channel().closeFuture().sync();
+ }
+ } catch (Exception e) {
+ logger.warn("PeerClient can't connect to {}:{} ({})", host, port, e.getMessage());
+ }
</file context>
Suggested change
| } catch (Exception e) { | |
| logger.warn("PeerClient can't connect to {}:{} ({})", host, port, e.getMessage()); | |
| } | |
| } catch (InterruptedException e) { | |
| Thread.currentThread().interrupt(); | |
| logger.warn("PeerClient can't connect to {}:{} ({})", host, port, e.getMessage()); | |
| } catch (Exception e) { | |
| logger.warn("PeerClient can't connect to {}:{} ({})", host, port, e.getMessage()); | |
| } |
|
|
||
| for (String key : existing.keySet()) { | ||
| if (!records.containsKey(key)) { | ||
| deleteRecord(existing.get(key).getRecordId()); |
There was a problem hiding this comment.
P2: Check the result of deleteRecord in submitChanges; failed deletes are currently ignored, so publish can report success with stale records still present.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At p2p/src/main/java/org/tron/p2p/dns/update/AliClient.java, line 203:
<comment>Check the result of `deleteRecord` in `submitChanges`; failed deletes are currently ignored, so publish can report success with stale records still present.</comment>
<file context>
@@ -0,0 +1,351 @@
+
+ for (String key : existing.keySet()) {
+ if (!records.containsKey(key)) {
+ deleteRecord(existing.get(key).getRecordId());
+ deleteCount++;
+ }
</file context>
| } | ||
|
|
||
| public synchronized Node addNode(Node n) { | ||
| if (n.getHostKey().equals(node.getHostKey())) { |
There was a problem hiding this comment.
P2: Guard preferred socket availability before calling getHostKey() to avoid NullPointerException in peer insertion.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At p2p/src/main/java/org/tron/p2p/discover/protocol/kad/table/NodeTable.java, line 33:
<comment>Guard preferred socket availability before calling `getHostKey()` to avoid `NullPointerException` in peer insertion.</comment>
<file context>
@@ -0,0 +1,114 @@
+ }
+
+ public synchronized Node addNode(Node n) {
+ if (n.getHostKey().equals(node.getHostKey())) {
+ return null;
+ }
</file context>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Internalize
io.github.tronprotocol:libp2p:2.2.7source code as a newp2p/gradle submodule, replacing the external Maven dependency. No functional changes to P2P behavior, protocol, or consensus.Why
The external libp2p repository had a 2.5-year development gap (2023.3 ~ 2025.10). Cross-repo maintenance creates friction:
p2p/package within the monorepo; this follows the same patternChanges
Commit 1: Add libp2p v2.2.7 source as p2p module
tronprotocol/libp2ptagv2.2.7p2p/build.gradlewith protobuf plugin configuration (consistent withprotocol/module)p2p/lombok.configto temporarily override root logger field name (logvslogger)StartApp) tosrc/test/java(not packaged in JAR, but available for reference)settings.gradleCommit 2: Apply java-tron code style
@Slf4j-generatedlogreferences withlogger(35 files) to matchlombok.config(lombok.log.fieldName=logger)p2p/lombok.configoverrideCommit 3: Switch dependency from external to local
common/build.gradle: replace 17 lines (Maven dependency + 11 exclude rules) withapi project(':p2p')io.github.tronprotocol:libp2p:2.2.7entry fromgradle/verification-metadata.xmlCommit 4: Fix checkstyle violations and enable checkstyle
google-java-formatfor bulk formatting (indentation, imports, whitespace)exclude '**/protos/**')Commit 5: Address review findings
BasicThreadFactory.builder()(requires commons-lang3 3.12+) withnew BasicThreadFactory.Builder()(available since 3.0) to prevent implicit global version upgrade from 3.4 to 3.18.0grpc-coredependency (p2p protos define only messages, no gRPC services); retaingrpc-nettyas Netty transport providerp2p/.gitignoreto exclude proto-generated code for all developersCommit 6: Fix CI failures
Math.min/maxwithStrictMath.min/maxin 5 files to satisfy CIcheck-mathrule (integer operations, results identical)NetUtilTest.testGetIPwith mockedURLConnectioninstead of calling external IP services, covering: valid IP, connection failure, invalid IP, empty responsetestExternalIp(covered by mock tests)testGetLanIPto not depend on external network connectivityKey Design Decisions
p2porg.tron.p2p, gethp2p/convention, project short-name styleprotocol/new Builder()works since 3.0**/protos/**Math→StrictMathcheck-mathrule bansjava.lang.Math; integer min/max results are identicaltestGetIPrewrite-xtest), issue only exposed after internalizationScope
org.tron.p2pare unchanged (package name preserved)git pulland rebuildKnown Issues (pre-existing in libp2p)
The following issues were identified during review but are NOT introduced by this PR — they exist in the original libp2p v2.2.7 codebase. They will be addressed in follow-up PRs:
java.util.Randominstead ofSecureRandom)InterruptedExceptionhandlers swallow the interrupt flagAlgorithm.compressPubKeydrops leading zeros from public key X-coordinateTest
checkstyleMain+checkstyleTest: PASS for all modules including p2pcheck-math: PASS