Skip to content

persistent storage#2075

Merged
alexcos20 merged 5 commits intomainfrom
feature/persistent_storage
Apr 17, 2026
Merged

persistent storage#2075
alexcos20 merged 5 commits intomainfrom
feature/persistent_storage

Conversation

@alexcos20
Copy link
Copy Markdown
Member

@alexcos20 alexcos20 commented Apr 16, 2026

Fixes # .

Changes proposed in this PR:

  • persistentStorage support in Ocean.js for both HTTP & P2P

@alexcos20 alexcos20 self-assigned this Apr 16, 2026
@alexcos20 alexcos20 marked this pull request as ready for review April 16, 2026 13:49
@alexcos20
Copy link
Copy Markdown
Member Author

alexcos20 commented Apr 16, 2026

depends on oceanprotocol/ocean-node#1329 and it's hardcoded in ci.yml, so before merging this, we need to wait for node and then clean up

@alexcos20
Copy link
Copy Markdown
Member Author

/run-security-scan

Copy link
Copy Markdown
Member Author

@alexcos20 alexcos20 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI automated code review (Gemini 3).

Overall risk: medium

Summary:
This pull request introduces comprehensive support for persistent storage features within ocean.js, allowing interaction with provider nodes for bucket and file management. It includes new type definitions, API methods in both HTTP and P2P providers, and integration tests to cover the core functionality. The implementation correctly handles authentication using signatures and nonces, and provides cross-environment streaming for file uploads.

Comments:
• [INFO][other] The NODE_VERSION: pr-1329 for Barge is very specific. While this might be a temporary or internal tag for CI, it's worth documenting or explaining its purpose if it's not self-evident. Does pr-1329 refer to a specific branch, commit, or Docker image version?
• [INFO][other] The Node.js version is bumped from 20 to 22. This is a significant update. Ensure all dependencies and underlying systems are compatible with Node.js 22. Given this is a client library, it mainly impacts the build/dev environment, but it's a good change for keeping up-to-date.
• [WARNING][style] The type PersistentStorageAccessList is defined as [key: string]: unknown. This is overly generic. In src/@types/Provider.ts, NodeStatus.persistentStorage.accessLists uses AccessList[]. If PersistentStorageAccessList is meant to represent an item within an access list (e.g., specific chain ID and addresses), it should be more strictly defined, potentially mirroring the AccessList interface or a more specific structure like interface PersistentStorageAccessListItem { chainId: string; addresses: string[] }. Currently, its definition as [key: string]: unknown makes it very permissive and could lead to runtime errors if the expected structure isn't unknown.
• [INFO][security] The nonce + 1 logic is crucial for preventing replay attacks for signed requests. This is correctly implemented in both HttpProvider and P2pProvider for persistent storage operations, which is good.
• [INFO][performance] The uploadPersistentStorageFile method includes robust cross-environment streaming logic (Node.js stream vs. Browser ReadableStream). This is well-implemented and handles various input chunk types, which is excellent for usability and compatibility.
• [INFO][other] Error handling typically involves throw new Error(await response.text()). While useful for debugging, in a production environment, exposing raw server error messages might be undesirable. Consider wrapping generic HTTP errors with more user-friendly messages, or only exposing the full server response for specific debugging scenarios, while logging full details internally.
• [INFO][security] Similar to HttpProvider, the nonce + 1 logic for P2P persistent storage operations ensures replay protection, which is a good security measure.
• [INFO][other] The list methods (getPersistentStorageBuckets, listPersistentStorageFiles) defensively return Array.isArray(result) ? result : []. This is a good practice to ensure type safety and prevent errors if the backend returns a non-array value for a list endpoint.
• [INFO][other] The test denies a non-owner not in bucket ACL is a critical security test case and is well-implemented, ensuring access control mechanisms are functioning as expected.

Comment thread src/services/providers/HttpProvider.ts
Comment thread src/@types/File.ts Outdated
Comment thread test/integration/Provider.test.ts Outdated
@alexcos20 alexcos20 requested a review from ndrpp April 17, 2026 10:44
@alexcos20 alexcos20 merged commit 2f1930f into main Apr 17, 2026
12 checks passed
@alexcos20 alexcos20 deleted the feature/persistent_storage branch April 17, 2026 11:23
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.

2 participants