Skip to content

Conversation

@alexcos20
Copy link
Member

@alexcos20 alexcos20 commented Jan 28, 2026

Refactor: Centralize Key and Blockchain Management in OceanNode

Summary

This PR refactors the OceanNode singleton to centralize key management and blockchain connection management. The refactoring introduces a new KeyManager component with a pluggable key provider architecture, and a BlockchainRegistry to manage blockchain instances per chain ID. This improves modularity, testability, and sets the foundation for future key management solutions (e.g., GCP KMS).

Architecture Changes

Before

  • Keys were scattered across the codebase, accessed directly from config.keys
  • Blockchain instances were created ad-hoc in 22+ locations
  • No centralized key or blockchain lifecycle management
  • Difficult to extend with new key sources (e.g., KMS)

After

OceanNode (singleton)
├── Libp2pNode (one)
├── KeyManager (one)
│   ├── IKeyProvider (pluggable)
│   │   └── RawPrivateKeyProvider (current implementation)
│   ├── peerId
│   ├── libp2pPrivateKey
│   ├── ethAddress
│   └── getEvmSigner(provider)
│
├── BlockchainRegistry (one)
│   ├── Blockchain(chainId=1)
│   │   └── signer = keyManager.getEvmSigner(provider)
│   ├── Blockchain(chainId=137)
│   ├── Blockchain(chainId=56)
│   └── getBlockchain(chainId) → delegates to BlockchainRegistry

Key Components

1. KeyManager (src/components/KeyManager/index.ts)

Centralizes all key management for OceanNode:

  • Provides access to peerId, libp2p keys, EVM address, and EVM signers
  • Uses a pluggable IKeyProvider interface to support multiple key sources
  • Currently implements RawPrivateKeyProvider for raw private keys
  • Future: Can easily add GcpKmsProvider or other key providers

Key Methods:

  • getPeerId(): Returns libp2p PeerId
  • getLibp2pPrivateKey(): Returns libp2p private key
  • getEthAddress(): Returns Ethereum address
  • getEvmSigner(provider): Returns cached EVM signer for a provider
  • encrypt(data, algorithm): Encrypts data using node keys
  • decrypt(data, algorithm): Decrypts data using node keys
  • encryptStream(stream, algorithm): Encrypts streams (AES/ECIES)
  • decryptStream(stream, algorithm): Decrypts streams (AES/ECIES)

2. IKeyProvider Interface (src/@types/KeyManager.ts)

Defines the contract for all key provider implementations:

  • getType(): Returns provider type ('raw' | 'gcp-kms')
  • getPeerId(), getLibp2pPrivateKey(), getPublicKey()
  • getEthAddress(), getEthWallet()
  • getRawPrivateKeyBytes(): For EVM signer creation
  • encrypt(), decrypt(): Data encryption/decryption
  • encryptStream(), decryptStream(): Stream encryption/decryption
  • cleanup?(): Optional cleanup method

3. RawPrivateKeyProvider (src/components/KeyManager/providers/RawPrivateKeyProvider.ts)

Implements IKeyProvider for raw private keys:

  • Initializes synchronously in constructor (no async initialize() needed)
  • Derives peerId, libp2p keys, and Ethereum address from raw private key
  • Supports AES and ECIES encryption/decryption for both data and streams
  • Stream encryption uses Node.js Transform streams for efficient processing

4. BlockchainRegistry (src/components/BlockchainRegistry/index.ts)

Manages Blockchain instances per chain ID:

  • Lazy initialization: Creates Blockchain instances on-demand
  • Centralized access: Single source of truth for blockchain connections
  • Caches instances: Reuses Blockchain instances for the same chain ID
  • Provides getBlockchain(chainId) method that returns Blockchain | null

5. Refactored Blockchain Class (src/utils/blockchain.ts)

Updated to work with the new architecture:

  • Constructor now accepts KeyManager and uses it to get signers
  • Removed dependency on OceanNodeConfig for signer creation
  • Maintains backward compatibility during migration phase
  • Signers are obtained via KeyManager.getEvmSigner(provider)

6. Updated OceanNode (src/OceanNode.ts)

Enhanced singleton with new components:

  • Constructor accepts KeyManager and BlockchainRegistry as parameters (dependency injection)
  • getInstance() method updated to require these components
  • New accessor methods:
    • getKeyManager(): Returns KeyManager instance
    • getBlockchainRegistry(): Returns BlockchainRegistry instance
    • getBlockchain(chainId): Convenience method delegating to BlockchainRegistry

Migration Changes

Handlers Updated

All handlers now use OceanNode.getBlockchain() instead of creating new Blockchain instances:

  • ddoHandler.ts: Uses oceanNode.getBlockchain(chainId) for blockchain access
  • downloadHandler.ts: Migrated to use centralized blockchain access
  • startCompute.ts: Updated multiple locations to use oceanNode.getBlockchain(chainId)
  • initialize.ts: Updated compute initialization to use centralized blockchain
  • collectFeesHandler.ts: Admin handler now uses oceanNode.getBlockchain(chainId)

Escrow Refactoring (src/components/core/utils/escrow.ts)

  • Constructor now requires BlockchainRegistry as a parameter
  • Introduced private getBlockchain(chainId) helper method
  • All methods (getPaymentAmountInWei, getNumberFromWei, getUserAvailableFunds, getLocks, getAuthorizations, createLock, claimLock, cancelExpiredLocks) now use the centralized helper
  • Removed all existingChain parameter patterns - cleaner API

Indexer Updates (src/components/Indexer/index.ts)

  • Constructor now requires BlockchainRegistry as a parameter
  • startThread() method retrieves Blockchain instances via blockchainRegistry.getBlockchain(chainID)
  • Removed fallback logic - relies on registry for blockchain access

P2P Updates (src/components/P2P/index.ts)

  • Constructor now accepts KeyManager as a parameter
  • Replaced direct access to config.keys.peerId, config.keys.publicKey, config.keys.privateKey with KeyManager methods:
    • keyManager.getPeerIdString()
    • keyManager.getLibp2pPublicKey()
    • keyManager.getLibp2pPrivateKey()

Main Initialization (src/index.ts)

  • Creates KeyManager instance: new KeyManager(config)
  • Creates BlockchainRegistry instance: new BlockchainRegistry(keyManager, config)
  • Passes both to OceanNode.getInstance()
  • Removed explicit await keyManager.initialize() call (initialization happens in constructor)

Configuration Changes

OceanNodeKeys Interface (src/@types/OceanNode.ts)

Enhanced to support key provider selection:

  • Added type?: 'raw' | 'gcp-kms': Specifies the key provider type
  • Added rawPrivateKey?: string: For raw private key configuration (future)
  • Added gcpKmsConfig?: For future GCP KMS integration

Note: The type field is currently inferred from the presence of privateKey in config, but the interface supports explicit type specification for future extensibility.

Stream Encryption/Decryption

Added support for streaming encryption/decryption in RawPrivateKeyProvider:

  • encryptStream(inputStream, algorithm): Encrypts a Readable stream
    • AES: Uses Node.js crypto.createCipheriv() for streaming encryption
    • ECIES: Collects all data first (ECIES doesn't support streaming), then encrypts
  • decryptStream(inputStream, algorithm): Decrypts a Readable stream
    • AES: Uses Node.js crypto.createDecipheriv() for streaming decryption
    • ECIES: Collects all data first, then decrypts

Benefits

  1. Centralized Key Management: All key access goes through KeyManager, making it easier to audit and secure
  2. Pluggable Architecture: Easy to add new key providers (GCP KMS, AWS KMS, etc.) without changing core logic
  3. Centralized Blockchain Management: Single source of truth for blockchain connections, reducing duplication
  4. Better Testability: Components can be easily mocked and tested in isolation
  5. Improved Maintainability: Clear separation of concerns and single responsibility principle
  6. Future-Proof: Architecture supports enterprise key management solutions

Testing

Updated test files to work with the new architecture:

  • src/test/integration/credentials.test.ts: Uses blockchainRegistry.getBlockchain()
  • src/test/integration/accessLists.test.ts: Updated to use new blockchain access pattern
  • src/test/unit/blockchain.test.ts: Tests BlockchainRegistry functionality

Files Changed

New Files

  • src/@types/KeyManager.ts: Type definitions for key provider architecture
  • src/components/KeyManager/index.ts: Main KeyManager class
  • src/components/KeyManager/providers/RawPrivateKeyProvider.ts: Raw private key provider implementation
  • src/components/BlockchainRegistry/index.ts: Blockchain registry implementation
  • docs/KeyManager.md: Documentation for KeyManager

Modified Files

  • src/OceanNode.ts: Added KeyManager and BlockchainRegistry integration
  • src/index.ts: Updated initialization flow
  • src/utils/blockchain.ts: Refactored to use KeyManager
  • src/@types/OceanNode.ts: Enhanced OceanNodeKeys interface
  • src/components/P2P/index.ts: Uses KeyManager for P2P keys
  • src/components/Indexer/index.ts: Uses BlockchainRegistry
  • src/components/core/utils/escrow.ts: Refactored to use BlockchainRegistry
  • src/components/core/handler/ddoHandler.ts: Uses OceanNode.getBlockchain()
  • src/components/core/handler/downloadHandler.ts: Uses OceanNode.getBlockchain()
  • src/components/core/compute/startCompute.ts: Uses OceanNode.getBlockchain()
  • src/components/core/compute/initialize.ts: Uses OceanNode.getBlockchain()
  • src/components/core/admin/collectFeesHandler.ts: Uses OceanNode.getBlockchain()
  • Multiple test files updated to work with new architecture

Related Issues

This refactoring addresses the need for:

  • Centralized key management
  • Support for enterprise key management solutions (KMS)
  • Reduced code duplication in blockchain instance creation
  • Better testability and maintainability

Later edit:

Rate limit refactor

Refactor rate limiting to pass caller as command parameter instead of using OceanNode state
This commit refactors rate limiting by:

  • Removing setRemoteCaller() and getRemoteCaller() from OceanNode
  • Adding caller?: string | string[] to the Command interface
  • Updating checkRateLimit() to accept caller as a parameter instead of reading from OceanNode
  • Updating HTTP routes and P2P handlers to set task.caller directly instead of calling setRemoteCaller()

This removes stateful caller tracking from OceanNode and makes the caller explicit in the command, improving clarity and testability.

@alexcos20
Copy link
Member Author

/run-security-scan

Copy link
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 a significant architectural refactoring focused on centralizing key management and blockchain interaction within OceanNode. It introduces two new core components: KeyManager and BlockchainRegistry. The KeyManager abstracts different key sources (e.g., raw private keys, future KMS integrations) and consolidates all cryptographic operations. The BlockchainRegistry provides a centralized, cached mechanism for obtaining Blockchain instances for specific chain IDs. The Blockchain class itself has been updated to use ethers.FallbackProvider for more robust RPC handling and now integrates with the KeyManager. The changes also include a cleaner approach to passing caller information (IP/PeerID) through command objects instead of a mutable remoteCaller property on the OceanNode singleton. This refactoring greatly improves the modularity, extensibility, and security posture of the node, while simplifying the Blockchain class by leveraging ethers.FallbackProvider's built-in retry logic.

Comments:
• [WARNING][bug] The PRIVATE_KEY environment variable is marked as required: false here, but src/utils/config/builder.ts (line 224 in current diff) explicitly throws an error if privateKey is missing when buildMergedConfig is called. If RawPrivateKeyProvider is the default and only currently working key provider, then PRIVATE_KEY is effectively required. This inconsistency should be resolved. Either required should be true or the buildMergedConfig logic needs to be adapted to allow alternative key sources to omit PRIVATE_KEY.
• [WARNING][style] The getLibp2pPrivateKey(): any return type is very broad. If a specific type from @libp2p/interface/keys is available for PrivateKey, it would be better to use that for stronger type safety.
• [WARNING][bug] The KeyProviderType enum is defined as 'raw' | 'gcp-kms'. However, docs/KeyManager.md mentions AwsKmsProvider as a future provider, implying 'aws' should also be part of this type. Please update the type definition to include all planned providers for completeness.
• [INFO][performance] The getEvmSigner uses chainId as the cache key. While FallbackProvider abstracts multiple RPCs into a single provider, if in the future the system needs to support multiple distinct FallbackProvider instances for the same chain ID (e.g., for different purposes or configurations), this cache key might lead to conflicts. The 'TO DO' comment indicates this is known. For now, it's likely fine, but keep this in mind for future enhancements if such a scenario arises.
• [INFO][performance] The encryptStream method for ECIES collects the entire stream into a buffer before encrypting and then pushing it as a single chunk. This means it's not truly streaming for ECIES and could consume significant memory for very large files. While ECIES often isn't stream-friendly by design, it's an important performance/memory characteristic to note, especially if large files are expected to be encrypted via ECIES. Consider adding a comment in the code to highlight this behavior.
• [STYLE][style] Please remove or replace the console.log('detectNetwork block', block) with a CORE_LOGGER.debug statement. console.log should generally be avoided in production code.
• [INFO][other] The OceanNode constructor has keyManager?: KeyManager, blockchainRegistry?: BlockchainRegistry as optional. However, if they are undefined, they are initialized using new KeyManager(config) and new BlockchainRegistry(this.keyManager, config). This is a common pattern, but it means that if external code relies on injecting specific KeyManager or BlockchainRegistry instances (e.g., for advanced testing or custom implementations), they must explicitly pass them. Otherwise, default instances are created. This is generally acceptable, but it's important to be aware of the default behavior.
• [INFO][style] The checkRateLimit method now correctly takes caller as a parameter, and the remoteCaller property on OceanNode has been removed. This refactoring is a positive change, as it promotes better separation of concerns by passing request-specific context directly through arguments rather than relying on mutable state on a global singleton.

@alexcos20 alexcos20 marked this pull request as ready for review January 29, 2026 10:27
@alexcos20
Copy link
Member Author

Will handle the ```[WARNING][bug]`` in separate issue

@alexcos20 alexcos20 linked an issue Jan 29, 2026 that may be closed by this pull request
1 task
@alexcos20 alexcos20 self-assigned this Jan 29, 2026
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.

Refactor Blockchain class use singleton pattern

2 participants