Skip to content

build(p2p): internalize libp2p as local module#6673

Open
barbatos2011 wants to merge 7 commits intotronprotocol:developfrom
barbatos2011:chore/internalize-libp2p
Open

build(p2p): internalize libp2p as local module#6673
barbatos2011 wants to merge 7 commits intotronprotocol:developfrom
barbatos2011:chore/internalize-libp2p

Conversation

@barbatos2011
Copy link
Copy Markdown

What

Internalize io.github.tronprotocol:libp2p:2.2.7 source code as a new p2p/ gradle submodule, replacing the external Maven dependency. No functional changes to P2P behavior, protocol, or consensus.

Why

The external libp2p repository had a 2.5-year development gap (2023.3 ~ 2025.10). Cross-repo maintenance creates friction:

  • Version coordination overhead: Modifying P2P behavior requires publishing libp2p first, then updating java-tron's dependency version
  • Debugging difficulty: IDE cannot jump into external artifact source; requires downloading source JARs
  • CI dependency: Build depends on external Maven/JitPack artifact availability
  • Industry alignment: geth (go-ethereum) maintains its p2p/ package within the monorepo; this follows the same pattern

Changes

Commit 1: chore(p2p): add libp2p v2.2.7 source as p2p module

  • Copy 90 main Java files (including embedded org.web3j crypto utils) + 2 proto files + 26 test files from tronprotocol/libp2p tag v2.2.7
  • Create p2p/build.gradle with protobuf plugin configuration
  • Add p2p/lombok.config to temporarily override root logger field name (log vs logger)
  • Move example code (StartApp) to src/test/java (not packaged in JAR, but available for reference)
  • Register module in settings.gradle, update gradle/verification-metadata.xml

Commit 2: style(p2p): replace log field with logger

  • Replace all @Slf4j-generated log references with logger (35 files) to match root lombok.config (lombok.log.fieldName=logger)
  • Remove temporary p2p/lombok.config override

Commit 3: build(common): switch from external libp2p to local p2p module

  • common/build.gradle: replace 17 lines (Maven dependency + 11 exclude rules) with single api project(':p2p')
  • Remove io.github.tronprotocol:libp2p:2.2.7 entry from gradle/verification-metadata.xml

Commit 4: style(p2p): fix checkstyle violations and enable checkstyle

  • Apply google-java-format for bulk formatting (indentation, imports, whitespace)
  • Manual fixes: lambda indentation, line length wrapping, empty catch blocks, star imports
  • Enable checkstyle plugin with proto-generated code excluded (exclude '**/protos/**')
  • ~1374 of 1454 violations were from proto-generated code; after exclusion only 80 remained in handwritten code

Commit 5: fix(p2p): address review findings

  • Replace BasicThreadFactory.builder() (requires commons-lang3 3.12+) with new BasicThreadFactory.Builder() (available since 3.0) to prevent implicit global version upgrade from 3.4 to 3.18.0
  • Remove unused grpc-core dependency; retain grpc-netty as Netty transport provider (p2p uses 30+ Netty imports for TCP/UDP but zero gRPC imports)
  • Remove gRPC protoc plugin from protobuf config (p2p protos define only messages, no services)
  • Add p2p/.gitignore to exclude proto-generated code for all developers

Commit 6: fix(p2p): replace Math with StrictMath and fix flaky network tests

  • Replace Math.min/max with StrictMath.min/max in 5 files to satisfy CI check-math rule (integer operations, results identical)
  • Rewrite NetUtilTest.testGetIP with mocked URLConnection — original test called 3 external IP services which failed in CI (libp2p's own CI never ran tests with -xtest)
  • New mock tests cover: valid IP response, connection failure, invalid IP, empty response
  • Remove testExternalIp (covered by mock tests), fix testGetLanIP to not depend on external connectivity

Key Design Decisions

Decision Choice Rationale
Module name p2p Matches package org.tron.p2p, geth p2p/ convention, project short-name style (common, crypto, etc.)
Proto files Stay in p2p module P2P protocol is separate from chain protocol in protocol/; avoids coupling
Example code Move to src/test/java Not packaged in jar, but accessible for external developers referencing the source
commons-lang3 3.4 (global consistent) Rewrite builder()new Builder() (3.0 API) instead of upgrading globally
grpc-netty retained Yes Provides Netty transitively; same as original libp2p; version auto-aligns with protocol module
grpc-core + grpc plugin removed Yes p2p protos have no service definitions, zero io.grpc imports in source
Checkstyle proto exclude **/protos/** Proto-generated code has ~1374 violations; consistent with protocol module approach
MathStrictMath All 5 call sites CI check-math rule; integer min/max results are identical
testGetIP mock rewrite URLConnection mock Original depended on external services; libp2p CI never ran tests

Scope

  • Does NOT change any P2P protocol behavior or message formats
  • Does NOT change consensus rules or transaction validation logic
  • Does NOT modify protobuf definitions
  • Does NOT require hard fork
  • All 38 files importing org.tron.p2p are unchanged (package name preserved)
  • Existing nodes upgrading need only git pull and rebuild

Known Issues (pre-existing in libp2p)

The following issues exist in the original libp2p v2.2.7 codebase and are NOT introduced by this PR. They will be addressed in follow-up PRs:

Critical:

  • Node ID generation uses weak PRNG (java.util.Random instead of SecureRandom) — eclipse attack vector
  • Handshake lacks cryptographic authentication (self-reported node ID, no signature verification)
  • Neighbor injection has no count limit per response

High:

  • TOCTOU race in ChannelManager.handMessage (double onConnect)
  • DNS sync uses predictable java.util.Random
  • Algorithm.compressPubKey drops leading zeros from public key X-coordinate

Medium (from Cubic AI review):

  • Node.equals compares byte[] via new String(id) instead of Arrays.equals
  • Channel.hashCode() NPE when inetSocketAddress is null
  • InterruptedException swallowed in DiscoverServer and PeerClient
  • Several minor NPE risks and unclosed resources

Test

  • p2p module: 63 tests (22 test classes), all pass
  • Full build: BUILD SUCCESSFUL (12m, ~2319 tests, 0 failures)
  • checkstyleMain + checkstyleTest: PASS for all modules including p2p
  • check-math: PASS
  • Node integration test: FullNode started with mainnet config, connected to peers, synced 14,000+ blocks successfully
  • Flaky net tests (PbftMsgHandler, TronStatsManager): confirmed pre-existing, pass on retry

Barbatos added 6 commits April 10, 2026 22:56
Copy source code from tronprotocol/libp2p v2.2.7 into a new p2p/
gradle submodule. This commit introduces the original source without
modifications, preserving the org.tron.p2p package structure.

- Add p2p/build.gradle with protobuf plugin and dependency declarations
- Add p2p/lombok.config to override root lombok field name (log vs logger)
- Move example code (StartApp) to src/test/java
- Proto generated code is not committed (built by protobuf plugin)
- Update gradle/verification-metadata.xml for new dependencies
Replace all @Slf4j-generated 'log' references with 'logger' to match
java-tron's lombok.config (lombok.log.fieldName=logger). Remove the
temporary p2p/lombok.config override added in the previous commit.

Checkstyle is not yet enabled for the p2p module due to ~1900
violations inherited from the original libp2p codebase. This will be
addressed in a follow-up PR.
Replace the Maven dependency on io.github.tronprotocol:libp2p:2.2.7
with a project dependency on the local :p2p module. This eliminates
11 exclude rules that were needed for the external artifact and
removes the libp2p entry from dependency verification metadata.

java-tron no longer depends on any external libp2p artifact.
Apply google-java-format for bulk formatting (indentation, imports,
whitespace), then fix remaining violations manually:
- Lambda indentation adjustments for checkstyle 8.7 compatibility
- Line length wrapping for long strings and license headers
- Empty catch blocks: add // expected comments
- Star import replacement, multiple variable declaration splits

Enable checkstyle plugin in p2p/build.gradle with proto-generated
code excluded. Both checkstyleMain and checkstyleTest now pass.
- Replace BasicThreadFactory.builder() with new BasicThreadFactory.Builder()
  to use commons-lang3 3.4-compatible API, preventing implicit global
  version upgrade from 3.4 to 3.18.0
- Remove unused grpc-core dependency (p2p protos define only messages,
  no gRPC services); retain grpc-netty as Netty transport provider
- Add p2p/.gitignore to exclude proto-generated code for all developers
- Update verification-metadata.xml for changed dependency tree
- Replace Math.min/max with StrictMath.min/max in NodeEntry and
  NodeTable to satisfy CI check-math rule (integer operations,
  results are identical)
- Rewrite NetUtilTest.testGetIP with mocked URLConnection instead of
  calling external IP services (checkip.amazonaws.com, ifconfig.me),
  covering: valid IP, connection failure, invalid IP, empty response
- Remove testExternalIp (covered by mock tests)
- Fix testGetLanIP to not depend on www.baidu.com connectivity
@kuny0707 kuny0707 requested review from xxo1shine and removed request for halibobo1205 April 10, 2026 14:57
Add comprehensive unit tests to meet CI coverage gate requirements
(changed files > 60%, overall delta < -0.1%).

New test files (27):
- connection/: Channel, ChannelManager, HandshakeService, KeepAlive,
  NodeDetect, ConnPoolService, UpgradeController, P2pProtobufDecoder,
  MessageHandler, PeerClient, P2pChannelInitializer, StatusMessage,
  P2pDisconnectMessage
- dns/: DnsManager, AliClient, AwsClient, PublishService
- discover/: FindNodeMessage, NeighborsMessage, Message, PacketDecoder,
  MessageHandler, DiscoverTask
- utils/web3j: Numeric, Strings, Hash, ECKeyPair
- stats: StatsManager
- exception: DnsException
- P2pService, P2pConfig

Exclude proto-generated code from jacoco coverage calculation
(consistent with checkstyle exclusion).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant