chore(p2p): internalize libp2p v2.2.9 as a local p2p module

[barbatos2011]

chore(p2p): internalize libp2p v2.2.9 as a local p2p module

Replaces the external io.github.tronprotocol:libp2p:2.2.9 Maven dependency with a local p2p/ Gradle module built from the same source.

Supersedes tronprotocol#6673, which was approved on the merits but closed because it touched too many files while 4.8.2 was already full. This is a fresh branch — no history is reused.

Reproducing the diff

Base	tronprotocol/java-tron develop @ 4a21592f95e37908b21bc3f611c6e7a1a67f09f3
Vendored	tronprotocol/libp2p tag v2.2.9 @ c564f263d310d7a964035d3b597634aba6bda86d

Commit 1 is byte-comparable to the upstream tag. It contains only git archive v2.2.9 src/main plus the new build files, so a reviewer can diff it directly against c564f263 and see that no vendored line was altered. Everything we changed about that source lands in commit 2, separately, for exactly this reason.

File count

	Files
p2p/src/main/java (vendored)	90
p2p/src/main/proto	2
p2p/src/test/java	62

Protobuf-generated sources under src/main/java/org/tron/p2p/protos/ are gitignored and regenerated at build time, so they are not in the diff.

Tests live in p2p/src/test/java/, next to the code they cover. An earlier revision of this branch put them in framework/src/test/ and justified it as a project convention; that was wrong — chainbase and consensus have no tests at all and actuator has one, so they are not evidence of anything, while the two modules that do have tests (common, plugins) keep them in the module. Moving them back also deleted two workarounds; see Module layout below.

StartApp stays in src/main/java as the entry point for driving the module without starting java-tron, and the jar declares it as Main-Class. The other three reference classes became a test — see Module layout below.

Problems encountered

Three of these did not exist when tronprotocol#6673 was written; all were confirmed by compiling.

H1 — Netty 4.2 split out netty-codec-protobuf

develop now resolves Netty 4.2.15.Final (via the gRPC 1.83 bump in tronprotocol#6891). io.netty.handler.codec.protobuf no longer arrives transitively, so ProtobufVarint32FrameDecoder / ProtobufVarint32LengthFieldPrepender — on every p2p channel pipeline — failed to resolve. Declared explicitly in p2p/build.gradle, mirroring what framework/build.gradle already does for the same reason, excludes included. Both now reference rootProject.nettyVersion rather than repeating the literal — see Build wiring cleanups below.

p2p tracks rootProject.grpcVersion rather than pinning libp2p's own gRPC version, so it cannot drift from the Netty the rest of the build resolves.

H2 — errorprone StringCaseLocaleUsage (the one behavioural deviation)

The root build enables exactly two errorprone rules as ERROR on every subproject except protocol/errorprone: StringCaseLocaleUsage and StringCaseLocaleUsageMethodRef. libp2p has 4 bare toLowerCase() / toUpperCase() calls, which are now toLowerCase(Locale.ROOT) / toUpperCase(Locale.ROOT), matching the project's own idiom (Args.java:1273).

This is the only change in this PR that is not purely mechanical. It is compile-forced and behaviour-identical for ASCII input, but it is a real semantic change under a Turkish locale, so it is called out here rather than buried in a style commit.

H3 — BasicThreadFactory.builder() needs commons-lang3 3.12+

The project pins commons-lang3 3.4 globally; v2.2.9 uses builder() in 13 places. Rewritten to new BasicThreadFactory.Builder() (the 3.0 API) rather than bumping commons-lang3, which would have been a silent global upgrade.

dom4j exclusion tail

The jaxen / stax-api / msv / xsdlib / relaxngDatatype / pull-parser / xpp3 exclusions used to sit on the libp2p dependency in common/build.gradle. That transitive tail comes from the Aliyun / Route53 SDKs, so internalizing moves it into p2p/build.gradle as a configurations.configureEach block.

An earlier revision also had to mirror this tail onto :framework's test configurations, because the DNS tests lived there and the SDKs are implementation-scope in :p2p. With the tests back in :p2p, testImplementation extends implementation covers it and that mirror is gone.

Task-dependency edges an external jar did not need

Adding a project to the dependency graph needs three explicit task edges that a Maven artifact did not. Gradle reported each as an implicit_dependency and responded by disabling execution optimizations "to ensure correctness":

• framework's buildFullNodeJar and plugins' binaryRelease both zip up runtimeClasspath and maintain a hand-written dependsOn list of project jars — the plugins one carries a comment explaining it exists so "partial / parallel builds cannot run binaryRelease before the dependency jars exist". :common now exposes p2p via api, so p2p-1.0.0.jar is on both classpaths, and neither list had been updated. Without the edge a parallel build could assemble the shipped fat jar before :p2p:jar is written.
• :p2p:processExampleResources read src/example/resources, which the protobuf plugin claimed as an output of generateExampleProto because generatedFilesBaseDir points at $projectDir/src. That edge went away with the example sourceSet itself.

A full build now reports zero implicit_dependency warnings.

Coverage attribution

An earlier revision bolted p2p's class and source dirs onto :framework:jacocoTestReport with additionalClassDirs/additionalSourceDirs, because :p2p had no test sourceSet and so produced no exec data of its own — leaving the module invisible to the coverage gate.

With the tests in :p2p, the module reports for itself. CI collects **/build/reports/jacoco/test/jacocoTestReport.xml across every module, so it is picked up with no wiring at all, and the **/protos/** exclusion moves into the module's own report block.

Dependency verification

Three components needed adding to gradle/verification-metadata.xml: org.bouncycastle:bcutil-jdk18on:1.84, com.google.code.gson:gson:2.9.0 and com.google.code.gson:gson-parent:2.9.0. Checksums were taken from Maven Central and cross-checked against the published .sha1.

gson 2.9.0 is older than the 2.14.0 used elsewhere. This does not change the assembled node: :p2p's isolated compile classpath sees 2.9.0, while :framework's runtimeClasspath still resolves gson:2.9.0 -> 2.14.0.

Version preservation across the switch

Removing a dependency also removes it as a version requester, so every version the libp2p POM declared was checked against what :p2p now declares. All twelve match except two deliberate differences:

	libp2p 2.2.9 declared	:p2p declares
commons-lang3	3.18.0 (runtime)	3.18.0
grpc-netty	1.81.0	rootProject.grpcVersion (1.83.0)

commons-lang3 is the one that nearly went wrong. libp2p declared 3.18.0 at runtime scope, which won conflict resolution against the root build's 3.4 and put 3.18.0 on :framework:runtimeClasspath. :p2p initially pinned 3.4 — the version the module needs to compile, since the source uses the 3.0-compatible new BasicThreadFactory.Builder() — which left no requester for anything newer and would have shipped a 2015 release, reintroducing CVE-2025-48924 (ClassUtils.getAbbreviatedName uncontrolled recursion, fixed in 3.18.0). :p2p now declares 3.18.0.

Verified with ./gradlew :framework:dependencyInsight --configuration runtimeClasspath: gson at 2.14.0 and commons-lang3 at 3.18.0 — the same versions the node shipped before.

Module layout — review feedback

Four changes came out of review and are separate commits.

Tests moved to p2p/src/test. Covered above. Beyond matching what common and plugins do, it deleted two workarounds that existed only because :p2p had no test sourceSet — the :framework test-classpath SDK re-declaration plus dom4j mirror, and the jacoco additionalClassDirs bolt-on. The relocated tests compiled first try with both removed, which is the evidence that neither was needed. It also lets Gradle run :p2p:test and :framework:test in parallel.

The example sourceSet is gone. DnsExample1, DnsExample2 and ImportUsing documented how an embedder configures this module, but they only compiled — each ended in a while (true) loop, bound a fixed port and pointed at live seed nodes, so nothing they showed was checked. Two of them carried real defects (TestMessage is not serializable, so ByteArray.fromObject returns null and Channel.send closes the channel; DnsExample1 held a signing private key in copyable code).

Porting them line by line would give three slow, network-dependent tests. ExampleUsageTest instead pins the contract they advertised — those configuration shapes are still accepted and still mean what the comments said. External embedders copy them, so a renamed setter is a breaking change even though nothing here calls them.

StartApp moved to src/main/java and stays as the standalone entry point. Three things surfaced:

• It left the checkstyle-exempt sourceSet, so it is linted for the first time — three over-long lines, wrapped.
• It also left the coverage-exempt sourceSet, adding 981 uncovered instructions to the denominator. org/tron/p2p/example/** is excluded from the module's jacoco report alongside **/protos/**; the two parsing helpers it owns are package-private and covered by StartAppArgsTest, so the exclusion does not take regression protection with it.
• java -jar needs a fat jar. An intermediate commit put Main-Class on the plain jar, which is thin — the entry point resolved and then died on NoClassDefFoundError: org/apache/commons/cli/ParseException. Worse than declaring nothing, since it advertises support that cannot work. The plain jar declares none again and fails honestly with "no main manifest attribute"; buildStandaloneJar produces p2p-standalone.jar with the runtime classpath bundled, wired exactly like :framework's FullNode.jar and :plugins' Toolkit.jar — same artifacts { archives(...) }, same -PbinaryRelease=false opt-out, same Bouncy Castle signature and dnsjava SPI exclusions. Verified by running both jars.

It also carried a real bug: --trust-ips is declared ip[,ip[...]] but resolved the whole comma-separated value as one hostname, so with more than one address none of the listed peers became trusted. Fixed, and pinned by StartAppArgsTest.

p2p/README.md added, promoted from src/example/resources/README.md where no reader would find it. It still read as upstream's document: four source links pointed at github.com/tronprotocol/libp2p, the invocations named libp2p.jar, and the prose described libp2p as a standalone project. Links are module-relative now, the prose is about this module, and the header states the provenance once — the single remaining upstream link — and says which of the two jars to use for what.

Duplicated versions removed. protobufVersion 3.25.8 was declared in both :protocol and :p2p; checkstyle 8.7 sat in a local versions map in :framework and :plugins and as a literal in :p2p. Both move to the root ext beside grpcVersion and nettyVersion, and all four modules reference them — removed rather than relocated. Resolution unchanged: protobuf-java:3.25.8 on :p2p's compile classpath.

.proto files moved to src/main/proto. src/main/protos with an explicit srcDir override builds fine, but IDEA's protobuf plugin does not read that override — it resolves against the plugin's default path — so import "Discover.proto" and every type it brings in showed as unresolved in the editor. The override is gone; generated sources still land in src/main/java/org/tron/p2p/protos and are still gitignored.

Flagging for a follow-up: :protocol has the identical setup and presumably the identical symptom, so the two modules now differ. Aligning it is the same one-directory rename, but it is core java-tron with a src/main/gen interplay and does not belong here.

logback.xml.example deleted. It came from libp2p as a standalone project; inside java-tron framework/src/main/resources/logback.xml is what applies, and the sample was being packaged into the jar for no reason.

Not done, by request: moving Connect.proto / Discover.proto into :protocol. It would make :p2p depend on :protocol and cost the module its leaf position in the dependency graph — currently it has zero project dependencies, which is what keeps vendored code from reaching back into java-tron internals and keeps upstream re-contribution possible.

Tests

All 23 of v2.2.9's own test files are ported, plus 4 new ones. Two upstream tests were unreliable by construction and were fixed rather than carried over as-is:

• NetUtilTest.testGetIP called three public IP-echo services and asserted all three returned the same string — a network dependency, and a coin flip on any host with more than one egress address. It now runs against a loopback HttpServer, which exercises the same fetch/parse/validate path deterministically and covers the rejection branches too. (libp2p's own CI never ran its tests, so this had not shown up.)
• ConnPoolServiceTest.getNodes_orderByUpdateTimeDesc asserted that getNodes() returns nodes ordered by updateTime descending. getNodes() sorts, truncates to max(limit * 10, 50) candidates, and then calls Collections.shuffle() — so with two nodes that assertion passes about half the time. It now asserts membership, and a new test (getNodes_prefersNewestAboveCandidateSize) covers the descending sort where it is actually observable: above the candidate bound.

One import was dropped: NodeHandlerTest had an unused org.checkerframework.checker.units.qual.N import — an IDE auto-import artifact that does not resolve on this classpath.

Coverage: p2p is at 78.62% line coverage (2,990/3,803), 79.32% instruction coverage (12,625/15,917), now reported by :p2p:jacocoTestReport itself. That report also reads :framework's exec data, because framework's own tests execute p2p code and that coverage was counted while p2p's classes hung off :framework:jacocoTestReport. v2.2.9's own tests reach about 35%. Getting from there to here took two rounds — the second was forced by the CI coverage gate and is written up in its own section below.

Two pre-existing test defects were fixed along the way, because they made coverage depend on fork scheduling rather than on what the tests assert:

• ConnPoolServiceTest and SocketTest bound fixed ports (10000 / 10001). PeerServer.start only logs on bind failure, so a collision let them pass while exercising nothing. Both now take a free port from PublicMethod.chooseRandomPort(), which is what java-tron's own tests use.
• NodeTableTest read Parameter.p2pConfig without ever setting it, so it depended on an earlier class in the same fork having done so. Running the class on its own failed all eleven methods — on this branch and on the unmodified one alike. It now sets up and restores its own config.

Where a collaborator is genuinely external — the Aliyun SDK, process-wide ChannelManager state — it is mocked, so the logic under test is real and only the transport is faked. What remains uncovered is code that needs a live connection: ConnPoolService.onConnect/onDisconnect/onMessage, NodeDetectService, PeerClient, Channel.init/send. Upstream's own SocketTest for exactly that is entirely commented out, so it would need integration tests with real channels rather than more unit tests.

Defects found while testing (not fixed here)

Two instances of the same shape: a DNS parse helper throws an unchecked exception past a catch (DnsException) that was clearly written to tolerate unparseable input, so one malformed TXT record aborts the whole operation instead of being skipped.

1. Short root entry. RootEntry.parseEntry does e.substring(rootPrefix.length()) with no length guard:

• p2p/src/main/java/org/tron/p2p/dns/tree/RootEntry.java:67

Any value shorter than the 13-character tree-root-v1: prefix throws StringIndexOutOfBoundsException. The caller at p2p/src/main/java/org/tron/p2p/dns/update/AwsClient.java:334 catches only DnsException, so it escapes and aborts computeChanges, failing the entire publish.

2. Malformed base64 in a nodes entry. Algorithm.decode64 calls Base64.getUrlDecoder().decode() directly:

• p2p/src/main/java/org/tron/p2p/dns/tree/Algorithm.java:121

which throws IllegalArgumentException on malformed input. NodesEntry.parseEntry converts only InvalidProtocolBufferException and UnknownHostException into DnsException, so the IllegalArgumentException escapes both it and the DnsException-only catch at p2p/src/main/java/org/tron/p2p/dns/update/AliClient.java:138, aborting the whole collectRecords — and with it the deploy() that called it.

A third, latent instance: BranchEntry.parseEntry (p2p/src/main/java/org/tron/p2p/dns/tree/BranchEntry.java:19) does the same unguarded substring(branchPrefix.length()), and unlike its siblings does not even declare throws DnsException. Its only caller checks txt.startsWith(branchPrefix) first, so it is not currently reachable with a short string — a hazard for the next caller, not a live bug. LinkEntry.parseEntry shows the correct shape: prefix check, length check, and catch (RuntimeException) around the base32 decode.

Reachability — these are operator-side, not peer-reachable

Traced every path that reaches these parsers:

Path	Guard	Result
TCP wire decode	Message.parse catch (Exception)	contained
UDP discovery decode	P2pPacketDecoder catch (Exception)	contained
DNS sync / iteration	RandomIterator.next() catch (Exception) around syncRandom() (ClientTree has no catch of its own)	contained
DNS publish / collect	only catch (DnsException)	escapes

So no remote peer can trigger these. Both live instances are on the operator's own publish path: they abort a DNS publish for whoever runs it, they do not give an attacker anything. That is worth stating plainly, because "unchecked exception on a parse path" in a networking module reads much worse than it is here.

All three are pre-existing in libp2p and out of scope for a no-functional-changes PR, so they are reported rather than fixed.

Instance 2 is now pinned by a test. AwsClientRecordsTest.malformedBase64InANodesEntryAbortsTheWholeCollection feeds a corrupt nodes entry through collectRecords and asserts the unchecked IllegalArgumentException escapes — that is, it asserts the defect, not the behaviour we want. It fails loudly if the escape is ever closed, which is the point. The tests covering instances 1 and 3 use inputs that reach the intended DnsException path and document the unchecked one in a comment.

Known security issues — pre-existing, deferred

These exist in libp2p today and are unchanged by this PR. Internalizing the source is what makes them fixable in-tree; each gets its own follow-up:

• Weak PRNG for node ID generation
• compressPubKey drops leading zeros
• Unauthenticated handshake
• Unbounded neighbour injection
• AwsClient swallows InterruptedException without restoring the interrupt flag (dns/update/AwsClient.java, now commented rather than silently empty)

Mixing any of them in would break the "no functional changes" claim, which is the only thing that makes a diff this size reviewable.

Build wiring cleanups

Build-only, no vendored code touched.

nettyVersion extracted to the root ext. :framework and :p2p each declare netty-codec-protobuf explicitly (see H1) and each carried the literal 4.2.15.Final. Netty itself is declared nowhere — it arrives transitively via grpc-netty, which :p2p tracks as rootProject.grpcVersion. So a grpc bump moves Netty while those two literals stay put, which is exactly the mismatch H1 describes. The version now sits next to grpcVersion so the coupling is visible in one place. Resolution is unchanged: 4.2.15.Final on both compile classpaths.

:framework now declares its direct :p2p dependency. framework uses org.tron.p2p in 17 files under src/main/java but declared nothing, relying on a three-hop transitive through :common -> :crypto -> :chainbase.

That export is not removable here, and it is worth being precise about why: CommonParameter publishes P2pConfig p2pConfig and PublishConfig dnsPublishConfig as public @Getter fields, so p2p types are part of :common's own API surface and api project(":p2p") is forced. Narrowing it to implementation would break every caller of getP2pConfig(). Genuinely decoupling the graph means moving those two fields out of CommonParameter, which is a functional refactor and deliberately out of scope. Until then, :p2p stays on the compile classpath of every module in the graph — as the external libp2p artifact did before this PR, for the same reason.

What is fixable now is the undeclared direct use, so it is declared. api, not implementation — framework does re-export p2p types: P2pEventHandlerImpl extends org.tron.p2p.P2pEventHandler, HelloMessage.getFrom() returns org.tron.p2p.discover.Node, PeerManager.add/remove take org.tron.p2p.connection.Channel, and Args.loadDnsPublishConfig returns PublishConfig. Declaring it implementation would compile today only because the transitive api chain still supplies those types to consumers — the moment that chain is narrowed, it breaks. No resolution change either way.

The coverage gate, and what it took to pass it

Worth reading before the verification table, because it is the one place where this PR's numbers moved a lot.

Wiring p2p into :framework:jacocoTestReport (see Coverage attribution above) is what makes the module visible to coverage at all. It also puts 16,568 instructions at 60.91% into the repo-wide denominator, and CI's overall-delta gate failed on exactly that:

	Before	After
Base overall	79.67%	79.69%
PR overall	78.49%	79.64%
Delta (threshold −0.1%)	−1.1746% ❌	−0.0500% ✅
Changed-line (threshold 60%)	61%	78% ✅

The cause was fully attributable. Downloading both jacoco reports from CI and recomputing:

剔除 p2p 后的 PR overall = 79.70%   (delta +0.04)

Every point of the drop came from vendored code now being counted — java-tron's own coverage went slightly up. So the choice was between hiding vendored code from the metric and actually testing it. This PR does the latter: p2p went from 60.91% to 78.88% instruction coverage, +3,168 covered instructions, and the delta gate now passes with the vendored code fully counted.

The added tests target the parts that need no live connection: the web3j copy (Numeric, Strings, Hash, Sign, ECKeyPair, ECDSASignature), every kad discovery message through its own wire bytes, P2pPacketDecoder's drop paths for hostile datagrams, Channel's send guard and exception classification, ChannelManager.processPeer's admission branches (ban list, global cap, per-IP cap, duplicate node id), Tree's signing and TXT output, AwsClient's Route53 batching limits, AliClient.deploy's threshold decision, and P2pService, which had no test at all.

What remains uncovered is still code that needs a real socket: PeerClient, P2pChannelInitializer, Channel.init/send against a live peer.

Scope — what this PR does not change

• No protocol or message-format change
• No consensus change
• No proto change (the .proto files move verbatim)
• No hard fork
• Package names preserved (org.tron.p2p.**), so imports in dependent code are untouched
• p2p remains usable standalone — java -cp … org.tron.p2p.example.StartApp runs the module on its own; see p2p/README.md

Verification

Gate	Result
:p2p:build (compile, test, checkstyle, jar)	PASS
java -jar p2p-standalone.jar --help	PASS — prints help
:framework:compileJava --rerun-tasks	PASS, tasks executed
project :p2p on framework runtimeClasspath	PASS
Zero external libp2p artifacts	PASS
gson:2.9.0 -> 2.14.0	PASS
:p2p:checkstyleMain	PASS, 0 violations (from 605)
:framework:checkstyleTest	PASS, 0 violations
:p2p:checkstyleTest	PASS, 0 violations
check-math	PASS
:p2p:test	357 tests, 0 failures, 3 skipped
commons-lang3 unchanged at 3.18.0	PASS
zero implicit_dependency warnings	PASS
:framework:test	PASS — 3,156 tests, 26 skipped
CI changed-line coverage (> 60%)	PASS — 78%
CI overall-delta coverage (≥ −0.1%)	PASS — −0.05%
node starts against mainnet config	PASS — HelloMessage exchanged, synced to block 2415

Three tests failed on their first attempt in the full local run and passed on retry, all pre-existing and none of them a p2p test: ValidateMultiSignContractTest.testTip854RejectsMalformedCalldata, AllowTvmLondonTest.testBaseFee, AllowTvmLondonTest.testStartWithEF. They also fail on the pre-change tree. :framework:test itself did not fail.

Separately, a BindException in Metrics.init can cascade through four framework test classes (SRMetricsTest, PrometheusApiServiceTest, JsonrpcServiceTest, RpcApiServicesTest) that each bind the same hard-coded Prometheus port 9527 while the test task runs up to 4 parallel forks. Verified pre-existing by running those four classes together with no p2p tests in the run: it reproduces, and hits a different class each time. The test classes this PR adds change how classes distribute across forks, which can make two of them land concurrently — the race is java-tron's, not p2p's, but this PR makes it more likely to surface.

Known remaining CI-reliability risk

Two ported upstream tests still need live DNS against a third-party zone and are left as-is rather than disabled, since they exercise real discovery behaviour:

• RandomTest.testRandomIterator and SyncTest both sync tree://…@nile.trondisco.net through hard-coded public resolvers. RandomTest already failed once in a local batch run and only went green via the test-retry plugin (maxRetries = 5). On a runner with restricted egress, or if that DNS tree is re-published or retired, they fail permanently.

LookUpTxtTest in the same package already @Ignores its network tests, so that is the precedent if reviewers would rather these be skipped than retried. Disabling them costs roughly 1.5 points of coverage, which still clears both gates.

The fixed-port problem that used to sit here — ConnPoolServiceTest and SocketTest binding 10000 / 10001 — is fixed in this PR; see Tests above.

[cubic-dev-ai]

34 issues found across 136 files

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/NodeManager.java">

<violation number="1" location="p2p/src/main/java/org/tron/p2p/discover/NodeManager.java:14">
P2: When `init()` is called again, this assignment abandons the running `KadService` while its executors and server remain active. Stop the existing discovery manager before replacing it; otherwise the new server cannot bind the UDP port and the new service cannot send discovery messages.</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:29">
P1: When shutdown runs while the asynchronous `start(port)` is still binding, this condition skips the close. The start thread then binds and waits on `closeFuture` indefinitely, leaving the listener and Netty threads active after `ChannelManager.close()` returns. Track shutdown state with proper synchronization, and have startup abort or close the channel when shutdown has already begun.</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:66">
P2: After `hasNext()` returns true, a normal `iterator.next()` performs a second DNS sync and discards the node prefetched into `cur`; it can even return `null` immediately after `hasNext()` returned true. Cache the look-ahead result and have `next()` consume it, or remove the `Iterator` implementation.</violation>

<violation number="2" location="p2p/src/main/java/org/tron/p2p/dns/sync/RandomIterator.java:90">
P2: When the iterator has no valid tree URLs, `pickTree()` calls `random.nextInt(0)` and every `next()` fails with `IllegalArgumentException`. Return `null` before selecting a random index when `clientTrees` is empty.</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:72">
P1: When `close()` runs before this asynchronous bind completes, it sees `channel == null` and returns after setting `shutdown`; this bind then still succeeds and waits indefinitely. Coordinate channel publication with shutdown, and close a channel bound after shutdown is requested or wait for the startup thread.</violation>
</file>

<file name="p2p/src/main/java/org/tron/p2p/dns/sync/SubtreeSync.java">

<violation number="1" location="p2p/src/main/java/org/tron/p2p/dns/sync/SubtreeSync.java:57">
P1: When a subtree TXT lookup temporarily returns no record, `resolveAll` discards that hash and marks the subtree complete. `ClientTree` then skips it while the root sequence is unchanged; throw on null before polling so the next sync retries it.</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:199">
P1: When a stale-record deletion fails after retries, `submitChanges` still reports publication success and leaves the old DNS record active. Check the return value and propagate a deletion failure before logging success.</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:31">
P1: When the public key’s X coordinate starts with a zero nibble, this slice drops that nibble and includes part of Y, so DNS tree publication produces an unusable public key. Left-pad the full public-key value to 128 hex characters before extracting X.</violation>
</file>

<file name="p2p/src/main/java/org/tron/p2p/utils/NetUtil.java">

<violation number="1" location="p2p/src/main/java/org/tron/p2p/utils/NetUtil.java:80">
P1: An advertised endpoint with a port outside 1–65535 passes `validNode`, then can throw `IllegalArgumentException` when discovery handles its socket address. Reject invalid ports in `validNode` before accepting the message.</violation>

<violation number="2" location="p2p/src/main/java/org/tron/p2p/utils/NetUtil.java:227">
P2: When a seed hostname is configured, `parseInetSocketAddress` performs a blocking JVM DNS lookup before `InetUtil` can submit its parallel, bounded lookups. Parse hostnames without resolving them here, or move hostname resolution entirely into `InetUtil`.</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">
P2: When a `DnsNode` is constructed with an ID, this call discards it, so `compress` omits `nodeId` and decompressed DNS entries cannot preserve peer identity. Pass the constructor's `id` to `Node`.</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:65">
P1: When an incumbent answers its eviction challenge, the replacement handler remains `ALIVE` without table membership and can never become active. Explicitly reject `replaceCandidate` when the incumbent survives.</violation>

<violation number="2" location="p2p/src/main/java/org/tron/p2p/discover/protocol/kad/NodeHandler.java:106">
P2: When an active peer fails the compatibility check, this transition marks its handler dead but leaves the peer in the routing table. Remove active entries when transitioning to `DEAD`, or keep the handler active until the table entry is removed.</violation>

<violation number="3" location="p2p/src/main/java/org/tron/p2p/discover/protocol/kad/NodeHandler.java:120">
P2: After a handler exhausts its initial ping retries, a later successful pong does not restore its retry budget. Reset `pingTrials` when a pong is accepted so recovered nodes receive the normal challenge budget.</violation>
</file>

<file name="p2p/src/main/java/org/tron/p2p/dns/tree/BranchEntry.java">

<violation number="1" location="p2p/src/main/java/org/tron/p2p/dns/tree/BranchEntry.java:24">
P2: When a DNS branch TXT value contains an empty or non-hash child, `parseEntry` accepts it and `SubtreeSync` queues it for resolution, so malformed publisher data can abort synchronization or silently omit leaves. Validate every child with the fixed-length base32 hash rules and reject the branch before returning it.</violation>
</file>

<file name="p2p/src/main/java/org/tron/p2p/utils/CollectionUtils.java">

<violation number="1" location="p2p/src/main/java/org/tron/p2p/utils/CollectionUtils.java:13">
P2: When `limit` is zero and `items` is non-empty, this loop returns every item instead of truncating to zero. Return an empty list before iterating when `limit == 0`.</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:58">
P2: When channel initialization throws, this catch logs and returns while leaving the Netty channel open with a partial pipeline. Close `ch` or rethrow the failure so Netty cannot retain an unusable peer connection.</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">
P1: When a remote UDP packet causes `eventHandler.handleEvent` to throw, this closes the shared UDP listener and takes discovery offline until `DiscoverServer` restarts it. Repeated packets can keep causing this outage; handle per-datagram failures without closing the listener.</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:34">
P1: When a bucket is full, `getLastSeen()` selects the newest entry instead of the least-recently-seen entry. Challenge the oldest entry (`sorted.get(sorted.size() - 1)`) so active peers are retained and stale peers are eligible for replacement.</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:66">
P2: After `close()` sets `isShutdown`, a later `init()` leaves it true, so `PeerClient.connectAsync()` returns null and the connection pool skips reconnection. Reset `isShutdown` during initialization.</violation>

<violation number="2" location="p2p/src/main/java/org/tron/p2p/connection/ChannelManager.java:94">
P1: When an old connection closes after a replacement to the same remote address is admitted, this removal deletes the replacement from `channels`. Remove the entry only when its mapped value is this `channel`.</violation>
</file>

<file name="p2p/src/main/java/org/tron/p2p/dns/sync/ClientTree.java">

<violation number="1" location="p2p/src/main/java/org/tron/p2p/dns/sync/ClientTree.java:113">
P1: When a referenced TXT record is temporarily unavailable, this line permanently drops its hash from `linkSync.missing` and treats the signed tree as complete. Keep unresolved hashes queued or fail the sync, and apply the same handling to the ENR removal in `syncNextRandomNode` so transient DNS propagation does not produce an incomplete node set.</violation>
</file>

<file name="p2p/src/main/java/org/tron/p2p/discover/protocol/kad/table/NodeEntry.java">

<violation number="1" location="p2p/src/main/java/org/tron/p2p/discover/protocol/kad/table/NodeEntry.java:14">
P2: When an existing handler learns a different node ID, this cached distance remains based on the old ID, so `NodeTable` keeps the peer in the wrong Kademlia bucket and may evict or fail-find the wrong peers. Recompute the distance from the current node ID, or update/rebucket the entry whenever the node ID changes.</violation>
</file>

<file name="p2p/src/main/java/org/tron/p2p/dns/update/PublishService.java">

<violation number="1" location="p2p/src/main/java/org/tron/p2p/dns/update/PublishService.java:138">
P1: When DNS publishing is enabled without `dnsPrivate`, `checkConfig` still accepts the configuration and `Tree.makeTree` skips signing, so the publisher emits a `tree://null@...`/unsigned tree. Reject missing private keys before starting the publisher.</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:67">
P1: When a root TXT value is shorter than `rootPrefix`, this line throws `StringIndexOutOfBoundsException`; prefixless values are also parsed as roots. `AwsClient.computeChanges` catches only `DnsException`, so malformed existing root data aborts publish; validate the prefix and length first.</violation>

<violation number="2" location="p2p/src/main/java/org/tron/p2p/dns/tree/RootEntry.java:70">
P2: When the outer root payload or embedded signature contains malformed Base64, `Algorithm.decode64` throws unchecked `IllegalArgumentException` instead of the declared `DnsException`. Catch decoder failures and convert them to the appropriate root or signature parse error so corrupt DNS data cannot escape root resolution and publishing paths.</violation>
</file>

<file name="p2p/src/main/java/org/tron/p2p/connection/business/keepalive/KeepAliveService.java">

<violation number="1" location="p2p/src/main/java/org/tron/p2p/connection/business/keepalive/KeepAliveService.java:40">
P1: When a peer answers before the following assignments run, `processMessage` clears `waitForPong`, then this code sets it back to true. Record `pingSent` and mark `waitForPong` before sending the ping, otherwise a healthy peer can be disconnected after 20 seconds.</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:115">
P2: When more than 16 peers share a distance bucket, `DistanceComparator` returns zero for all of them, so this sort preserves arbitrary `HashMap` order. The following truncation can omit XOR-closer peers from `getClosestNodes`; sort by the full XOR distance before limiting the response.</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:155">
P1: When a peer keeps the same ID but advertises a different endpoint, equal `Node` objects produce different hashes, so `HashSet` lookups and deduplication fail. Hash the same byte-array identity used by `equals`.</violation>

<violation number="2" location="p2p/src/main/java/org/tron/p2p/discover/Node.java:169">
P1: When two peer IDs contain different invalid UTF-8 byte sequences, `getIdString()` can make them equal and the node table can treat distinct peers as the same node. Compare the ID byte arrays directly with `Arrays.equals` instead of decoding them to `String`.</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:99">
P1: When the zone ID is omitted and nested Route53 zones exist, `findZoneID` can select the parent zone because it returns the first suffix match. Keep the longest matching hosted-zone name, or require an explicit zone ID.</violation>
</file>

<file name="p2p/src/main/java/org/web3j/utils/Numeric.java">

<violation number="1" location="p2p/src/main/java/org/web3j/utils/Numeric.java:43">
P2: When `decodeQuantity` receives a negative or unprefixed value, it returns it instead of rejecting it. Validate the quantity digits and require the `0x` form before parsing.</violation>

<violation number="2" location="p2p/src/main/java/org/web3j/utils/Numeric.java:213">
P2: When `hexStringToByteArray` receives a non-hex character, it silently transforms the input into bytes, so `Hash.sha3(String)` hashes the wrong data. Reject either nibble before constructing each byte.</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:38">
P2: `connect` blocks until the peer disconnects, so callers cannot continue after the connection is established. Wait only for the connect future and return.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[cubic-dev-ai]

P1: When DNS publishing is enabled without dnsPrivate, checkConfig still accepts the configuration and Tree.makeTree skips signing, so the publisher emits a tree://null@.../unsigned tree. Reject missing private keys before starting the publisher.

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/PublishService.java, line 138:

<comment>When DNS publishing is enabled without `dnsPrivate`, `checkConfig` still accepts the configuration and `Tree.makeTree` skips signing, so the publisher emits a `tree://null@...`/unsigned tree. Reject missing private keys before starting the publisher.</comment>

<file context>
@@ -0,0 +1,146 @@
+      logger.error("The configuration items related to the AwsRoute53 dns server cannot be empty");
+      return false;
+    }
+    return true;
+  }
+
</file context>

[cubic-dev-ai]

P1: When a root TXT value is shorter than rootPrefix, this line throws StringIndexOutOfBoundsException; prefixless values are also parsed as roots. AwsClient.computeChanges catches only DnsException, so malformed existing root data aborts publish; validate the prefix and length first.

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/RootEntry.java, line 67:

<comment>When a root TXT value is shorter than `rootPrefix`, this line throws `StringIndexOutOfBoundsException`; prefixless values are also parsed as roots. `AwsClient.computeChanges` catches only `DnsException`, so malformed existing root data aborts publish; validate the prefix and length first.</comment>

<file context>
@@ -0,0 +1,113 @@
+  }
+
+  public static RootEntry parseEntry(String e) throws DnsException {
+    String value = e.substring(rootPrefix.length());
+    DnsRoot dnsRoot1;
+    try {
</file context>

[cubic-dev-ai]

P1: When a stale-record deletion fails after retries, submitChanges still reports publication success and leaves the old DNS record active. Check the return value and propagate a deletion failure before logging success.

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 199:

<comment>When a stale-record deletion fails after retries, `submitChanges` still reports publication success and leaves the old DNS record active. Check the return value and propagate a deletion failure before logging success.</comment>

<file context>
@@ -0,0 +1,341 @@
+
+    for (String key : existing.keySet()) {
+      if (!records.containsKey(key)) {
+        deleteRecord(existing.get(key).getRecordId());
+        deleteCount++;
+      }
</file context>

[cubic-dev-ai]

P1: When a peer keeps the same ID but advertises a different endpoint, equal Node objects produce different hashes, so HashSet lookups and deduplication fail. Hash the same byte-array identity used by equals.

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>When a peer keeps the same ID but advertises a different endpoint, equal `Node` objects produce different hashes, so `HashSet` lookups and deduplication fail. Hash the same byte-array identity used by `equals`.</comment>

<file context>
@@ -0,0 +1,197 @@
+
+  @Override
+  public int hashCode() {
+    return this.format().hashCode();
+  }
+
</file context>

[cubic-dev-ai]

P1: When the public key’s X coordinate starts with a zero nibble, this slice drops that nibble and includes part of Y, so DNS tree publication produces an unusable public key. Left-pad the full public-key value to 128 hex characters before extracting X.

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 31:

<comment>When the public key’s X coordinate starts with a zero nibble, this slice drops that nibble and includes part of Y, so DNS tree publication produces an unusable public key. Left-pad the full public-key value to 128 hex characters before extracting X.</comment>

<file context>
@@ -0,0 +1,150 @@
+  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;
+    return hexPub;
</file context>

[cubic-dev-ai]

P2: When decodeQuantity receives a negative or unprefixed value, it returns it instead of rejecting it. Validate the quantity digits and require the 0x form before parsing.

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/web3j/utils/Numeric.java, line 43:

<comment>When `decodeQuantity` receives a negative or unprefixed value, it returns it instead of rejecting it. Validate the quantity digits and require the `0x` form before parsing.</comment>

<file context>
@@ -0,0 +1,252 @@
+  }
+
+  public static BigInteger decodeQuantity(String value) {
+    if (isLongValue(value)) {
+      return BigInteger.valueOf(Long.parseLong(value));
+    }
</file context>

[cubic-dev-ai]

P2: When hexStringToByteArray receives a non-hex character, it silently transforms the input into bytes, so Hash.sha3(String) hashes the wrong data. Reject either nibble before constructing each byte.

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/web3j/utils/Numeric.java, line 213:

<comment>When `hexStringToByteArray` receives a non-hex character, it silently transforms the input into bytes, so `Hash.sha3(String)` hashes the wrong data. Reject either nibble before constructing each byte.</comment>

<file context>
@@ -0,0 +1,252 @@
+    int startIdx;
+    if (len % 2 != 0) {
+      data = new byte[(len / 2) + 1];
+      data[0] = (byte) Character.digit(cleanInput.charAt(0), 16);
+      startIdx = 1;
+    } else {
</file context>

[cubic-dev-ai]

P2: When channel initialization throws, this catch logs and returns while leaving the Netty channel open with a partial pipeline. Close ch or rethrow the failure so Netty cannot retain an unusable peer connection.

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 58:

<comment>When channel initialization throws, this catch logs and returns while leaving the Netty channel open with a partial pipeline. Close `ch` or rethrow the failure so Netty cannot retain an unusable peer connection.</comment>

<file context>
@@ -0,0 +1,62 @@
+      });
+
+    } catch (Exception e) {
+      logger.error("Unexpected initChannel error", e);
+    }
+  }
</file context>

[cubic-dev-ai]

P2: When an existing handler learns a different node ID, this cached distance remains based on the old ID, so NodeTable keeps the peer in the wrong Kademlia bucket and may evict or fail-find the wrong peers. Recompute the distance from the current node ID, or update/rebucket the entry whenever the node ID changes.

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/NodeEntry.java, line 14:

<comment>When an existing handler learns a different node ID, this cached distance remains based on the old ID, so `NodeTable` keeps the peer in the wrong Kademlia bucket and may evict or fail-find the wrong peers. Recompute the distance from the current node ID, or update/rebucket the entry whenever the node ID changes.</comment>

<file context>
@@ -0,0 +1,88 @@
+  public NodeEntry(byte[] ownerId, Node n) {
+    this.node = n;
+    entryId = n.getHostKey();
+    distance = distance(ownerId, n.getId());
+    touch();
+  }
</file context>

[cubic-dev-ai]

All reported issues were addressed across 3 files (changes from recent commits).

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[cubic-dev-ai]

1 issue found across 33 files (changes from recent commits).

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="framework/src/test/java/org/tron/p2p/connection/message/detect/StatusMessageTest.java">

<violation number="1" location="framework/src/test/java/org/tron/p2p/connection/message/detect/StatusMessageTest.java:28">
P3: This test rewrites the process-wide singleton `Parameter.p2pConfig` and clears the shared `ChannelManager.channels` map in setUp/tearDown. `Parameter.p2pConfig` is `volatile` and read by the live p2p stack (getHomeNode, processPeer, StatusMessage), and `channels` is the production connection map; mutating them makes the test order- and concurrency-sensitive and lets it clobber state that other tests or a running Stack may rely on.</violation>
</file>

<sub>Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic</sub>

[cubic-dev-ai]

P3: This test rewrites the process-wide singleton Parameter.p2pConfig and clears the shared ChannelManager.channels map in setUp/tearDown. Parameter.p2pConfig is volatile and read by the live p2p stack (getHomeNode, processPeer, StatusMessage), and channels is the production connection map; mutating them makes the test order- and concurrency-sensitive and lets it clobber state that other tests or a running Stack may rely on.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At framework/src/test/java/org/tron/p2p/connection/message/detect/StatusMessageTest.java, line 28:

<comment>This test rewrites the process-wide singleton `Parameter.p2pConfig` and clears the shared `ChannelManager.channels` map in setUp/tearDown. `Parameter.p2pConfig` is `volatile` and read by the live p2p stack (getHomeNode, processPeer, StatusMessage), and `channels` is the production connection map; mutating them makes the test order- and concurrency-sensitive and lets it clobber state that other tests or a running Stack may rely on.</comment>

<file context>
@@ -0,0 +1,70 @@
+    config.setIp("127.0.0.1");
+    config.setNetworkId(11111);
+    config.setMaxConnections(30);
+    Parameter.p2pConfig = config;
+    ChannelManager.getChannels().clear();
+  }
</file context>

[cubic-dev-ai]

7 issues found across 33 files (changes from recent commits).

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="framework/src/test/java/org/tron/p2p/stats/StatsManagerTest.java">

<violation number="1" location="framework/src/test/java/org/tron/p2p/stats/StatsManagerTest.java:26">
P3: snapshotIsDetachedFromLaterTraffic permanently mutates the shared static TrafficStats.tcp counter and never restores it. Because TrafficStats.tcp is a JVM-global @Sharable singleton also attached to real TCP pipelines, the +1 leaks into any later test or path that relies on an absolute baseline. Restore the counter (or use a fresh TrafficStatHandler) after asserting to keep the test isolated.</violation>
</file>

<file name="framework/src/test/java/org/tron/p2p/discover/message/DiscoverMessageTest.java">

<violation number="1" location="framework/src/test/java/org/tron/p2p/discover/message/DiscoverMessageTest.java:19">
P3: The class comment claims the two Message.parse rejection paths are covered, but only the NO_SUCH_MESSAGE default branch is. unparseableBodyIsRejected throws from Discover.PingMessage.parseFrom (InvalidProtocolBufferException) before valid() runs, so no test hits the valid()==false BAD_MESSAGE rejection. Either fix the comment or add a test that sends a well-formed datagram with an invalid node (e.g., empty host) and asserts P2pException.TypeEnum.BAD_MESSAGE.</violation>
</file>

<file name="framework/src/test/java/org/tron/p2p/connection/socket/MessageHandlerTest.java">

<violation number="1" location="framework/src/test/java/org/tron/p2p/connection/socket/MessageHandlerTest.java:123">
P3: This test asserts NO_SUCH_MESSAGE for type byte 0x01, which depends on the global static Parameter.handlerMap having no registered handler for 0x01. Any other test (or the application wiring) that registers a P2pEventHandler with type 0x01 through addP2pEventHandle makes this assertion fail, and because handlerMap is never reset by setUp/tearDown the test is order- and suite-dependent. Save and restore handlerMap (clear it in setUp, restore in tearDown) or register the handler under test explicitly so the test does not depend on unspecified global state.</violation>
</file>

<file name="framework/src/test/java/org/tron/p2p/connection/business/pool/ConnPoolLifecycleTest.java">

<violation number="1" location="framework/src/test/java/org/tron/p2p/connection/business/pool/ConnPoolLifecycleTest.java:61">
P2: Each test method registers a new ConnPoolService into the global static Parameter.handlerList (added by the ConnPoolService constructor) and never removes it or calls close(). ChannelManager.onConnect/onDisconnect iterate that list, so every leaked instance keeps receiving connection events and accumulating across the suite, polluting other p2p tests in the same JVM. Remove the handler (Parameter.handlerList.remove(service)) and shut down the executors in tearDown, or restore the list to its pre-test contents.</violation>
</file>

<file name="framework/src/test/java/org/tron/p2p/utils/NetUtilAddressTest.java">

<violation number="1" location="framework/src/test/java/org/tron/p2p/utils/NetUtilAddressTest.java:35">
P3: `tolerantOfSurroundingWhitespace` trims its input (`.trim()`) before calling `parseInetSocketAddress`, so it never exercises the method's own whitespace handling and would pass even if `parseInetSocketAddress` dropped its internal `para.trim()`. Pass the un-trimmed value so the test actually verifies the claimed behavior it documents.</violation>
</file>

<file name="framework/src/test/java/org/tron/p2p/connection/ChannelCoreTest.java">

<violation number="1" location="framework/src/test/java/org/tron/p2p/connection/ChannelCoreTest.java:106">
P3: The assertion `channel.getLastSendTime() > 0` cannot fail: `Channel.lastSendTime` is initialized to `System.currentTimeMillis()` at field construction, so it is always non-zero even if `send` never ran. To actually verify that `send` updates the timestamp, reset it (e.g. set lastSendTime to 0) before calling send, then assert it became non-zero after the write.</violation>
</file>

<file name="framework/src/test/java/org/tron/p2p/P2pServiceTest.java">

<violation number="1" location="framework/src/test/java/org/tron/p2p/P2pServiceTest.java:47">
P3: Each test calls service.start(config), and P2pService.start() registers a fresh Runtime shutdown hook (new Thread(this::close)) that is never removed. Running this 5-test class accumulates 5 hook threads that persist until the JVM exits and hold references to otherwise-discarded service instances. Start the service once per class (or refactor so the hook is not registered per instance) to avoid leaking a hook per test.</violation>
</file>

<sub>Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic</sub>

[cubic-dev-ai]

P2: Each test method registers a new ConnPoolService into the global static Parameter.handlerList (added by the ConnPoolService constructor) and never removes it or calls close(). ChannelManager.onConnect/onDisconnect iterate that list, so every leaked instance keeps receiving connection events and accumulating across the suite, polluting other p2p tests in the same JVM. Remove the handler (Parameter.handlerList.remove(service)) and shut down the executors in tearDown, or restore the list to its pre-test contents.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At framework/src/test/java/org/tron/p2p/connection/business/pool/ConnPoolLifecycleTest.java, line 61:

<comment>Each test method registers a new ConnPoolService into the global static Parameter.handlerList (added by the ConnPoolService constructor) and never removes it or calls close(). ChannelManager.onConnect/onDisconnect iterate that list, so every leaked instance keeps receiving connection events and accumulating across the suite, polluting other p2p tests in the same JVM. Remove the handler (Parameter.handlerList.remove(service)) and shut down the executors in tearDown, or restore the list to its pre-test contents.</comment>

<file context>
@@ -0,0 +1,133 @@
+    config.setMaxConnectionsWithSameIp(5);
+    Parameter.p2pConfig = config;
+    ChannelManager.getChannels().clear();
+    service = new ConnPoolService();
+  }
+
</file context>

[cubic-dev-ai]

P3: snapshotIsDetachedFromLaterTraffic permanently mutates the shared static TrafficStats.tcp counter and never restores it. Because TrafficStats.tcp is a JVM-global @sharable singleton also attached to real TCP pipelines, the +1 leaks into any later test or path that relies on an absolute baseline. Restore the counter (or use a fresh TrafficStatHandler) after asserting to keep the test isolated.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At framework/src/test/java/org/tron/p2p/stats/StatsManagerTest.java, line 26:

<comment>snapshotIsDetachedFromLaterTraffic permanently mutates the shared static TrafficStats.tcp counter and never restores it. Because TrafficStats.tcp is a JVM-global @Sharable singleton also attached to real TCP pipelines, the +1 leaks into any later test or path that relies on an absolute baseline. Restore the counter (or use a fresh TrafficStatHandler) after asserting to keep the test isolated.</comment>

<file context>
@@ -0,0 +1,32 @@
+  public void snapshotIsDetachedFromLaterTraffic() {
+    P2pStats before = new StatsManager().getP2pStats();
+    long recorded = before.getTcpInPackets();
+    TrafficStats.tcp.getInPackets().incrementAndGet();
+
+    // The old snapshot must not move with the counter.
</file context>

[cubic-dev-ai]

P3: The class comment claims the two Message.parse rejection paths are covered, but only the NO_SUCH_MESSAGE default branch is. unparseableBodyIsRejected throws from Discover.PingMessage.parseFrom (InvalidProtocolBufferException) before valid() runs, so no test hits the valid()==false BAD_MESSAGE rejection. Either fix the comment or add a test that sends a well-formed datagram with an invalid node (e.g., empty host) and asserts P2pException.TypeEnum.BAD_MESSAGE.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At framework/src/test/java/org/tron/p2p/discover/message/DiscoverMessageTest.java, line 19:

<comment>The class comment claims the two Message.parse rejection paths are covered, but only the NO_SUCH_MESSAGE default branch is. unparseableBodyIsRejected throws from Discover.PingMessage.parseFrom (InvalidProtocolBufferException) before valid() runs, so no test hits the valid()==false BAD_MESSAGE rejection. Either fix the comment or add a test that sends a well-formed datagram with an invalid node (e.g., empty host) and asserts P2pException.TypeEnum.BAD_MESSAGE.</comment>

<file context>
@@ -0,0 +1,128 @@
+import org.tron.p2p.utils.NetUtil;
+
+/**
+ * Message.parse is the entry point for every inbound discovery datagram, so its
+ * dispatch table and its two rejection paths are what a hostile packet meets
+ * first.
</file context>

[cubic-dev-ai]

P3: This test asserts NO_SUCH_MESSAGE for type byte 0x01, which depends on the global static Parameter.handlerMap having no registered handler for 0x01. Any other test (or the application wiring) that registers a P2pEventHandler with type 0x01 through addP2pEventHandle makes this assertion fail, and because handlerMap is never reset by setUp/tearDown the test is order- and suite-dependent. Save and restore handlerMap (clear it in setUp, restore in tearDown) or register the handler under test explicitly so the test does not depend on unspecified global state.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At framework/src/test/java/org/tron/p2p/connection/socket/MessageHandlerTest.java, line 123:

<comment>This test asserts NO_SUCH_MESSAGE for type byte 0x01, which depends on the global static Parameter.handlerMap having no registered handler for 0x01. Any other test (or the application wiring) that registers a P2pEventHandler with type 0x01 through addP2pEventHandle makes this assertion fail, and because handlerMap is never reset by setUp/tearDown the test is order- and suite-dependent. Save and restore handlerMap (clear it in setUp, restore in tearDown) or register the handler under test explicitly so the test does not depend on unspecified global state.</comment>

<file context>
@@ -0,0 +1,139 @@
+    // registered handler map rather than going through Message.parse. With no
+    // handler registered for 0x01 that path raises NO_SUCH_MESSAGE too, so a
+    // peer probing unused type bytes is disconnected the same way.
+    RecordingChannel channel = feed(new byte[] {0x01, 1, 2, 3});
+    Assert.assertEquals(DisconnectReason.NO_SUCH_MESSAGE, channel.onlyReason());
+  }
</file context>

[cubic-dev-ai]

P3: tolerantOfSurroundingWhitespace trims its input (.trim()) before calling parseInetSocketAddress, so it never exercises the method's own whitespace handling and would pass even if parseInetSocketAddress dropped its internal para.trim(). Pass the un-trimmed value so the test actually verifies the claimed behavior it documents.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At framework/src/test/java/org/tron/p2p/utils/NetUtilAddressTest.java, line 35:

<comment>`tolerantOfSurroundingWhitespace` trims its input (`.trim()`) before calling `parseInetSocketAddress`, so it never exercises the method's own whitespace handling and would pass even if `parseInetSocketAddress` dropped its internal `para.trim()`. Pass the un-trimmed value so the test actually verifies the claimed behavior it documents.</comment>

<file context>
@@ -0,0 +1,103 @@
+  @Test
+  public void tolerantOfSurroundingWhitespace() {
+    Assert.assertEquals(18888,
+        NetUtil.parseInetSocketAddress(" 127.0.0.1:18888 ".trim()).getPort());
+  }
+
</file context>

[cubic-dev-ai]

P3: The assertion channel.getLastSendTime() > 0 cannot fail: Channel.lastSendTime is initialized to System.currentTimeMillis() at field construction, so it is always non-zero even if send never ran. To actually verify that send updates the timestamp, reset it (e.g. set lastSendTime to 0) before calling send, then assert it became non-zero after the write.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At framework/src/test/java/org/tron/p2p/connection/ChannelCoreTest.java, line 106:

<comment>The assertion `channel.getLastSendTime() > 0` cannot fail: `Channel.lastSendTime` is initialized to `System.currentTimeMillis()` at field construction, so it is always non-zero even if `send` never ran. To actually verify that `send` updates the timestamp, reset it (e.g. set lastSendTime to 0) before calling send, then assert it became non-zero after the write.</comment>

<file context>
@@ -0,0 +1,173 @@
+    ByteBuf written = netty.readOutbound();
+    Assert.assertNotNull("a ping should have been written", written);
+    Assert.assertEquals(MessageType.KEEP_ALIVE_PING.getType(), written.getByte(0));
+    Assert.assertTrue(channel.getLastSendTime() > 0);
+    netty.finishAndReleaseAll();
+  }
</file context>

[cubic-dev-ai]

P3: Each test calls service.start(config), and P2pService.start() registers a fresh Runtime shutdown hook (new Thread(this::close)) that is never removed. Running this 5-test class accumulates 5 hook threads that persist until the JVM exits and hold references to otherwise-discarded service instances. Start the service once per class (or refactor so the hook is not registered per instance) to avoid leaking a hook per test.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At framework/src/test/java/org/tron/p2p/P2pServiceTest.java, line 47:

<comment>Each test calls service.start(config), and P2pService.start() registers a fresh Runtime shutdown hook (new Thread(this::close)) that is never removed. Running this 5-test class accumulates 5 hook threads that persist until the JVM exits and hold references to otherwise-discarded service instances. Start the service once per class (or refactor so the hook is not registered per instance) to avoid leaking a hook per test.</comment>

<file context>
@@ -0,0 +1,118 @@
+    savedHandlerMap = new HashMap<>(Parameter.handlerMap);
+
+    service = new P2pService();
+    service.start(config);
+  }
+
</file context>

[cubic-dev-ai]

5 issues found across 71 files (changes from recent commits).

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/test/java/org/tron/p2p/example/ExampleUsageTest.java">

<violation number="1" location="p2p/src/test/java/org/tron/p2p/example/ExampleUsageTest.java:43">
P2: These two tests register a handler for hardcoded byte 0x01 into the process-wide static Parameter.handlerList/handlerMap and clean up only by restoring a snapshot taken in @Before. If any prior test in the shared fork leaves a 0x01 handler in that static map, Parameter.addP2pEventHandle throws TYPE_ALREADY_REGISTERED and importUsingLifecycleRunsEndToEnd fails spuriously. Reset the handlers to a clean state in @Before (or use a type unique to this class) instead of relying on every class's @After restoring the global registry exactly.</violation>
</file>

<file name="p2p/build.gradle">

<violation number="1" location="p2p/build.gradle:165">
P2: The documented standalone launch fails because this task's command-substitution output is contaminated by the root build's configuration-time `println`. Emit a machine-readable classpath without other Gradle stdout, or otherwise remove the configuration log from this command's output.</violation>
</file>

<file name="p2p/README.md">

<violation number="1" location="p2p/README.md:437">
P3: The unchanged "For details please" line now dangles in front of the added text, reading "For details please The former `ImportUsing`...". Drop the dangling lead-in so the sentence reads cleanly.</violation>
</file>

<file name="p2p/src/test/java/org/tron/p2p/utils/TestPort.java">

<violation number="1" location="p2p/src/test/java/org/tron/p2p/utils/TestPort.java:31">
P3: The catch (IOException e) in choose() is unreachable: available() swallows every IOException internally and returns false, so it never propagates. The try/catch wrapper adds no behavior; remove it and the now-unused IOException import.</violation>

<violation number="2" location="p2p/src/test/java/org/tron/p2p/utils/TestPort.java:42">
P2: available() frees the port the moment the check returns, so between that check and the test's actual bind another Gradle fork or process can take the port — the exact false-pass the comment warns about. This is the classic check-then-use TOCTOU race; checking a port and handing out its number cannot guarantee it stays free. Prefer returning a held ServerSocket that the test binds against (keeping it open), or at least acknowledge that the availability check does not remove the collision window.</violation>
</file>

<sub>Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic</sub>

[cubic-dev-ai]

P2: available() frees the port the moment the check returns, so between that check and the test's actual bind another Gradle fork or process can take the port — the exact false-pass the comment warns about. This is the classic check-then-use TOCTOU race; checking a port and handing out its number cannot guarantee it stays free. Prefer returning a held ServerSocket that the test binds against (keeping it open), or at least acknowledge that the availability check does not remove the collision window.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At p2p/src/test/java/org/tron/p2p/utils/TestPort.java, line 42:

<comment>available() frees the port the moment the check returns, so between that check and the test's actual bind another Gradle fork or process can take the port — the exact false-pass the comment warns about. This is the classic check-then-use TOCTOU race; checking a port and handing out its number cannot guarantee it stays free. Prefer returning a held ServerSocket that the test binds against (keeping it open), or at least acknowledge that the availability check does not remove the collision window.</comment>

<file context>
@@ -0,0 +1,49 @@
+  }
+
+  private static boolean available(int port) throws IOException {
+    try (ServerSocket socket = new ServerSocket(port)) {
+      socket.setReuseAddress(true);
+      return true;
</file context>

[cubic-dev-ai]

P2: These two tests register a handler for hardcoded byte 0x01 into the process-wide static Parameter.handlerList/handlerMap and clean up only by restoring a snapshot taken in @before. If any prior test in the shared fork leaves a 0x01 handler in that static map, Parameter.addP2pEventHandle throws TYPE_ALREADY_REGISTERED and importUsingLifecycleRunsEndToEnd fails spuriously. Reset the handlers to a clean state in @before (or use a type unique to this class) instead of relying on every class's @after restoring the global registry exactly.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At p2p/src/test/java/org/tron/p2p/example/ExampleUsageTest.java, line 43:

<comment>These two tests register a handler for hardcoded byte 0x01 into the process-wide static Parameter.handlerList/handlerMap and clean up only by restoring a snapshot taken in @Before. If any prior test in the shared fork leaves a 0x01 handler in that static map, Parameter.addP2pEventHandle throws TYPE_ALREADY_REGISTERED and importUsingLifecycleRunsEndToEnd fails spuriously. Reset the handlers to a clean state in @Before (or use a type unique to this class) instead of relying on every class's @After restoring the global registry exactly.</comment>

<file context>
@@ -0,0 +1,224 @@
+  private List<P2pEventHandler> savedHandlerList;
+  private java.util.Map<Byte, P2pEventHandler> savedHandlerMap;
+
+  private static final byte TEST_MESSAGE_TYPE = (byte) 0x01;
+
+  @Before
</file context>

[cubic-dev-ai]

P3: The unchanged "For details please" line now dangles in front of the added text, reading "For details please The former ImportUsing...". Drop the dangling lead-in so the sentence reads cleanly.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At p2p/README.md, line 437:

<comment>The unchanged "For details please" line now dangles in front of the added text, reading "For details please The former `ImportUsing`...". Drop the dangling lead-in so the sentence reads cleanly.</comment>

<file context>
@@ -416,6 +434,9 @@ p2pService.start(config);
 
 For details please
-check [ImportUsing](ImportUsing.java), [DnsExample1](DnsExample1.java), [DnsExample2](DnsExample2.java)
+The former `ImportUsing`, `DnsExample1` and `DnsExample2` reference classes have
+been replaced by
+[ExampleUsageTest](src/test/java/org/tron/p2p/example/ExampleUsageTest.java),
</file context>

[cubic-dev-ai]

P3: The catch (IOException e) in choose() is unreachable: available() swallows every IOException internally and returns false, so it never propagates. The try/catch wrapper adds no behavior; remove it and the now-unused IOException import.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At p2p/src/test/java/org/tron/p2p/utils/TestPort.java, line 31:

<comment>The catch (IOException e) in choose() is unreachable: available() swallows every IOException internally and returns false, so it never propagates. The try/catch wrapper adds no behavior; remove it and the now-unused IOException import.</comment>

<file context>
@@ -0,0 +1,49 @@
+      while (!available(port)) {
+        port = next();
+      }
+    } catch (IOException e) {
+      return next();
+    }
</file context>

[cubic-dev-ai]

1 issue found across 3 files (changes from recent commits).

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/test/java/org/tron/p2p/example/StartAppArgsTest.java">

<violation number="1" location="p2p/src/test/java/org/tron/p2p/example/StartAppArgsTest.java:48">
P3: trustIpsSkipsWhatItCannotResolve forces a real DNS lookup for "no-such-host.invalid" on every run. The .invalid TLD is RFC 2606-reserved so it is deterministic in normal environments, but this is still network I/O inside a unit test; in a network-less or slow-DNS CI it can hang on the resolver timeout or behave unexpectedly. Consider stubbing/mocking the resolver or removing the hostname case and testing the unresolvable path with a value that needs no network.</violation>
</file>

<sub>Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic</sub>

[cubic-dev-ai]

P3: trustIpsSkipsWhatItCannotResolve forces a real DNS lookup for "no-such-host.invalid" on every run. The .invalid TLD is RFC 2606-reserved so it is deterministic in normal environments, but this is still network I/O inside a unit test; in a network-less or slow-DNS CI it can hang on the resolver timeout or behave unexpectedly. Consider stubbing/mocking the resolver or removing the hostname case and testing the unresolvable path with a value that needs no network.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At p2p/src/test/java/org/tron/p2p/example/StartAppArgsTest.java, line 48:

<comment>trustIpsSkipsWhatItCannotResolve forces a real DNS lookup for "no-such-host.invalid" on every run. The .invalid TLD is RFC 2606-reserved so it is deterministic in normal environments, but this is still network I/O inside a unit test; in a network-less or slow-DNS CI it can hang on the resolver timeout or behave unexpectedly. Consider stubbing/mocking the resolver or removing the hostname case and testing the unresolvable path with a value that needs no network.</comment>

<file context>
@@ -0,0 +1,71 @@
+  public void trustIpsSkipsWhatItCannotResolve() {
+    // An unresolvable entry is logged and dropped rather than aborting the rest.
+    List<InetAddress> parsed =
+        app.parseInetAddressList("127.0.0.2,no-such-host.invalid,127.0.0.3");
+    Assert.assertEquals(2, parsed.size());
+    Assert.assertEquals("127.0.0.2", parsed.get(0).getHostAddress());
</file context>

[cubic-dev-ai]

1 issue found across 6 files (changes from recent commits).

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/README.md">

<violation number="1" location="p2p/README.md:28">
P2: The documented `-PbinaryRelease=false` opt-out does not take effect. The Gradle property is `binaryRelease` (lowercase 'b') in p2p/build.gradle:29, and `-P` property names are case-sensitive, so passing the capital-B flag leaves the property unset and the default 'true' builds `p2p-standalone.jar` anyway. Use `-PbinaryRelease=false` to match the modules it claims to mirror (`:framework`, `:plugins`).</violation>
</file>

<sub>Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic</sub>

[cubic-dev-ai]

P2: The documented -PbinaryRelease=false opt-out does not take effect. The Gradle property is binaryRelease (lowercase 'b') in p2p/build.gradle:29, and -P property names are case-sensitive, so passing the capital-B flag leaves the property unset and the default 'true' builds p2p-standalone.jar anyway. Use -PbinaryRelease=false to match the modules it claims to mirror (:framework, :plugins).

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At p2p/README.md, line 28:

<comment>The documented `-PbinaryRelease=false` opt-out does not take effect. The Gradle property is `binaryRelease` (lowercase 'b') in p2p/build.gradle:29, and `-P` property names are case-sensitive, so passing the capital-B flag leaves the property unset and the default 'true' builds `p2p-standalone.jar` anyway. Use `-PbinaryRelease=false` to match the modules it claims to mirror (`:framework`, `:plugins`).</comment>

<file context>
@@ -1,29 +1,40 @@
+java -jar p2p/build/libs/p2p-standalone.jar --help
+```
+
+`-PbinaryRelease=false` skips building it, matching `:framework` and `:plugins`.
+
+This module can run on its own, or be used as a library.
</file context>