Skip to content

Conversation

@fbac
Copy link
Collaborator

@fbac fbac commented Jul 30, 2025

There’s been some back and forth on this.

We initially switched from HTTP RPC URLs to WSS URLs to simplify xmtpd configuration, since a WebSocket connection allow both JSON-RPC requests and subscriptions over a single connection.

However, it turns out that our RPC providers optimize their infrastructure for JSON-RPC requests specifically over HTTP. These performance improvements don’t apply when JSON-RPC is served over WebSocket.

With this change, we’re reverting to making all JSON-RPC requests over HTTP, while continuing to use WSS solely for subscriptions.

@fbac fbac requested a review from a team as a code owner July 30, 2025 14:06
@graphite-app
Copy link

graphite-app bot commented Jul 30, 2025

How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • Queue - adds this PR to the back of the merge queue
  • Hotfix - for urgent hot fixes, skip the queue and merge this PR next

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

@macroscopeapp
Copy link

macroscopeapp bot commented Jul 30, 2025

Reintroduce HTTP JSON-RPC client by replacing WebSocket blockchain client connections with HTTP RPC clients across CLI commands, indexers, and server components

The codebase transitions from WebSocket-based blockchain connections to HTTP JSON-RPC clients for most operations while maintaining separate WebSocket clients for live subscriptions. Key changes include:

  • Configuration adds RPCURL fields to AppChainOptions and SettlementChainOptions structs in pkg/config/options.go with corresponding validation in pkg/config/validation.go
  • Blockchain client API splits into NewRPCClient for HTTP connections and NewWebsocketClient for WebSocket connections in pkg/blockchain/client.go
  • Indexers for app chain and settlement chain now use dual clients - HTTP RPC for queries/backfill and WebSocket for live subscriptions in pkg/indexer/app_chain/app_chain.go and pkg/indexer/settlement_chain/settlement_chain.go
  • CLI commands and server components switch from WebSocket to HTTP RPC connections for registry operations and contract interactions
  • Test utilities update to support both URL types with StartAnvil returning both WebSocket and HTTP endpoints

📍Where to Start

Start with the NewRPCClient and NewWebsocketClient functions in pkg/blockchain/client.go to understand the new client creation API, then examine the dual client usage in pkg/indexer/app_chain/app_chain.go.

Changes since #1001 opened

  • Replaced websocket client creation with RPC client creation in blockchain service setup functions [aa5ca5a]
  • Updated test helper functions to provide both websocket and RPC URLs for contract configuration [aa5ca5a]

Macroscope summarized aa5ca5a.

Copy link

Choose a reason for hiding this comment

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

Issue on line in pkg/indexer/app_chain/app_chain.go:65:

The error handling in NewAppChain will dereference a nil rpcClient if blockchain.NewRPCClient fails, causing a panic. The code calls rpcClient.Close() even though rpcClient is nil on initialization failure. Consider guarding the call to rpcClient.Close() so it only runs when rpcClient is non-nil, preventing a nil pointer dereference.

-       cancel()
-       rpcClient.Close()
+       cancel()
+       if rpcClient != nil {
+           rpcClient.Close()
+       }

🚀 Reply to ask Macroscope to explain or update this suggestion.

👍 Helpful? React to give us feedback.

@fbac fbac merged commit 09466d1 into main Aug 11, 2025
10 checks passed
@fbac fbac deleted the 07-30-reintroduce_rpc_url branch August 11, 2025 15:52
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