Skip to content

Conversation

jiexi
Copy link
Contributor

@jiexi jiexi commented Oct 14, 2025

Explanation

  • Rename RPC_URLS_MAP to RpcUrlsMap
  • Favor api.readonlyRPCMap entries over built in infura endpoint entries
  • Rename RpcClient to RequestRouter
  • Add new RpcClient and move rpc node request logic there
  • Throw MissingRpcEndpointErr from RpcClient when no endpoint exists for target scope
  • Reroute requests to wallet when RpcClient is unable to serve the request

References

See: https://consensyssoftware.atlassian.net/browse/WAPI-780

Checklist

  • I've updated the test suite for new or updated code as appropriate
  • I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate
  • I've highlighted breaking changes using the "BREAKING" category above as appropriate

Note

Adds a RequestRouter and RpcClient to handle readonly RPC methods via configured RPC endpoints (favoring custom map over Infura), renames RPC_URLS_MAP to RpcUrlsMap, and updates tests accordingly.

  • Core routing:
    • Introduces RequestRouter to dispatch RPC calls: routes RPC_HANDLED_METHODS to RPC nodes, SDK_HANDLED_METHODS to SDK (placeholder), others to wallet; falls back to wallet if no RPC endpoint.
    • Adds RpcClient to perform JSON-RPC over HTTP to endpoints derived from api.readonlyRPCMap (preferred) and Infura via getInfuraRpcUrls.
    • MultichainSDK.invokeMethod now uses RequestRouter + RpcClient instead of RPCClient.
  • API/Types:
    • Renames RPC_URLS_MAP -> RpcUrlsMap and keys by CaipChainId; updates usages in constants, infura, and MultichainOptions.
    • Exposes RPC_HANDLED_METHODS and SDK_HANDLED_METHODS in api/constants.
  • Tests:
    • Replaces old rpc/client.test with new rpc/handlers/rpcClient.test and adds rpc/requestRouter.test; updates invoke.test to use RequestRouter.
  • Misc:
    • Minor code cleanups/formatting; small platform/utils adjustments.

Written by Cursor Bugbot for commit 9fdc2bb. This will update automatically on new commits. Configure here.

@jiexi jiexi requested a review from a team as a code owner October 14, 2025 20:43
@jiexi
Copy link
Contributor Author

jiexi commented Oct 14, 2025

I'm not seeing where eth_accounts and eth_chainId are cached. Both actually shouldn't be callable via wallet_invokeMethod in the first place though

@adonesky1
Copy link
Contributor

I'm not seeing where eth_accounts and eth_chainId are cached. Both actually shouldn't be callable via wallet_invokeMethod in the first place though

Good point. We will need to do the caching and expose "simulations" of these methods in our backwards compatibility layer

Comment on lines 234 to 249
t.it('should redirect to provider for methods in METHODS_TO_REDIRECT', async () => {
const redirectOptions: InvokeMethodOptions = {
scope: 'eip155:1' as Scope,
request: {
method: RPC_METHODS.ETH_SENDTRANSACTION,
params: { to: '0x123', value: '0x100' },
},
};
mockTransport.request.mockResolvedValue({ result: '0xhash' });
const result = await rpcClient.invokeMethod(redirectOptions);
t.expect(result).toBe('0xhash');
t.expect(mockTransport.request).toHaveBeenCalledWith({
method: 'wallet_invokeMethod',
params: redirectOptions,
});
t.expect(mockFetch).not.toHaveBeenCalled();
Copy link
Contributor

Choose a reason for hiding this comment

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

should we preserve this test but make it
should redirect to provider for EVM methods not in EVM_RPC_PASSTHROUGH_METHODS ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

there's one that tests this via personal_sign. Perhaps we just rename that test title?

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah lets rename it

Copy link

codecov bot commented Oct 14, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.99%. Comparing base (aa521c8) to head (9fdc2bb).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1370   +/-   ##
=======================================
  Coverage   74.99%   74.99%           
=======================================
  Files         184      184           
  Lines        4519     4519           
  Branches     1108     1108           
=======================================
  Hits         3389     3389           
  Misses       1130     1130           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

jiexi added 4 commits October 16, 2025 14:11
…ead-calls

# Conflicts:
#	packages/sdk-multichain/src/domain/multichain/api/constants.ts
#	packages/sdk-multichain/src/multichain/rpc/client.test.ts
#	packages/sdk-multichain/src/multichain/rpc/client.ts
cursor[bot]

This comment was marked as outdated.

Comment on lines 32 to 34
if (SDK_HANDLED_METHODS.has(method)) {
return this.handleWithSdkState(options);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm okay leaving this here for now, but we should consider whether to pull this into a separate method since these methods don't technically go through wallet_invokeMethod

Copy link

@jiexi
Copy link
Contributor Author

jiexi commented Oct 17, 2025

Going to wait for new monorepo and then migrate these changes over

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