feat(walrus-sui): skip JSON-RPC client when fully migrated to gRPC by nikos-terzo · Pull Request #3186 · MystenLabs/walrus

nikos-terzo · 2026-03-24T13:58:53Z

Summary

Skip JSON-RPC SuiClient creation in DualClient::new() when WALRUS_GRPC_MIGRATION_LEVEL >= 4. Add GrpcMigrationLevel::is_fully_migrated() helper.

Motivation

sui-forking only exposes gRPC — no JSON-RPC. DualClient::new() always calls SuiClientBuilder::build() which probes with rpc.discover, failing with 404 on gRPC-only nodes. This blocks using site-builder against forked networks.

The infrastructure was designed for this — the existing comment on sui_client() says: "when the migration is complete, we will not create SuiClients." This PR implements that.

Needed by: MystenLabs/walrus-sites#690

Test plan

WALRUS_GRPC_MIGRATION_LEVEL=4 site-builder publish against sui-forking succeeds (previously 404)
No behavior change when unset or < 4 (default is 0)

🤖 Generated with Claude Code

When WALRUS_GRPC_MIGRATION_LEVEL is at its maximum (4), DualClient::new() now skips creating the SuiClient (JSON-RPC). This allows connecting to gRPC-only nodes like sui-forking, which don't serve JSON-RPC endpoints. Previously, DualClient always called SuiClientBuilder::build() which probes the server with an rpc.discover JSON-RPC call — failing with 404 on gRPC-only nodes. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

This PR updates walrus-sui’s dual client initialization to avoid constructing the JSON-RPC SuiClient when the gRPC migration is considered complete, enabling compatibility with gRPC-only nodes (e.g. sui-forking) where JSON-RPC discovery fails.

Changes:

Add GrpcMigrationLevel::is_fully_migrated() helper.
Update DualClient::new() to skip JSON-RPC SuiClient creation when WALRUS_GRPC_MIGRATION_LEVEL >= 4.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
`crates/walrus-sui/src/client/retry_client/retriable_sui_client.rs`	Adds `is_fully_migrated()` helper on `GrpcMigrationLevel`.
`crates/walrus-sui/src/client/dual_client.rs`	Conditionally avoids building the JSON-RPC `SuiClient` at high migration levels to support gRPC-only endpoints.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-24T14:03:18Z

crates/walrus-sui/src/client/dual_client.rs

+        let sui_client = if GrpcMigrationLevel::default().is_fully_migrated() {
+            None
+        } else {
+            let mut client_builder = SuiClientBuilder::default();
+            if let Some(request_timeout) = request_timeout {
+                client_builder = client_builder.request_timeout(request_timeout);
+            }
+            Some(client_builder.build(rpc_url).await?)
+        };


DualClient::new() can now set sui_client to None when WALRUS_GRPC_MIGRATION_LEVEL >= 4, but there are still call paths in the crate that unconditionally call DualClient::sui_client() (which panics when sui_client is None), e.g. RetriableSuiClient::query_transaction_blocks / get_owned_objects. This turns a config change into a runtime panic. Consider either (a) only skipping JSON-RPC client creation once all JSON-RPC-dependent methods are gated off at that migration level, or (b) making JSON-RPC client creation lazy / fallible at the point of use and returning a structured error instead of panicking.

Copilot · 2026-03-24T14:03:18Z

crates/walrus-sui/src/client/dual_client.rs

 use crate::{
-    client::SuiClientError,
+    client::{SuiClientError, retry_client::retriable_sui_client::GrpcMigrationLevel},
    coin::Coin,


Importing GrpcMigrationLevel from retry_client::retriable_sui_client makes dual_client depend on the retry-layer module that already depends on DualClient, increasing coupling and risking cyclic-module tangles. Consider moving GrpcMigrationLevel (and its env parsing) into a lower-level/shared module (e.g. under client/ root) and re-exporting it, so both DualClient and retry code depend on the same shared definition.

Copilot · 2026-03-24T14:03:19Z

crates/walrus-sui/src/client/retry_client/retriable_sui_client.rs

+    /// Returns true if the migration level is high enough that all RPC calls use gRPC
+    /// and the JSON-RPC SuiClient is not needed.


is_fully_migrated() is documented as meaning "all RPC calls use gRPC and the JSON-RPC SuiClient is not needed", but the threshold used (GRPC_MIGRATION_LEVEL_GET_BALANCE) only reflects migration of balance retrieval. There are still JSON-RPC-only methods in the codebase (e.g. transaction queries / owned-object listing), so this helper name/doc are misleading and (when used to skip client creation) can trigger panics later. Consider renaming/rewording this helper to match what level 4 actually guarantees, or updating the threshold/levels so that "fully migrated" truly implies no JSON-RPC usage.

Suggested change

/// Returns true if the migration level is high enough that all RPC calls use gRPC

/// and the JSON-RPC SuiClient is not needed.

/// Returns true if the migration level is high enough that balance-related RPC calls

/// (including coin selection and balance retrieval) use gRPC.

///

/// Note: This does *not* guarantee that all RPC calls have been migrated to gRPC. Some

/// operations (for example, certain transaction queries or owned-object listing) may

/// still rely on the JSON-RPC `SuiClient`. Callers MUST NOT use this helper to decide

/// that a JSON-RPC client is unnecessary.

#[deprecated(

note = "is_fully_migrated only reflects that balance-related RPCs are on gRPC; \

do not use it to assume JSON-RPC is no longer required."

)]

nikos-terzo · 2026-03-24T14:12:09Z

As copilot correctly discovered, the change isn't ready. Started looking for a solution in sui-forking for now.

wbbradley · 2026-03-24T15:10:28Z

@nikos-terzo Yes, Walrus gRPC migration is not yet complete. If there are sui nodes that are disabling JSON RPC, then Walrus will not be supported by those nodes until we can complete the gRPC switchover within Walrus. cc @bmwill

wbbradley

Walrus is not ready for gRPC-only usage.

nikos-terzo requested review from Copilot, stefan-mysten and wbbradley March 24, 2026 13:58

Copilot started reviewing on behalf of nikos-terzo March 24, 2026 13:59 View session

Copilot AI reviewed Mar 24, 2026

View reviewed changes

nikos-terzo marked this pull request as draft March 24, 2026 14:07

wbbradley requested changes Mar 24, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(walrus-sui): skip JSON-RPC client when fully migrated to gRPC#3186

feat(walrus-sui): skip JSON-RPC client when fully migrated to gRPC#3186
nikos-terzo wants to merge 1 commit intomainfrom
grpc-only-dual-client

nikos-terzo commented Mar 24, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 24, 2026

Uh oh!

Copilot AI Mar 24, 2026

Uh oh!

Copilot AI Mar 24, 2026

Uh oh!

nikos-terzo commented Mar 24, 2026

Uh oh!

wbbradley commented Mar 24, 2026

Uh oh!

wbbradley left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		/// Returns true if the migration level is high enough that all RPC calls use gRPC
		/// and the JSON-RPC SuiClient is not needed.

-    /// Returns true if the migration level is high enough that all RPC calls use gRPC
-    /// and the JSON-RPC SuiClient is not needed.
+    /// Returns true if the migration level is high enough that balance-related RPC calls
+    /// (including coin selection and balance retrieval) use gRPC.
+    ///
+    /// Note: This does *not* guarantee that all RPC calls have been migrated to gRPC. Some
+    /// operations (for example, certain transaction queries or owned-object listing) may
+    /// still rely on the JSON-RPC `SuiClient`. Callers MUST NOT use this helper to decide
+    /// that a JSON-RPC client is unnecessary.
+    #[deprecated(
+        note = "is_fully_migrated only reflects that balance-related RPCs are on gRPC; \
+                do not use it to assume JSON-RPC is no longer required."
+    )]

Conversation

nikos-terzo commented Mar 24, 2026

Summary

Motivation

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

nikos-terzo commented Mar 24, 2026

Uh oh!

wbbradley commented Mar 24, 2026

Uh oh!

wbbradley left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants