Skip to content

Feat/add rpc discover #159

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from
Open

Feat/add rpc discover #159

wants to merge 1 commit into from

Conversation

shanejonas
Copy link
Contributor

@shanejonas shanejonas commented May 6, 2025

User description

This adds

This adds rpc.discover method for OpenRPC service discovery. This enables clients to retrieve the OpenRPC schema via JSON-RPC.

Notes:

  • It has to use this other way of programatically adding the method to jayson because it doesnt support rpc.

PR Type

Enhancement


Description

  • Add rpc.discover OpenRPC service discovery method

  • Integrate OpenRPC schema via @shardeum-foundation/api-specs

  • Update method whitelist to allow rpc.discover

  • Register rpc.discover in JSON-RPC server initialization


Changes walkthrough 📝

Relevant files
Enhancement
rpcDiscover.ts
Add rpcDiscover method for OpenRPC schema retrieval           

src/methods/rpcDiscover.ts

  • Introduce rpcDiscover method returning OpenRPC document
  • Import OpenRPC schema from @shardeum-foundation/api-specs
  • Export the new method for server use
  • +7/-0     
    methodWhitelist.ts
    Allow rpc.discover in method whitelist                                     

    src/middlewares/methodWhitelist.ts

  • Add rpc.discover to allowed JSON-RPC methods
  • Ensure requests to rpc.discover are permitted
  • +1/-1     
    server.ts
    Register rpc.discover in JSON-RPC server                                 

    src/server.ts

  • Import and register rpcDiscover in JSON-RPC server
  • Add custom handler for rpc.discover method
  • Integrate with existing server initialization
  • +8/-1     
    Dependencies
    package.json
    Add OpenRPC schema dependency to project                                 

    package.json

  • Add @shardeum-foundation/api-specs as a dependency
  • Maintain existing dependencies and formatting
  • +4/-3     
    Additional files
    openrpc.json [link]   

    Need help?
  • Type /help how to ... in the comments thread for any questions about PR-Agent usage.
  • Check out the documentation for more information.
  • Copy link

    github-actions bot commented May 6, 2025

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    🎫 Ticket compliance analysis ✅

    131 - Fully compliant

    Compliant requirements:

    • Add rpc.discover OpenRPC service discovery method.
    • Integrate OpenRPC schema via @shardeum-foundation/api-specs.
    • Update method whitelist to allow rpc.discover.
    • Register rpc.discover in JSON-RPC server initialization.
    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🏅 Score: 85
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Version Management Policy

    Updates to @shardeum-foundation package versions are included in this PR. Per project policy, these should be in a separate PR or commit to avoid merge conflicts between branch lines with different versions.

    "@shardeum-foundation/api-specs": "^1.0.1",
    "@shardeum-foundation/lib-archiver-discovery": "1.3.0-prerelease.1",
    "@shardeum-foundation/lib-crypto-utils": "4.3.0-prerelease.1",
    "@shardeum-foundation/lib-types": "1.4.0-prerelease.2",
    Missing Unit Tests

    There are no unit tests added for the new rpc.discover method. Ensure that this method is covered by appropriate tests, including correct schema return and error handling.

    import ShardeumOpenRPCDocument from "@shardeum-foundation/api-specs"
    
    const rpcDiscover = async (): Promise<unknown> => {
      return ShardeumOpenRPCDocument
    }
    
    export default rpcDiscover

    Comment on lines +54 to +60
    server._methods['rpc.discover'] = new jayson.Method((_args: unknown, done: jayson.JSONRPCCallbackTypePlain) => {
    rpcDiscover().then((res) => {
    done(null, res)
    }).catch((err) => {
    done(err, null)
    })
    });
    Copy link

    Choose a reason for hiding this comment

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

    Suggestion: The current implementation exposes the rpc.discover method without any authentication or rate limiting, which could allow abuse or information leakage. Consider adding appropriate access controls or rate limiting to this method to prevent misuse. [security, importance: 8]

    Suggested change
    server._methods['rpc.discover'] = new jayson.Method((_args: unknown, done: jayson.JSONRPCCallbackTypePlain) => {
    rpcDiscover().then((res) => {
    done(null, res)
    }).catch((err) => {
    done(err, null)
    })
    });
    // Apply rate limiting or authentication as appropriate before exposing rpc.discover
    server._methods['rpc.discover'] = new jayson.Method((_args: unknown, done: jayson.JSONRPCCallbackTypePlain) => {
    // Example: check authentication or apply rate limiting here
    rpcDiscover().then((res) => {
    done(null, res)
    }).catch((err) => {
    done(err, null)
    })
    });

    @shanejonas shanejonas force-pushed the feat/add-rpc-discover branch from 5f59866 to 57b03f6 Compare May 21, 2025 21:04
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    1 participant