Skip to content

Commit cdbc5aa

Browse files
committed
fix(rpc): isolate circuit breaker per chain+host
The RPC provider circuit name was derived from the RPS-limiter key, which is empty when the limiter is disabled (e.g. the smart proxy). An empty circuit name makes the circuit breaker execute the provider directly without a circuit, so a provider that consistently fails for a given chain (e.g. a proxy with no backend for that chain) keeps being retried on every request instead of being short-circuited to the fallback. Always derive a non-empty circuit name isolated per chain+host so the breaker can open for a consistently failing endpoint without affecting other chains that share the same host. The RPS limiter remains intentionally shared per host.
1 parent 56577af commit cdbc5aa

2 files changed

Lines changed: 64 additions & 6 deletions

File tree

internal/rpc/client.go

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -234,23 +234,31 @@ func (c *Client) getClientUsingCache(chainID uint64) (chain.ClientInterface, err
234234
return client, nil
235235
}
236236

237+
// getProviderRPCLimiter returns the (optional) RPS limiter for the provider and the circuit
238+
// name to use for the circuit breaker.
239+
// The circuit name is always non-empty and isolated per chain+host, so the circuit breaker can
240+
// short-circuit a consistently failing endpoint without affecting other chains that share the
241+
// same host (e.g. the smart proxy). The RPS limiter, in contrast, is intentionally shared per
242+
// host so we throttle the whole host regardless of chain.
237243
func (c *Client) getProviderRPCLimiter(provider params.RpcProvider) (*rpclimiter.RPCRpsLimiter, string, error) {
244+
circuitName := fmt.Sprintf("%s-%d", provider.GetHost(), provider.ChainID)
245+
238246
c.rpsLimiterMutex.Lock()
239247
defer c.rpsLimiterMutex.Unlock()
240248
if !provider.EnableRPSLimiter {
241-
return nil, "", nil
249+
return nil, circuitName, nil
242250
}
243-
// Generate a unique key for the provider based on its host
251+
// Generate a unique key for the limiter based on its host (shared across chains)
244252
limiterKey := provider.GetHost()
245253

246254
// Check if the limiter already exists
247255
if limiter, ok := c.limiterPerProvider[limiterKey]; ok {
248-
return limiter, limiterKey, nil
256+
return limiter, circuitName, nil
249257
}
250258

251259
limiter := rpclimiter.NewRPCRpsLimiter(&c.PauseBroadcaster)
252260
c.limiterPerProvider[limiterKey] = limiter
253-
return limiter, limiterKey, nil
261+
return limiter, circuitName, nil
254262
}
255263

256264
// SetPaused stops/resumes the 1s tickers of every RPC rate limiter owned by this Client.
@@ -282,14 +290,14 @@ func (c *Client) getEthClients(network *params.Network) []ethclient.RPSLimitedEt
282290
continue
283291
}
284292

285-
rpcLimiter, limiterKey, err := c.getProviderRPCLimiter(provider)
293+
rpcLimiter, circuitName, err := c.getProviderRPCLimiter(provider)
286294
if err != nil {
287295
c.logger.Error("get RPC limiter failed", zap.String("provider", provider.Name), zap.Error(err))
288296
continue
289297
}
290298

291299
// Create ethclient with RPS limiter. If limiter is not enabled, it will be nil
292-
ethClient := ethclient.NewRPSLimitedEthClient(rpcClient, rpcLimiter, limiterKey, provider.Name)
300+
ethClient := ethclient.NewRPSLimitedEthClient(rpcClient, rpcLimiter, circuitName, provider.Name)
293301
ethClients = append(ethClients, ethClient)
294302
}
295303

internal/rpc/client_test.go

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import (
1919
"github.com/ethereum/go-ethereum/common"
2020

2121
"github.com/status-im/status-go/internal/db/appdatabase"
22+
"github.com/status-im/status-go/internal/rpc/chain/rpclimiter"
2223
"github.com/status-im/status-go/internal/testutils"
2324
"github.com/status-im/status-go/params"
2425
"github.com/status-im/status-go/params/networkhelper"
@@ -120,3 +121,52 @@ func TestGetClientsUsingCache(t *testing.T) {
120121
func TestUserAgent(t *testing.T) {
121122
require.True(t, strings.HasPrefix(rpcUserAgentName, "procuratee-desktop/"))
122123
}
124+
125+
func TestGetProviderRPCLimiterCircuitName(t *testing.T) {
126+
const host = "test.eth-rpc.status.im"
127+
128+
newProvider := func(chainID uint64, rpsLimiter bool) params.RpcProvider {
129+
return params.RpcProvider{
130+
Name: "smart-proxy",
131+
ChainID: chainID,
132+
URL: security.NewSensitiveString(fmt.Sprintf("https://%s/ethereum/mainnet/", host)),
133+
Type: params.EmbeddedEthRpcProxyProviderType,
134+
AuthType: params.NoAuth,
135+
EnableRPSLimiter: rpsLimiter,
136+
Enabled: true,
137+
}
138+
}
139+
140+
c := &Client{limiterPerProvider: make(map[string]*rpclimiter.RPCRpsLimiter)}
141+
142+
// Circuit name must be non-empty even when the RPS limiter is disabled, so the
143+
// circuit breaker can short-circuit a consistently failing provider.
144+
limiter, circuitChain1, err := c.getProviderRPCLimiter(newProvider(1, false))
145+
require.NoError(t, err)
146+
require.Nil(t, limiter)
147+
require.Equal(t, fmt.Sprintf("%s-%d", host, 1), circuitChain1)
148+
149+
// Same host but a different chain must yield a different circuit name, so a provider
150+
// that fails for one chain does not open the circuit for other chains sharing the host.
151+
limiter, circuitChain747474, err := c.getProviderRPCLimiter(newProvider(747474, false))
152+
require.NoError(t, err)
153+
require.Nil(t, limiter)
154+
require.Equal(t, fmt.Sprintf("%s-%d", host, 747474), circuitChain747474)
155+
require.NotEqual(t, circuitChain1, circuitChain747474)
156+
157+
// With the RPS limiter enabled, the circuit name is still isolated per chain+host,
158+
// while the limiter itself is shared per host across chains.
159+
limiterChain1, circuitChain1Rps, err := c.getProviderRPCLimiter(newProvider(1, true))
160+
require.NoError(t, err)
161+
require.NotNil(t, limiterChain1)
162+
require.Equal(t, fmt.Sprintf("%s-%d", host, 1), circuitChain1Rps)
163+
164+
limiterChain2, circuitChain2Rps, err := c.getProviderRPCLimiter(newProvider(2, true))
165+
require.NoError(t, err)
166+
require.NotNil(t, limiterChain2)
167+
require.Equal(t, fmt.Sprintf("%s-%d", host, 2), circuitChain2Rps)
168+
require.NotEqual(t, circuitChain1Rps, circuitChain2Rps)
169+
170+
// Limiter is shared per host regardless of chain.
171+
require.Same(t, limiterChain1, limiterChain2)
172+
}

0 commit comments

Comments
 (0)