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

Closed
BA
1mo ago
+17381-32

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/java-tron#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

Basetronprotocol/java-tron develop @ 4a21592f95e37908b21bc3f611c6e7a1a67f09f3
Vendoredtronprotocol/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/proto2
p2p/src/test/java62

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/java-tron#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/java-tron#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-lang33.18.0 (runtime)3.18.0
grpc-netty1.81.0rootProject.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:

PathGuardResult
TCP wire decodeMessage.parse catch (Exception)contained
UDP discovery decodeP2pPacketDecoder catch (Exception)contained
DNS sync / iterationRandomIterator.next() catch (Exception) around syncRandom() (ClientTree has no catch of its own)contained
DNS publish / collectonly 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:

BeforeAfter
Base overall79.67%79.69%
PR overall78.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

GateResult
:p2p:build (compile, test, checkstyle, jar)PASS
java -jar p2p-standalone.jar --helpPASS — prints help
:framework:compileJava --rerun-tasksPASS, tasks executed
project :p2p on framework runtimeClasspathPASS
Zero external libp2p artifactsPASS
gson:2.9.0 -> 2.14.0PASS
:p2p:checkstyleMainPASS, 0 violations (from 605)
:framework:checkstyleTestPASS, 0 violations
:p2p:checkstyleTestPASS, 0 violations
check-mathPASS
:p2p:test357 tests, 0 failures, 3 skipped
commons-lang3 unchanged at 3.18.0PASS
zero implicit_dependency warningsPASS
:framework:testPASS — 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 configPASS — 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.