feat(network): migrate to libp2p v3#8856
feat(network): migrate to libp2p v3#8856lodestradamus wants to merge 19 commits intoChainSafe:unstablefrom
Conversation
Summary of ChangesHello @lodestradamus, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request undertakes a comprehensive upgrade of the network layer to libp2p v3, aiming to modernize the underlying peer-to-peer communication infrastructure. The changes introduce a cleaner, more robust architecture for GossipSub and ReqResp protocols, improving maintainability and aligning with the latest libp2p standards. This migration streamlines data handling and protocol interactions, enhancing the overall stability and performance of the network components. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request upgrades libp2p dependencies to version 3, leading to a significant refactoring of network communication logic. Key changes include updating Eth2Gossipsub to wrap the new GossipSub instance and migrating ReqResp stream handling from it-pipe to libp2p v3's byteStream API for imperative read/write operations, which also involved removing the TTFB_TIMEOUT as per spec relaxation. Review comments highlight that several as any type assertions and Multiaddr type casts were introduced as temporary workarounds due to dependency version mismatches, with a recommendation to address these in future follow-up tasks to ensure full type safety.
| // Type assertion needed due to multiaddr version differences between libp2p and lodestar | ||
| const status = this.handleDiscoveredPeer(id, multiaddrs[0] as Multiaddr, attnets, syncnets, undefined); |
There was a problem hiding this comment.
This type assertion to Multiaddr seems to be another case of dependency version mismatch. As with the other type assertions in this PR, it would be beneficial to track the removal of this cast in a follow-up issue to restore full type safety.
There was a problem hiding this comment.
Agreed. The Multiaddr type mismatch comes from @multiformats/multiaddr version differences between libp2p's internal types and what lodestar imports directly.
This will resolve itself when we align @multiformats/multiaddr versions across the dependency tree. I'll add a TODO comment referencing the cleanup.
packages/reqresp/src/ReqResp.ts
Outdated
| // libp2p v3: protocol handlers now have signature (stream, connection) => Promise<void> | ||
| // Note: type assertion needed due to libp2p interface version mismatch between types and runtime | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| return this.libp2p.handle(protocolID, this.getRequestHandler(protocol, protocolID) as any, {force: true}); |
There was a problem hiding this comment.
The as any cast for the request handler hides potential type errors. This seems related to the broader dependency version issues. Let's ensure this is covered by a follow-up ticket to be addressed once types are compatible.
There was a problem hiding this comment.
Yes, this is due to the libp2p v3 handler signature change from ({ stream, connection }) to (stream, connection). The types in @libp2p/interface@3.x haven't fully propagated through all transitive dependencies yet.
Once the libp2p ecosystem stabilizes on v3 types, this cast can be removed. The runtime behavior is correct — it's purely a type-level mismatch.
There was a problem hiding this comment.
this seems wrong? can you investigate exactly which types and which parts of the types are mismatching to cause this?
| // Use byteStream for imperative read/write with libp2p v3 | ||
| // Note: type assertion needed due to libp2p interface version mismatch | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| const bytes = byteStream(stream as any); |
There was a problem hiding this comment.
Casting stream as any here is another instance where we're bypassing the type system due to dependency version conflicts. It's good that there's a comment explaining it, but we should aim to remove this. Please make sure this is part of a follow-up technical debt ticket.
There was a problem hiding this comment.
The byteStream() function from @libp2p/utils expects its own internal Stream type, which differs slightly from @libp2p/interface's Stream. Both are structurally compatible at runtime.
This is a common pattern in libp2p v3 migrations — the types will align once the ecosystem converges. I've documented the cast with comments explaining the reason.
| // Use byteStream for imperative read/write with libp2p v3 | ||
| // Note: type assertion needed due to libp2p interface version mismatch | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| const bytes = byteStream(stream as any); |
There was a problem hiding this comment.
Similar to other files, the stream as any cast here is a workaround for type incompatibilities. It's important to track these and remove them in the future to maintain type safety.
There was a problem hiding this comment.
Same situation as the request side — byteStream() type expects a slightly different Stream interface than what @libp2p/interface exports.
To track all these type assertion cleanups, I'll create a follow-up issue once this PR lands. The casts are safe (runtime compatible) but we should remove them when libp2p v3 types stabilize across packages.
lodestradamus
force-pushed
the
feat/libp2p-v3-migration
branch
from
961421d to
cfb974f
Compare
|
Re: @wemeetagain's question about the Good point, let me dig into this properly. The mismatch is between what
type StreamHandler = (stream: Stream, connection: Connection) => void | Promise<void>But the The problem is Root cause: libp2p v3 ships with its own bundled interface types that don't match the separately installed Options:
Happy to investigate option 1 further if you prefer. The runtime behavior is correct since the types are structurally identical. |
|
@lodestradamus the problem should go away when our installed version of libp2p/interface matches the version installed by libp2p |
|
Makes sense — will clean up the cast once the versions align. |
lodestradamus
force-pushed
the
feat/libp2p-v3-migration
branch
from
35a6486 to
cc82afa
Compare
- Updated all libp2p dependencies to v3-compatible versions:
- libp2p: 2.9.0 → 3.1.3
- @libp2p/interface: ^2.7.0 → ^3.1.0
- @libp2p/utils: ^7.0.10 (new)
- @chainsafe/libp2p-gossipsub → @libp2p/gossipsub: ^15.0.12
- @chainsafe/libp2p-noise: ^16.1.5 → ^17.0.0
- All other @libp2p/* packages updated to v3-compatible versions
- Rewrote reqresp package for libp2p v3 API:
- Use byteStream() from @libp2p/utils for imperative read/write
- Protocol handlers changed from ({ stream, connection }) to async (stream, connection)
- Removed it-pipe, abortableSource, bufferedSource patterns
- Removed TTFB timeout tracking (spec relaxed) - single timeout via AbortSignal
- Changed encoders from async generators to synchronous Uint8ArrayList returns
- Updated snappy compression to return Uint8ArrayList synchronously
- Files removed (no longer needed):
- utils/abortableSource.ts
- utils/bufferedSource.ts
- utils/collectExactOne.ts
- utils/collectMaxResponse.ts
- utils/onChunk.ts
- utils/snappy.ts
- Updated tests to use new API:
- Updated MockLibP2pStream for v3 compatibility
- Updated encoder tests to use byteStream API
- Simplified timeout tests (no TTFB tracking)
Wire format unchanged (spec compliant):
request ::= <varint-length> | <snappy-frames(ssz-payload)>
response ::= <response_chunk>*
response_chunk ::= <result> | <context-bytes>? | <varint-length> | <snappy-frames(ssz-payload)>
- sendData.test.ts: Use SecuredConnection.connection instead of .conn Cast to DuplexStream for pipe-based benchmark compatibility Convert sync generator to async generator for type compatibility - attnetsService.test.ts: Cast gossipStub to any since mesh property is added dynamically via Object.defineProperty
- Update decode.ts with new bytes reading API - Update response handling for v3 stream - Update utils for v3 compatibility
- Update MockStream for v3 compatibility - Fix encode/decode test helpers - Update request test mocks
- Update gossipsub subscription handlers for v3 events - Update encoding for v3 message types - Update scoring parameters for v3 API - Update interface types for v3 compatibility
- Update networkCore for v3 peer events - Update networkCoreWorkerHandler for v3 types - Update network events for v3 compatibility - Update libp2p initialization for v3
- Update peer discovery for v3 events - Update prioritizePeers for v3 types
- Update gossipValidatorFn for v3 types - Update processor types for v3 - Update node utils for v3 peer info - Update workerEvents for v3 types
- Remove unused 'it-all' import in reqrespEncode test
lodestradamus
force-pushed
the
feat/libp2p-v3-migration
branch
from
cc82afa to
24b48d0
Compare
- Remove it-all import from blockArchive.ts, use inline for-await-of loop - Add it-all ^3.0.4 as devDependency for test files that still use it Fixes CI build failure: Cannot find module 'it-all'
Fixed issues: - Buffer vs Uint8Array mismatches in test expectations - Timeout error wrapping - convert native TimeoutError to RequestError with RESP_TIMEOUT code - Multi-chunk response edge cases - fixed frame-by-frame snappy decoding to avoid consuming bytes from next chunk - Error message decoding - properly handle both snappy-encoded and raw error messages - UnexpectedEOFError handling - wrap libp2p errors in SszSnappyError(SOURCE_ABORTED) - Updated libp2p v3 stream.close() usage (replaces closeWrite()) - Fixed snappy compression Buffer type compatibility
- Add nodeInfo configuration to createLibp2p for proper agent identification (replaces deprecated agentVersion option in identify service) - Fix peerManager test mock to include required Connection methods for libp2p v3 (close, abort, timeline, remoteAddr) This fixes compatibility issues with the libp2p v3 migration where: 1. The identify protocol now reads agent info from nodeInfo instead of agentVersion 2. Connection objects require additional methods that weren't in the test mocks
This enables automatic identify protocol execution on new connections, which should ensure topology notifications are properly triggered and allow the gossipsub mesh to form correctly after the libp2p v3 migration. Fixes peer discovery issue where nodes were only getting 1 connection instead of expected 4.
lodestradamus
force-pushed
the
feat/libp2p-v3-migration
branch
from
8cc51ed to
a851a59
Compare
- Update reqrespEncode.test.ts to use byteStream() wrapper - Fix reqresp package stream handling for v3 - Fix era package snappy compression Fixes e2e test hanging due to old v2 stream API
lodestradamus
force-pushed
the
feat/libp2p-v3-migration
branch
from
1966e57 to
bfa4a6a
Compare
Adds the 'direct' Set property to track direct peers at runtime. The property was being used by addDirectPeer/removeDirectPeer/getDirectPeers methods but was not declared on the class. Fixes TypeScript error: Property 'direct' does not exist on type 'Eth2Gossipsub' Lines affected: 513, 523, 534
- Remove unused biome suppression comment from light-client browser test - Fix lint formatting in reqresp request/index.ts - Fix import organization in reqresp utils/index.ts - Add missing parseProtocolID export - Fix TimeoutError handling for AbortSignal.timeout in reqresp
2 participants
Summary
Complete migration to libp2p v3 with clean architecture.
Changes
libp2p Core (2.9.0 → 3.1.3)
({ stream, connection })→(stream, connection)byteStream()from@libp2p/utilsfor imperative read/writenodeInfoconfiguration (moved from identify options)GossipSub (@ChainSafe → @libp2p/gossipsub)
@libp2p/gossipsubpackagegossipsub(opts)(components)GossipSubInternalinterface that accessed undocumented internalsReqResp Rewrite
it-pipewithbyteStream()read/writePackage Updates
libp2p@libp2p/interface@libp2p/gossipsub@libp2p/utils@chainsafe/libp2p-gossipsubit-pipeTesting
pnpm buildpassespnpm lintpassesContext
Continues work from #8805. Key improvement: GossipSub wrapper now uses only public APIs (
getTopics(),getMeshPeers(),getScore()) instead of accessing internal properties.