Skip to content

MCP Fixes & Improvements#10

Merged
giurgiur99 merged 17 commits into
mainfrom
feature-c2d-authoring-and-key-disclaimer
Jun 2, 2026
Merged

MCP Fixes & Improvements#10
giurgiur99 merged 17 commits into
mainfrom
feature-c2d-authoring-and-key-disclaimer

Conversation

@giurgiur99

@giurgiur99 giurgiur99 commented May 27, 2026

Copy link
Copy Markdown
Contributor

Fixes # .

Changes proposed in this PR:

  • C2D rawcode authoring: new ocean://docs/c2d-algorithm-authoring resource + reworked workflow for inline meta.rawcode against prebuilt
    oceanprotocol/c2d_examples images (no Docker build); relaxed validate_algo_structure to warnings
  • create_auth_token redesign: takes ephemeral or privateKey instead of completeSignature, returns consumerAddress, and never echoes private
    keys (+ unit tests)
  • Base migration: switched defaults/docs from Sepolia to Base (8453) and added COMPY alongside USDC as a fee token
  • Compute guidance: added payment-vs-escrow, polling, recommended-nodes, and fileObject shape guides; documented bare-fileObject datasets
  • New get_erc20_token_info tool to denominate raw token amounts before showing costs
    Paid-compute flow: computeStart now directs users without a token to the dashboard (deposit + authorize) rather than minting one, and clarifies
    maxJobDuration/validUntil are container run time, not the escrow lock
  • Persistent storage: documented consumer-scoped bucket ownership (reuse the same authToken) and the
    /data/persistentStorage/<bucketId>/<fileName> mount path
  • Auth reuse: guidance to mint the JWT once per session and reuse it, since buckets/jobs/results are owned by the consumer address
  • Startup clarity: the auto-generated PRIVATE_KEY warning now explains it's only the libp2p server identity, not the key that funds or authorizes
    jobs

@giurgiur99

Copy link
Copy Markdown
Contributor Author

/run-security-scan

@giurgiur99 giurgiur99 marked this pull request as ready for review May 27, 2026 06:09

@alexcos20 alexcos20 left a comment

Copy link
Copy Markdown
Member

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: low

Summary:
This PR introduces a crucial disclaimer mechanism when a disposable private key is used, leveraging MCP elicitation to surface warnings to the client. It also extends the resource catalog with comprehensive Markdown documentation for C2D algorithm authoring. The design is solid and improves both developer experience and security awareness.

Comments:
• [INFO][bug] It is a good practice to pass any potential arguments to the previous handler and await it in case it performs asynchronous operations. This prevents subtle race conditions and ensures full compatibility if the underlying MCP SDK evolves or if multiple handlers are chained.

-  server.server.oninitialized = () => {
-    previous?.()
+  server.server.oninitialized = async (...args: any[]) => {
+    if (previous) await previous(...args)

• [INFO][style] Good clear warning structure. Using MCP elicitation to present security warnings to users directly in the client context is an excellent utilization of the protocol features.

@greptile-apps

greptile-apps Bot commented May 27, 2026

Copy link
Copy Markdown

Greptile Summary

This PR expands the Ocean Protocol MCP server with C2D algorithm authoring guidance, a redesigned create_auth_token tool, migration of defaults from Sepolia to Base (chain 8453), a new get_erc20_token_info ERC-20 utility, and comprehensive compute guidance covering payment/escrow semantics, job polling, and persistent storage ownership.

  • create_auth_token redesign: replaces the completeSignature flow with ephemeral/privateKey inputs; the tool now handles nonce + signing internally, returns consumerAddress, scrubs user keys from all error paths, and ships full unit test coverage for the no-echo guarantee.
  • New get_erc20_token_info tool: reads decimals, symbol, and name from any ERC-20 contract and optionally formats a raw token amount — used by LLM agents to denominate payment.amount before presenting costs.
  • C2D rawcode authoring resource: ocean://docs/c2d-algorithm-authoring documents the prebuilt oceanprotocol/c2d_examples tag catalog, filesystem contract, and constraints; validate_algo_structure is relaxed from errors to warnings for the DIDS/input-path checks to accommodate URL-fileObject and persistent-storage algorithms.

Confidence Score: 5/5

Safe to merge; the key-scrubbing logic on the privateKey path is well-tested and correct, and the Base chain migration is mechanically straightforward.

The redesigned create_auth_token handler correctly scrubs user-supplied keys from all error paths and is backed by thorough unit tests. The new get_erc20_token_info tool wraps all contract calls in a try/catch. The two flagged issues are documentation contradictions in LLM-facing guidance strings that do not affect runtime behavior.

registerDocsTools.ts (checksum listed in write_c2d_algo step 5) and p2pProviderTools.ts (conflicting getPersistentStorageFileObject guidance in computeAssetList) contain minor documentation inconsistencies worth tidying before the guidance confuses an LLM agent.

Important Files Changed

Filename Overview
src/tools/p2pProviderTools.ts Redesigns create_auth_token to ephemeral/privateKey flow with key-scrubbing error handling; adds compute guidance strings to tool descriptions; reworks computeAssetList to document bare-fileObject datasets. Minor internal guidance contradiction on whether getPersistentStorageFileObject is required.
src/tools/evmContractTools.ts Adds get_erc20_token_info tool: reads decimals/symbol/name from any ERC-20 via read-only contract call; optionally formats a raw amount string using BigInt+formatUnits. All errors caught and returned as isError.
src/utils/c2dAlgorithmAuthoring.ts New markdown resource documenting the rawcode + prebuilt image C2D path; covers image tag catalog, algorithm object shape, filesystem contract, constraints, and free-compute auth.
src/tools/p2pSchemas.ts Adds STORAGE_OBJECT_SHAPE_GUIDE, OCEAN_RECOMMENDED_NODES, P2P_RECOMMENDED_NODES_GUIDE, EPHEMERAL_CONSUMER_KEY_DISCLAIMER, P2P_COMPUTE_PAYMENT_GUIDE, and P2P_COMPUTE_POLLING_GUIDE constants; guidance is detailed and internally consistent.
src/tools/registerDocsTools.ts Migrates Sepolia to Base (8453), adds COMPY alongside USDC as accepted fee tokens, updates write_c2d_algo workflow steps; relaxes validateAlgoCode to warnings; write_c2d_algo step 5 lists checksum in the algorithm object shape contradicting the omit-checksum guidance elsewhere.
src/test/unit/tools/createAuthToken.test.ts New unit tests cover ephemeral path, privateKey path, no-source rejection, dual-source rejection, invalid key rejection, and downstream mint failure — all asserting key non-exposure.
src/clients/nodeClient.ts Adds createAuthTokenWithSigner method that wraps generateAuthToken with a Signer; cleanly delegates nonce fetch and signing to ocean.js internally.
src/resources/resourceCatalog.ts Registers the new c2d-algorithm-authoring builtin resource URI and serves its markdown content correctly.
src/prompts/registerPrompts.ts Updates payment error diagnostics and env template to reference Base fee tokens (USDC + COMPY) and removes Sepolia from default RPCS config.

Sequence Diagram

sequenceDiagram
    participant U as LLM Agent
    participant T as create_auth_token
    participant W as ethers Wallet
    participant N as NodeClient

    alt ephemeral: true
        U->>T: "nodeId, ephemeral=true"
        T->>W: Wallet.createRandom()
        W-->>T: ephemeralWallet
        T->>N: createAuthTokenWithSigner(node, ephemeralWallet)
        N-->>T: JWT string
        T-->>U: token, consumerAddress, privateKey, disclaimer
    else privateKey provided
        U->>T: nodeId, privateKey
        T->>W: new Wallet(privateKey)
        W-->>T: wallet address only
        T->>N: createAuthTokenWithSigner(node, wallet)
        N-->>T: JWT string
        T-->>U: token, consumerAddress only
    end

    Note over U,N: Paid compute flow after auth
    U->>N: getComputeEnvironments
    N-->>U: envs with minJobDuration
    U->>N: initializeCompute
    N-->>U: payment amount and minLockSeconds
    U->>T: get_erc20_token_info with rawAmount
    T-->>U: formattedAmount and symbol
    U->>N: computeStart with authToken
    loop poll every 5-10s
        U->>N: computeStatus
        N-->>U: C2DStatusNumber
    end
    U->>N: getComputeResult
Loading

Reviews (12): Last reviewed commit: "clearer guidance for buckets" | Re-trigger Greptile

Comment thread src/utils/c2dAlgorithmAuthoring.ts
Comment thread src/index.ts Outdated
Comment thread src/tools/registerDocsTools.ts
Comment thread src/utils/c2dAlgorithmAuthoring.ts Outdated
@greptile-apps

greptile-apps Bot commented May 27, 2026

Copy link
Copy Markdown

Want your agent to iterate on Greptile's feedback? Try greploops.

@giurgiur99

Copy link
Copy Markdown
Contributor Author

/run-security-scan

@giurgiur99 giurgiur99 marked this pull request as draft May 28, 2026 07:27

@alexcos20 alexcos20 left a comment

Copy link
Copy Markdown
Member

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: low

Summary:
This PR introduces excellent improvements to C2D algorithm authoring documentation, adds a new get_erc20_token_info tool for proper token denomination, and refactors create_auth_token to handle wallet signing directly on the server side instead of delegating signature construction to the client. The security precautions taken to explicitly suppress and sanitize exceptions to prevent private key leakage are top-notch. LGTM!

Comments:
• [INFO][security] Excellent job meticulously avoiding the exposure of the user's private key in both ethers.js parsing errors and downstream node API errors. Masking the raw error here is a very strong security practice!
• [INFO][bug] Since rawAmount represents base token units and must be an integer, you could enforce this at the Zod schema level. This provides a clearer framework-level validation error if an LLM incorrectly attempts to pass a float (e.g., "1.5"), which would otherwise cause BigInt() to throw a generic error later.

         rawAmount: z
           .string()
+          .regex(/^\d+$/, 'Must be an integer string')
           .optional()
           .describe(

• [INFO][style] While validating the exclusivity of ephemeral and privateKey in the handler works perfectly, you can optionally enforce this using Zod's .refine() at the schema level to let the MCP server handle the rejection automatically.

-      }
+      }).refine(data => [data.ephemeral === true, !!data.privateKey].filter(Boolean).length === 1, {
+        message: 'Provide exactly one of: ephemeral or privateKey'
+      })

@giurgiur99

Copy link
Copy Markdown
Contributor Author

/run-security-scan

@alexcos20 alexcos20 left a comment

Copy link
Copy Markdown
Member

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: low

Summary:
This PR excellently updates the default configurations and workflows from Sepolia to Base, introduces C2D algorithm authoring guides, adds an ERC20 token info utility, and significantly improves prompt/schema documentation for LLM interactions. The refactoring of the auth token creation to securely handle private keys with strong error-masking is highly commendable. LGTM!

Comments:
• [INFO][security] Excellent job catching the Wallet instantiation and masking the error to prevent the private key from being accidentally echoed back in the exception message. This is a very strong security practice when passing credentials through LLMs/MCP interfaces.
• [INFO][other] The implementation of get_erc20_token_info correctly utilizes Promise.all for performance and provides a much-needed utility for formatting token units cleanly in the client.
• [INFO][style] Refining the algorithm validation checks to account for usesDataInputs and usesPersistentStorage, and demoting the DIDS check to a warning, significantly improves the developer experience for cases that do not strictly require it.

@giurgiur99 giurgiur99 marked this pull request as ready for review May 29, 2026 11:19
@giurgiur99 giurgiur99 changed the title c2d authoring MCP Fixes & Improvements May 29, 2026
@dnsi0

dnsi0 commented Jun 2, 2026

Copy link
Copy Markdown

LGTM

@giurgiur99 giurgiur99 merged commit b4419ee into main Jun 2, 2026
10 checks passed
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.

3 participants