p2p impl

[giurgiur99]

Fixes #2053

Changes proposed in this PR:

• Split Provider.ts into BaseProvider (routing), HttpProvider, P2pProvider — auto-routes by URI format
• Run CI tests with HTTP & P2P
• Full P2P transport over libp2p: KadDHT, bootstrap, mDNS, TLS, retry on dropped connections
• enableTcp flag in P2PConfig for Node.js (off by default for browsers)
• getDownloadUrl() streams binary via lpStream → DownloadResponse
• getComputeResult() new streaming method → AsyncGenerator with resumable offset
• computeStatus() takes signerOrAuthToken consistently across all layers
• computeStart payment body includes resources in both transports
• Aquarius.resolve/waitForIndexer/validate route through P2P when URL is a multiaddr
• NFT.setMetadata stores bare peer ID on-chain (strips multiaddr prefix)

[giurgiur99]

/run-security-scan

[alexcos20]

AI automated code review (Gemini 3).

Overall risk: medium

Summary:
This pull request introduces P2P communication capabilities to the ocean.js library by refactoring the Provider service. The original Provider logic is now encapsulated within an HttpProvider, and a new P2pProvider handles libp2p based communication. A BaseProvider acts as a facade, routing requests to the appropriate underlying provider based on the nodeUri. The CI pipeline has been updated to test both HTTP and P2P integration paths. Key changes include adding libp2p dependencies, updating Node.js versions in CI, and modifying NFT.ts and Aquarius.ts to leverage the new P2P routing for metadata decryption and DDO operations. While the architectural change is positive, there are some areas for careful review, particularly around P2P stream handling and return type consistency.

Comments:
• [INFO][other] Node.js version updated from 20 to 22 across multiple jobs. While Node.js 22 is generally backward-compatible, it's good practice to monitor for any unexpected regressions in the CI or runtime with this new version.
• [WARNING][bug] The regex /p2p/([^/]+)/ is used to extract the peer ID from metadataDecryptorUrl. This regex assumes a very specific multiaddr format. If multiaddrs can have different structures (e.g., /p2p/Qm.../tcp/4001), this regex might not be robust enough. Consider using a dedicated multiaddr parsing library function for more reliable extraction, or refine the regex to account for more variations.
• [INFO][style] The refactoring of the Provider class into BaseProvider, HttpProvider, and P2pProvider using a facade pattern is a good architectural improvement. This will make the codebase more modular and extensible for future transport mechanisms.
• [WARNING][other] The isP2pUri function relies on startsWith('/'), startsWith('12D3'), startsWith('16Uiu'), or startsWith('Qm'). While pragmatic for common peer IDs/multiaddrs, this could be brittle if new peer ID prefixes emerge or if multiaddr formats vary in unexpected ways. A more explicit check for /p2p/ within the string might be more robust, potentially leveraging multiaddr library's parsing capabilities.
• [INFO][other] The bufToHex utility function handles various buffer representations (JSON string, object, indexed object). Given its use for fields like hash, publicKey, r, s in validateDdo, which are often hex strings, ensure the Buffer.from(data).toString() conversion correctly yields hex (e.g., if data bytes are actual ASCII hex characters, not raw bytes of the hex value). If the data is truly raw bytes that represent a hex string, toString('hex') might be more appropriate. Clarification on the exact input/output intent would be beneficial.
• [WARNING][other] The libp2pNode and lastBootstrapKey are global variables. This means there will only be one libp2p instance for the entire application, which could lead to issues in scenarios requiring multiple distinct P2P connections or different configurations, or in a testing environment where state needs to be reset between tests. Consider managing the libp2pNode lifecycle per P2pProvider instance or within a more controlled singleton pattern if global state is strictly intended.
• [ERROR][bug] In dialAndStream, the stream is closed immediately after writing the payload (await stream.close()). For a typical request/response pattern over lpStream, the client would write the request, then read the response from the same stream, and only then close it. Closing the stream immediately might prevent the client from receiving the full response or cause errors on the server side if it's still trying to write to a closed stream. This pattern needs to be carefully verified with the expected libp2p stream usage and the lpStream library's behavior. If lpStream detaches read/write, it might be fine, but it seems suspicious.
• [ERROR][bug] The getComputeResultUrl method in P2pProvider returns Promise<any> which resolves to an AsyncGenerator<Uint8Array>, while the HttpProvider counterpart returns Promise<string> (a URL). The BaseProvider facade's signature for this method is Promise<string>, creating a type mismatch at compile time for P2pProvider and a potential runtime error for callers expecting a string (URL) when interacting with a P2P nodeUri. The return type of BaseProvider.getComputeResultUrl needs to be updated to reflect both possibilities (e.g., Promise<string | AsyncGenerator<Uint8Array>>), and all call sites in the library need to be updated to handle the AsyncGenerator type when a P2P URI is used.
• [INFO][style] Removed console.log statements for nonce, which is good cleanup for production code.

[giurgiur99]

/run-security-scan

[alexcos20]

AI automated code review (Gemini 3).

Overall risk: medium

Summary:
This pull request introduces P2P communication support to ocean.js, refactoring the core Provider logic into a BaseProvider, HttpProvider, and P2pProvider. It updates the CI to Node.js 22, adds necessary libp2p dependencies, and modifies Aquarius and NFT services to leverage the new P2P capabilities. Integration tests are updated to cover both HTTP and P2P transports. The architectural changes are a significant improvement for supporting multiple communication protocols.

Comments:
• [INFO][other] The CI workflow is updated to Node.js 22 across multiple jobs. This is a good move to stay current, but ensure all project dependencies have been thoroughly tested for full compatibility with Node.js 22.
• [WARNING][other] The test_integration job for the p2p transport matrix entry explicitly runs npm run test:integration instead of npm run test:integration:cover. Additionally, the 'Upload coverage' step is conditional (if: matrix.transport == 'http'). This means P2P integration tests are not contributing to code coverage. Please clarify if this is a temporary measure or a known limitation, and if there are plans to include P2P test coverage in the future to ensure comprehensive testing.
• [INFO][style] The change from import ... assert { type: 'json' } to import ... with { type: 'json' } is correct for Node.js 22's adoption of 'import attributes' over 'import assertions'. This aligns with the Node.js version update in CI.
• [INFO][other] The logic to extract the peer ID from metadataDecryptorUrl when it's a P2P multiaddr is a necessary and well-commented change to ensure correct routing for P2P decrypt operations within the indexer. Good job anticipating the indexer's requirements.
• [INFO][architecture] The refactoring of the large Provider class into BaseProvider, HttpProvider, and P2pProvider is an excellent architectural improvement. It significantly enhances modularity, maintainability, and extensibility, making it easier to manage different communication protocols. This is a strong positive for the codebase.
• [INFO][other] The Aquarius service now correctly routes DDO resolution and validation requests to the appropriate provider (HTTP or P2P) using isP2pUri and ProviderInstance. This is a critical integration point for the new P2P functionality.
• [WARNING][other] The isP2pUri function's check for multiaddrs starting with Qm might be overly broad. While Qm often indicates a CID (which can be part of a P2P multiaddr), it's not exclusively for multiaddrs and could potentially lead to false positives. Consider if a more precise check (e.g., validating the full multiaddr structure or looking for /p2p/ explicitly) is feasible and desirable.
• [WARNING][other] The getComputeResultUrl method in P2pProvider returns any (an async generator of Uint8Array chunks), which differs significantly from HttpProvider's Promise<string> (a URL). While the BaseProvider's getDownloadUrl properly handles string | DownloadResponse, this specific method in BaseProvider still returns Promise<string>. This inconsistency in return types for getComputeResultUrl between HTTP and P2P paths could be a source of confusion or bugs for consumers. This method in BaseProvider should probably return Promise<string | AsyncGenerator<Uint8Array>> and be handled accordingly in calling code.
• [INFO][performance] The getOrCreateLibp2pNode function correctly reuses the libp2pNode instance based on the bootstrap key. This is good for performance and resource management, preventing unnecessary node creation and destruction.
• [INFO][other] The sendP2pCommand includes retry logic with MAX_RETRIES and RETRY_DELAY_MS for connection closed/reset errors. This is a good practice for improving the resilience of P2P communication against transient network issues.
• [INFO][other] In P2pProvider.getEndpoints, the STATUS command is used. It's important to note that this returns a different structure (address, providerAddress, providerAddresses) than the HTTP provider's getEndpoints which returns serviceEndpoints. This is handled by BaseProvider but could still be a nuance for callers inspecting the raw response.
• [INFO][other] The downloadFile function is updated to gracefully handle both HTTP URLs (strings) and pre-collected DownloadResponse objects (from P2P). This demonstrates good adaptability for the new P2P download flow, providing a unified interface for consumers.
• [INFO][style] Changing delay(delayTimeout) to an it('should delay', ...) block with setTimeout is a good practice for asynchronous testing in Mocha. It ensures that the delay is properly awaited and test timeouts are respected, making the test suite more reliable.

[giurgiur99]

/run-security-scan

[alexcos20]

AI automated code review (Gemini 3).

Overall risk: medium

Summary:
This pull request introduces a significant architectural change by integrating P2P transport capabilities into the ocean.js library. The Provider class has been refactored into a BaseProvider facade that dynamically selects between HttpProvider and a new P2pProvider based on the provided node URI. This enables DDO resolution, file info retrieval, and compute operations over P2P.

Key changes include:

• P2P Integration: Full libp2p implementation for a new transport layer.
• Refactoring: Provider.ts is now a lightweight dispatcher to HttpProvider.ts (existing logic) and P2pProvider.ts (new logic).
• CI/CD Updates: Node.js version bumped to 22, and integration tests are matrixed to cover both HTTP and P2P scenarios.
• Dependency Management: Extensive additions of libp2p related packages.
• Syntax Modernization: import assertions updated from assert to with.

Overall, this is a substantial enhancement adding a critical new feature, but it also increases complexity and introduces a new set of dependencies and potential failure modes.

Comments:
• [WARNING][other] Updating Node.js to version 22 is a significant jump. While keeping up-to-date is good, Node.js 20 is the current LTS. Moving to an even newer non-LTS (or recently-LTS'd) version might introduce compatibility issues with existing dependencies or build tools that haven't fully adapted yet. Consider if Node.js 20 LTS would be a more stable choice for CI, or if Node.js 22 is strictly necessary for specific features.
• [WARNING][bug] The coverage upload is conditional on matrix.transport == 'http'. This means code paths specific to the new P2P implementation will not have their coverage reported. It's crucial to ensure P2P code paths are also adequately covered by integration tests and included in coverage reports to prevent regressions and ensure code quality for this new, complex feature.
• [INFO][other] Good catch to extract the bare peer ID for P2P decrypt routing. This demonstrates understanding of the different formats expected by HTTP vs. P2P endpoints.
• [WARNING][other] The Aquarius class now acts as a dispatcher for DDO resolution, delegating to ProviderInstance if aquariusURL is a P2P URI. This means aquariusURL can now effectively be a 'nodeUrl' capable of P2P. This is a significant conceptual shift for Aquarius's role. It would be beneficial to add comments explaining this, and ensure ProviderInstance's P2P setup (setupP2P) is always called before Aquarius is used with a P2P URI.
• [INFO][style] This is a massive refactor of Provider.ts, moving all the implementation details into HttpProvider.ts and P2pProvider.ts. This is a very clean and maintainable approach for supporting multiple transport protocols. Great job on the abstraction!
• [INFO][performance] connectionManager: { maxConnections: 100 } - This is a reasonable default. Depending on usage patterns and network characteristics, this value might need tuning in production for optimal performance and resource usage.
• [WARNING][other] Silently catching and ignoring errors during node.dial for newly discovered peers (.catch(() => {})) could mask important connection issues during P2P discovery. While it's in the event listener for 'peer:discovery', it might be better to log these errors at a debug or info level at least, to aid in troubleshooting P2P connectivity problems.
• [WARNING][bug] The bufToHex function's logic if (typeof val === 'string') { try { val = JSON.parse(val) } catch { return val } } followed by const bytes = Array.isArray(val?.data) ? val.data : typeof val?.[0] === 'number' ? Object.values(val) : null seems overly complex and potentially brittle. If val is expected to be a JSON string representation of a Buffer, it should probably just Buffer.from(JSON.parse(val).data).toString('hex'). If it's expected to be a Buffer or Uint8Array, then Buffer.from(val).toString('hex') is sufficient. This current implementation tries to handle multiple inconsistent input types, which might lead to unexpected behavior. It also uses Object.values(val) which is not guaranteed to maintain order for integer keys in older JS engines, although modern ones do. Let's simplify or clarify the expected input val.
• [INFO][performance] The retry mechanism for P2P commands is a good addition for network resilience. DEFAULT_MAX_RETRIES and DEFAULT_RETRY_DELAY_MS are set, but it would be good to make these configurable via P2PConfig to allow consumers to tune their P2P interaction strategy.
• [INFO][other] The getDownloadUrl in P2pProvider now returns a DownloadResponse (containing an ArrayBuffer), which is directly consumable by FetchHelper.downloadFile. This is a good design, ensuring consistency in the download interface across HTTP and P2P, despite the underlying mechanism differences.
• [INFO][other] Removing computeRoutePath and hasFreeComputeSupport from examples is good, as the ProviderInstance now abstracts away the transport choice. This simplifies client-side code, which is a major benefit of the refactoring.
• [INFO][other] The removal of the freeComputeRouteSupport flag and the conditional logic simplifies the ComputeFlow test significantly, showcasing the benefit of the new BaseProvider abstraction. This implies the ProviderInstance is now expected to handle the route determination internally.
• [WARNING][bug] The hardcoded IP address 172.15.0.5 for local P2P bootstrap in the test setup (_P2PWarmup.test.ts) is specific to a Docker Compose network configuration. This makes the test less portable and prone to failure if the local environment deviates from this setup. It should ideally be retrieved dynamically (e.g., from an environment variable or Docker network inspection) or made configurable in P2PConfig for testing purposes.