From e6a1dcb3c850e4264384867bc74b3af807b91361 Mon Sep 17 00:00:00 2001 From: bdchatham Date: Fri, 3 Apr 2026 15:55:25 -0700 Subject: [PATCH 1/2] fix: handle both JSON-RPC envelope and flat seid responses PR #60 assumed all CometBFT RPC endpoints return JSON-RPC envelopes ({"jsonrpc":"2.0","result":{...}}), but seid's CometBFT fork returns flat JSON ({"node_info":{...}}). This caused Client.Get() to return "empty result" for all seid queries, breaking peer discovery and await-condition height polling. Fix Client.Get() to handle both formats: if the response has a "result" key, unwrap it (standard CometBFT); otherwise return the body as-is (seid flat format). Also add logging in peer discovery when instances are found but unreachable, preventing silent failures. Co-Authored-By: Claude Opus 4.6 (1M context) --- sidecar/rpc/client.go | 29 ++++++++++++++++++++--------- sidecar/rpc/client_test.go | 21 ++++++++++++++------- sidecar/tasks/peers.go | 7 ++++++- 3 files changed, 40 insertions(+), 17 deletions(-) diff --git a/sidecar/rpc/client.go b/sidecar/rpc/client.go index c8d407e..5131826 100644 --- a/sidecar/rpc/client.go +++ b/sidecar/rpc/client.go @@ -23,8 +23,9 @@ type rpcError struct { Data string `json:"data"` } -// envelope is the JSON-RPC response wrapper returned by all CometBFT -// HTTP RPC endpoints (e.g. /status, /block, /block_results). +// envelope is the JSON-RPC response wrapper returned by standard CometBFT +// HTTP RPC endpoints. Note: seid's CometBFT fork returns flat JSON without +// this wrapper — Client.Get handles both formats. type envelope struct { Result json.RawMessage `json:"result"` Error *rpcError `json:"error,omitempty"` @@ -61,8 +62,15 @@ func (c *Client) SetTimeout(d time.Duration) { c.timeout = d } // Endpoint returns the configured RPC base URL. func (c *Client) Endpoint() string { return c.endpoint } -// Get performs an HTTP GET to endpoint+path, unwraps the JSON-RPC -// envelope, and returns the inner "result" as raw JSON. +// Get performs an HTTP GET to endpoint+path and returns the inner result +// as raw JSON. It handles both response formats: +// - JSON-RPC envelope (standard CometBFT): {"jsonrpc":"2.0","result":{...}} +// → returns the unwrapped "result" value +// - Flat JSON (seid): {"node_info":{...},"sync_info":{...}} +// → returns the body as-is +// +// This dual-format support is necessary because seid's CometBFT fork +// returns flat responses while standard CometBFT uses JSON-RPC envelopes. func (c *Client) Get(ctx context.Context, path string) (json.RawMessage, error) { body, err := c.doGet(ctx, path) if err != nil { @@ -71,17 +79,20 @@ func (c *Client) Get(ctx context.Context, path string) (json.RawMessage, error) var env envelope if err := json.Unmarshal(body, &env); err != nil { - return nil, fmt.Errorf("decoding JSON-RPC envelope from %s: %w", path, err) + return nil, fmt.Errorf("decoding JSON response from %s: %w", path, err) } if env.Error != nil { return nil, fmt.Errorf("JSON-RPC error from %s: %s (code %d, data: %s)", path, env.Error.Message, env.Error.Code, env.Error.Data) } - if len(env.Result) == 0 { - return nil, fmt.Errorf("empty result in JSON-RPC response from %s", path) - } - return env.Result, nil + // If the response had a "result" key, return the unwrapped inner value + // (standard CometBFT JSON-RPC envelope). Otherwise the response is + // flat JSON (seid format) — return the entire body. + if len(env.Result) > 0 { + return env.Result, nil + } + return json.RawMessage(body), nil } // GetRaw performs an HTTP GET and returns the entire response body diff --git a/sidecar/rpc/client_test.go b/sidecar/rpc/client_test.go index cfe9521..8c2caec 100644 --- a/sidecar/rpc/client_test.go +++ b/sidecar/rpc/client_test.go @@ -30,19 +30,26 @@ func TestClient_Get_UnwrapsEnvelope(t *testing.T) { } } -func TestClient_Get_EmptyResult(t *testing.T) { +func TestClient_Get_FlatJSON_SeidFormat(t *testing.T) { + // seid returns flat JSON without the JSON-RPC envelope. + flat := `{"node_info":{"id":"abc123"},"sync_info":{"latest_block_height":"42","catching_up":false}}` srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - _, _ = w.Write([]byte(`{"jsonrpc":"2.0","id":-1}`)) + _, _ = w.Write([]byte(flat)) })) defer srv.Close() c := NewClient(srv.URL, nil) - _, err := c.Get(context.Background(), "/status") - if err == nil { - t.Fatal("expected error for empty result") + raw, err := c.Get(context.Background(), "/status") + if err != nil { + t.Fatalf("Get: %v", err) } - if !strings.Contains(err.Error(), "empty result") { - t.Errorf("unexpected error: %v", err) + + got := string(raw) + if !strings.Contains(got, "abc123") { + t.Errorf("expected flat result containing node ID, got %s", got) + } + if !strings.Contains(got, "latest_block_height") { + t.Errorf("expected flat result containing latest_block_height, got %s", got) } } diff --git a/sidecar/tasks/peers.go b/sidecar/tasks/peers.go index 518c035..e9fb688 100644 --- a/sidecar/tasks/peers.go +++ b/sidecar/tasks/peers.go @@ -125,11 +125,15 @@ func (s *EC2TagsSource) Discover(ctx context.Context) ([]string, error) { return nil, fmt.Errorf("ec2 DescribeInstances: %w", err) } + var instanceCount int var peers []string for _, reservation := range output.Reservations { for _, instance := range reservation.Instances { + instanceCount++ peer, err := buildPeerAddress(ctx, querier, instance) if err != nil { + peerLog.Info("skipping unreachable EC2 instance", + "ip", instanceIP(instance), "error", err) continue } peers = append(peers, peer) @@ -137,7 +141,8 @@ func (s *EC2TagsSource) Discover(ctx context.Context) ([]string, error) { } if len(peers) == 0 { - return nil, fmt.Errorf("no reachable peers found via ec2Tags in region %s", s.Region) + return nil, fmt.Errorf("no reachable peers found via ec2Tags in region %s (%d instances returned, 0 reachable)", + s.Region, instanceCount) } return peers, nil From 2179f188b02918ef3ec802d223ad9e73ba658703 Mon Sep 17 00:00:00 2001 From: bdchatham Date: Fri, 3 Apr 2026 16:00:40 -0700 Subject: [PATCH 2/2] review: use jsonrpc field as envelope discriminator, add ambiguous case test Use "jsonrpc":"2.0" presence to distinguish JSON-RPC envelopes from flat seid responses, instead of checking for a "result" key. This prevents false-positive unwrapping if seid ever returns a flat response with a top-level "result" data field. Error checking (env.Error) now only applies inside the JSON-RPC branch. Added TestClient_Get_FlatJSON_WithResultKey to prove a flat response containing "result" is returned as-is, not unwrapped. Co-Authored-By: Claude Opus 4.6 (1M context) --- sidecar/rpc/client.go | 25 +++++++++++++++---------- sidecar/rpc/client_test.go | 25 +++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 10 deletions(-) diff --git a/sidecar/rpc/client.go b/sidecar/rpc/client.go index 5131826..60a5ffa 100644 --- a/sidecar/rpc/client.go +++ b/sidecar/rpc/client.go @@ -27,8 +27,9 @@ type rpcError struct { // HTTP RPC endpoints. Note: seid's CometBFT fork returns flat JSON without // this wrapper — Client.Get handles both formats. type envelope struct { - Result json.RawMessage `json:"result"` - Error *rpcError `json:"error,omitempty"` + JSONRPC string `json:"jsonrpc"` + Result json.RawMessage `json:"result"` + Error *rpcError `json:"error,omitempty"` } // Client performs HTTP GET requests against a CometBFT RPC endpoint @@ -81,17 +82,21 @@ func (c *Client) Get(ctx context.Context, path string) (json.RawMessage, error) if err := json.Unmarshal(body, &env); err != nil { return nil, fmt.Errorf("decoding JSON response from %s: %w", path, err) } - if env.Error != nil { - return nil, fmt.Errorf("JSON-RPC error from %s: %s (code %d, data: %s)", - path, env.Error.Message, env.Error.Code, env.Error.Data) - } - // If the response had a "result" key, return the unwrapped inner value - // (standard CometBFT JSON-RPC envelope). Otherwise the response is - // flat JSON (seid format) — return the entire body. - if len(env.Result) > 0 { + // Discriminate by the presence of "jsonrpc":"2.0" — only real JSON-RPC + // envelopes carry this field. Seid's flat responses never will. + if env.JSONRPC == "2.0" { + if env.Error != nil { + return nil, fmt.Errorf("JSON-RPC error from %s: %s (code %d, data: %s)", + path, env.Error.Message, env.Error.Code, env.Error.Data) + } + if len(env.Result) == 0 { + return nil, fmt.Errorf("empty result in JSON-RPC response from %s", path) + } return env.Result, nil } + + // Flat JSON (seid format) — return the body as-is. return json.RawMessage(body), nil } diff --git a/sidecar/rpc/client_test.go b/sidecar/rpc/client_test.go index 8c2caec..21256a7 100644 --- a/sidecar/rpc/client_test.go +++ b/sidecar/rpc/client_test.go @@ -53,6 +53,31 @@ func TestClient_Get_FlatJSON_SeidFormat(t *testing.T) { } } +func TestClient_Get_FlatJSON_WithResultKey(t *testing.T) { + // A flat response that happens to contain a "result" data key must NOT + // be mistaken for a JSON-RPC envelope. + flat := `{"result":{"code":0,"log":"ok"},"hash":"ABC123"}` + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write([]byte(flat)) + })) + defer srv.Close() + + c := NewClient(srv.URL, nil) + raw, err := c.Get(context.Background(), "/tx") + if err != nil { + t.Fatalf("Get: %v", err) + } + + got := string(raw) + // Should return the full body, not just the inner "result" value. + if !strings.Contains(got, "hash") { + t.Errorf("expected full flat body with hash field, got %s", got) + } + if !strings.Contains(got, "ABC123") { + t.Errorf("expected full flat body with hash value, got %s", got) + } +} + func TestClient_Get_HTTPError(t *testing.T) { srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusBadGateway)