From f8bec03e36eb98ce7b9b6db5bd03aa4bd290a89a Mon Sep 17 00:00:00 2001 From: SeUkKim Date: Thu, 24 Jul 2025 17:09:59 +0900 Subject: [PATCH 01/23] feat(rpc): add rpc pool for endpoint fallback --- README.md | 48 +++++ challenger/README.md | 10 +- challenger/types/config.go | 12 +- executor/README.md | 15 +- executor/batchsubmitter/batch_test.go | 6 +- executor/batchsubmitter/handler_test.go | 3 +- executor/types/config.go | 20 +- node/node.go | 2 +- node/rpcclient/client.go | 241 ++++++++++++++++++++++-- node/rpcclient/rpcpool.go | 183 ++++++++++++++++++ node/rpcclient/rpcpool_test.go | 195 +++++++++++++++++++ node/types/config.go | 4 +- 12 files changed, 697 insertions(+), 42 deletions(-) create mode 100644 node/rpcclient/rpcpool.go create mode 100644 node/rpcclient/rpcpool_test.go diff --git a/README.md b/README.md index 6827094e..1dc7d0de 100644 --- a/README.md +++ b/README.md @@ -100,3 +100,51 @@ Challenger node types: - host - child ``` + +## RPC Pool Configuration + +To enhance reliability, the bots support multiple RPC endpoints for L1, L2, and DA nodes. This allows for automatic fallback and retries if an endpoint becomes unavailable. + +### Endpoint Configuration + +In your configuration file (`~/.opinit/[bot-name].json`), you can specify a list of RPC addresses for each node: + +```json +{ + "l1_node": { + "chain_id": "testnet-l1-1", + "bech32_prefix": "init", + "rpc_address": [ + "tcp://doi-rpc:26657", + "tcp://another-l1-rpc:26657" + ] + }, + "l2_node": { + "chain_id": "testnet-l2-1", + "bech32_prefix": "init", + "rpc_address": [ + "tcp://rpc:27657", + "tcp://another-l2-rpc:27657" + ] + }, + "da_node": { + "chain_id": "testnet-l1-1", + "bech32_prefix": "init", + "rpc_address": [ + "tcp://rpc:26657", + "tcp://another-da-rpc:26657" + ] + } +} +``` + +The bot will try the endpoints in the order they are listed. If a request to the first endpoint fails, it will automatically fall back to the next one in the list. + +### Timeout Configuration + +You can configure the timeout for each RPC request using the `RPC_TIMEOUT_SECONDS` environment variable. The value is in seconds. If the variable is not set, the default timeout is 5 seconds. + +```bash +export RPC_TIMEOUT_SECONDS=10 +opinitd start [bot-name] +``` diff --git a/challenger/README.md b/challenger/README.md index a79269db..f820cfe1 100644 --- a/challenger/README.md +++ b/challenger/README.md @@ -28,12 +28,18 @@ To configure the Challenger, fill in the values in the `~/.opinit/challenger.jso "l1_node": { "chain_id": "testnet-l1-1", "bech32_prefix": "init", - "rpc_address": "tcp://localhost:26657", + "rpc_address": [ + "tcp://doi-rpc:26657", + "tcp://localhost:26657" + ], }, "l2_node": { "chain_id": "testnet-l2-1", "bech32_prefix": "init", - "rpc_address": "tcp://localhost:27657", + "rpc_address": [ + "tcp://another-rpc:26657", + "tcp://localhost:27657" + ], }, // DisableAutoSetL1Height is the flag to disable the automatic setting of the l1 height. // If it is false, it will finds the optimal height and sets l1_start_height automatically diff --git a/challenger/types/config.go b/challenger/types/config.go index 220a3a24..e0b08657 100644 --- a/challenger/types/config.go +++ b/challenger/types/config.go @@ -8,9 +8,9 @@ import ( ) type NodeConfig struct { - ChainID string `json:"chain_id"` - Bech32Prefix string `json:"bech32_prefix"` - RPCAddress string `json:"rpc_address"` + ChainID string `json:"chain_id"` + Bech32Prefix string `json:"bech32_prefix"` + RPCAddress []string `json:"rpc_address"` } func (nc NodeConfig) Validate() error { @@ -20,7 +20,7 @@ func (nc NodeConfig) Validate() error { if nc.Bech32Prefix == "" { return errors.New("bech32 prefix is required") } - if nc.RPCAddress == "" { + if len(nc.RPCAddress) == 0 { return errors.New("RPC address is required") } return nil @@ -66,13 +66,13 @@ func DefaultConfig() *Config { L1Node: NodeConfig{ ChainID: "testnet-l1-1", Bech32Prefix: "init", - RPCAddress: "tcp://localhost:26657", + RPCAddress: []string{"tcp://localhost:26657"}, }, L2Node: NodeConfig{ ChainID: "testnet-l2-1", Bech32Prefix: "init", - RPCAddress: "tcp://localhost:27657", + RPCAddress: []string{"tcp://localhost:27657"}, }, DisableAutoSetL1Height: false, L1StartHeight: 1, diff --git a/executor/README.md b/executor/README.md index 85e6299d..9d4e2183 100644 --- a/executor/README.md +++ b/executor/README.md @@ -26,7 +26,10 @@ To configure the Executor, fill in the values in the `~/.opinit/executor.json` f "l1_node": { "chain_id": "testnet-l1-1", "bech32_prefix": "init", - "rpc_address": "tcp://localhost:26657", + "rpc_address": [ + "tcp://doi-rpc.com", + "tcp://localhost:26657" + ], "gas_price": "0.15uinit", "gas_adjustment": 1.5, "tx_timeout": 60 @@ -34,7 +37,10 @@ To configure the Executor, fill in the values in the `~/.opinit/executor.json` f "l2_node": { "chain_id": "testnet-l2-1", "bech32_prefix": "init", - "rpc_address": "tcp://localhost:27657", + "rpc_address": [ + "tcp://another-rpc:27657", + "tcp://localhost:27657" + ], "gas_price": "", "gas_adjustment": 1.5, "tx_timeout": 60 @@ -42,7 +48,10 @@ To configure the Executor, fill in the values in the `~/.opinit/executor.json` f "da_node": { "chain_id": "testnet-l1-1", "bech32_prefix": "init", - "rpc_address": "tcp://localhost:26657", + "rpc_address": [ + "tcp://another-rpc:26657", + "tcp://localhost:26657" + ], "gas_price": "0.15uinit", "gas_adjustment": 1.5, "tx_timeout": 60 diff --git a/executor/batchsubmitter/batch_test.go b/executor/batchsubmitter/batch_test.go index a205b151..13f2ba77 100644 --- a/executor/batchsubmitter/batch_test.go +++ b/executor/batchsubmitter/batch_test.go @@ -272,7 +272,8 @@ func TestFinalizeBatch(t *testing.T) { require.NoError(t, err) mockCaller := mockclient.NewMockCaller() - rpcClient := rpcclient.NewRPCClientWithClient(appCodec, client.NewWithCaller(mockCaller)) + rpcClient, err := rpcclient.NewRPCClientWithClient(appCodec, client.NewWithCaller(mockCaller), []string{"http://localhost:26657"}) + require.NoError(t, err) batchNode := node.NewTestNode(nodetypes.NodeConfig{}, batchDB, appCodec, txConfig, rpcClient, nil) hostCdc, _, err := hostprovider.GetCodec("init") @@ -753,7 +754,8 @@ func TestSubmitGenesis(t *testing.T) { require.NoError(t, err) mockCaller := mockclient.NewMockCaller() - rpcClient := rpcclient.NewRPCClientWithClient(appCodec, client.NewWithCaller(mockCaller)) + rpcClient, err := rpcclient.NewRPCClientWithClient(appCodec, client.NewWithCaller(mockCaller), []string{"http://localhost:26657"}) + require.NoError(t, err) batchNode := node.NewTestNode(nodetypes.NodeConfig{}, batchDB, appCodec, txConfig, rpcClient, nil) hostCdc, _, err := hostprovider.GetCodec("init") diff --git a/executor/batchsubmitter/handler_test.go b/executor/batchsubmitter/handler_test.go index 600b3a8f..b07281dd 100644 --- a/executor/batchsubmitter/handler_test.go +++ b/executor/batchsubmitter/handler_test.go @@ -46,7 +46,8 @@ func TestRawBlockHandler(t *testing.T) { require.NoError(t, err) mockCaller := mockclient.NewMockCaller() - rpcClient := rpcclient.NewRPCClientWithClient(appCodec, client.NewWithCaller(mockCaller)) + rpcClient, err := rpcclient.NewRPCClientWithClient(appCodec, client.NewWithCaller(mockCaller), []string{"http://localhost:26657"}) + require.NoError(t, err) batchNode := node.NewTestNode(nodetypes.NodeConfig{}, batchDB, appCodec, txConfig, rpcClient, nil) hostCdc, _, err := hostprovider.GetCodec("init") diff --git a/executor/types/config.go b/executor/types/config.go index 115839ee..b98552e7 100644 --- a/executor/types/config.go +++ b/executor/types/config.go @@ -11,12 +11,12 @@ import ( ) type NodeConfig struct { - ChainID string `json:"chain_id"` - Bech32Prefix string `json:"bech32_prefix"` - RPCAddress string `json:"rpc_address"` - GasPrice string `json:"gas_price"` - GasAdjustment float64 `json:"gas_adjustment"` - TxTimeout int64 `json:"tx_timeout"` // seconds + ChainID string `json:"chain_id"` + Bech32Prefix string `json:"bech32_prefix"` + RPCAddress []string `json:"rpc_address"` + GasPrice string `json:"gas_price"` + GasAdjustment float64 `json:"gas_adjustment"` + TxTimeout int64 `json:"tx_timeout"` // seconds } func (nc NodeConfig) Validate() error { @@ -26,7 +26,7 @@ func (nc NodeConfig) Validate() error { if nc.Bech32Prefix == "" { return errors.New("bech32 prefix is required") } - if nc.RPCAddress == "" { + if len(nc.RPCAddress) == 0 { return errors.New("RPC address is required") } return nil @@ -109,7 +109,7 @@ func DefaultConfig() *Config { L1Node: NodeConfig{ ChainID: "testnet-l1-1", Bech32Prefix: "init", - RPCAddress: "tcp://localhost:26657", + RPCAddress: []string{"tcp://localhost:26657"}, GasPrice: "0.15uinit", GasAdjustment: 1.5, TxTimeout: 60, @@ -118,7 +118,7 @@ func DefaultConfig() *Config { L2Node: NodeConfig{ ChainID: "testnet-l2-1", Bech32Prefix: "init", - RPCAddress: "tcp://localhost:27657", + RPCAddress: []string{"tcp://localhost:27657"}, GasPrice: "", GasAdjustment: 1.5, TxTimeout: 60, @@ -127,7 +127,7 @@ func DefaultConfig() *Config { DANode: NodeConfig{ ChainID: "testnet-l1-1", Bech32Prefix: "init", - RPCAddress: "tcp://localhost:26657", + RPCAddress: []string{"tcp://localhost:26657"}, GasPrice: "0.15uinit", GasAdjustment: 1.5, TxTimeout: 60, diff --git a/node/node.go b/node/node.go index 30f914e8..64ab7bab 100644 --- a/node/node.go +++ b/node/node.go @@ -51,7 +51,7 @@ func NewNode(cfg nodetypes.NodeConfig, db types.DB, cdc codec.Codec, txConfig cl rpcClient, err := rpcclient.NewRPCClient(cdc, cfg.RPC) if err != nil { - return nil, err + return nil, errors.Wrap(err, "failed to create RPC client") } n := &Node{ diff --git a/node/rpcclient/client.go b/node/rpcclient/client.go index 4e6f5b8b..80103264 100644 --- a/node/rpcclient/client.go +++ b/node/rpcclient/client.go @@ -2,7 +2,6 @@ package rpcclient import ( "context" - "errors" "fmt" "reflect" "strconv" @@ -17,7 +16,10 @@ import ( abci "github.com/cometbft/cometbft/abci/types" client2 "github.com/cometbft/cometbft/rpc/client" coretypes "github.com/cometbft/cometbft/rpc/core/types" + cmttypes "github.com/cometbft/cometbft/types" gogogrpc "github.com/cosmos/gogoproto/grpc" + "github.com/pkg/errors" + "go.uber.org/zap" "github.com/cosmos/cosmos-sdk/codec" "github.com/cosmos/cosmos-sdk/codec/types" @@ -35,11 +37,26 @@ var protoCodec = encoding.GetCodec(proto.Name) type RPCClient struct { *clienthttp.HTTP - cdc codec.Codec + cdc codec.Codec + pool *RPCPool } -func NewRPCClient(cdc codec.Codec, rpcAddr string) (*RPCClient, error) { - client, err := clienthttp.New(rpcAddr, "/websocket") +func NewRPCClient(cdc codec.Codec, rpcAddrs []string) (*RPCClient, error) { + if len(rpcAddrs) == 0 { + return nil, errors.New("no RPC addresses provided") + } + + // Create logger + logger, err := zap.NewProduction() + if err != nil { + return nil, errors.Wrap(err, "failed to create logger") + } + + // Create RPC pool + pool := NewRPCPool(rpcAddrs, logger) + + // Create HTTP client with the first endpoint + client, err := clienthttp.New(pool.GetCurrentEndpoint(), "/websocket") if err != nil { return nil, err } @@ -47,14 +64,29 @@ func NewRPCClient(cdc codec.Codec, rpcAddr string) (*RPCClient, error) { return &RPCClient{ HTTP: client, cdc: cdc, + pool: pool, }, nil } -func NewRPCClientWithClient(cdc codec.Codec, client *clienthttp.HTTP) *RPCClient { +func NewRPCClientWithClient(cdc codec.Codec, client *clienthttp.HTTP, endpoints []string) (*RPCClient, error) { + if len(endpoints) == 0 { + return nil, errors.New("no RPC endpoints provided") + } + + // Create logger + logger, err := zap.NewProduction() + if err != nil { + return nil, errors.Wrap(err, "failed to create logger") + } + + // Create RPC pool + pool := NewRPCPool(endpoints, logger) + return &RPCClient{ HTTP: client, cdc: cdc, - } + pool: pool, + }, nil } // Invoke implements the grpc ClientConq.Invoke method @@ -153,9 +185,16 @@ func (q RPCClient) QueryABCI(ctx context.Context, req abci.RequestQuery) (abci.R Prove: req.Prove, } - result, err := q.ABCIQueryWithOptions(ctx, req.Path, req.Data, opts) - if err != nil { - return abci.ResponseQuery{}, err + var result *coretypes.ResultABCIQuery + var err error + + execErr := q.ExecuteWithFallback(ctx, func(ctx context.Context) error { + result, err = q.ABCIQueryWithOptions(ctx, req.Path, req.Data, opts) + return err + }) + + if execErr != nil { + return abci.ResponseQuery{}, execErr } if !result.Response.IsOK() { @@ -189,18 +228,190 @@ func GetQueryContext(ctx context.Context, height int64) (context.Context, contex func (q RPCClient) QueryRawCommit(ctx context.Context, height int64) ([]byte, error) { ctx, cancel := GetQueryContext(ctx, height) defer cancel() - return q.RawCommit(ctx, &height) + + var result []byte + var err error + + execErr := q.ExecuteWithFallback(ctx, func(ctx context.Context) error { + result, err = q.RawCommit(ctx, &height) + return err + }) + + if execErr != nil { + return nil, execErr + } + + return result, nil } -// QueryBlockBulk queries blocks in bulk. -func (q RPCClient) QueryBlockBulk(ctx context.Context, start int64, end int64) ([][]byte, error) { +// QueryBlockBulk queries blocks in bulk with fallback and retry logic. +func (q *RPCClient) QueryBlockBulk(ctx context.Context, start int64, end int64) ([][]byte, error) { ctx, cancel := GetQueryContext(ctx, 0) defer cancel() - return q.BlockBulk(ctx, &start, &end) + + var result [][]byte + var err error + + execErr := q.ExecuteWithFallback(ctx, func(ctx context.Context) error { + result, err = q.BlockBulk(ctx, &start, &end) + return err + }) + + if execErr != nil { + return nil, execErr + } + + return result, nil +} + +// ExecuteWithFallback executes the given function with fallback to other endpoints if it fails +func (q *RPCClient) ExecuteWithFallback(ctx context.Context, fn func(context.Context) error) error { + return q.pool.ExecuteWithFallback(ctx, func(ctx context.Context) error { + // Update HTTP client to current endpoint before executing + if err := q.updateHTTPClient(); err != nil { + return err + } + return fn(ctx) + }) +} + +// updateHTTPClient updates the HTTP client to use the current endpoint from the pool +func (q *RPCClient) updateHTTPClient() error { + // If this is a mock client (created with NewWithCaller), don't replace it + // Mock clients have empty remote and nil rpc field + if q.HTTP.Remote() == "" { + return nil + } + + currentEndpoint := q.pool.GetCurrentEndpoint() + if q.HTTP.Remote() != currentEndpoint { + // Create new HTTP client with current endpoint + client, err := clienthttp.New(currentEndpoint, "/websocket") + if err != nil { + return err + } + q.HTTP = client + } + return nil +} + +// Status returns the status of the node with fallback and retry logic +func (q *RPCClient) Status(ctx context.Context) (*coretypes.ResultStatus, error) { + var result *coretypes.ResultStatus + var err error + + execErr := q.ExecuteWithFallback(ctx, func(ctx context.Context) error { + result, err = q.HTTP.Status(ctx) + return err + }) + + if execErr != nil { + return nil, execErr + } + + return result, nil } -func (q RPCClient) QueryTx(ctx context.Context, txHash []byte) (*coretypes.ResultTx, error) { +// Block returns the block at the given height with fallback and retry logic +func (q *RPCClient) Block(ctx context.Context, height *int64) (*coretypes.ResultBlock, error) { + var result *coretypes.ResultBlock + var err error + + execErr := q.ExecuteWithFallback(ctx, func(ctx context.Context) error { + result, err = q.HTTP.Block(ctx, height) + return err + }) + + if execErr != nil { + return nil, execErr + } + + return result, nil +} + +// BlockResults returns the block results at the given height with fallback and retry logic +func (q *RPCClient) BlockResults(ctx context.Context, height *int64) (*coretypes.ResultBlockResults, error) { + var result *coretypes.ResultBlockResults + var err error + + execErr := q.ExecuteWithFallback(ctx, func(ctx context.Context) error { + result, err = q.HTTP.BlockResults(ctx, height) + return err + }) + + if execErr != nil { + return nil, execErr + } + + return result, nil +} + +func (q *RPCClient) QueryTx(ctx context.Context, txHash []byte) (*coretypes.ResultTx, error) { ctx, cancel := GetQueryContext(ctx, 0) defer cancel() - return q.Tx(ctx, txHash, false) + + var result *coretypes.ResultTx + var err error + + execErr := q.ExecuteWithFallback(ctx, func(ctx context.Context) error { + result, err = q.Tx(ctx, txHash, false) + return err + }) + + if execErr != nil { + return nil, execErr + } + + return result, nil +} + +// Tx returns the transaction with the given hash with fallback and retry logic +func (q *RPCClient) Tx(ctx context.Context, hash []byte, prove bool) (*coretypes.ResultTx, error) { + var result *coretypes.ResultTx + var err error + + execErr := q.ExecuteWithFallback(ctx, func(ctx context.Context) error { + result, err = q.HTTP.Tx(ctx, hash, prove) + return err + }) + + if execErr != nil { + return nil, execErr + } + + return result, nil +} + +// BroadcastTxSync broadcasts a transaction synchronously with fallback and retry logic +func (q *RPCClient) BroadcastTxSync(ctx context.Context, tx cmttypes.Tx) (*coretypes.ResultBroadcastTx, error) { + var result *coretypes.ResultBroadcastTx + var err error + + execErr := q.ExecuteWithFallback(ctx, func(ctx context.Context) error { + result, err = q.HTTP.BroadcastTxSync(ctx, tx) + return err + }) + + if execErr != nil { + return nil, execErr + } + + return result, nil +} + +// BroadcastTxAsync broadcasts a transaction asynchronously with fallback and retry logic +func (q *RPCClient) BroadcastTxAsync(ctx context.Context, tx cmttypes.Tx) (*coretypes.ResultBroadcastTx, error) { + var result *coretypes.ResultBroadcastTx + var err error + + execErr := q.ExecuteWithFallback(ctx, func(ctx context.Context) error { + result, err = q.HTTP.BroadcastTxAsync(ctx, tx) + return err + }) + + if execErr != nil { + return nil, execErr + } + + return result, nil } diff --git a/node/rpcclient/rpcpool.go b/node/rpcclient/rpcpool.go new file mode 100644 index 00000000..3a5c4d2e --- /dev/null +++ b/node/rpcclient/rpcpool.go @@ -0,0 +1,183 @@ +package rpcclient + +import ( + "context" + "fmt" + "math" + "os" + "strconv" + "sync" + "time" + + "github.com/pkg/errors" + "go.uber.org/zap" + + clienthttp "github.com/initia-labs/opinit-bots/client" + "github.com/cosmos/cosmos-sdk/codec" +) + +const ( + // DefaultRPCTimeout is the default timeout for RPC requests in seconds + DefaultRPCTimeout = 5 + // DefaultMaxRetries is the default maximum number of retries for RPC requests + DefaultMaxRetries = 3 +) + +// RPCPool manages multiple RPC endpoints with fallback and retry logic +type RPCPool struct { + endpoints []string + currentIndex int + mu sync.RWMutex + rpcTimeout time.Duration + logger *zap.Logger + maxRetries int + retryInterval time.Duration +} + +// NewRPCPool creates a new RPC pool with the given endpoints +func NewRPCPool(endpoints []string, logger *zap.Logger) *RPCPool { + // Get timeout from environment variable or use default + timeoutStr := os.Getenv("RPC_TIMEOUT_SECONDS") + timeout := DefaultRPCTimeout + if timeoutStr != "" { + if t, err := strconv.Atoi(timeoutStr); err == nil && t > 0 { + timeout = t + } else { + logger.Warn("Invalid RPC_TIMEOUT_SECONDS value, using default", + zap.String("value", timeoutStr), + zap.Int("default", DefaultRPCTimeout)) + } + } + + return &RPCPool{ + endpoints: endpoints, + currentIndex: 0, + mu: sync.RWMutex{}, + rpcTimeout: time.Duration(timeout) * time.Second, + logger: logger, + maxRetries: DefaultMaxRetries, + retryInterval: 1 * time.Second, + } +} + +// GetCurrentEndpoint returns the current RPC endpoint +func (p *RPCPool) GetCurrentEndpoint() string { + p.mu.RLock() + defer p.mu.RUnlock() + return p.endpoints[p.currentIndex] +} + +// MoveToNextEndpoint moves to the next RPC endpoint +func (p *RPCPool) MoveToNextEndpoint() string { + p.mu.Lock() + defer p.mu.Unlock() + p.currentIndex = (p.currentIndex + 1) % len(p.endpoints) + endpoint := p.endpoints[p.currentIndex] + p.logger.Info("Switching to next RPC endpoint", zap.String("endpoint", endpoint)) + return endpoint +} + +// ExecuteWithFallback executes the given function with fallback to other endpoints if it fails +func (p *RPCPool) ExecuteWithFallback(ctx context.Context, fn func(context.Context) error) error { + // Try all endpoints + for i := 0; i < len(p.endpoints); i++ { + currentEndpoint := p.GetCurrentEndpoint() + + // Create a timeout context + timeoutCtx, cancel := context.WithTimeout(ctx, p.rpcTimeout) + defer cancel() + + p.logger.Debug("Trying RPC endpoint", zap.String("endpoint", currentEndpoint)) + + err := fn(timeoutCtx) + if err == nil { + return nil + } + + p.logger.Warn("RPC request failed, trying next endpoint", + zap.String("endpoint", currentEndpoint), + zap.String("error", err.Error())) + + // Move to the next endpoint + p.MoveToNextEndpoint() + } + + // If all endpoints failed, retry with exponential backoff + return p.retryWithBackoff(ctx, fn) +} + +// retryWithBackoff retries the given function with exponential backoff +func (p *RPCPool) retryWithBackoff(ctx context.Context, fn func(context.Context) error) error { + var lastErr error + + for retry := 0; retry < p.maxRetries; retry++ { + // Calculate backoff duration + backoffDuration := time.Duration(math.Pow(2, float64(retry))) * p.retryInterval + + p.logger.Info("All RPC endpoints failed, retrying after backoff", + zap.Duration("backoff", backoffDuration), + zap.Int("retry", retry+1), + zap.Int("max_retries", p.maxRetries)) + + // Wait for backoff duration + select { + case <-ctx.Done(): + return ctx.Err() + case <-time.After(backoffDuration): + } + + // Try all endpoints again + for i := 0; i < len(p.endpoints); i++ { + currentEndpoint := p.GetCurrentEndpoint() + + // Create a timeout context + timeoutCtx, cancel := context.WithTimeout(ctx, p.rpcTimeout) + defer cancel() + + p.logger.Debug("Retrying RPC endpoint", + zap.String("endpoint", currentEndpoint), + zap.Int("retry", retry+1)) + + err := fn(timeoutCtx) + if err == nil { + return nil + } + + lastErr = err + p.logger.Warn("RPC request failed during retry, trying next endpoint", + zap.String("endpoint", currentEndpoint), + zap.String("error", err.Error()), + zap.Int("retry", retry+1)) + + // Move to the next endpoint + p.MoveToNextEndpoint() + } + } + + return fmt.Errorf("all RPC endpoints failed after %d retries: %w", p.maxRetries, lastErr) +} + +// CreateRPCClient creates a new RPC client with the given codec and RPC addresses +func CreateRPCClient(cdc codec.Codec, rpcAddresses []string, logger *zap.Logger) (*RPCClient, error) { + if len(rpcAddresses) == 0 { + return nil, errors.New("no RPC addresses provided") + } + + // Create RPC pool + pool := NewRPCPool(rpcAddresses, logger) + + // Create HTTP client with the first endpoint + client, err := clienthttp.New(pool.GetCurrentEndpoint(), "/websocket") + if err != nil { + return nil, err + } + + // Create RPC client + rpcClient := &RPCClient{ + HTTP: client, + cdc: cdc, + pool: pool, + } + + return rpcClient, nil +} \ No newline at end of file diff --git a/node/rpcclient/rpcpool_test.go b/node/rpcclient/rpcpool_test.go new file mode 100644 index 00000000..9cb7854c --- /dev/null +++ b/node/rpcclient/rpcpool_test.go @@ -0,0 +1,195 @@ +package rpcclient + +import ( + "context" + "errors" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "go.uber.org/zap" + "go.uber.org/zap/zaptest" + "go.uber.org/zap/zaptest/observer" +) + +func TestRPCPool_GetCurrentEndpoint(t *testing.T) { + logger := zaptest.NewLogger(t) + endpoints := []string{"doi", "moro", "rene"} + pool := NewRPCPool(endpoints, logger) + + // Initial endpoint should be the first one + assert.Equal(t, "doi", pool.GetCurrentEndpoint()) +} + +func TestRPCPool_MoveToNextEndpoint(t *testing.T) { + logger := zaptest.NewLogger(t) + endpoints := []string{"doi", "moro", "rene"} + pool := NewRPCPool(endpoints, logger) + + // Move to next endpoint + assert.Equal(t, "moro", pool.MoveToNextEndpoint()) + assert.Equal(t, "moro", pool.GetCurrentEndpoint()) + + // Move to next endpoint again + assert.Equal(t, "rene", pool.MoveToNextEndpoint()) + assert.Equal(t, "rene", pool.GetCurrentEndpoint()) + + // Move to next endpoint should wrap around + assert.Equal(t, "doi", pool.MoveToNextEndpoint()) + assert.Equal(t, "doi", pool.GetCurrentEndpoint()) +} + +func TestRPCPool_ExecuteWithFallback_Success(t *testing.T) { + logger := zaptest.NewLogger(t) + endpoints := []string{"doi", "moro", "rene"} + pool := NewRPCPool(endpoints, logger) + + // Function succeeds on first try + callCount := 0 + err := pool.ExecuteWithFallback(context.Background(), func(ctx context.Context) error { + callCount++ + return nil + }) + + assert.NoError(t, err) + assert.Equal(t, 1, callCount) + assert.Equal(t, "doi", pool.GetCurrentEndpoint()) +} + +func TestRPCPool_ExecuteWithFallback_FallbackSuccess(t *testing.T) { + logger := zaptest.NewLogger(t) + endpoints := []string{"doi", "moro", "rene"} + pool := NewRPCPool(endpoints, logger) + + // Function fails on first endpoint, succeeds on second + callCount := 0 + err := pool.ExecuteWithFallback(context.Background(), func(ctx context.Context) error { + callCount++ + if callCount == 1 { + return errors.New("first endpoint failed") + } + return nil + }) + + assert.NoError(t, err) + assert.Equal(t, 2, callCount) + assert.Equal(t, "moro", pool.GetCurrentEndpoint()) +} + +func TestRPCPool_ExecuteWithFallback_AllFail(t *testing.T) { + logger := zaptest.NewLogger(t) + endpoints := []string{"doi", "moro"} + pool := NewRPCPool(endpoints, logger) + pool.maxRetries = 1 // Set to 1 for faster test + + // All endpoints fail + callCount := 0 + err := pool.ExecuteWithFallback(context.Background(), func(ctx context.Context) error { + callCount++ + return errors.New("endpoint failed") + }) + + assert.Error(t, err) + assert.Contains(t, err.Error(), "all RPC endpoints failed after 1 retries") + // 2 endpoints + 2 more for 1 retry = 4 calls + assert.Equal(t, 4, callCount) +} + +func TestRPCPool_ExecuteWithFallback_Timeout(t *testing.T) { + logger := zaptest.NewLogger(t) + endpoints := []string{"doi"} + pool := NewRPCPool(endpoints, logger) + pool.rpcTimeout = 100 * time.Millisecond + + // Function takes too long + err := pool.ExecuteWithFallback(context.Background(), func(ctx context.Context) error { + select { + case <-ctx.Done(): + return ctx.Err() + case <-time.After(200 * time.Millisecond): + return nil + } + }) + + assert.Error(t, err) + assert.Contains(t, err.Error(), "context deadline exceeded") +} + +func TestRPCPool_ExecuteWithFallback_RetrySuccess(t *testing.T) { + logger := zaptest.NewLogger(t) + endpoints := []string{"doi"} + pool := NewRPCPool(endpoints, logger) + pool.maxRetries = 2 + pool.retryInterval = 10 * time.Millisecond + + // Function fails on first try, succeeds on retry + callCount := 0 + err := pool.ExecuteWithFallback(context.Background(), func(ctx context.Context) error { + callCount++ + if callCount <= 1 { + return errors.New("first try failed") + } + return nil + }) + + assert.NoError(t, err) + assert.Equal(t, 2, callCount) +} + +func TestRPCPool_ExecuteWithFallback_ContextCancellation(t *testing.T) { + logger := zaptest.NewLogger(t) + endpoints := []string{"doi", "moro"} + pool := NewRPCPool(endpoints, logger) + + // Create a context that will be cancelled + ctx, cancel := context.WithCancel(context.Background()) + + // Cancel the context after a short delay + go func() { + time.Sleep(50 * time.Millisecond) + cancel() + }() + + // Function should return context cancelled error + err := pool.ExecuteWithFallback(ctx, func(ctx context.Context) error { + select { + case <-ctx.Done(): + return ctx.Err() + case <-time.After(200 * time.Millisecond): + return nil + } + }) + + assert.Error(t, err) + assert.Contains(t, err.Error(), "context canceled") +} + +func TestRPCPool_Logging(t *testing.T) { + // Create a logger that captures logs + core, recorded := observer.New(zap.InfoLevel) + logger := zap.New(core) + + endpoints := []string{"doi", "moro"} + pool := NewRPCPool(endpoints, logger) + + // Function fails on first endpoint, succeeds on second + _ = pool.ExecuteWithFallback(context.Background(), func(ctx context.Context) error { + if pool.GetCurrentEndpoint() == "doi" { + return errors.New("first endpoint failed") + } + return nil + }) + + // Check that the failure was logged + logs := recorded.All() + assert.True(t, len(logs) > 0) + + foundFailureLog := false + for _, log := range logs { + if log.Message == "RPC request failed, trying next endpoint" { + foundFailureLog = true + break + } + } + assert.True(t, foundFailureLog, "Should have logged endpoint failure") +} diff --git a/node/types/config.go b/node/types/config.go index 3cb6cf05..373638c0 100644 --- a/node/types/config.go +++ b/node/types/config.go @@ -19,7 +19,7 @@ type NodeConfig struct { ChainID string // RPC is the RPC address of the chain. - RPC string + RPC []string // BlockProcessType is the type of block process. ProcessType BlockProcessType @@ -36,7 +36,7 @@ func (nc NodeConfig) Validate() error { return fmt.Errorf("chain ID is empty") } - if nc.RPC == "" { + if len(nc.RPC) == 0 { return fmt.Errorf("rpc is empty") } From 0bb912aa478c67a8796c9a49faacc5f9864c4a53 Mon Sep 17 00:00:00 2001 From: SeUkKim Date: Thu, 24 Jul 2025 17:34:04 +0900 Subject: [PATCH 02/23] chore: fix typo --- node/rpcclient/rpcpool_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/node/rpcclient/rpcpool_test.go b/node/rpcclient/rpcpool_test.go index 9cb7854c..cd897b64 100644 --- a/node/rpcclient/rpcpool_test.go +++ b/node/rpcclient/rpcpool_test.go @@ -141,7 +141,7 @@ func TestRPCPool_ExecuteWithFallback_ContextCancellation(t *testing.T) { endpoints := []string{"doi", "moro"} pool := NewRPCPool(endpoints, logger) - // Create a context that will be cancelled + // Create a context that will be canceled ctx, cancel := context.WithCancel(context.Background()) // Cancel the context after a short delay @@ -150,7 +150,7 @@ func TestRPCPool_ExecuteWithFallback_ContextCancellation(t *testing.T) { cancel() }() - // Function should return context cancelled error + // Function should return context canceled error err := pool.ExecuteWithFallback(ctx, func(ctx context.Context) error { select { case <-ctx.Done(): From 46f740a015495754ef4d4855e2e9012965b09320 Mon Sep 17 00:00:00 2001 From: SeUkKim Date: Thu, 24 Jul 2025 17:40:02 +0900 Subject: [PATCH 03/23] chore: sort imports. --- node/rpcclient/rpcpool.go | 68 +++++++++++++++++++-------------------- 1 file changed, 34 insertions(+), 34 deletions(-) diff --git a/node/rpcclient/rpcpool.go b/node/rpcclient/rpcpool.go index 3a5c4d2e..7ca61ec8 100644 --- a/node/rpcclient/rpcpool.go +++ b/node/rpcclient/rpcpool.go @@ -12,8 +12,8 @@ import ( "github.com/pkg/errors" "go.uber.org/zap" - clienthttp "github.com/initia-labs/opinit-bots/client" "github.com/cosmos/cosmos-sdk/codec" + clienthttp "github.com/initia-labs/opinit-bots/client" ) const ( @@ -43,8 +43,8 @@ func NewRPCPool(endpoints []string, logger *zap.Logger) *RPCPool { if t, err := strconv.Atoi(timeoutStr); err == nil && t > 0 { timeout = t } else { - logger.Warn("Invalid RPC_TIMEOUT_SECONDS value, using default", - zap.String("value", timeoutStr), + logger.Warn("Invalid RPC_TIMEOUT_SECONDS value, using default", + zap.String("value", timeoutStr), zap.Int("default", DefaultRPCTimeout)) } } @@ -82,26 +82,26 @@ func (p *RPCPool) ExecuteWithFallback(ctx context.Context, fn func(context.Conte // Try all endpoints for i := 0; i < len(p.endpoints); i++ { currentEndpoint := p.GetCurrentEndpoint() - + // Create a timeout context timeoutCtx, cancel := context.WithTimeout(ctx, p.rpcTimeout) defer cancel() - + p.logger.Debug("Trying RPC endpoint", zap.String("endpoint", currentEndpoint)) - + err := fn(timeoutCtx) if err == nil { return nil } - - p.logger.Warn("RPC request failed, trying next endpoint", - zap.String("endpoint", currentEndpoint), + + p.logger.Warn("RPC request failed, trying next endpoint", + zap.String("endpoint", currentEndpoint), zap.String("error", err.Error())) - + // Move to the next endpoint p.MoveToNextEndpoint() } - + // If all endpoints failed, retry with exponential backoff return p.retryWithBackoff(ctx, fn) } @@ -109,51 +109,51 @@ func (p *RPCPool) ExecuteWithFallback(ctx context.Context, fn func(context.Conte // retryWithBackoff retries the given function with exponential backoff func (p *RPCPool) retryWithBackoff(ctx context.Context, fn func(context.Context) error) error { var lastErr error - + for retry := 0; retry < p.maxRetries; retry++ { // Calculate backoff duration backoffDuration := time.Duration(math.Pow(2, float64(retry))) * p.retryInterval - - p.logger.Info("All RPC endpoints failed, retrying after backoff", - zap.Duration("backoff", backoffDuration), - zap.Int("retry", retry+1), + + p.logger.Info("All RPC endpoints failed, retrying after backoff", + zap.Duration("backoff", backoffDuration), + zap.Int("retry", retry+1), zap.Int("max_retries", p.maxRetries)) - + // Wait for backoff duration select { case <-ctx.Done(): return ctx.Err() case <-time.After(backoffDuration): } - + // Try all endpoints again for i := 0; i < len(p.endpoints); i++ { currentEndpoint := p.GetCurrentEndpoint() - + // Create a timeout context timeoutCtx, cancel := context.WithTimeout(ctx, p.rpcTimeout) defer cancel() - - p.logger.Debug("Retrying RPC endpoint", - zap.String("endpoint", currentEndpoint), + + p.logger.Debug("Retrying RPC endpoint", + zap.String("endpoint", currentEndpoint), zap.Int("retry", retry+1)) - + err := fn(timeoutCtx) if err == nil { return nil } - + lastErr = err - p.logger.Warn("RPC request failed during retry, trying next endpoint", - zap.String("endpoint", currentEndpoint), - zap.String("error", err.Error()), + p.logger.Warn("RPC request failed during retry, trying next endpoint", + zap.String("endpoint", currentEndpoint), + zap.String("error", err.Error()), zap.Int("retry", retry+1)) - + // Move to the next endpoint p.MoveToNextEndpoint() } } - + return fmt.Errorf("all RPC endpoints failed after %d retries: %w", p.maxRetries, lastErr) } @@ -162,22 +162,22 @@ func CreateRPCClient(cdc codec.Codec, rpcAddresses []string, logger *zap.Logger) if len(rpcAddresses) == 0 { return nil, errors.New("no RPC addresses provided") } - + // Create RPC pool pool := NewRPCPool(rpcAddresses, logger) - + // Create HTTP client with the first endpoint client, err := clienthttp.New(pool.GetCurrentEndpoint(), "/websocket") if err != nil { return nil, err } - + // Create RPC client rpcClient := &RPCClient{ HTTP: client, cdc: cdc, pool: pool, } - + return rpcClient, nil -} \ No newline at end of file +} From a4478971149bfc1c148627f7d0370825a6782757 Mon Sep 17 00:00:00 2001 From: SeUkKim Date: Fri, 25 Jul 2025 02:18:18 +0900 Subject: [PATCH 04/23] wip(rpc): apply feedbacks --- challenger/challenger.go | 33 +++++- cmd/opinitd/db.go | 2 +- cmd/opinitd/tx.go | 6 +- executor/batchsubmitter/batch_test.go | 6 +- executor/batchsubmitter/handler_test.go | 3 +- executor/executor.go | 12 ++ node/node.go | 45 ++++---- node/rpcclient/client.go | 26 ++--- node/rpcclient/rpcpool.go | 144 ++++++++++++++++-------- provider/child/child.go | 40 ++++++- provider/host/host.go | 41 ++++++- 11 files changed, 258 insertions(+), 100 deletions(-) diff --git a/challenger/challenger.go b/challenger/challenger.go index ed37a206..dd3d59f6 100644 --- a/challenger/challenger.go +++ b/challenger/challenger.go @@ -84,6 +84,12 @@ func NewChallenger(cfg *challengertypes.Config, db types.DB, sv *server.Server) } func (c *Challenger) Initialize(ctx types.Context) error { + // Initialize the child's query client first so we can query bridge info + err := c.child.InitializeQueryClient(ctx) + if err != nil { + return errors.Wrap(err, "failed to initialize child query client") + } + childBridgeInfo, err := c.child.QueryBridgeInfo(ctx) if err != nil { return err @@ -92,6 +98,12 @@ func (c *Challenger) Initialize(ctx types.Context) error { return errors.New("bridge info is not set") } + // Initialize the host's query client so we can query bridge config + err = c.host.InitializeQueryClient(ctx) + if err != nil { + return errors.Wrap(err, "failed to initialize host query client") + } + bridgeInfo, err := c.host.QueryBridgeConfig(ctx, childBridgeInfo.BridgeId) if err != nil { return err @@ -179,8 +191,25 @@ func (c *Challenger) Start(ctx types.Context) error { return c.challengeHandler(ctx) }) - c.host.Start(ctx) - c.child.Start(ctx) + // Start components in separate goroutines to ensure proper shutdown handling + ctx.ErrGrp().Go(func() (err error) { + defer func() { + ctx.Logger().Info("host stopped") + }() + c.host.Start(ctx) + <-ctx.Done() + return nil + }) + + ctx.ErrGrp().Go(func() (err error) { + defer func() { + ctx.Logger().Info("child stopped") + }() + c.child.Start(ctx) + <-ctx.Done() + return nil + }) + return ctx.ErrGrp().Wait() } diff --git a/cmd/opinitd/db.go b/cmd/opinitd/db.go index 56eb322c..b75dddb0 100644 --- a/cmd/opinitd/db.go +++ b/cmd/opinitd/db.go @@ -79,7 +79,7 @@ v0.1.9-2: Fill block hash of finalized tree return err } - rpcClient, err := rpcclient.NewRPCClient(cdc, l2Config.RPC) + rpcClient, err := rpcclient.NewRPCClient(cdc, l2Config.RPC, baseCtx.Logger().Named("migration-rpcclient")) if err != nil { return err } diff --git a/cmd/opinitd/tx.go b/cmd/opinitd/tx.go index 86d26f0f..1ed03d66 100644 --- a/cmd/opinitd/tx.go +++ b/cmd/opinitd/tx.go @@ -226,7 +226,7 @@ func QueryBridgeId(ctx types.Context, cfg *executortypes.Config) (uint64, error) return 0, err } - l2RpcClient, err := rpcclient.NewRPCClient(cdc, l2Config.RPC) + l2RpcClient, err := rpcclient.NewRPCClient(cdc, l2Config.RPC, ctx.Logger().Named("l2-rpcclient")) if err != nil { return 0, err } @@ -247,7 +247,7 @@ func l1ProposerAccount(ctx types.Context, cfg *executortypes.Config, bridgeId ui return nil, err } - rpcClient, err := rpcclient.NewRPCClient(cdc, l1Config.RPC) + rpcClient, err := rpcclient.NewRPCClient(cdc, l1Config.RPC, ctx.Logger().Named("l1-rpcclient")) if err != nil { return nil, err } @@ -272,7 +272,7 @@ func l2BroadcasterAccount(ctx types.Context, cfg *executortypes.Config) (*broadc return nil, err } - rpcClient, err := rpcclient.NewRPCClient(cdc, l2Config.RPC) + rpcClient, err := rpcclient.NewRPCClient(cdc, l2Config.RPC, ctx.Logger().Named("l2-rpcclient")) if err != nil { return nil, err } diff --git a/executor/batchsubmitter/batch_test.go b/executor/batchsubmitter/batch_test.go index 13f2ba77..c750f97a 100644 --- a/executor/batchsubmitter/batch_test.go +++ b/executor/batchsubmitter/batch_test.go @@ -272,7 +272,8 @@ func TestFinalizeBatch(t *testing.T) { require.NoError(t, err) mockCaller := mockclient.NewMockCaller() - rpcClient, err := rpcclient.NewRPCClientWithClient(appCodec, client.NewWithCaller(mockCaller), []string{"http://localhost:26657"}) + testLogger, _ := zap.NewDevelopment() + rpcClient, err := rpcclient.NewRPCClientWithClient(appCodec, client.NewWithCaller(mockCaller), []string{"http://localhost:26657"}, testLogger) require.NoError(t, err) batchNode := node.NewTestNode(nodetypes.NodeConfig{}, batchDB, appCodec, txConfig, rpcClient, nil) @@ -754,7 +755,8 @@ func TestSubmitGenesis(t *testing.T) { require.NoError(t, err) mockCaller := mockclient.NewMockCaller() - rpcClient, err := rpcclient.NewRPCClientWithClient(appCodec, client.NewWithCaller(mockCaller), []string{"http://localhost:26657"}) + testLogger, _ := zap.NewDevelopment() + rpcClient, err := rpcclient.NewRPCClientWithClient(appCodec, client.NewWithCaller(mockCaller), []string{"http://localhost:26657"}, testLogger) require.NoError(t, err) batchNode := node.NewTestNode(nodetypes.NodeConfig{}, batchDB, appCodec, txConfig, rpcClient, nil) diff --git a/executor/batchsubmitter/handler_test.go b/executor/batchsubmitter/handler_test.go index b07281dd..92b0ab55 100644 --- a/executor/batchsubmitter/handler_test.go +++ b/executor/batchsubmitter/handler_test.go @@ -46,7 +46,8 @@ func TestRawBlockHandler(t *testing.T) { require.NoError(t, err) mockCaller := mockclient.NewMockCaller() - rpcClient, err := rpcclient.NewRPCClientWithClient(appCodec, client.NewWithCaller(mockCaller), []string{"http://localhost:26657"}) + testLogger, _ := zap.NewDevelopment() + rpcClient, err := rpcclient.NewRPCClientWithClient(appCodec, client.NewWithCaller(mockCaller), []string{"http://localhost:26657"}, testLogger) require.NoError(t, err) batchNode := node.NewTestNode(nodetypes.NodeConfig{}, batchDB, appCodec, txConfig, rpcClient, nil) diff --git a/executor/executor.go b/executor/executor.go index c7c1da09..abcbafdf 100644 --- a/executor/executor.go +++ b/executor/executor.go @@ -70,6 +70,12 @@ func NewExecutor(cfg *executortypes.Config, db types.DB, sv *server.Server) *Exe } func (ex *Executor) Initialize(ctx types.Context) error { + // Initialize the child's query client first so we can query bridge info + err := ex.child.InitializeQueryClient(ctx) + if err != nil { + return errors.Wrap(err, "failed to initialize child query client") + } + childBridgeInfo, err := ex.child.QueryBridgeInfo(ctx) if err != nil { return err @@ -78,6 +84,12 @@ func (ex *Executor) Initialize(ctx types.Context) error { return errors.New("bridge info is not set") } + // Initialize the host's query client so we can query bridge config + err = ex.host.InitializeQueryClient(ctx) + if err != nil { + return errors.Wrap(err, "failed to initialize host query client") + } + bridgeInfo, err := ex.host.QueryBridgeConfig(ctx, childBridgeInfo.BridgeId) if err != nil { return err diff --git a/node/node.go b/node/node.go index 64ab7bab..de99d376 100644 --- a/node/node.go +++ b/node/node.go @@ -49,14 +49,7 @@ func NewNode(cfg nodetypes.NodeConfig, db types.DB, cdc codec.Codec, txConfig cl return nil, err } - rpcClient, err := rpcclient.NewRPCClient(cdc, cfg.RPC) - if err != nil { - return nil, errors.Wrap(err, "failed to create RPC client") - } - n := &Node{ - rpcClient: rpcClient, - cfg: cfg, db: db, @@ -68,19 +61,6 @@ func NewNode(cfg nodetypes.NodeConfig, db types.DB, cdc codec.Codec, txConfig cl startOnce: &sync.Once{}, syncing: true, } - // create broadcaster - if n.cfg.BroadcasterConfig != nil { - n.broadcaster, err = broadcaster.NewBroadcaster( - *n.cfg.BroadcasterConfig, - n.db, - n.cdc, - n.txConfig, - rpcClient, - ) - if err != nil { - return nil, errors.Wrap(err, "failed to create broadcaster") - } - } syncedHeight, err := GetSyncInfo(n.db) if errors.Is(err, dbtypes.ErrNotFound) { @@ -96,6 +76,28 @@ func NewNode(cfg nodetypes.NodeConfig, db types.DB, cdc codec.Codec, txConfig cl // If it is 0, the latest height is used. // If the latest height exists in the database, this is ignored. func (n *Node) Initialize(ctx types.Context, processedHeight int64, keyringConfig []btypes.KeyringConfig) (err error) { + // Create RPC client with shared logger from context + if n.rpcClient == nil { + n.rpcClient, err = rpcclient.NewRPCClient(n.cdc, n.cfg.RPC, ctx.Logger().Named("rpcclient")) + if err != nil { + return errors.Wrap(err, "failed to create RPC client") + } + } + + // Create broadcaster if needed + if n.cfg.BroadcasterConfig != nil && n.broadcaster == nil { + n.broadcaster, err = broadcaster.NewBroadcaster( + *n.cfg.BroadcasterConfig, + n.db, + n.cdc, + n.txConfig, + n.rpcClient, + ) + if err != nil { + return errors.Wrap(err, "failed to create broadcaster") + } + } + // check if node is catching up status, err := n.rpcClient.Status(ctx) if err != nil { @@ -233,6 +235,9 @@ func (n Node) MustGetBroadcaster() *broadcaster.Broadcaster { } func (n Node) GetRPCClient() *rpcclient.RPCClient { + if n.rpcClient == nil { + panic("RPC client not initialized - call Initialize() first") + } return n.rpcClient } diff --git a/node/rpcclient/client.go b/node/rpcclient/client.go index 80103264..efe546cc 100644 --- a/node/rpcclient/client.go +++ b/node/rpcclient/client.go @@ -41,20 +41,14 @@ type RPCClient struct { pool *RPCPool } -func NewRPCClient(cdc codec.Codec, rpcAddrs []string) (*RPCClient, error) { +func NewRPCClient(cdc codec.Codec, rpcAddrs []string, logger *zap.Logger) (*RPCClient, error) { if len(rpcAddrs) == 0 { return nil, errors.New("no RPC addresses provided") } - // Create logger - logger, err := zap.NewProduction() - if err != nil { - return nil, errors.Wrap(err, "failed to create logger") - } - // Create RPC pool pool := NewRPCPool(rpcAddrs, logger) - + // Create HTTP client with the first endpoint client, err := clienthttp.New(pool.GetCurrentEndpoint(), "/websocket") if err != nil { @@ -68,17 +62,11 @@ func NewRPCClient(cdc codec.Codec, rpcAddrs []string) (*RPCClient, error) { }, nil } -func NewRPCClientWithClient(cdc codec.Codec, client *clienthttp.HTTP, endpoints []string) (*RPCClient, error) { +func NewRPCClientWithClient(cdc codec.Codec, client *clienthttp.HTTP, endpoints []string, logger *zap.Logger) (*RPCClient, error) { if len(endpoints) == 0 { return nil, errors.New("no RPC endpoints provided") } - // Create logger - logger, err := zap.NewProduction() - if err != nil { - return nil, errors.Wrap(err, "failed to create logger") - } - // Create RPC pool pool := NewRPCPool(endpoints, logger) @@ -228,7 +216,7 @@ func GetQueryContext(ctx context.Context, height int64) (context.Context, contex func (q RPCClient) QueryRawCommit(ctx context.Context, height int64) ([]byte, error) { ctx, cancel := GetQueryContext(ctx, height) defer cancel() - + var result []byte var err error @@ -248,7 +236,7 @@ func (q RPCClient) QueryRawCommit(ctx context.Context, height int64) ([]byte, er func (q *RPCClient) QueryBlockBulk(ctx context.Context, start int64, end int64) ([][]byte, error) { ctx, cancel := GetQueryContext(ctx, 0) defer cancel() - + var result [][]byte var err error @@ -282,7 +270,7 @@ func (q *RPCClient) updateHTTPClient() error { if q.HTTP.Remote() == "" { return nil } - + currentEndpoint := q.pool.GetCurrentEndpoint() if q.HTTP.Remote() != currentEndpoint { // Create new HTTP client with current endpoint @@ -349,7 +337,7 @@ func (q *RPCClient) BlockResults(ctx context.Context, height *int64) (*coretypes func (q *RPCClient) QueryTx(ctx context.Context, txHash []byte) (*coretypes.ResultTx, error) { ctx, cancel := GetQueryContext(ctx, 0) defer cancel() - + var result *coretypes.ResultTx var err error diff --git a/node/rpcclient/rpcpool.go b/node/rpcclient/rpcpool.go index 7ca61ec8..cf656650 100644 --- a/node/rpcclient/rpcpool.go +++ b/node/rpcclient/rpcpool.go @@ -14,13 +14,12 @@ import ( "github.com/cosmos/cosmos-sdk/codec" clienthttp "github.com/initia-labs/opinit-bots/client" + "github.com/initia-labs/opinit-bots/types" ) const ( // DefaultRPCTimeout is the default timeout for RPC requests in seconds DefaultRPCTimeout = 5 - // DefaultMaxRetries is the default maximum number of retries for RPC requests - DefaultMaxRetries = 3 ) // RPCPool manages multiple RPC endpoints with fallback and retry logic @@ -36,6 +35,10 @@ type RPCPool struct { // NewRPCPool creates a new RPC pool with the given endpoints func NewRPCPool(endpoints []string, logger *zap.Logger) *RPCPool { + if len(endpoints) == 0 { + panic("endpoints slice cannot be empty") + } + // Get timeout from environment variable or use default timeoutStr := os.Getenv("RPC_TIMEOUT_SECONDS") timeout := DefaultRPCTimeout @@ -55,7 +58,7 @@ func NewRPCPool(endpoints []string, logger *zap.Logger) *RPCPool { mu: sync.RWMutex{}, rpcTimeout: time.Duration(timeout) * time.Second, logger: logger, - maxRetries: DefaultMaxRetries, + maxRetries: types.MaxRetryCount, retryInterval: 1 * time.Second, } } @@ -77,39 +80,112 @@ func (p *RPCPool) MoveToNextEndpoint() string { return endpoint } -// ExecuteWithFallback executes the given function with fallback to other endpoints if it fails -func (p *RPCPool) ExecuteWithFallback(ctx context.Context, fn func(context.Context) error) error { - // Try all endpoints - for i := 0; i < len(p.endpoints); i++ { - currentEndpoint := p.GetCurrentEndpoint() +// getCurrentIndex returns the current index (thread-safe) +func (p *RPCPool) getCurrentIndex() int { + p.mu.RLock() + defer p.mu.RUnlock() + return p.currentIndex +} + +// setCurrentIndex sets the current index (thread-safe) +func (p *RPCPool) setCurrentIndex(index int) { + p.mu.Lock() + defer p.mu.Unlock() + p.currentIndex = index +} + +// tryAllEndpoints tries the given function on all endpoints once +// Returns nil on first success or the last encountered error if all endpoints fail +func (p *RPCPool) tryAllEndpoints(ctx context.Context, fn func(context.Context) error, retryAttempt int) error { + // Try current endpoint first (which should be the last successful one) + currentEndpoint := p.GetCurrentEndpoint() + startIndex := p.getCurrentIndex() + + // Create a timeout context + timeoutCtx, cancel := context.WithTimeout(ctx, p.rpcTimeout) + + if retryAttempt == 0 { + p.logger.Debug("Trying RPC endpoint", zap.String("endpoint", currentEndpoint)) + } else { + p.logger.Debug("Retrying RPC endpoint", + zap.String("endpoint", currentEndpoint), + zap.Int("retry", retryAttempt)) + } + + err := fn(timeoutCtx) + cancel() + if err == nil { + // Current endpoint worked, no need to try others + return nil + } + + var lastErr = err + if retryAttempt == 0 { + p.logger.Warn("RPC request failed, trying next endpoint", + zap.String("endpoint", currentEndpoint), + zap.String("error", err.Error())) + } else { + p.logger.Warn("RPC request failed during retry, trying next endpoint", + zap.String("endpoint", currentEndpoint), + zap.String("error", err.Error()), + zap.Int("retry", retryAttempt)) + } + + // Current endpoint failed, try the remaining endpoints + for i := 1; i < len(p.endpoints); i++ { + p.MoveToNextEndpoint() + currentEndpoint = p.GetCurrentEndpoint() // Create a timeout context timeoutCtx, cancel := context.WithTimeout(ctx, p.rpcTimeout) - defer cancel() - p.logger.Debug("Trying RPC endpoint", zap.String("endpoint", currentEndpoint)) + if retryAttempt == 0 { + p.logger.Debug("Trying RPC endpoint", zap.String("endpoint", currentEndpoint)) + } else { + p.logger.Debug("Retrying RPC endpoint", + zap.String("endpoint", currentEndpoint), + zap.Int("retry", retryAttempt)) + } err := fn(timeoutCtx) + cancel() if err == nil { + // This endpoint worked, keep it as current for future requests return nil } - p.logger.Warn("RPC request failed, trying next endpoint", - zap.String("endpoint", currentEndpoint), - zap.String("error", err.Error())) + lastErr = err + if retryAttempt == 0 { + p.logger.Warn("RPC request failed, trying next endpoint", + zap.String("endpoint", currentEndpoint), + zap.String("error", err.Error())) + } else { + p.logger.Warn("RPC request failed during retry, trying next endpoint", + zap.String("endpoint", currentEndpoint), + zap.String("error", err.Error()), + zap.Int("retry", retryAttempt)) + } + } - // Move to the next endpoint - p.MoveToNextEndpoint() + // Reset to original position if all endpoints failed + if retryAttempt == 0 { + p.setCurrentIndex(startIndex) } - // If all endpoints failed, retry with exponential backoff - return p.retryWithBackoff(ctx, fn) + return lastErr } -// retryWithBackoff retries the given function with exponential backoff -func (p *RPCPool) retryWithBackoff(ctx context.Context, fn func(context.Context) error) error { - var lastErr error +// ExecuteWithFallback executes the given function with fallback to other endpoints if it fails +// and retries with exponential backoff if all endpoints fail +func (p *RPCPool) ExecuteWithFallback(ctx context.Context, fn func(context.Context) error) error { + // First attempt: try all endpoints once + err := p.tryAllEndpoints(ctx, fn, 0) + if err == nil { + return nil + } + // If all endpoints failed, retry with exponential backoff + var lastErr error for retry := 0; retry < p.maxRetries; retry++ { // Calculate backoff duration backoffDuration := time.Duration(math.Pow(2, float64(retry))) * p.retryInterval @@ -127,31 +203,11 @@ func (p *RPCPool) retryWithBackoff(ctx context.Context, fn func(context.Context) } // Try all endpoints again - for i := 0; i < len(p.endpoints); i++ { - currentEndpoint := p.GetCurrentEndpoint() - - // Create a timeout context - timeoutCtx, cancel := context.WithTimeout(ctx, p.rpcTimeout) - defer cancel() - - p.logger.Debug("Retrying RPC endpoint", - zap.String("endpoint", currentEndpoint), - zap.Int("retry", retry+1)) - - err := fn(timeoutCtx) - if err == nil { - return nil - } - - lastErr = err - p.logger.Warn("RPC request failed during retry, trying next endpoint", - zap.String("endpoint", currentEndpoint), - zap.String("error", err.Error()), - zap.Int("retry", retry+1)) - - // Move to the next endpoint - p.MoveToNextEndpoint() + err := p.tryAllEndpoints(ctx, fn, retry+1) + if err == nil { + return nil } + lastErr = err } return fmt.Errorf("all RPC endpoints failed after %d retries: %w", p.maxRetries, lastErr) diff --git a/provider/child/child.go b/provider/child/child.go index 1de7740f..7ecff81b 100644 --- a/provider/child/child.go +++ b/provider/child/child.go @@ -76,7 +76,7 @@ func NewBaseChildV1( cfg: cfg, - opchildQueryClient: opchildtypes.NewQueryClient(node.GetRPCClient()), + // opchildQueryClient will be initialized in Initialize() method after node is initialized processedMsgs: make([]btypes.ProcessedMsgs, 0), msgQueue: make(map[string][]sdk.Msg), @@ -101,6 +101,25 @@ func GetCodec(bech32Prefix string) (codec.Codec, client.TxConfig, error) { return codec, txConfig, err } +// InitializeQueryClient initializes just the RPC client and query client for basic queries +// This should be called before QueryBridgeInfo is needed, before full initialization +func (b *BaseChild) InitializeQueryClient(ctx types.Context) error { + if b.opchildQueryClient != nil { + return nil // Already initialized + } + + // Initialize the node's RPC client with minimal configuration + err := b.node.Initialize(ctx, 0, []btypes.KeyringConfig{}) + if err != nil { + return errors.Wrap(err, "failed to initialize node for query client") + } + + // Initialize the opchild query client + b.opchildQueryClient = opchildtypes.NewQueryClient(b.node.GetRPCClient()) + + return nil +} + // Initialize initializes the child node. // if the synced height of the node is initialized, it will delete the future working trees and set initializeTreeFn. func (b *BaseChild) Initialize( @@ -114,9 +133,22 @@ func (b *BaseChild) Initialize( ) (uint64, error) { b.SetBridgeInfo(bridgeInfo) - err := b.node.Initialize(ctx, processedHeight, b.keyringConfigs(keyringConfig, oracleKeyringConfig)) - if err != nil { - return 0, err + // Initialize the node if not already done by InitializeQueryClient + var err error + if b.opchildQueryClient == nil { + err = b.node.Initialize(ctx, processedHeight, b.keyringConfigs(keyringConfig, oracleKeyringConfig)) + if err != nil { + return 0, err + } + + // Initialize the opchild query client after the node's RPC client is ready + b.opchildQueryClient = opchildtypes.NewQueryClient(b.node.GetRPCClient()) + } else { + // Node was already initialized by InitializeQueryClient, just reinitialize with proper config + err = b.node.Initialize(ctx, processedHeight, b.keyringConfigs(keyringConfig, oracleKeyringConfig)) + if err != nil { + return 0, err + } } var l2Sequence uint64 diff --git a/provider/host/host.go b/provider/host/host.go index 4cda29f4..8cb8378c 100644 --- a/provider/host/host.go +++ b/provider/host/host.go @@ -53,7 +53,7 @@ func NewBaseHostV1(cfg nodetypes.NodeConfig, db types.DB) *BaseHost { cfg: cfg, - ophostQueryClient: ophosttypes.NewQueryClient(node.GetRPCClient()), + // ophostQueryClient will be initialized in Initialize() method after node is initialized processedMsgs: make([]btypes.ProcessedMsgs, 0), msgQueue: make(map[string][]sdk.Msg), @@ -73,11 +73,44 @@ func GetCodec(bech32Prefix string) (codec.Codec, client.TxConfig, error) { return codec, txConfig, err } -func (b *BaseHost) Initialize(ctx types.Context, processedHeight int64, bridgeInfo ophosttypes.QueryBridgeResponse, keyringConfig *btypes.KeyringConfig) error { - err := b.node.Initialize(ctx, processedHeight, b.keyringConfigs(keyringConfig)) +// InitializeQueryClient initializes just the RPC client and query client for basic queries +// This should be called before QueryBridgeConfig is needed, before full initialization +func (b *BaseHost) InitializeQueryClient(ctx types.Context) error { + if b.ophostQueryClient != nil { + return nil // Already initialized + } + + // Initialize the node's RPC client with minimal configuration + err := b.node.Initialize(ctx, 0, []btypes.KeyringConfig{}) if err != nil { - return errors.Wrap(err, "failed to initialize node") + return errors.Wrap(err, "failed to initialize node for query client") + } + + // Initialize the ophost query client + b.ophostQueryClient = ophosttypes.NewQueryClient(b.node.GetRPCClient()) + + return nil +} + +func (b *BaseHost) Initialize(ctx types.Context, processedHeight int64, bridgeInfo ophosttypes.QueryBridgeResponse, keyringConfig *btypes.KeyringConfig) error { + // Initialize the node if not already done by InitializeQueryClient + var err error + if b.ophostQueryClient == nil { + err = b.node.Initialize(ctx, processedHeight, b.keyringConfigs(keyringConfig)) + if err != nil { + return errors.Wrap(err, "failed to initialize node") + } + + // Initialize the ophost query client after the node's RPC client is ready + b.ophostQueryClient = ophosttypes.NewQueryClient(b.node.GetRPCClient()) + } else { + // Node was already initialized by InitializeQueryClient, just reinitialize with proper config + err = b.node.Initialize(ctx, processedHeight, b.keyringConfigs(keyringConfig)) + if err != nil { + return errors.Wrap(err, "failed to initialize node") + } } + b.SetBridgeInfo(bridgeInfo) return nil } From eac5605e107026de4a75089a4607677a294b543e Mon Sep 17 00:00:00 2001 From: SeUkKim Date: Fri, 25 Jul 2025 02:44:46 +0900 Subject: [PATCH 05/23] refactor(rpc): deduplicate retry logic in ExecuteWithFallback using types.SleepWithRetry and consolidating duplicate logging logic in rpcpool --- node/rpcclient/rpcpool.go | 82 +++++++++++++++++---------------------- 1 file changed, 35 insertions(+), 47 deletions(-) diff --git a/node/rpcclient/rpcpool.go b/node/rpcclient/rpcpool.go index cf656650..dc754278 100644 --- a/node/rpcclient/rpcpool.go +++ b/node/rpcclient/rpcpool.go @@ -3,7 +3,6 @@ package rpcclient import ( "context" "fmt" - "math" "os" "strconv" "sync" @@ -94,6 +93,31 @@ func (p *RPCPool) setCurrentIndex(index int) { p.currentIndex = index } +// logEndpointAttempt logs the attempt to use an RPC endpoint +func (p *RPCPool) logEndpointAttempt(endpoint string, retryAttempt int) { + if retryAttempt == 0 { + p.logger.Debug("Trying RPC endpoint", zap.String("endpoint", endpoint)) + } else { + p.logger.Debug("Retrying RPC endpoint", + zap.String("endpoint", endpoint), + zap.Int("retry", retryAttempt)) + } +} + +// logEndpointFailure logs the failure of an RPC endpoint +func (p *RPCPool) logEndpointFailure(endpoint string, err error, retryAttempt int) { + if retryAttempt == 0 { + p.logger.Warn("RPC request failed, trying next endpoint", + zap.String("endpoint", endpoint), + zap.String("error", err.Error())) + } else { + p.logger.Warn("RPC request failed during retry, trying next endpoint", + zap.String("endpoint", endpoint), + zap.String("error", err.Error()), + zap.Int("retry", retryAttempt)) + } +} + // tryAllEndpoints tries the given function on all endpoints once // Returns nil on first success or the last encountered error if all endpoints fail func (p *RPCPool) tryAllEndpoints(ctx context.Context, fn func(context.Context) error, retryAttempt int) error { @@ -104,13 +128,7 @@ func (p *RPCPool) tryAllEndpoints(ctx context.Context, fn func(context.Context) // Create a timeout context timeoutCtx, cancel := context.WithTimeout(ctx, p.rpcTimeout) - if retryAttempt == 0 { - p.logger.Debug("Trying RPC endpoint", zap.String("endpoint", currentEndpoint)) - } else { - p.logger.Debug("Retrying RPC endpoint", - zap.String("endpoint", currentEndpoint), - zap.Int("retry", retryAttempt)) - } + p.logEndpointAttempt(currentEndpoint, retryAttempt) err := fn(timeoutCtx) cancel() @@ -120,16 +138,7 @@ func (p *RPCPool) tryAllEndpoints(ctx context.Context, fn func(context.Context) } var lastErr = err - if retryAttempt == 0 { - p.logger.Warn("RPC request failed, trying next endpoint", - zap.String("endpoint", currentEndpoint), - zap.String("error", err.Error())) - } else { - p.logger.Warn("RPC request failed during retry, trying next endpoint", - zap.String("endpoint", currentEndpoint), - zap.String("error", err.Error()), - zap.Int("retry", retryAttempt)) - } + p.logEndpointFailure(currentEndpoint, err, retryAttempt) // Current endpoint failed, try the remaining endpoints for i := 1; i < len(p.endpoints); i++ { @@ -139,13 +148,7 @@ func (p *RPCPool) tryAllEndpoints(ctx context.Context, fn func(context.Context) // Create a timeout context timeoutCtx, cancel := context.WithTimeout(ctx, p.rpcTimeout) - if retryAttempt == 0 { - p.logger.Debug("Trying RPC endpoint", zap.String("endpoint", currentEndpoint)) - } else { - p.logger.Debug("Retrying RPC endpoint", - zap.String("endpoint", currentEndpoint), - zap.Int("retry", retryAttempt)) - } + p.logEndpointAttempt(currentEndpoint, retryAttempt) err := fn(timeoutCtx) cancel() @@ -155,16 +158,7 @@ func (p *RPCPool) tryAllEndpoints(ctx context.Context, fn func(context.Context) } lastErr = err - if retryAttempt == 0 { - p.logger.Warn("RPC request failed, trying next endpoint", - zap.String("endpoint", currentEndpoint), - zap.String("error", err.Error())) - } else { - p.logger.Warn("RPC request failed during retry, trying next endpoint", - zap.String("endpoint", currentEndpoint), - zap.String("error", err.Error()), - zap.Int("retry", retryAttempt)) - } + p.logEndpointFailure(currentEndpoint, err, retryAttempt) } // Reset to original position if all endpoints failed @@ -184,26 +178,20 @@ func (p *RPCPool) ExecuteWithFallback(ctx context.Context, fn func(context.Conte return nil } - // If all endpoints failed, retry with exponential backoff + // If all endpoints failed, retry with exponential backoff using SleepWithRetry var lastErr error - for retry := 0; retry < p.maxRetries; retry++ { - // Calculate backoff duration - backoffDuration := time.Duration(math.Pow(2, float64(retry))) * p.retryInterval - + for retry := 1; retry <= p.maxRetries; retry++ { p.logger.Info("All RPC endpoints failed, retrying after backoff", - zap.Duration("backoff", backoffDuration), - zap.Int("retry", retry+1), + zap.Int("retry", retry), zap.Int("max_retries", p.maxRetries)) - // Wait for backoff duration - select { - case <-ctx.Done(): + // Use SleepWithRetry for exponential backoff with jitter + if cancelled := types.SleepWithRetry(ctx, retry); cancelled { return ctx.Err() - case <-time.After(backoffDuration): } // Try all endpoints again - err := p.tryAllEndpoints(ctx, fn, retry+1) + err := p.tryAllEndpoints(ctx, fn, retry) if err == nil { return nil } From 54e90de4a218d915f13a0681cf3e5694a1b861fc Mon Sep 17 00:00:00 2001 From: SeUkKim Date: Fri, 25 Jul 2025 02:55:48 +0900 Subject: [PATCH 06/23] refactor(config): rename RPCAddress to RPCAddresses for clarity --- challenger/types/config.go | 24 ++++++++++++------------ e2e/helper.go | 6 +++--- executor/README.md | 2 +- executor/types/config.go | 16 ++++++++-------- 4 files changed, 24 insertions(+), 24 deletions(-) diff --git a/challenger/types/config.go b/challenger/types/config.go index e0b08657..7b7be07a 100644 --- a/challenger/types/config.go +++ b/challenger/types/config.go @@ -8,9 +8,9 @@ import ( ) type NodeConfig struct { - ChainID string `json:"chain_id"` - Bech32Prefix string `json:"bech32_prefix"` - RPCAddress []string `json:"rpc_address"` + ChainID string `json:"chain_id"` + Bech32Prefix string `json:"bech32_prefix"` + RPCAddresses []string `json:"rpc_addresses"` } func (nc NodeConfig) Validate() error { @@ -20,7 +20,7 @@ func (nc NodeConfig) Validate() error { if nc.Bech32Prefix == "" { return errors.New("bech32 prefix is required") } - if len(nc.RPCAddress) == 0 { + if len(nc.RPCAddresses) == 0 { return errors.New("RPC address is required") } return nil @@ -64,15 +64,15 @@ func DefaultConfig() *Config { }, L1Node: NodeConfig{ - ChainID: "testnet-l1-1", - Bech32Prefix: "init", - RPCAddress: []string{"tcp://localhost:26657"}, + ChainID: "testnet-l1-1", + Bech32Prefix: "init", + RPCAddresses: []string{"tcp://localhost:26657"}, }, L2Node: NodeConfig{ - ChainID: "testnet-l2-1", - Bech32Prefix: "init", - RPCAddress: []string{"tcp://localhost:27657"}, + ChainID: "testnet-l2-1", + Bech32Prefix: "init", + RPCAddresses: []string{"tcp://localhost:27657"}, }, DisableAutoSetL1Height: false, L1StartHeight: 1, @@ -114,7 +114,7 @@ func (cfg Config) Validate() error { func (cfg Config) L1NodeConfig() nodetypes.NodeConfig { nc := nodetypes.NodeConfig{ ChainID: cfg.L1Node.ChainID, - RPC: cfg.L1Node.RPCAddress, + RPC: cfg.L1Node.RPCAddresses, ProcessType: nodetypes.PROCESS_TYPE_DEFAULT, Bech32Prefix: cfg.L1Node.Bech32Prefix, } @@ -124,7 +124,7 @@ func (cfg Config) L1NodeConfig() nodetypes.NodeConfig { func (cfg Config) L2NodeConfig() nodetypes.NodeConfig { nc := nodetypes.NodeConfig{ ChainID: cfg.L2Node.ChainID, - RPC: cfg.L2Node.RPCAddress, + RPC: cfg.L2Node.RPCAddresses, ProcessType: nodetypes.PROCESS_TYPE_DEFAULT, Bech32Prefix: cfg.L2Node.Bech32Prefix, } diff --git a/e2e/helper.go b/e2e/helper.go index bed5c220..93b5f98c 100644 --- a/e2e/helper.go +++ b/e2e/helper.go @@ -684,7 +684,7 @@ func (op OPTestHelper) ExecutorConfig() *executortypes.Config { L1Node: executortypes.NodeConfig{ ChainID: op.Initia.Config().ChainID, Bech32Prefix: op.Initia.Config().Bech32Prefix, - RPCAddress: fmt.Sprintf("http://%s:26657", op.Initia.GetFullNode().HostName()), + RPCAddresses: []string{fmt.Sprintf("http://%s:26657", op.Initia.GetFullNode().HostName())}, GasPrice: op.Initia.Config().GasPrices, GasAdjustment: op.Initia.Config().GasAdjustment, TxTimeout: 60, @@ -693,7 +693,7 @@ func (op OPTestHelper) ExecutorConfig() *executortypes.Config { L2Node: executortypes.NodeConfig{ ChainID: op.Minitia.Config().ChainID, Bech32Prefix: op.Minitia.Config().Bech32Prefix, - RPCAddress: fmt.Sprintf("http://%s:26657", op.Minitia.GetFullNode().HostName()), + RPCAddresses: []string{fmt.Sprintf("http://%s:26657", op.Minitia.GetFullNode().HostName())}, GasPrice: "", GasAdjustment: op.Minitia.Config().GasAdjustment, TxTimeout: 60, @@ -702,7 +702,7 @@ func (op OPTestHelper) ExecutorConfig() *executortypes.Config { DANode: executortypes.NodeConfig{ ChainID: op.DA.Config().ChainID, Bech32Prefix: op.DA.Config().Bech32Prefix, - RPCAddress: fmt.Sprintf("http://%s:26657", op.DA.GetFullNode().HostName()), + RPCAddresses: []string{fmt.Sprintf("http://%s:26657", op.DA.GetFullNode().HostName())}, GasPrice: op.DA.Config().GasPrices, GasAdjustment: op.DA.Config().GasAdjustment, TxTimeout: 60, diff --git a/executor/README.md b/executor/README.md index 9d4e2183..5593a8e7 100644 --- a/executor/README.md +++ b/executor/README.md @@ -329,7 +329,7 @@ If the batch info registered in the chain is changed to change the account or DA ```go { - RPCAddress string `json:"rpc_address"` + RPCAddresses []string `json:"rpc_addresses"` GasPrice string `json:"gas_price"` GasAdjustment string `json:"gas_adjustment"` ChainID string `json:"chain_id"` diff --git a/executor/types/config.go b/executor/types/config.go index b98552e7..f1c5c2e4 100644 --- a/executor/types/config.go +++ b/executor/types/config.go @@ -13,7 +13,7 @@ import ( type NodeConfig struct { ChainID string `json:"chain_id"` Bech32Prefix string `json:"bech32_prefix"` - RPCAddress []string `json:"rpc_address"` + RPCAddresses []string `json:"rpc_addresses"` GasPrice string `json:"gas_price"` GasAdjustment float64 `json:"gas_adjustment"` TxTimeout int64 `json:"tx_timeout"` // seconds @@ -26,7 +26,7 @@ func (nc NodeConfig) Validate() error { if nc.Bech32Prefix == "" { return errors.New("bech32 prefix is required") } - if len(nc.RPCAddress) == 0 { + if len(nc.RPCAddresses) == 0 { return errors.New("RPC address is required") } return nil @@ -109,7 +109,7 @@ func DefaultConfig() *Config { L1Node: NodeConfig{ ChainID: "testnet-l1-1", Bech32Prefix: "init", - RPCAddress: []string{"tcp://localhost:26657"}, + RPCAddresses: []string{"tcp://localhost:26657"}, GasPrice: "0.15uinit", GasAdjustment: 1.5, TxTimeout: 60, @@ -118,7 +118,7 @@ func DefaultConfig() *Config { L2Node: NodeConfig{ ChainID: "testnet-l2-1", Bech32Prefix: "init", - RPCAddress: []string{"tcp://localhost:27657"}, + RPCAddresses: []string{"tcp://localhost:27657"}, GasPrice: "", GasAdjustment: 1.5, TxTimeout: 60, @@ -127,7 +127,7 @@ func DefaultConfig() *Config { DANode: NodeConfig{ ChainID: "testnet-l1-1", Bech32Prefix: "init", - RPCAddress: []string{"tcp://localhost:26657"}, + RPCAddresses: []string{"tcp://localhost:26657"}, GasPrice: "0.15uinit", GasAdjustment: 1.5, TxTimeout: 60, @@ -204,7 +204,7 @@ func (cfg *Config) Validate() error { func (cfg Config) L1NodeConfig() nodetypes.NodeConfig { nc := nodetypes.NodeConfig{ ChainID: cfg.L1Node.ChainID, - RPC: cfg.L1Node.RPCAddress, + RPC: cfg.L1Node.RPCAddresses, ProcessType: nodetypes.PROCESS_TYPE_DEFAULT, Bech32Prefix: cfg.L1Node.Bech32Prefix, } @@ -225,7 +225,7 @@ func (cfg Config) L1NodeConfig() nodetypes.NodeConfig { func (cfg Config) L2NodeConfig() nodetypes.NodeConfig { nc := nodetypes.NodeConfig{ ChainID: cfg.L2Node.ChainID, - RPC: cfg.L2Node.RPCAddress, + RPC: cfg.L2Node.RPCAddresses, ProcessType: nodetypes.PROCESS_TYPE_DEFAULT, Bech32Prefix: cfg.L2Node.Bech32Prefix, } @@ -246,7 +246,7 @@ func (cfg Config) L2NodeConfig() nodetypes.NodeConfig { func (cfg Config) DANodeConfig() nodetypes.NodeConfig { nc := nodetypes.NodeConfig{ ChainID: cfg.DANode.ChainID, - RPC: cfg.DANode.RPCAddress, + RPC: cfg.DANode.RPCAddresses, ProcessType: nodetypes.PROCESS_TYPE_ONLY_BROADCAST, Bech32Prefix: cfg.DANode.Bech32Prefix, } From 79a23d692928657222b1bb04848dfc61b440a021 Mon Sep 17 00:00:00 2001 From: SeUkKim Date: Fri, 25 Jul 2025 02:57:47 +0900 Subject: [PATCH 07/23] chore: add missed files --- README.md | 10 +++++----- challenger/README.md | 6 +++--- executor/README.md | 10 +++++----- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index 1dc7d0de..bdc9641d 100644 --- a/README.md +++ b/README.md @@ -114,7 +114,7 @@ In your configuration file (`~/.opinit/[bot-name].json`), you can specify a list "l1_node": { "chain_id": "testnet-l1-1", "bech32_prefix": "init", - "rpc_address": [ + "rpc_addresses": [ "tcp://doi-rpc:26657", "tcp://another-l1-rpc:26657" ] @@ -122,16 +122,16 @@ In your configuration file (`~/.opinit/[bot-name].json`), you can specify a list "l2_node": { "chain_id": "testnet-l2-1", "bech32_prefix": "init", - "rpc_address": [ - "tcp://rpc:27657", + "rpc_addresses": [ + "tcp://moro-rpc:27657", "tcp://another-l2-rpc:27657" ] }, "da_node": { "chain_id": "testnet-l1-1", "bech32_prefix": "init", - "rpc_address": [ - "tcp://rpc:26657", + "rpc_addresses": [ + "tcp://rene-rpc:26657", "tcp://another-da-rpc:26657" ] } diff --git a/challenger/README.md b/challenger/README.md index f820cfe1..a4dccdf5 100644 --- a/challenger/README.md +++ b/challenger/README.md @@ -28,7 +28,7 @@ To configure the Challenger, fill in the values in the `~/.opinit/challenger.jso "l1_node": { "chain_id": "testnet-l1-1", "bech32_prefix": "init", - "rpc_address": [ + "rpc_addresses": [ "tcp://doi-rpc:26657", "tcp://localhost:26657" ], @@ -36,8 +36,8 @@ To configure the Challenger, fill in the values in the `~/.opinit/challenger.jso "l2_node": { "chain_id": "testnet-l2-1", "bech32_prefix": "init", - "rpc_address": [ - "tcp://another-rpc:26657", + "rpc_addresses": [ + "tcp://moro-rpc:26657", "tcp://localhost:27657" ], }, diff --git a/executor/README.md b/executor/README.md index 5593a8e7..14acddd9 100644 --- a/executor/README.md +++ b/executor/README.md @@ -26,7 +26,7 @@ To configure the Executor, fill in the values in the `~/.opinit/executor.json` f "l1_node": { "chain_id": "testnet-l1-1", "bech32_prefix": "init", - "rpc_address": [ + "rpc_addresses": [ "tcp://doi-rpc.com", "tcp://localhost:26657" ], @@ -37,8 +37,8 @@ To configure the Executor, fill in the values in the `~/.opinit/executor.json` f "l2_node": { "chain_id": "testnet-l2-1", "bech32_prefix": "init", - "rpc_address": [ - "tcp://another-rpc:27657", + "rpc_addresses": [ + "tcp://moro-rpc:27657", "tcp://localhost:27657" ], "gas_price": "", @@ -48,8 +48,8 @@ To configure the Executor, fill in the values in the `~/.opinit/executor.json` f "da_node": { "chain_id": "testnet-l1-1", "bech32_prefix": "init", - "rpc_address": [ - "tcp://another-rpc:26657", + "rpc_addresses": [ + "tcp://rene-rpc:26657", "tcp://localhost:26657" ], "gas_price": "0.15uinit", From a81e174946697e2cea2e1ba9dd1b8d506bd9c7c7 Mon Sep 17 00:00:00 2001 From: SeUkKim Date: Fri, 25 Jul 2025 03:30:45 +0900 Subject: [PATCH 08/23] fix(rpc): add mutex to ensure tread-safe access to HTTP client. --- node/rpcclient/client.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/node/rpcclient/client.go b/node/rpcclient/client.go index efe546cc..3ea8308e 100644 --- a/node/rpcclient/client.go +++ b/node/rpcclient/client.go @@ -5,6 +5,7 @@ import ( "fmt" "reflect" "strconv" + "sync" "time" "google.golang.org/grpc" @@ -39,6 +40,7 @@ type RPCClient struct { cdc codec.Codec pool *RPCPool + mu sync.RWMutex } func NewRPCClient(cdc codec.Codec, rpcAddrs []string, logger *zap.Logger) (*RPCClient, error) { @@ -278,7 +280,9 @@ func (q *RPCClient) updateHTTPClient() error { if err != nil { return err } + q.mu.Lock() q.HTTP = client + q.mu.Unlock() } return nil } From a5310459f3aaa55be0cca1a3b2159f95436d4d55 Mon Sep 17 00:00:00 2001 From: SeUkKim Date: Fri, 25 Jul 2025 03:46:03 +0900 Subject: [PATCH 09/23] fix(rpc): use pointer receivers for RCPClient methods --- node/rpcclient/client.go | 10 +++++----- node/rpcclient/rpcpool.go | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/node/rpcclient/client.go b/node/rpcclient/client.go index 3ea8308e..5828b427 100644 --- a/node/rpcclient/client.go +++ b/node/rpcclient/client.go @@ -80,7 +80,7 @@ func NewRPCClientWithClient(cdc codec.Codec, client *clienthttp.HTTP, endpoints } // Invoke implements the grpc ClientConq.Invoke method -func (q RPCClient) Invoke(ctx context.Context, method string, req, reply interface{}, opts ...grpc.CallOption) (err error) { +func (q *RPCClient) Invoke(ctx context.Context, method string, req, reply interface{}, opts ...grpc.CallOption) (err error) { // In both cases, we don't allow empty request req (it will panic unexpectedly). if reflect.ValueOf(req).IsNil() { return sdkerrors.Wrap(legacyerrors.ErrInvalidRequest, "request cannot be nil") @@ -113,7 +113,7 @@ func (q RPCClient) Invoke(ctx context.Context, method string, req, reply interfa } // NewStream implements the grpc ClientConq.NewStream method -func (q RPCClient) NewStream(context.Context, *grpc.StreamDesc, string, ...grpc.CallOption) (grpc.ClientStream, error) { +func (q *RPCClient) NewStream(context.Context, *grpc.StreamDesc, string, ...grpc.CallOption) (grpc.ClientStream, error) { return nil, fmt.Errorf("streaming rpc not supported") } @@ -121,7 +121,7 @@ func (q RPCClient) NewStream(context.Context, *grpc.StreamDesc, string, ...grpc. // arguments for the gRPC method, and returns the ABCI response. It is used // to factorize code between client (Invoke) and server (RegisterGRPCServer) // gRPC handlers. -func (q RPCClient) RunGRPCQuery(ctx context.Context, method string, req interface{}, md metadata.MD) (abci.ResponseQuery, metadata.MD, error) { +func (q *RPCClient) RunGRPCQuery(ctx context.Context, method string, req interface{}, md metadata.MD) (abci.ResponseQuery, metadata.MD, error) { reqBz, err := protoCodec.Marshal(req) if err != nil { return abci.ResponseQuery{}, nil, err @@ -169,7 +169,7 @@ func (q RPCClient) RunGRPCQuery(ctx context.Context, method string, req interfac } // QueryABCI performs an ABCI query and returns the appropriate response and error sdk error code. -func (q RPCClient) QueryABCI(ctx context.Context, req abci.RequestQuery) (abci.ResponseQuery, error) { +func (q *RPCClient) QueryABCI(ctx context.Context, req abci.RequestQuery) (abci.ResponseQuery, error) { opts := client2.ABCIQueryOptions{ Height: req.Height, Prove: req.Prove, @@ -215,7 +215,7 @@ func GetQueryContext(ctx context.Context, height int64) (context.Context, contex } // QueryRawCommit queries the raw commit at a given height. -func (q RPCClient) QueryRawCommit(ctx context.Context, height int64) ([]byte, error) { +func (q *RPCClient) QueryRawCommit(ctx context.Context, height int64) ([]byte, error) { ctx, cancel := GetQueryContext(ctx, height) defer cancel() diff --git a/node/rpcclient/rpcpool.go b/node/rpcclient/rpcpool.go index dc754278..55579a2e 100644 --- a/node/rpcclient/rpcpool.go +++ b/node/rpcclient/rpcpool.go @@ -186,7 +186,7 @@ func (p *RPCPool) ExecuteWithFallback(ctx context.Context, fn func(context.Conte zap.Int("max_retries", p.maxRetries)) // Use SleepWithRetry for exponential backoff with jitter - if cancelled := types.SleepWithRetry(ctx, retry); cancelled { + if canceled := types.SleepWithRetry(ctx, retry); canceled { return ctx.Err() } From ccfe38eafeab9db618ccb095c207a94963056329 Mon Sep 17 00:00:00 2001 From: SeUkKim Date: Fri, 25 Jul 2025 12:33:12 +0900 Subject: [PATCH 10/23] chore: fix lint error --- challenger/challenger.go | 2 +- challenger/types/config.go | 14 +++++++------- provider/host/host.go | 10 +++++----- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/challenger/challenger.go b/challenger/challenger.go index dd3d59f6..a4479e3f 100644 --- a/challenger/challenger.go +++ b/challenger/challenger.go @@ -89,7 +89,7 @@ func (c *Challenger) Initialize(ctx types.Context) error { if err != nil { return errors.Wrap(err, "failed to initialize child query client") } - + childBridgeInfo, err := c.child.QueryBridgeInfo(ctx) if err != nil { return err diff --git a/challenger/types/config.go b/challenger/types/config.go index 7b7be07a..0e3a68b1 100644 --- a/challenger/types/config.go +++ b/challenger/types/config.go @@ -8,9 +8,9 @@ import ( ) type NodeConfig struct { - ChainID string `json:"chain_id"` - Bech32Prefix string `json:"bech32_prefix"` - RPCAddresses []string `json:"rpc_addresses"` + ChainID string `json:"chain_id"` + Bech32Prefix string `json:"bech32_prefix"` + RPCAddresses []string `json:"rpc_addresses"` } func (nc NodeConfig) Validate() error { @@ -64,14 +64,14 @@ func DefaultConfig() *Config { }, L1Node: NodeConfig{ - ChainID: "testnet-l1-1", - Bech32Prefix: "init", + ChainID: "testnet-l1-1", + Bech32Prefix: "init", RPCAddresses: []string{"tcp://localhost:26657"}, }, L2Node: NodeConfig{ - ChainID: "testnet-l2-1", - Bech32Prefix: "init", + ChainID: "testnet-l2-1", + Bech32Prefix: "init", RPCAddresses: []string{"tcp://localhost:27657"}, }, DisableAutoSetL1Height: false, diff --git a/provider/host/host.go b/provider/host/host.go index 8cb8378c..00af0b33 100644 --- a/provider/host/host.go +++ b/provider/host/host.go @@ -79,16 +79,16 @@ func (b *BaseHost) InitializeQueryClient(ctx types.Context) error { if b.ophostQueryClient != nil { return nil // Already initialized } - + // Initialize the node's RPC client with minimal configuration err := b.node.Initialize(ctx, 0, []btypes.KeyringConfig{}) if err != nil { return errors.Wrap(err, "failed to initialize node for query client") } - + // Initialize the ophost query client b.ophostQueryClient = ophosttypes.NewQueryClient(b.node.GetRPCClient()) - + return nil } @@ -100,7 +100,7 @@ func (b *BaseHost) Initialize(ctx types.Context, processedHeight int64, bridgeIn if err != nil { return errors.Wrap(err, "failed to initialize node") } - + // Initialize the ophost query client after the node's RPC client is ready b.ophostQueryClient = ophosttypes.NewQueryClient(b.node.GetRPCClient()) } else { @@ -110,7 +110,7 @@ func (b *BaseHost) Initialize(ctx types.Context, processedHeight int64, bridgeIn return errors.Wrap(err, "failed to initialize node") } } - + b.SetBridgeInfo(bridgeInfo) return nil } From a78ae668ce84a160ff9d6215d268113905c376d7 Mon Sep 17 00:00:00 2001 From: SeUkKim Date: Fri, 25 Jul 2025 13:42:58 +0900 Subject: [PATCH 11/23] refactor(rpcclient): pass rpc-timeout via context instead of env var --- cmd/opinitd/db.go | 11 +++++++++-- cmd/opinitd/start.go | 10 +++++++++- cmd/opinitd/tx.go | 6 +++--- executor/batchsubmitter/batch_test.go | 6 ++++-- executor/batchsubmitter/handler_test.go | 3 ++- node/node.go | 2 +- node/rpcclient/client.go | 9 +++++---- node/rpcclient/rpcpool.go | 25 ++++++++---------------- node/rpcclient/rpcpool_test.go | 26 ++++++++++++++++--------- types/context.go | 10 ++++++++++ 10 files changed, 68 insertions(+), 40 deletions(-) diff --git a/cmd/opinitd/db.go b/cmd/opinitd/db.go index b75dddb0..7467f06b 100644 --- a/cmd/opinitd/db.go +++ b/cmd/opinitd/db.go @@ -58,8 +58,14 @@ v0.1.9-2: Fill block hash of finalized tree return err } + rpcTimeout, err := cmd.Flags().GetDuration(flagRPCTimeout) + if err != nil { + return err + } + baseCtx := types.NewContext(cmdCtx, ctx.logger.Named(string(bottypes.BotTypeExecutor)), ctx.homePath). - WithPollingInterval(interval) + WithPollingInterval(interval). + WithRPCTimeout(rpcTimeout) configPath, err := getConfigPath(cmd, ctx.homePath, string(bottypes.BotTypeExecutor)) if err != nil { @@ -79,7 +85,7 @@ v0.1.9-2: Fill block hash of finalized tree return err } - rpcClient, err := rpcclient.NewRPCClient(cdc, l2Config.RPC, baseCtx.Logger().Named("migration-rpcclient")) + rpcClient, err := rpcclient.NewRPCClient(baseCtx, cdc, l2Config.RPC, baseCtx.Logger().Named("migration-rpcclient")) if err != nil { return err } @@ -106,5 +112,6 @@ v0.1.9-2: Fill block hash of finalized tree } cmd = configFlag(ctx.v, cmd) cmd.Flags().Duration(flagPollingInterval, 100*time.Millisecond, "Polling interval in milliseconds") + cmd.Flags().Duration(flagRPCTimeout, 5*time.Second, "RPC timeout duration") return cmd } diff --git a/cmd/opinitd/start.go b/cmd/opinitd/start.go index f6850778..2d01af28 100644 --- a/cmd/opinitd/start.go +++ b/cmd/opinitd/start.go @@ -19,6 +19,7 @@ import ( const ( flagPollingInterval = "polling-interval" + flagRPCTimeout = "rpc-timeout" ) func startCmd(cmdCtx *cmdContext) *cobra.Command { @@ -62,9 +63,15 @@ Currently supported bots: return err } + rpcTimeout, err := cmd.Flags().GetDuration(flagRPCTimeout) + if err != nil { + return err + } + baseCtx := types.NewContext(ctx, cmdCtx.logger.Named(string(botType)), cmdCtx.homePath). WithErrGrp(errGrp). - WithPollingInterval(interval) + WithPollingInterval(interval). + WithRPCTimeout(rpcTimeout) err = bot.Initialize(baseCtx) if err != nil { return err @@ -75,6 +82,7 @@ Currently supported bots: cmd = configFlag(cmdCtx.v, cmd) cmd.Flags().Duration(flagPollingInterval, 100*time.Millisecond, "Polling interval in milliseconds") + cmd.Flags().Duration(flagRPCTimeout, 5*time.Second, "RPC timeout duration") return cmd } diff --git a/cmd/opinitd/tx.go b/cmd/opinitd/tx.go index 1ed03d66..1feb8863 100644 --- a/cmd/opinitd/tx.go +++ b/cmd/opinitd/tx.go @@ -226,7 +226,7 @@ func QueryBridgeId(ctx types.Context, cfg *executortypes.Config) (uint64, error) return 0, err } - l2RpcClient, err := rpcclient.NewRPCClient(cdc, l2Config.RPC, ctx.Logger().Named("l2-rpcclient")) + l2RpcClient, err := rpcclient.NewRPCClient(ctx, cdc, l2Config.RPC, ctx.Logger().Named("l2-rpcclient")) if err != nil { return 0, err } @@ -247,7 +247,7 @@ func l1ProposerAccount(ctx types.Context, cfg *executortypes.Config, bridgeId ui return nil, err } - rpcClient, err := rpcclient.NewRPCClient(cdc, l1Config.RPC, ctx.Logger().Named("l1-rpcclient")) + rpcClient, err := rpcclient.NewRPCClient(ctx, cdc, l1Config.RPC, ctx.Logger().Named("l1-rpcclient")) if err != nil { return nil, err } @@ -272,7 +272,7 @@ func l2BroadcasterAccount(ctx types.Context, cfg *executortypes.Config) (*broadc return nil, err } - rpcClient, err := rpcclient.NewRPCClient(cdc, l2Config.RPC, ctx.Logger().Named("l2-rpcclient")) + rpcClient, err := rpcclient.NewRPCClient(ctx, cdc, l2Config.RPC, ctx.Logger().Named("l2-rpcclient")) if err != nil { return nil, err } diff --git a/executor/batchsubmitter/batch_test.go b/executor/batchsubmitter/batch_test.go index c750f97a..3ff72dff 100644 --- a/executor/batchsubmitter/batch_test.go +++ b/executor/batchsubmitter/batch_test.go @@ -273,7 +273,8 @@ func TestFinalizeBatch(t *testing.T) { mockCaller := mockclient.NewMockCaller() testLogger, _ := zap.NewDevelopment() - rpcClient, err := rpcclient.NewRPCClientWithClient(appCodec, client.NewWithCaller(mockCaller), []string{"http://localhost:26657"}, testLogger) + testCtx := types.NewContext(context.Background(), testLogger, "/tmp").WithRPCTimeout(5 * time.Second) + rpcClient, err := rpcclient.NewRPCClientWithClient(testCtx, appCodec, client.NewWithCaller(mockCaller), []string{"http://localhost:26657"}, testLogger) require.NoError(t, err) batchNode := node.NewTestNode(nodetypes.NodeConfig{}, batchDB, appCodec, txConfig, rpcClient, nil) @@ -756,7 +757,8 @@ func TestSubmitGenesis(t *testing.T) { mockCaller := mockclient.NewMockCaller() testLogger, _ := zap.NewDevelopment() - rpcClient, err := rpcclient.NewRPCClientWithClient(appCodec, client.NewWithCaller(mockCaller), []string{"http://localhost:26657"}, testLogger) + testCtx := types.NewContext(context.Background(), testLogger, "/tmp").WithRPCTimeout(5 * time.Second) + rpcClient, err := rpcclient.NewRPCClientWithClient(testCtx, appCodec, client.NewWithCaller(mockCaller), []string{"http://localhost:26657"}, testLogger) require.NoError(t, err) batchNode := node.NewTestNode(nodetypes.NodeConfig{}, batchDB, appCodec, txConfig, rpcClient, nil) diff --git a/executor/batchsubmitter/handler_test.go b/executor/batchsubmitter/handler_test.go index 92b0ab55..4b1e25f0 100644 --- a/executor/batchsubmitter/handler_test.go +++ b/executor/batchsubmitter/handler_test.go @@ -47,7 +47,8 @@ func TestRawBlockHandler(t *testing.T) { mockCaller := mockclient.NewMockCaller() testLogger, _ := zap.NewDevelopment() - rpcClient, err := rpcclient.NewRPCClientWithClient(appCodec, client.NewWithCaller(mockCaller), []string{"http://localhost:26657"}, testLogger) + testCtx := types.NewContext(context.Background(), testLogger, "/tmp").WithRPCTimeout(5 * time.Second) + rpcClient, err := rpcclient.NewRPCClientWithClient(testCtx, appCodec, client.NewWithCaller(mockCaller), []string{"http://localhost:26657"}, testLogger) require.NoError(t, err) batchNode := node.NewTestNode(nodetypes.NodeConfig{}, batchDB, appCodec, txConfig, rpcClient, nil) diff --git a/node/node.go b/node/node.go index de99d376..b0ec9349 100644 --- a/node/node.go +++ b/node/node.go @@ -78,7 +78,7 @@ func NewNode(cfg nodetypes.NodeConfig, db types.DB, cdc codec.Codec, txConfig cl func (n *Node) Initialize(ctx types.Context, processedHeight int64, keyringConfig []btypes.KeyringConfig) (err error) { // Create RPC client with shared logger from context if n.rpcClient == nil { - n.rpcClient, err = rpcclient.NewRPCClient(n.cdc, n.cfg.RPC, ctx.Logger().Named("rpcclient")) + n.rpcClient, err = rpcclient.NewRPCClient(ctx, n.cdc, n.cfg.RPC, ctx.Logger().Named("rpcclient")) if err != nil { return errors.Wrap(err, "failed to create RPC client") } diff --git a/node/rpcclient/client.go b/node/rpcclient/client.go index 5828b427..299f43fc 100644 --- a/node/rpcclient/client.go +++ b/node/rpcclient/client.go @@ -28,6 +28,7 @@ import ( grpctypes "github.com/cosmos/cosmos-sdk/types/grpc" clienthttp "github.com/initia-labs/opinit-bots/client" + opTypes "github.com/initia-labs/opinit-bots/types" ) var _ gogogrpc.ClientConn = &RPCClient{} @@ -43,13 +44,13 @@ type RPCClient struct { mu sync.RWMutex } -func NewRPCClient(cdc codec.Codec, rpcAddrs []string, logger *zap.Logger) (*RPCClient, error) { +func NewRPCClient(ctx opTypes.Context, cdc codec.Codec, rpcAddrs []string, logger *zap.Logger) (*RPCClient, error) { if len(rpcAddrs) == 0 { return nil, errors.New("no RPC addresses provided") } // Create RPC pool - pool := NewRPCPool(rpcAddrs, logger) + pool := NewRPCPool(ctx, rpcAddrs, logger) // Create HTTP client with the first endpoint client, err := clienthttp.New(pool.GetCurrentEndpoint(), "/websocket") @@ -64,13 +65,13 @@ func NewRPCClient(cdc codec.Codec, rpcAddrs []string, logger *zap.Logger) (*RPCC }, nil } -func NewRPCClientWithClient(cdc codec.Codec, client *clienthttp.HTTP, endpoints []string, logger *zap.Logger) (*RPCClient, error) { +func NewRPCClientWithClient(ctx opTypes.Context, cdc codec.Codec, client *clienthttp.HTTP, endpoints []string, logger *zap.Logger) (*RPCClient, error) { if len(endpoints) == 0 { return nil, errors.New("no RPC endpoints provided") } // Create RPC pool - pool := NewRPCPool(endpoints, logger) + pool := NewRPCPool(ctx, endpoints, logger) return &RPCClient{ HTTP: client, diff --git a/node/rpcclient/rpcpool.go b/node/rpcclient/rpcpool.go index 55579a2e..4ed9c175 100644 --- a/node/rpcclient/rpcpool.go +++ b/node/rpcclient/rpcpool.go @@ -3,8 +3,6 @@ package rpcclient import ( "context" "fmt" - "os" - "strconv" "sync" "time" @@ -33,29 +31,22 @@ type RPCPool struct { } // NewRPCPool creates a new RPC pool with the given endpoints -func NewRPCPool(endpoints []string, logger *zap.Logger) *RPCPool { +func NewRPCPool(ctx types.Context, endpoints []string, logger *zap.Logger) *RPCPool { if len(endpoints) == 0 { panic("endpoints slice cannot be empty") } - // Get timeout from environment variable or use default - timeoutStr := os.Getenv("RPC_TIMEOUT_SECONDS") - timeout := DefaultRPCTimeout - if timeoutStr != "" { - if t, err := strconv.Atoi(timeoutStr); err == nil && t > 0 { - timeout = t - } else { - logger.Warn("Invalid RPC_TIMEOUT_SECONDS value, using default", - zap.String("value", timeoutStr), - zap.Int("default", DefaultRPCTimeout)) - } + // Get timeout from context or use default + rpcTimeout := ctx.RPCTimeout() + if rpcTimeout == 0 { + rpcTimeout = time.Duration(DefaultRPCTimeout) * time.Second } return &RPCPool{ endpoints: endpoints, currentIndex: 0, mu: sync.RWMutex{}, - rpcTimeout: time.Duration(timeout) * time.Second, + rpcTimeout: rpcTimeout, logger: logger, maxRetries: types.MaxRetryCount, retryInterval: 1 * time.Second, @@ -202,13 +193,13 @@ func (p *RPCPool) ExecuteWithFallback(ctx context.Context, fn func(context.Conte } // CreateRPCClient creates a new RPC client with the given codec and RPC addresses -func CreateRPCClient(cdc codec.Codec, rpcAddresses []string, logger *zap.Logger) (*RPCClient, error) { +func CreateRPCClient(ctx types.Context, cdc codec.Codec, rpcAddresses []string, logger *zap.Logger) (*RPCClient, error) { if len(rpcAddresses) == 0 { return nil, errors.New("no RPC addresses provided") } // Create RPC pool - pool := NewRPCPool(rpcAddresses, logger) + pool := NewRPCPool(ctx, rpcAddresses, logger) // Create HTTP client with the first endpoint client, err := clienthttp.New(pool.GetCurrentEndpoint(), "/websocket") diff --git a/node/rpcclient/rpcpool_test.go b/node/rpcclient/rpcpool_test.go index cd897b64..7b277a00 100644 --- a/node/rpcclient/rpcpool_test.go +++ b/node/rpcclient/rpcpool_test.go @@ -10,12 +10,20 @@ import ( "go.uber.org/zap" "go.uber.org/zap/zaptest" "go.uber.org/zap/zaptest/observer" + + "github.com/initia-labs/opinit-bots/types" ) +// createTestContext creates a test context with default RPC timeout +func createTestContext(logger *zap.Logger) types.Context { + return types.NewContext(context.Background(), logger, "/tmp"). + WithRPCTimeout(5 * time.Second) +} + func TestRPCPool_GetCurrentEndpoint(t *testing.T) { logger := zaptest.NewLogger(t) endpoints := []string{"doi", "moro", "rene"} - pool := NewRPCPool(endpoints, logger) + pool := NewRPCPool(createTestContext(logger), endpoints, logger) // Initial endpoint should be the first one assert.Equal(t, "doi", pool.GetCurrentEndpoint()) @@ -24,7 +32,7 @@ func TestRPCPool_GetCurrentEndpoint(t *testing.T) { func TestRPCPool_MoveToNextEndpoint(t *testing.T) { logger := zaptest.NewLogger(t) endpoints := []string{"doi", "moro", "rene"} - pool := NewRPCPool(endpoints, logger) + pool := NewRPCPool(createTestContext(logger), endpoints, logger) // Move to next endpoint assert.Equal(t, "moro", pool.MoveToNextEndpoint()) @@ -42,7 +50,7 @@ func TestRPCPool_MoveToNextEndpoint(t *testing.T) { func TestRPCPool_ExecuteWithFallback_Success(t *testing.T) { logger := zaptest.NewLogger(t) endpoints := []string{"doi", "moro", "rene"} - pool := NewRPCPool(endpoints, logger) + pool := NewRPCPool(createTestContext(logger), endpoints, logger) // Function succeeds on first try callCount := 0 @@ -59,7 +67,7 @@ func TestRPCPool_ExecuteWithFallback_Success(t *testing.T) { func TestRPCPool_ExecuteWithFallback_FallbackSuccess(t *testing.T) { logger := zaptest.NewLogger(t) endpoints := []string{"doi", "moro", "rene"} - pool := NewRPCPool(endpoints, logger) + pool := NewRPCPool(createTestContext(logger), endpoints, logger) // Function fails on first endpoint, succeeds on second callCount := 0 @@ -79,7 +87,7 @@ func TestRPCPool_ExecuteWithFallback_FallbackSuccess(t *testing.T) { func TestRPCPool_ExecuteWithFallback_AllFail(t *testing.T) { logger := zaptest.NewLogger(t) endpoints := []string{"doi", "moro"} - pool := NewRPCPool(endpoints, logger) + pool := NewRPCPool(createTestContext(logger), endpoints, logger) pool.maxRetries = 1 // Set to 1 for faster test // All endpoints fail @@ -98,7 +106,7 @@ func TestRPCPool_ExecuteWithFallback_AllFail(t *testing.T) { func TestRPCPool_ExecuteWithFallback_Timeout(t *testing.T) { logger := zaptest.NewLogger(t) endpoints := []string{"doi"} - pool := NewRPCPool(endpoints, logger) + pool := NewRPCPool(createTestContext(logger), endpoints, logger) pool.rpcTimeout = 100 * time.Millisecond // Function takes too long @@ -118,7 +126,7 @@ func TestRPCPool_ExecuteWithFallback_Timeout(t *testing.T) { func TestRPCPool_ExecuteWithFallback_RetrySuccess(t *testing.T) { logger := zaptest.NewLogger(t) endpoints := []string{"doi"} - pool := NewRPCPool(endpoints, logger) + pool := NewRPCPool(createTestContext(logger), endpoints, logger) pool.maxRetries = 2 pool.retryInterval = 10 * time.Millisecond @@ -139,7 +147,7 @@ func TestRPCPool_ExecuteWithFallback_RetrySuccess(t *testing.T) { func TestRPCPool_ExecuteWithFallback_ContextCancellation(t *testing.T) { logger := zaptest.NewLogger(t) endpoints := []string{"doi", "moro"} - pool := NewRPCPool(endpoints, logger) + pool := NewRPCPool(createTestContext(logger), endpoints, logger) // Create a context that will be canceled ctx, cancel := context.WithCancel(context.Background()) @@ -170,7 +178,7 @@ func TestRPCPool_Logging(t *testing.T) { logger := zap.New(core) endpoints := []string{"doi", "moro"} - pool := NewRPCPool(endpoints, logger) + pool := NewRPCPool(createTestContext(logger), endpoints, logger) // Function fails on first endpoint, succeeds on second _ = pool.ExecuteWithFallback(context.Background(), func(ctx context.Context) error { diff --git a/types/context.go b/types/context.go index ed9b61bc..70f4ec61 100644 --- a/types/context.go +++ b/types/context.go @@ -16,6 +16,7 @@ type Context struct { errGrp *errgroup.Group pollingInterval time.Duration txTimeout time.Duration + rpcTimeout time.Duration homePath string } @@ -71,6 +72,11 @@ func (c Context) WithTxTimeout(timeout time.Duration) Context { return c } +func (c Context) WithRPCTimeout(timeout time.Duration) Context { + c.rpcTimeout = timeout + return c +} + func (c Context) WithHomePath(homePath string) Context { c.homePath = homePath return c @@ -96,6 +102,10 @@ func (c Context) TxTimeout() time.Duration { return c.txTimeout } +func (c Context) RPCTimeout() time.Duration { + return c.rpcTimeout +} + func (c Context) HomePath() string { return c.homePath } From 44aa89a242e4b3de5c5c772ffc959059e4c38955 Mon Sep 17 00:00:00 2001 From: SeUkKim Date: Fri, 25 Jul 2025 15:44:21 +0900 Subject: [PATCH 12/23] chore(challenger): update comments to reflect challenger --- challenger/challenger.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/challenger/challenger.go b/challenger/challenger.go index a4479e3f..d131f5dd 100644 --- a/challenger/challenger.go +++ b/challenger/challenger.go @@ -22,9 +22,13 @@ import ( var _ bottypes.Bot = &Challenger{} -// Executor charges the execution of the bridge between the host and the child chain -// - relay l1 deposit messages to l2 -// - generate l2 output root and submit to l1 +// Challenger is responsible for: +// Verifying that the MsgInitiateTokenDeposit event is properly relayed to MsgFinalizeTokenDeposit +// Checking whether MsgInitiateTokenDeposit was relayed on time +// Verifying that the Oracle data is properly relayed to MsgUpdateOracle +// Checking whether Oracle was relayed on time +// Verifying that the OutputRoot submitted with MsgProposeOutput is correct +// Checking whether next MsgProposeOutput was submitted on time type Challenger struct { host *host.Host child *child.Child From 754728034f88c45bec1a1bf226c577461f8f9cdf Mon Sep 17 00:00:00 2001 From: SeUkKim Date: Wed, 6 Aug 2025 16:54:30 +0900 Subject: [PATCH 13/23] feat(rpcclient): implement advanced RPC pool with health scoring --- node/rpcclient/client.go | 67 ++--- node/rpcclient/rpcpool.go | 392 ++++++++++++++++++++++----- node/rpcclient/rpcpool_test.go | 38 +-- node/rpcclient/scoring_test.go | 469 +++++++++++++++++++++++++++++++++ 4 files changed, 836 insertions(+), 130 deletions(-) create mode 100644 node/rpcclient/scoring_test.go diff --git a/node/rpcclient/client.go b/node/rpcclient/client.go index 299f43fc..9728bb1c 100644 --- a/node/rpcclient/client.go +++ b/node/rpcclient/client.go @@ -45,24 +45,7 @@ type RPCClient struct { } func NewRPCClient(ctx opTypes.Context, cdc codec.Codec, rpcAddrs []string, logger *zap.Logger) (*RPCClient, error) { - if len(rpcAddrs) == 0 { - return nil, errors.New("no RPC addresses provided") - } - - // Create RPC pool - pool := NewRPCPool(ctx, rpcAddrs, logger) - - // Create HTTP client with the first endpoint - client, err := clienthttp.New(pool.GetCurrentEndpoint(), "/websocket") - if err != nil { - return nil, err - } - - return &RPCClient{ - HTTP: client, - cdc: cdc, - pool: pool, - }, nil + return CreateRPCClient(ctx, cdc, rpcAddrs, logger) } func NewRPCClientWithClient(ctx opTypes.Context, cdc codec.Codec, client *clienthttp.HTTP, endpoints []string, logger *zap.Logger) (*RPCClient, error) { @@ -70,8 +53,15 @@ func NewRPCClientWithClient(ctx opTypes.Context, cdc codec.Codec, client *client return nil, errors.New("no RPC endpoints provided") } - // Create RPC pool - pool := NewRPCPool(ctx, endpoints, logger) + // For test clients with mocked HTTP client, don't create a pool + // This allows tests to bypass the pool fallback logic + var pool *RPCPool + if client != nil { + // If a specific HTTP client is provided (likely for testing), don't create pool + pool = nil + } else { + pool = NewRPCPool(ctx, endpoints, logger) + } return &RPCClient{ HTTP: client, @@ -257,35 +247,26 @@ func (q *RPCClient) QueryBlockBulk(ctx context.Context, start int64, end int64) // ExecuteWithFallback executes the given function with fallback to other endpoints if it fails func (q *RPCClient) ExecuteWithFallback(ctx context.Context, fn func(context.Context) error) error { - return q.pool.ExecuteWithFallback(ctx, func(ctx context.Context) error { - // Update HTTP client to current endpoint before executing - if err := q.updateHTTPClient(); err != nil { - return err - } + // If pool is nil, this is likely a test client with mocked HTTP client + // Execute directly without pool fallback logic + if q.pool == nil { return fn(ctx) - }) -} - -// updateHTTPClient updates the HTTP client to use the current endpoint from the pool -func (q *RPCClient) updateHTTPClient() error { - // If this is a mock client (created with NewWithCaller), don't replace it - // Mock clients have empty remote and nil rpc field - if q.HTTP.Remote() == "" { - return nil } - currentEndpoint := q.pool.GetCurrentEndpoint() - if q.HTTP.Remote() != currentEndpoint { - // Create new HTTP client with current endpoint - client, err := clienthttp.New(currentEndpoint, "/websocket") - if err != nil { - return err + return q.pool.ExecuteWithFallback(ctx, func(ctx context.Context) error { + // Get current HTTP client from pool + currentClient := q.pool.GetCurrentClient() + if currentClient == nil || currentClient.client == nil { + return fmt.Errorf("no healthy HTTP client available") } + + // Update the RPCClient to use the current pool client q.mu.Lock() - q.HTTP = client + q.HTTP = currentClient.client q.mu.Unlock() - } - return nil + + return fn(ctx) + }) } // Status returns the status of the node with fallback and retry logic diff --git a/node/rpcclient/rpcpool.go b/node/rpcclient/rpcpool.go index 4ed9c175..4e5a29f5 100644 --- a/node/rpcclient/rpcpool.go +++ b/node/rpcclient/rpcpool.go @@ -3,6 +3,7 @@ package rpcclient import ( "context" "fmt" + "sort" "sync" "time" @@ -17,17 +18,41 @@ import ( const ( // DefaultRPCTimeout is the default timeout for RPC requests in seconds DefaultRPCTimeout = 5 + + // Scoring system constants + DefaultInitialScore = 100.0 // Initial score for new endpoints + ScoreDecayOnFailure = 10.0 // Points to subtract on failure + ScoreDecayOnTimeout = 20.0 // Points to subtract on timeout (more severe) + ScoreIncreaseOnSuccess = 5.0 // Points to add on success + MinScore = 1.0 // Minimum score to prevent complete exclusion + MaxScore = 200.0 // Maximum score to prevent unbounded growth + ScoreResetInterval = 5 * time.Minute // How often to reset scores ) -// RPCPool manages multiple RPC endpoints with fallback and retry logic +// RPCClientInfo holds information about an RPC client and its health status +type RPCClientInfo struct { + client *clienthttp.HTTP + endpoint string + healthy bool + lastError error + lastCheck time.Time + score float64 // Endpoint score for prioritization (higher is better) + successCount int64 // Number of successful requests + failureCount int64 // Number of failed requests + timeoutCount int64 // Number of timeout failures + lastReset time.Time // Last time scores were reset +} + +// RPCPool manages multiple RPC endpoints with persistent HTTP clients, fallback and retry logic type RPCPool struct { - endpoints []string - currentIndex int - mu sync.RWMutex - rpcTimeout time.Duration - logger *zap.Logger - maxRetries int - retryInterval time.Duration + clients []*RPCClientInfo + currentIndex int + mu sync.RWMutex + rpcTimeout time.Duration + logger *zap.Logger + maxRetries int + retryInterval time.Duration + lastScoreReset time.Time // Last time scores were reset across all endpoints } // NewRPCPool creates a new RPC pool with the given endpoints @@ -42,46 +67,250 @@ func NewRPCPool(ctx types.Context, endpoints []string, logger *zap.Logger) *RPCP rpcTimeout = time.Duration(DefaultRPCTimeout) * time.Second } + // Create HTTP clients for each endpoint + clients := make([]*RPCClientInfo, len(endpoints)) + now := time.Now() + for i, endpoint := range endpoints { + client, err := clienthttp.New(endpoint, "/websocket") + if err != nil { + logger.Warn("Failed to create HTTP client for endpoint", + zap.String("endpoint", endpoint), + zap.Error(err)) + // Mark as unhealthy but still include in pool + clients[i] = &RPCClientInfo{ + client: nil, + endpoint: endpoint, + healthy: false, + lastError: err, + lastCheck: now, + score: DefaultInitialScore, + successCount: 0, + failureCount: 0, + timeoutCount: 0, + lastReset: now, + } + } else { + clients[i] = &RPCClientInfo{ + client: client, + endpoint: endpoint, + healthy: true, + lastError: nil, + lastCheck: now, + score: DefaultInitialScore, + successCount: 0, + failureCount: 0, + timeoutCount: 0, + lastReset: now, + } + } + } + return &RPCPool{ - endpoints: endpoints, - currentIndex: 0, - mu: sync.RWMutex{}, - rpcTimeout: rpcTimeout, - logger: logger, - maxRetries: types.MaxRetryCount, - retryInterval: 1 * time.Second, + clients: clients, + currentIndex: 0, + mu: sync.RWMutex{}, + rpcTimeout: rpcTimeout, + logger: logger, + maxRetries: types.MaxRetryCount, + retryInterval: 1 * time.Second, + lastScoreReset: now, } } -// GetCurrentEndpoint returns the current RPC endpoint -func (p *RPCPool) GetCurrentEndpoint() string { +// GetCurrentClient returns the current RPC client info +func (p *RPCPool) GetCurrentClient() *RPCClientInfo { p.mu.RLock() defer p.mu.RUnlock() - return p.endpoints[p.currentIndex] + return p.clients[p.currentIndex] +} + +// MoveToNextHealthyClient moves to the next healthy RPC client, returns nil if none available +func (p *RPCPool) MoveToNextHealthyClient() *RPCClientInfo { + p.mu.Lock() + defer p.mu.Unlock() + + startIndex := p.currentIndex + for i := 0; i < len(p.clients); i++ { + p.currentIndex = (p.currentIndex + 1) % len(p.clients) + client := p.clients[p.currentIndex] + if client.healthy && client.client != nil { + p.logger.Info("Switching to next healthy RPC endpoint", zap.String("endpoint", client.endpoint)) + return client + } + } + + // Reset to original position if no healthy clients found + p.currentIndex = startIndex + return nil } -// MoveToNextEndpoint moves to the next RPC endpoint -func (p *RPCPool) MoveToNextEndpoint() string { +// MarkClientUnhealthy marks the current client as unhealthy +func (p *RPCPool) MarkClientUnhealthy(err error) { p.mu.Lock() defer p.mu.Unlock() - p.currentIndex = (p.currentIndex + 1) % len(p.endpoints) - endpoint := p.endpoints[p.currentIndex] - p.logger.Info("Switching to next RPC endpoint", zap.String("endpoint", endpoint)) - return endpoint + + client := p.clients[p.currentIndex] + client.healthy = false + client.lastError = err + client.lastCheck = time.Now() + + p.logger.Warn("Marked RPC client as unhealthy", + zap.String("endpoint", client.endpoint), + zap.Error(err)) +} + +// AttemptClientRecovery attempts to recover an unhealthy client by recreating the HTTP client +func (p *RPCPool) AttemptClientRecovery(clientInfo *RPCClientInfo) bool { + newClient, err := clienthttp.New(clientInfo.endpoint, "/websocket") + if err != nil { + clientInfo.lastError = err + clientInfo.lastCheck = time.Now() + p.logger.Debug("Failed to recover RPC client", + zap.String("endpoint", clientInfo.endpoint), + zap.Error(err)) + return false + } + + // Replace the old client + clientInfo.client = newClient + clientInfo.healthy = true + clientInfo.lastError = nil + clientInfo.lastCheck = time.Now() + + p.logger.Info("Successfully recovered RPC client", + zap.String("endpoint", clientInfo.endpoint)) + return true } -// getCurrentIndex returns the current index (thread-safe) -func (p *RPCPool) getCurrentIndex() int { +// GetHealthyClientCount returns the number of healthy clients +func (p *RPCPool) GetHealthyClientCount() int { p.mu.RLock() defer p.mu.RUnlock() - return p.currentIndex + + count := 0 + for _, client := range p.clients { + if client.healthy && client.client != nil { + count++ + } + } + return count +} + +// updateScore updates the score for a client based on success or failure +func (p *RPCPool) updateScore(clientInfo *RPCClientInfo, success bool, isTimeout bool) { + if success { + clientInfo.successCount++ + // Update score on every success + clientInfo.score += ScoreIncreaseOnSuccess + if clientInfo.score > MaxScore { + clientInfo.score = MaxScore + } + } else { + clientInfo.failureCount++ + if isTimeout { + clientInfo.timeoutCount++ + clientInfo.score -= ScoreDecayOnTimeout + } else { + clientInfo.score -= ScoreDecayOnFailure + } + if clientInfo.score < MinScore { + clientInfo.score = MinScore + } + } +} + +// UpdateScoreOnSuccess updates the score for the current client on successful request +func (p *RPCPool) UpdateScoreOnSuccess() { + p.mu.Lock() + defer p.mu.Unlock() + + currentClient := p.clients[p.currentIndex] + p.updateScore(currentClient, true, false) + + p.logger.Info("Updated endpoint score on success", + zap.String("endpoint", currentClient.endpoint), + zap.Float64("score", currentClient.score), + zap.Int64("success_count", currentClient.successCount)) } -// setCurrentIndex sets the current index (thread-safe) -func (p *RPCPool) setCurrentIndex(index int) { +// UpdateScoreOnFailure updates the score for the current client on failed request +func (p *RPCPool) UpdateScoreOnFailure(err error, isTimeout bool) { p.mu.Lock() defer p.mu.Unlock() - p.currentIndex = index + + currentClient := p.clients[p.currentIndex] + p.updateScore(currentClient, false, isTimeout) + + p.logger.Info("Updated endpoint score on failure", + zap.String("endpoint", currentClient.endpoint), + zap.Float64("score", currentClient.score), + zap.Int64("failure_count", currentClient.failureCount), + zap.Int64("timeout_count", currentClient.timeoutCount), + zap.Bool("is_timeout", isTimeout), + zap.Error(err)) +} + +// TryRecoverUnhealthyClients attempts to recover all unhealthy clients +func (p *RPCPool) TryRecoverUnhealthyClients() { + p.mu.Lock() + defer p.mu.Unlock() + + for _, client := range p.clients { + if !client.healthy || client.client == nil { + // Only attempt recovery if enough time has passed since last check + if time.Since(client.lastCheck) > p.retryInterval { + p.AttemptClientRecovery(client) + } + } + } +} + +// ResetScoresIfNeeded resets all endpoint scores if enough time has passed +func (p *RPCPool) ResetScoresIfNeeded() { + p.mu.Lock() + defer p.mu.Unlock() + + now := time.Now() + if now.Sub(p.lastScoreReset) >= ScoreResetInterval { + p.resetAllScores(now) + p.lastScoreReset = now + + p.logger.Info("Reset all endpoint scores", + zap.Duration("interval", ScoreResetInterval), + zap.Int("endpoint_count", len(p.clients))) + } +} + +// resetAllScores resets scores for all endpoints (must be called with lock held) +func (p *RPCPool) resetAllScores(now time.Time) { + for _, client := range p.clients { + client.score = DefaultInitialScore + client.successCount = 0 + client.failureCount = 0 + client.timeoutCount = 0 + client.lastReset = now + + p.logger.Debug("Reset endpoint score", + zap.String("endpoint", client.endpoint), + zap.Float64("score", client.score)) + } +} + +// getSortedClientsByScore returns a copy of clients sorted by score (highest first) +func (p *RPCPool) getSortedClientsByScore() []*RPCClientInfo { + // Create a copy of the clients slice + sortedClients := make([]*RPCClientInfo, len(p.clients)) + copy(sortedClients, p.clients) + + // Sort by score (highest first), then by endpoint name for consistency + sort.Slice(sortedClients, func(i, j int) bool { + if sortedClients[i].score == sortedClients[j].score { + return sortedClients[i].endpoint < sortedClients[j].endpoint + } + return sortedClients[i].score > sortedClients[j].score + }) + + return sortedClients } // logEndpointAttempt logs the attempt to use an RPC endpoint @@ -109,52 +338,58 @@ func (p *RPCPool) logEndpointFailure(endpoint string, err error, retryAttempt in } } -// tryAllEndpoints tries the given function on all endpoints once -// Returns nil on first success or the last encountered error if all endpoints fail -func (p *RPCPool) tryAllEndpoints(ctx context.Context, fn func(context.Context) error, retryAttempt int) error { - // Try current endpoint first (which should be the last successful one) - currentEndpoint := p.GetCurrentEndpoint() - startIndex := p.getCurrentIndex() - - // Create a timeout context - timeoutCtx, cancel := context.WithTimeout(ctx, p.rpcTimeout) - - p.logEndpointAttempt(currentEndpoint, retryAttempt) +// tryAllEndpointsWithScoring tries all endpoints prioritized by score until one succeeds or all fail +func (p *RPCPool) tryAllEndpointsWithScoring(ctx context.Context, fn func(context.Context) error, retryAttempt int) error { + p.mu.Lock() + sortedClients := p.getSortedClientsByScore() + p.mu.Unlock() - err := fn(timeoutCtx) - cancel() - if err == nil { - // Current endpoint worked, no need to try others - return nil - } + var lastErr error + var isTimeout bool - var lastErr = err - p.logEndpointFailure(currentEndpoint, err, retryAttempt) + // Try endpoints in order of their scores (highest first) + for _, client := range sortedClients { + // Skip unhealthy clients + if !client.healthy || client.client == nil { + continue + } - // Current endpoint failed, try the remaining endpoints - for i := 1; i < len(p.endpoints); i++ { - p.MoveToNextEndpoint() - currentEndpoint = p.GetCurrentEndpoint() + // Update current index to this client + p.mu.Lock() + for i, c := range p.clients { + if c == client { + p.currentIndex = i + break + } + } + p.mu.Unlock() // Create a timeout context timeoutCtx, cancel := context.WithTimeout(ctx, p.rpcTimeout) - p.logEndpointAttempt(currentEndpoint, retryAttempt) + p.logEndpointAttempt(client.endpoint, retryAttempt) err := fn(timeoutCtx) cancel() + + // Check if error is due to timeout + isTimeout = err != nil && (errors.Is(timeoutCtx.Err(), context.DeadlineExceeded)) + if err == nil { - // This endpoint worked, keep it as current for future requests + p.UpdateScoreOnSuccess() return nil } + // Failure - update score negatively and mark as unhealthy + p.UpdateScoreOnFailure(err, isTimeout) + p.MarkClientUnhealthy(err) lastErr = err - p.logEndpointFailure(currentEndpoint, err, retryAttempt) + p.logEndpointFailure(client.endpoint, err, retryAttempt) } - // Reset to original position if all endpoints failed - if retryAttempt == 0 { - p.setCurrentIndex(startIndex) + // If no healthy clients were available, return appropriate error + if lastErr == nil { + return fmt.Errorf("no healthy RPC clients available") } return lastErr @@ -163,8 +398,14 @@ func (p *RPCPool) tryAllEndpoints(ctx context.Context, fn func(context.Context) // ExecuteWithFallback executes the given function with fallback to other endpoints if it fails // and retries with exponential backoff if all endpoints fail func (p *RPCPool) ExecuteWithFallback(ctx context.Context, fn func(context.Context) error) error { - // First attempt: try all endpoints once - err := p.tryAllEndpoints(ctx, fn, 0) + // Reset scores periodically to allow recovered endpoints to regain priority + p.ResetScoresIfNeeded() + + // Try to recover unhealthy clients before starting + p.TryRecoverUnhealthyClients() + + // First attempt: try all endpoints once using score-based prioritization + err := p.tryAllEndpointsWithScoring(ctx, fn, 0) if err == nil { return nil } @@ -174,15 +415,19 @@ func (p *RPCPool) ExecuteWithFallback(ctx context.Context, fn func(context.Conte for retry := 1; retry <= p.maxRetries; retry++ { p.logger.Info("All RPC endpoints failed, retrying after backoff", zap.Int("retry", retry), - zap.Int("max_retries", p.maxRetries)) + zap.Int("max_retries", p.maxRetries), + zap.Int("healthy_clients", p.GetHealthyClientCount())) // Use SleepWithRetry for exponential backoff with jitter if canceled := types.SleepWithRetry(ctx, retry); canceled { return ctx.Err() } - // Try all endpoints again - err := p.tryAllEndpoints(ctx, fn, retry) + // Try to recover unhealthy clients before retrying + p.TryRecoverUnhealthyClients() + + // Try all endpoints again with score-based prioritization + err := p.tryAllEndpointsWithScoring(ctx, fn, retry) if err == nil { return nil } @@ -198,18 +443,23 @@ func CreateRPCClient(ctx types.Context, cdc codec.Codec, rpcAddresses []string, return nil, errors.New("no RPC addresses provided") } - // Create RPC pool + // Create RPC pool with persistent HTTP clients pool := NewRPCPool(ctx, rpcAddresses, logger) - // Create HTTP client with the first endpoint - client, err := clienthttp.New(pool.GetCurrentEndpoint(), "/websocket") - if err != nil { - return nil, err + // Get the first healthy client from the pool + currentClient := pool.GetCurrentClient() + if currentClient == nil || currentClient.client == nil { + // Try to find any healthy client + healthyClient := pool.MoveToNextHealthyClient() + if healthyClient == nil { + return nil, errors.New("no healthy RPC clients available") + } + currentClient = healthyClient } - // Create RPC client + // Create RPC client with the healthy HTTP client from pool rpcClient := &RPCClient{ - HTTP: client, + HTTP: currentClient.client, cdc: cdc, pool: pool, } diff --git a/node/rpcclient/rpcpool_test.go b/node/rpcclient/rpcpool_test.go index 7b277a00..2a303312 100644 --- a/node/rpcclient/rpcpool_test.go +++ b/node/rpcclient/rpcpool_test.go @@ -26,25 +26,31 @@ func TestRPCPool_GetCurrentEndpoint(t *testing.T) { pool := NewRPCPool(createTestContext(logger), endpoints, logger) // Initial endpoint should be the first one - assert.Equal(t, "doi", pool.GetCurrentEndpoint()) + assert.Equal(t, "doi", pool.GetCurrentClient().endpoint) } -func TestRPCPool_MoveToNextEndpoint(t *testing.T) { +func TestRPCPool_MoveToNextHealthyClient(t *testing.T) { logger := zaptest.NewLogger(t) endpoints := []string{"doi", "moro", "rene"} pool := NewRPCPool(createTestContext(logger), endpoints, logger) - // Move to next endpoint - assert.Equal(t, "moro", pool.MoveToNextEndpoint()) - assert.Equal(t, "moro", pool.GetCurrentEndpoint()) - - // Move to next endpoint again - assert.Equal(t, "rene", pool.MoveToNextEndpoint()) - assert.Equal(t, "rene", pool.GetCurrentEndpoint()) - - // Move to next endpoint should wrap around - assert.Equal(t, "doi", pool.MoveToNextEndpoint()) - assert.Equal(t, "doi", pool.GetCurrentEndpoint()) + // Move to next healthy client + client := pool.MoveToNextHealthyClient() + assert.NotNil(t, client) + assert.Equal(t, "moro", client.endpoint) + assert.Equal(t, "moro", pool.GetCurrentClient().endpoint) + + // Move to next healthy client again + client = pool.MoveToNextHealthyClient() + assert.NotNil(t, client) + assert.Equal(t, "rene", client.endpoint) + assert.Equal(t, "rene", pool.GetCurrentClient().endpoint) + + // Move to next healthy client should wrap around + client = pool.MoveToNextHealthyClient() + assert.NotNil(t, client) + assert.Equal(t, "doi", client.endpoint) + assert.Equal(t, "doi", pool.GetCurrentClient().endpoint) } func TestRPCPool_ExecuteWithFallback_Success(t *testing.T) { @@ -61,7 +67,7 @@ func TestRPCPool_ExecuteWithFallback_Success(t *testing.T) { assert.NoError(t, err) assert.Equal(t, 1, callCount) - assert.Equal(t, "doi", pool.GetCurrentEndpoint()) + assert.Equal(t, "doi", pool.GetCurrentClient().endpoint) } func TestRPCPool_ExecuteWithFallback_FallbackSuccess(t *testing.T) { @@ -81,7 +87,7 @@ func TestRPCPool_ExecuteWithFallback_FallbackSuccess(t *testing.T) { assert.NoError(t, err) assert.Equal(t, 2, callCount) - assert.Equal(t, "moro", pool.GetCurrentEndpoint()) + assert.Equal(t, "moro", pool.GetCurrentClient().endpoint) } func TestRPCPool_ExecuteWithFallback_AllFail(t *testing.T) { @@ -182,7 +188,7 @@ func TestRPCPool_Logging(t *testing.T) { // Function fails on first endpoint, succeeds on second _ = pool.ExecuteWithFallback(context.Background(), func(ctx context.Context) error { - if pool.GetCurrentEndpoint() == "doi" { + if pool.GetCurrentClient().endpoint == "doi" { return errors.New("first endpoint failed") } return nil diff --git a/node/rpcclient/scoring_test.go b/node/rpcclient/scoring_test.go new file mode 100644 index 00000000..691ec9c7 --- /dev/null +++ b/node/rpcclient/scoring_test.go @@ -0,0 +1,469 @@ +package rpcclient + +import ( + "context" + "fmt" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.uber.org/zap" + + "github.com/initia-labs/opinit-bots/types" +) + +// createTestRPCPool creates a test RPC pool with mock endpoints +func createTestRPCPool(t *testing.T, endpoints []string) *RPCPool { + ctx := types.NewContext(context.Background(), zap.NewNop(), "") + logger := zap.NewNop() + + pool := NewRPCPool(ctx, endpoints, logger) + + // Mark all clients as healthy for testing (since endpoints are fake) + pool.mu.Lock() + for _, client := range pool.clients { + client.healthy = true + // The scoring functions don't actually use the HTTP client + } + pool.mu.Unlock() + + return pool +} + +func TestRPCPool_InitialScores(t *testing.T) { + endpoints := []string{"http://endpoint1:26657", "http://endpoint2:26657", "http://endpoint3:26657"} + pool := createTestRPCPool(t, endpoints) + + // Check that all endpoints start with default initial score + pool.mu.RLock() + for _, client := range pool.clients { + assert.Equal(t, DefaultInitialScore, client.score, "Initial score should be DefaultInitialScore") + assert.Equal(t, int64(0), client.successCount, "Initial success count should be 0") + assert.Equal(t, int64(0), client.failureCount, "Initial failure count should be 0") + assert.Equal(t, int64(0), client.timeoutCount, "Initial timeout count should be 0") + } + pool.mu.RUnlock() +} + +func TestRPCPool_UpdateScoreOnSuccess(t *testing.T) { + endpoints := []string{"http://endpoint1:26657", "http://endpoint2:26657"} + pool := createTestRPCPool(t, endpoints) + + initialScore := pool.clients[0].score + + // Update score on success + pool.UpdateScoreOnSuccess() + + pool.mu.RLock() + currentClient := pool.clients[pool.currentIndex] + assert.Equal(t, initialScore+ScoreIncreaseOnSuccess, currentClient.score, "Score should increase on success") + assert.Equal(t, int64(1), currentClient.successCount, "Success count should increment") + assert.Equal(t, int64(0), currentClient.failureCount, "Failure count should remain 0") + pool.mu.RUnlock() +} + +func TestRPCPool_UpdateScoreOnFailure(t *testing.T) { + endpoints := []string{"http://endpoint1:26657", "http://endpoint2:26657"} + pool := createTestRPCPool(t, endpoints) + + initialScore := pool.clients[0].score + testErr := fmt.Errorf("test error") + + // Test regular failure + pool.UpdateScoreOnFailure(testErr, false) + + pool.mu.RLock() + currentClient := pool.clients[pool.currentIndex] + assert.Equal(t, initialScore-ScoreDecayOnFailure, currentClient.score, "Score should decrease on failure") + assert.Equal(t, int64(0), currentClient.successCount, "Success count should remain 0") + assert.Equal(t, int64(1), currentClient.failureCount, "Failure count should increment") + assert.Equal(t, int64(0), currentClient.timeoutCount, "Timeout count should remain 0") + pool.mu.RUnlock() +} + +func TestRPCPool_UpdateScoreOnTimeout(t *testing.T) { + endpoints := []string{"http://endpoint1:26657", "http://endpoint2:26657"} + pool := createTestRPCPool(t, endpoints) + + initialScore := pool.clients[0].score + testErr := fmt.Errorf("timeout error") + + // Test timeout failure + pool.UpdateScoreOnFailure(testErr, true) + + pool.mu.RLock() + currentClient := pool.clients[pool.currentIndex] + assert.Equal(t, initialScore-ScoreDecayOnTimeout, currentClient.score, "Score should decrease more on timeout") + assert.Equal(t, int64(0), currentClient.successCount, "Success count should remain 0") + assert.Equal(t, int64(1), currentClient.failureCount, "Failure count should increment") + assert.Equal(t, int64(1), currentClient.timeoutCount, "Timeout count should increment") + pool.mu.RUnlock() +} + +func TestRPCPool_ScoreBounds(t *testing.T) { + endpoints := []string{"http://endpoint1:26657"} + pool := createTestRPCPool(t, endpoints) + + // Test maximum score bound + for i := 0; i < 50; i++ { + pool.UpdateScoreOnSuccess() + } + + pool.mu.RLock() + assert.Equal(t, MaxScore, pool.clients[0].score, "Score should not exceed MaxScore") + pool.mu.RUnlock() + + // Reset score to test minimum bound + pool.mu.Lock() + pool.clients[0].score = DefaultInitialScore + pool.mu.Unlock() + + // Test minimum score bound + testErr := fmt.Errorf("test error") + for i := 0; i < 50; i++ { + pool.UpdateScoreOnFailure(testErr, false) + } + + pool.mu.RLock() + assert.Equal(t, MinScore, pool.clients[0].score, "Score should not go below MinScore") + pool.mu.RUnlock() +} + +func TestRPCPool_GetSortedClientsByScore(t *testing.T) { + endpoints := []string{"http://endpoint1:26657", "http://endpoint2:26657", "http://endpoint3:26657"} + pool := createTestRPCPool(t, endpoints) + + // Set different scores for each endpoint + pool.mu.Lock() + pool.clients[0].score = 150.0 // highest + pool.clients[1].score = 80.0 // lowest + pool.clients[2].score = 120.0 // middle + pool.mu.Unlock() + + sortedClients := pool.getSortedClientsByScore() + + // Verify sorting (highest first) + assert.Equal(t, 150.0, sortedClients[0].score, "First client should have highest score") + assert.Equal(t, 120.0, sortedClients[1].score, "Second client should have middle score") + assert.Equal(t, 80.0, sortedClients[2].score, "Third client should have lowest score") + + // Verify endpoints are correctly ordered + assert.Equal(t, "http://endpoint1:26657", sortedClients[0].endpoint) + assert.Equal(t, "http://endpoint3:26657", sortedClients[1].endpoint) + assert.Equal(t, "http://endpoint2:26657", sortedClients[2].endpoint) +} + +func TestRPCPool_GetBestHealthyClient(t *testing.T) { + endpoints := []string{"http://endpoint1:26657", "http://endpoint2:26657", "http://endpoint3:26657"} + pool := createTestRPCPool(t, endpoints) + + // Set different scores + pool.mu.Lock() + pool.clients[0].score = 80.0 + pool.clients[1].score = 150.0 // highest, should be selected + pool.clients[2].score = 120.0 + pool.mu.Unlock() + + // Use getSortedClientsByScore and find the best healthy client + sortedClients := pool.getSortedClientsByScore() + var bestClient *RPCClientInfo + for _, client := range sortedClients { + if client.healthy && client.client != nil { + bestClient = client + break + } + } + + require.NotNil(t, bestClient, "Should return a client") + assert.Equal(t, 150.0, bestClient.score, "Should return client with highest score") + assert.Equal(t, "http://endpoint2:26657", bestClient.endpoint) +} + +func TestRPCPool_GetBestHealthyClient_OnlyHealthy(t *testing.T) { + endpoints := []string{"http://endpoint1:26657", "http://endpoint2:26657", "http://endpoint3:26657"} + pool := createTestRPCPool(t, endpoints) + + // Set different scores and mark some as unhealthy + pool.mu.Lock() + pool.clients[0].score = 80.0 + pool.clients[0].healthy = false // unhealthy, should be skipped + pool.clients[1].score = 150.0 // highest but unhealthy + pool.clients[1].healthy = false // unhealthy, should be skipped + pool.clients[2].score = 120.0 // healthy, should be selected + pool.clients[2].healthy = true + pool.mu.Unlock() + + // Use getSortedClientsByScore and find the best healthy client + sortedClients := pool.getSortedClientsByScore() + var bestClient *RPCClientInfo + for _, client := range sortedClients { + if client.healthy && client.client != nil { + bestClient = client + break + } + } + + require.NotNil(t, bestClient, "Should return a healthy client") + assert.Equal(t, 120.0, bestClient.score, "Should return healthy client with highest score") + assert.Equal(t, "http://endpoint3:26657", bestClient.endpoint) +} + +func TestRPCPool_GetBestHealthyClient_NoHealthyClients(t *testing.T) { + endpoints := []string{"http://endpoint1:26657", "http://endpoint2:26657"} + pool := createTestRPCPool(t, endpoints) + + // Mark all clients as unhealthy + pool.mu.Lock() + for _, client := range pool.clients { + client.healthy = false + } + pool.mu.Unlock() + + // Use getSortedClientsByScore and find the best healthy client + sortedClients := pool.getSortedClientsByScore() + var bestClient *RPCClientInfo + for _, client := range sortedClients { + if client.healthy && client.client != nil { + bestClient = client + break + } + } + + assert.Nil(t, bestClient, "Should return nil when no healthy clients available") +} + +func TestRPCPool_ResetScores(t *testing.T) { + endpoints := []string{"http://endpoint1:26657", "http://endpoint2:26657"} + pool := createTestRPCPool(t, endpoints) + + // Modify scores and counters + pool.mu.Lock() + pool.clients[0].score = 150.0 + pool.clients[0].successCount = 10 + pool.clients[0].failureCount = 5 + pool.clients[0].timeoutCount = 2 + pool.clients[1].score = 80.0 + pool.clients[1].successCount = 3 + pool.clients[1].failureCount = 8 + pool.clients[1].timeoutCount = 4 + pool.mu.Unlock() + + // Reset scores + now := time.Now() + pool.mu.Lock() + pool.resetAllScores(now) + pool.mu.Unlock() + + // Verify reset + pool.mu.RLock() + for _, client := range pool.clients { + assert.Equal(t, DefaultInitialScore, client.score, "Score should be reset to default") + assert.Equal(t, int64(0), client.successCount, "Success count should be reset to 0") + assert.Equal(t, int64(0), client.failureCount, "Failure count should be reset to 0") + assert.Equal(t, int64(0), client.timeoutCount, "Timeout count should be reset to 0") + assert.Equal(t, now, client.lastReset, "Last reset time should be updated") + } + pool.mu.RUnlock() +} + +func TestRPCPool_ResetScoresIfNeeded(t *testing.T) { + endpoints := []string{"http://endpoint1:26657"} + pool := createTestRPCPool(t, endpoints) + + // Set last reset time to past the interval + pool.mu.Lock() + pool.lastScoreReset = time.Now().Add(-ScoreResetInterval - time.Minute) + pool.clients[0].score = 150.0 + pool.mu.Unlock() + + // Call ResetScoresIfNeeded + pool.ResetScoresIfNeeded() + + // Verify scores were reset + pool.mu.RLock() + assert.Equal(t, DefaultInitialScore, pool.clients[0].score, "Score should be reset") + assert.True(t, time.Since(pool.lastScoreReset) < time.Minute, "Last reset time should be updated") + pool.mu.RUnlock() +} + +func TestRPCPool_ResetScoresIfNeeded_NotYetTime(t *testing.T) { + endpoints := []string{"http://endpoint1:26657"} + pool := createTestRPCPool(t, endpoints) + + // Set last reset time to recent + pool.mu.Lock() + pool.lastScoreReset = time.Now().Add(-time.Minute) // Only 1 minute ago + pool.clients[0].score = 150.0 + pool.mu.Unlock() + + // Call ResetScoresIfNeeded + pool.ResetScoresIfNeeded() + + // Verify scores were NOT reset + pool.mu.RLock() + assert.Equal(t, 150.0, pool.clients[0].score, "Score should not be reset yet") + pool.mu.RUnlock() +} + +func TestRPCPool_TryAllEndpointsWithScoring(t *testing.T) { + endpoints := []string{"http://endpoint1:26657", "http://endpoint2:26657", "http://endpoint3:26657"} + pool := createTestRPCPool(t, endpoints) + + // Set different scores + pool.mu.Lock() + pool.clients[0].score = 80.0 // lowest + pool.clients[1].score = 150.0 // highest + pool.clients[2].score = 120.0 // middle + pool.mu.Unlock() + + callOrder := []string{} + + // Test function that records which endpoint was called + testFn := func(ctx context.Context) error { + currentEndpoint := pool.GetCurrentClient().endpoint + callOrder = append(callOrder, currentEndpoint) + + // Fail for all except the highest scored endpoint + if currentEndpoint == "http://endpoint2:26657" { + return nil // Success for highest scored endpoint + } + return fmt.Errorf("endpoint %s failed", currentEndpoint) + } + + err := pool.tryAllEndpointsWithScoring(context.Background(), testFn, 0) + + assert.NoError(t, err, "Should succeed when highest scored endpoint works") + + // Verify that endpoints were tried in score order (highest first) + require.Len(t, callOrder, 1, "Should only call one endpoint (the successful one)") + assert.Equal(t, "http://endpoint2:26657", callOrder[0], "Should try highest scored endpoint first") +} + +func TestRPCPool_TryAllEndpointsWithScoring_AllFail(t *testing.T) { + endpoints := []string{"http://endpoint1:26657", "http://endpoint2:26657"} + pool := createTestRPCPool(t, endpoints) + + // Set different scores + pool.mu.Lock() + pool.clients[0].score = 80.0 + pool.clients[1].score = 150.0 + pool.mu.Unlock() + + callOrder := []string{} + + // Test function that always fails but records call order + testFn := func(ctx context.Context) error { + currentEndpoint := pool.GetCurrentClient().endpoint + callOrder = append(callOrder, currentEndpoint) + return fmt.Errorf("endpoint %s failed", currentEndpoint) + } + + err := pool.tryAllEndpointsWithScoring(context.Background(), testFn, 0) + + assert.Error(t, err, "Should fail when all endpoints fail") + + // Verify that endpoints were tried in score order (highest first) + require.Len(t, callOrder, 2, "Should try both endpoints") + assert.Equal(t, "http://endpoint2:26657", callOrder[0], "Should try highest scored endpoint first") + assert.Equal(t, "http://endpoint1:26657", callOrder[1], "Should try lower scored endpoint second") +} + +func TestRPCPool_ScoreInflationPrevention(t *testing.T) { + endpoints := []string{"http://endpoint1:26657"} + pool := createTestRPCPool(t, endpoints) + + initialScore := pool.clients[0].score + + // Test multiple successful requests - score should only increase once + pool.UpdateScoreOnSuccess() + pool.mu.RLock() + firstSuccessScore := pool.clients[0].score + firstSuccessCount := pool.clients[0].successCount + pool.mu.RUnlock() + + assert.Equal(t, initialScore+ScoreIncreaseOnSuccess, firstSuccessScore, "Score should increase on first success") + assert.Equal(t, int64(1), firstSuccessCount, "Success count should be 1") + + // Additional successful requests should continue to increase score + for i := 0; i < 5; i++ { + pool.UpdateScoreOnSuccess() + } + + pool.mu.RLock() + finalScore := pool.clients[0].score + finalSuccessCount := pool.clients[0].successCount + pool.mu.RUnlock() + + expectedFinalScore := initialScore + (6 * ScoreIncreaseOnSuccess) // 6 total successes + assert.Equal(t, expectedFinalScore, finalScore, "Score should increase with each success") + assert.Equal(t, int64(6), finalSuccessCount, "Success count should continue incrementing") + + // Test that failure resets the success state + testErr := fmt.Errorf("test error") + pool.UpdateScoreOnFailure(testErr, false) + + pool.mu.RLock() + afterFailureScore := pool.clients[0].score + pool.mu.RUnlock() + + assert.Equal(t, expectedFinalScore-ScoreDecayOnFailure, afterFailureScore, "Score should decrease on failure") + + // Test that score can increase again after failure + pool.UpdateScoreOnSuccess() + + pool.mu.RLock() + recoveryScore := pool.clients[0].score + pool.mu.RUnlock() + + assert.Equal(t, afterFailureScore+ScoreIncreaseOnSuccess, recoveryScore, "Score should increase on success after failure") +} + +func TestRPCPool_ConcurrentScoreUpdates(t *testing.T) { + endpoints := []string{"http://endpoint1:26657"} + pool := createTestRPCPool(t, endpoints) + + // Run concurrent score updates + done := make(chan bool, 2) + + // Goroutine 1: Success updates + go func() { + for i := 0; i < 50; i++ { + pool.UpdateScoreOnSuccess() + } + done <- true + }() + + // Goroutine 2: Failure updates + go func() { + testErr := fmt.Errorf("test error") + for i := 0; i < 10; i++ { + pool.UpdateScoreOnFailure(testErr, false) + } + done <- true + }() + + // Wait for both goroutines to complete + <-done + <-done + + // Verify final state is consistent + pool.mu.RLock() + client := pool.clients[0] + assert.Equal(t, int64(50), client.successCount, "Success count should be 50") + assert.Equal(t, int64(10), client.failureCount, "Failure count should be 10") + + // Score should be initial + (50 * success) - (10 * failure), bounded by MinScore and MaxScore + expectedScore := DefaultInitialScore + (50 * ScoreIncreaseOnSuccess) - (10 * ScoreDecayOnFailure) + if expectedScore < MinScore { + expectedScore = MinScore + } else if expectedScore > MaxScore { + expectedScore = MaxScore + } + assert.Equal(t, expectedScore, client.score, "Score should reflect all updates within bounds") + + // Also verify the score is within valid bounds + assert.GreaterOrEqual(t, client.score, MinScore, "Score should not be below MinScore") + assert.LessOrEqual(t, client.score, MaxScore, "Score should not exceed MaxScore") + pool.mu.RUnlock() +} From 7f7520dba30cc561360fa34674cf4883bc92c038 Mon Sep 17 00:00:00 2001 From: SeUkKim Date: Wed, 6 Aug 2025 17:32:24 +0900 Subject: [PATCH 14/23] fix(rpcclient): fix race condition when accessing client score --- node/rpcclient/rpcpool.go | 4 ++-- node/rpcclient/scoring_test.go | 8 ++++++++ 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/node/rpcclient/rpcpool.go b/node/rpcclient/rpcpool.go index 4e5a29f5..035e2d49 100644 --- a/node/rpcclient/rpcpool.go +++ b/node/rpcclient/rpcpool.go @@ -227,7 +227,7 @@ func (p *RPCPool) UpdateScoreOnSuccess() { currentClient := p.clients[p.currentIndex] p.updateScore(currentClient, true, false) - p.logger.Info("Updated endpoint score on success", + p.logger.Debug("Updated endpoint score on success", zap.String("endpoint", currentClient.endpoint), zap.Float64("score", currentClient.score), zap.Int64("success_count", currentClient.successCount)) @@ -241,7 +241,7 @@ func (p *RPCPool) UpdateScoreOnFailure(err error, isTimeout bool) { currentClient := p.clients[p.currentIndex] p.updateScore(currentClient, false, isTimeout) - p.logger.Info("Updated endpoint score on failure", + p.logger.Debug("Updated endpoint score on failure", zap.String("endpoint", currentClient.endpoint), zap.Float64("score", currentClient.score), zap.Int64("failure_count", currentClient.failureCount), diff --git a/node/rpcclient/scoring_test.go b/node/rpcclient/scoring_test.go index 691ec9c7..c6c7e3fe 100644 --- a/node/rpcclient/scoring_test.go +++ b/node/rpcclient/scoring_test.go @@ -50,7 +50,9 @@ func TestRPCPool_UpdateScoreOnSuccess(t *testing.T) { endpoints := []string{"http://endpoint1:26657", "http://endpoint2:26657"} pool := createTestRPCPool(t, endpoints) + pool.mu.RLock() initialScore := pool.clients[0].score + pool.mu.RUnlock() // Update score on success pool.UpdateScoreOnSuccess() @@ -67,7 +69,9 @@ func TestRPCPool_UpdateScoreOnFailure(t *testing.T) { endpoints := []string{"http://endpoint1:26657", "http://endpoint2:26657"} pool := createTestRPCPool(t, endpoints) + pool.mu.RLock() initialScore := pool.clients[0].score + pool.mu.RUnlock() testErr := fmt.Errorf("test error") // Test regular failure @@ -86,7 +90,9 @@ func TestRPCPool_UpdateScoreOnTimeout(t *testing.T) { endpoints := []string{"http://endpoint1:26657", "http://endpoint2:26657"} pool := createTestRPCPool(t, endpoints) + pool.mu.RLock() initialScore := pool.clients[0].score + pool.mu.RUnlock() testErr := fmt.Errorf("timeout error") // Test timeout failure @@ -373,7 +379,9 @@ func TestRPCPool_ScoreInflationPrevention(t *testing.T) { endpoints := []string{"http://endpoint1:26657"} pool := createTestRPCPool(t, endpoints) + pool.mu.RLock() initialScore := pool.clients[0].score + pool.mu.RUnlock() // Test multiple successful requests - score should only increase once pool.UpdateScoreOnSuccess() From dc458b2fb66efeb72827e6505356a1d8fc472677 Mon Sep 17 00:00:00 2001 From: SeUkKim Date: Thu, 14 Aug 2025 14:30:00 +0900 Subject: [PATCH 15/23] fix(rpcpool): delete invalid endpoints from the slice instead of marking them as unhealthy --- node/rpcclient/rpcpool.go | 56 ++++++++++++++++++--------------------- 1 file changed, 26 insertions(+), 30 deletions(-) diff --git a/node/rpcclient/rpcpool.go b/node/rpcclient/rpcpool.go index 035e2d49..74464216 100644 --- a/node/rpcclient/rpcpool.go +++ b/node/rpcclient/rpcpool.go @@ -67,42 +67,38 @@ func NewRPCPool(ctx types.Context, endpoints []string, logger *zap.Logger) *RPCP rpcTimeout = time.Duration(DefaultRPCTimeout) * time.Second } - // Create HTTP clients for each endpoint - clients := make([]*RPCClientInfo, len(endpoints)) + // Create HTTP clients for each endpoint, filtering out invalid ones + var clients []*RPCClientInfo now := time.Now() - for i, endpoint := range endpoints { + + for _, endpoint := range endpoints { client, err := clienthttp.New(endpoint, "/websocket") if err != nil { - logger.Warn("Failed to create HTTP client for endpoint", + // Log the error and remove invalid endpoint from the pool + logger.Warn("Removing invalid endpoint from pool", zap.String("endpoint", endpoint), zap.Error(err)) - // Mark as unhealthy but still include in pool - clients[i] = &RPCClientInfo{ - client: nil, - endpoint: endpoint, - healthy: false, - lastError: err, - lastCheck: now, - score: DefaultInitialScore, - successCount: 0, - failureCount: 0, - timeoutCount: 0, - lastReset: now, - } - } else { - clients[i] = &RPCClientInfo{ - client: client, - endpoint: endpoint, - healthy: true, - lastError: nil, - lastCheck: now, - score: DefaultInitialScore, - successCount: 0, - failureCount: 0, - timeoutCount: 0, - lastReset: now, - } + continue // Skip this endpoint entirely } + + // Only add valid endpoints and their clients to the pool + clients = append(clients, &RPCClientInfo{ + client: client, + endpoint: endpoint, + healthy: true, + lastError: nil, + lastCheck: now, + score: DefaultInitialScore, + successCount: 0, + failureCount: 0, + timeoutCount: 0, + lastReset: now, + }) + } + + // Ensure we have at least one valid endpoint + if len(clients) == 0 { + panic("no valid endpoints found - all endpoints failed to create HTTP clients") } return &RPCPool{ From 395f427d9ca4e29a91ac8b3e3ceb9ac8d6268918 Mon Sep 17 00:00:00 2001 From: SeUkKim Date: Thu, 14 Aug 2025 14:53:45 +0900 Subject: [PATCH 16/23] docs(rpcpool): add a more descriptive comment for function --- node/rpcclient/rpcpool.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/node/rpcclient/rpcpool.go b/node/rpcclient/rpcpool.go index 74464216..a2a1578e 100644 --- a/node/rpcclient/rpcpool.go +++ b/node/rpcclient/rpcpool.go @@ -155,7 +155,9 @@ func (p *RPCPool) MarkClientUnhealthy(err error) { zap.Error(err)) } -// AttemptClientRecovery attempts to recover an unhealthy client by recreating the HTTP client +// AttemptClientRecovery attempts to recover an unhealthy client by recreating the HTTP client. +// This is useful for resolving issues like transient network errors, stale connections, or +// when an RPC endpoint comes back online after a period of unavailability. func (p *RPCPool) AttemptClientRecovery(clientInfo *RPCClientInfo) bool { newClient, err := clienthttp.New(clientInfo.endpoint, "/websocket") if err != nil { From d721e33fe08f594877ec6cedc4135dd267d4ea14 Mon Sep 17 00:00:00 2001 From: SeUkKim Date: Thu, 14 Aug 2025 16:27:49 +0900 Subject: [PATCH 17/23] test(rpcpool): add and fix tests. --- node/rpcclient/client.go | 6 +- node/rpcclient/rpcpool.go | 16 ++++-- node/rpcclient/rpcpool_test.go | 102 ++++++++++++++++++++++++++++----- node/rpcclient/scoring_test.go | 3 +- 4 files changed, 105 insertions(+), 22 deletions(-) diff --git a/node/rpcclient/client.go b/node/rpcclient/client.go index 9728bb1c..e120a31a 100644 --- a/node/rpcclient/client.go +++ b/node/rpcclient/client.go @@ -60,7 +60,11 @@ func NewRPCClientWithClient(ctx opTypes.Context, cdc codec.Codec, client *client // If a specific HTTP client is provided (likely for testing), don't create pool pool = nil } else { - pool = NewRPCPool(ctx, endpoints, logger) + var err error + pool, err = NewRPCPool(ctx, endpoints, logger) + if err != nil { + return nil, err + } } return &RPCClient{ diff --git a/node/rpcclient/rpcpool.go b/node/rpcclient/rpcpool.go index a2a1578e..13dcf1e8 100644 --- a/node/rpcclient/rpcpool.go +++ b/node/rpcclient/rpcpool.go @@ -55,8 +55,11 @@ type RPCPool struct { lastScoreReset time.Time // Last time scores were reset across all endpoints } -// NewRPCPool creates a new RPC pool with the given endpoints -func NewRPCPool(ctx types.Context, endpoints []string, logger *zap.Logger) *RPCPool { +// NewRPCPool creates a new RPC pool with the given endpoints. +// Invalid endpoints (those that fail clienthttp.New) are automatically dropped during initialization +// and logged as warnings. Returns an error if all provided endpoints are invalid and no valid +// endpoints remain after filtering. +func NewRPCPool(ctx types.Context, endpoints []string, logger *zap.Logger) (*RPCPool, error) { if len(endpoints) == 0 { panic("endpoints slice cannot be empty") } @@ -98,7 +101,7 @@ func NewRPCPool(ctx types.Context, endpoints []string, logger *zap.Logger) *RPCP // Ensure we have at least one valid endpoint if len(clients) == 0 { - panic("no valid endpoints found - all endpoints failed to create HTTP clients") + return nil, errors.New("no valid endpoints found - all endpoints failed to create HTTP clients") } return &RPCPool{ @@ -110,7 +113,7 @@ func NewRPCPool(ctx types.Context, endpoints []string, logger *zap.Logger) *RPCP maxRetries: types.MaxRetryCount, retryInterval: 1 * time.Second, lastScoreReset: now, - } + }, nil } // GetCurrentClient returns the current RPC client info @@ -442,7 +445,10 @@ func CreateRPCClient(ctx types.Context, cdc codec.Codec, rpcAddresses []string, } // Create RPC pool with persistent HTTP clients - pool := NewRPCPool(ctx, rpcAddresses, logger) + pool, err := NewRPCPool(ctx, rpcAddresses, logger) + if err != nil { + return nil, err + } // Get the first healthy client from the pool currentClient := pool.GetCurrentClient() diff --git a/node/rpcclient/rpcpool_test.go b/node/rpcclient/rpcpool_test.go index 2a303312..f967478a 100644 --- a/node/rpcclient/rpcpool_test.go +++ b/node/rpcclient/rpcpool_test.go @@ -23,7 +23,8 @@ func createTestContext(logger *zap.Logger) types.Context { func TestRPCPool_GetCurrentEndpoint(t *testing.T) { logger := zaptest.NewLogger(t) endpoints := []string{"doi", "moro", "rene"} - pool := NewRPCPool(createTestContext(logger), endpoints, logger) + pool, err := NewRPCPool(createTestContext(logger), endpoints, logger) + assert.NoError(t, err) // Initial endpoint should be the first one assert.Equal(t, "doi", pool.GetCurrentClient().endpoint) @@ -32,7 +33,8 @@ func TestRPCPool_GetCurrentEndpoint(t *testing.T) { func TestRPCPool_MoveToNextHealthyClient(t *testing.T) { logger := zaptest.NewLogger(t) endpoints := []string{"doi", "moro", "rene"} - pool := NewRPCPool(createTestContext(logger), endpoints, logger) + pool, err := NewRPCPool(createTestContext(logger), endpoints, logger) + assert.NoError(t, err) // Move to next healthy client client := pool.MoveToNextHealthyClient() @@ -56,11 +58,12 @@ func TestRPCPool_MoveToNextHealthyClient(t *testing.T) { func TestRPCPool_ExecuteWithFallback_Success(t *testing.T) { logger := zaptest.NewLogger(t) endpoints := []string{"doi", "moro", "rene"} - pool := NewRPCPool(createTestContext(logger), endpoints, logger) + pool, err := NewRPCPool(createTestContext(logger), endpoints, logger) + assert.NoError(t, err) // Function succeeds on first try callCount := 0 - err := pool.ExecuteWithFallback(context.Background(), func(ctx context.Context) error { + err = pool.ExecuteWithFallback(context.Background(), func(ctx context.Context) error { callCount++ return nil }) @@ -73,11 +76,12 @@ func TestRPCPool_ExecuteWithFallback_Success(t *testing.T) { func TestRPCPool_ExecuteWithFallback_FallbackSuccess(t *testing.T) { logger := zaptest.NewLogger(t) endpoints := []string{"doi", "moro", "rene"} - pool := NewRPCPool(createTestContext(logger), endpoints, logger) + pool, err := NewRPCPool(createTestContext(logger), endpoints, logger) + assert.NoError(t, err) // Function fails on first endpoint, succeeds on second callCount := 0 - err := pool.ExecuteWithFallback(context.Background(), func(ctx context.Context) error { + err = pool.ExecuteWithFallback(context.Background(), func(ctx context.Context) error { callCount++ if callCount == 1 { return errors.New("first endpoint failed") @@ -93,12 +97,13 @@ func TestRPCPool_ExecuteWithFallback_FallbackSuccess(t *testing.T) { func TestRPCPool_ExecuteWithFallback_AllFail(t *testing.T) { logger := zaptest.NewLogger(t) endpoints := []string{"doi", "moro"} - pool := NewRPCPool(createTestContext(logger), endpoints, logger) + pool, err := NewRPCPool(createTestContext(logger), endpoints, logger) + assert.NoError(t, err) pool.maxRetries = 1 // Set to 1 for faster test // All endpoints fail callCount := 0 - err := pool.ExecuteWithFallback(context.Background(), func(ctx context.Context) error { + err = pool.ExecuteWithFallback(context.Background(), func(ctx context.Context) error { callCount++ return errors.New("endpoint failed") }) @@ -112,11 +117,12 @@ func TestRPCPool_ExecuteWithFallback_AllFail(t *testing.T) { func TestRPCPool_ExecuteWithFallback_Timeout(t *testing.T) { logger := zaptest.NewLogger(t) endpoints := []string{"doi"} - pool := NewRPCPool(createTestContext(logger), endpoints, logger) + pool, err := NewRPCPool(createTestContext(logger), endpoints, logger) + assert.NoError(t, err) pool.rpcTimeout = 100 * time.Millisecond // Function takes too long - err := pool.ExecuteWithFallback(context.Background(), func(ctx context.Context) error { + err = pool.ExecuteWithFallback(context.Background(), func(ctx context.Context) error { select { case <-ctx.Done(): return ctx.Err() @@ -132,13 +138,14 @@ func TestRPCPool_ExecuteWithFallback_Timeout(t *testing.T) { func TestRPCPool_ExecuteWithFallback_RetrySuccess(t *testing.T) { logger := zaptest.NewLogger(t) endpoints := []string{"doi"} - pool := NewRPCPool(createTestContext(logger), endpoints, logger) + pool, err := NewRPCPool(createTestContext(logger), endpoints, logger) + assert.NoError(t, err) pool.maxRetries = 2 pool.retryInterval = 10 * time.Millisecond // Function fails on first try, succeeds on retry callCount := 0 - err := pool.ExecuteWithFallback(context.Background(), func(ctx context.Context) error { + err = pool.ExecuteWithFallback(context.Background(), func(ctx context.Context) error { callCount++ if callCount <= 1 { return errors.New("first try failed") @@ -153,7 +160,8 @@ func TestRPCPool_ExecuteWithFallback_RetrySuccess(t *testing.T) { func TestRPCPool_ExecuteWithFallback_ContextCancellation(t *testing.T) { logger := zaptest.NewLogger(t) endpoints := []string{"doi", "moro"} - pool := NewRPCPool(createTestContext(logger), endpoints, logger) + pool, err := NewRPCPool(createTestContext(logger), endpoints, logger) + assert.NoError(t, err) // Create a context that will be canceled ctx, cancel := context.WithCancel(context.Background()) @@ -165,7 +173,7 @@ func TestRPCPool_ExecuteWithFallback_ContextCancellation(t *testing.T) { }() // Function should return context canceled error - err := pool.ExecuteWithFallback(ctx, func(ctx context.Context) error { + err = pool.ExecuteWithFallback(ctx, func(ctx context.Context) error { select { case <-ctx.Done(): return ctx.Err() @@ -184,7 +192,8 @@ func TestRPCPool_Logging(t *testing.T) { logger := zap.New(core) endpoints := []string{"doi", "moro"} - pool := NewRPCPool(createTestContext(logger), endpoints, logger) + pool, err := NewRPCPool(createTestContext(logger), endpoints, logger) + assert.NoError(t, err) // Function fails on first endpoint, succeeds on second _ = pool.ExecuteWithFallback(context.Background(), func(ctx context.Context) error { @@ -207,3 +216,66 @@ func TestRPCPool_Logging(t *testing.T) { } assert.True(t, foundFailureLog, "Should have logged endpoint failure") } + +// TestNewRPCPool_ValidEndpoints tests NewRPCPool with valid endpoints +func TestNewRPCPool_ValidEndpoints(t *testing.T) { + logger := zaptest.NewLogger(t) + ctx := createTestContext(logger) + + validEndpoints := []string{"http://localhost:26657", "http://localhost:26658"} + pool, err := NewRPCPool(ctx, validEndpoints, logger) + + assert.NoError(t, err, "NewRPCPool should succeed with valid endpoints") + assert.NotNil(t, pool, "Pool should not be nil") + assert.Equal(t, len(validEndpoints), len(pool.clients), "Pool should have all valid endpoints") +} + +// TestNewRPCPool_InvalidEndpoints tests NewRPCPool with invalid endpoints +func TestNewRPCPool_InvalidEndpoints(t *testing.T) { + logger := zaptest.NewLogger(t) + ctx := createTestContext(logger) + + invalidEndpoints := []string{"://malformed-url", "://another-malformed-url"} + pool, err := NewRPCPool(ctx, invalidEndpoints, logger) + + assert.Error(t, err, "NewRPCPool should fail with invalid endpoints") + assert.Nil(t, pool, "Pool should be nil when all endpoints are invalid") + assert.Contains(t, err.Error(), "no valid endpoints found", "Error should indicate no valid endpoints") +} + +// TestNewRPCPool_MixedEndpoints tests NewRPCPool with mixed valid and invalid endpoints +func TestNewRPCPool_MixedEndpoints(t *testing.T) { + // Create a logger with observer to capture warning logs + core, recorded := observer.New(zap.WarnLevel) + logger := zap.New(core) + ctx := createTestContext(logger) + + mixedEndpoints := []string{"http://localhost:26657", "://malformed-url", "http://localhost:26658"} + pool, err := NewRPCPool(ctx, mixedEndpoints, logger) + + assert.NoError(t, err, "NewRPCPool should succeed with mixed endpoints (invalid ones filtered out)") + assert.NotNil(t, pool, "Pool should not be nil") + assert.Equal(t, 2, len(pool.clients), "Pool should have only the valid endpoints") + + // Check that invalid endpoints were logged as warnings + logs := recorded.All() + foundWarning := false + for _, log := range logs { + if log.Message == "Removing invalid endpoint from pool" { + foundWarning = true + break + } + } + assert.True(t, foundWarning, "Should have logged warning for invalid endpoint") +} + +// TestNewRPCPool_EmptyEndpoints tests NewRPCPool with empty endpoints (should panic) +func TestNewRPCPool_EmptyEndpoints(t *testing.T) { + logger := zaptest.NewLogger(t) + ctx := createTestContext(logger) + + assert.Panics(t, func() { + emptyEndpoints := []string{} + _, _ = NewRPCPool(ctx, emptyEndpoints, logger) + }, "NewRPCPool should panic with empty endpoints") +} diff --git a/node/rpcclient/scoring_test.go b/node/rpcclient/scoring_test.go index c6c7e3fe..f8fdff32 100644 --- a/node/rpcclient/scoring_test.go +++ b/node/rpcclient/scoring_test.go @@ -18,7 +18,8 @@ func createTestRPCPool(t *testing.T, endpoints []string) *RPCPool { ctx := types.NewContext(context.Background(), zap.NewNop(), "") logger := zap.NewNop() - pool := NewRPCPool(ctx, endpoints, logger) + pool, err := NewRPCPool(ctx, endpoints, logger) + require.NoError(t, err) // Mark all clients as healthy for testing (since endpoints are fake) pool.mu.Lock() From c3f4e03eeff3e361c81a4e62736fb32bffe830fa Mon Sep 17 00:00:00 2001 From: SeUkKim Date: Mon, 18 Aug 2025 20:51:59 +0900 Subject: [PATCH 18/23] fix(rpcpool): mark client healthy for retry rather than recreating it --- node/rpcclient/rpcpool.go | 34 ++++++++-------------------------- 1 file changed, 8 insertions(+), 26 deletions(-) diff --git a/node/rpcclient/rpcpool.go b/node/rpcclient/rpcpool.go index 13dcf1e8..61b860a9 100644 --- a/node/rpcclient/rpcpool.go +++ b/node/rpcclient/rpcpool.go @@ -158,30 +158,6 @@ func (p *RPCPool) MarkClientUnhealthy(err error) { zap.Error(err)) } -// AttemptClientRecovery attempts to recover an unhealthy client by recreating the HTTP client. -// This is useful for resolving issues like transient network errors, stale connections, or -// when an RPC endpoint comes back online after a period of unavailability. -func (p *RPCPool) AttemptClientRecovery(clientInfo *RPCClientInfo) bool { - newClient, err := clienthttp.New(clientInfo.endpoint, "/websocket") - if err != nil { - clientInfo.lastError = err - clientInfo.lastCheck = time.Now() - p.logger.Debug("Failed to recover RPC client", - zap.String("endpoint", clientInfo.endpoint), - zap.Error(err)) - return false - } - - // Replace the old client - clientInfo.client = newClient - clientInfo.healthy = true - clientInfo.lastError = nil - clientInfo.lastCheck = time.Now() - - p.logger.Info("Successfully recovered RPC client", - zap.String("endpoint", clientInfo.endpoint)) - return true -} // GetHealthyClientCount returns the number of healthy clients func (p *RPCPool) GetHealthyClientCount() int { @@ -257,10 +233,16 @@ func (p *RPCPool) TryRecoverUnhealthyClients() { defer p.mu.Unlock() for _, client := range p.clients { - if !client.healthy || client.client == nil { + if !client.healthy && client.client != nil { // Only attempt recovery if enough time has passed since last check if time.Since(client.lastCheck) > p.retryInterval { - p.AttemptClientRecovery(client) + // Mark client as healthy again for retry - net/http handles connection recovery + client.healthy = true + client.lastError = nil + client.lastCheck = time.Now() + + p.logger.Info("Marked RPC client as healthy for retry", + zap.String("endpoint", client.endpoint)) } } } From f46d2f504b4a2a696e001e47b149faeb4f8233c2 Mon Sep 17 00:00:00 2001 From: SeUkKim Date: Mon, 18 Aug 2025 21:00:18 +0900 Subject: [PATCH 19/23] style(rpcpool): format code with gofmt --- node/rpcclient/rpcpool.go | 1 - 1 file changed, 1 deletion(-) diff --git a/node/rpcclient/rpcpool.go b/node/rpcclient/rpcpool.go index 61b860a9..ce223f19 100644 --- a/node/rpcclient/rpcpool.go +++ b/node/rpcclient/rpcpool.go @@ -158,7 +158,6 @@ func (p *RPCPool) MarkClientUnhealthy(err error) { zap.Error(err)) } - // GetHealthyClientCount returns the number of healthy clients func (p *RPCPool) GetHealthyClientCount() int { p.mu.RLock() From 114601b3695dac824e547d44d72c1a87e8a4593c Mon Sep 17 00:00:00 2001 From: SeUkKim Date: Mon, 18 Aug 2025 21:15:30 +0900 Subject: [PATCH 20/23] fix(rpcpool): fix data race --- node/rpcclient/rpcpool.go | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/node/rpcclient/rpcpool.go b/node/rpcclient/rpcpool.go index ce223f19..93a97f6b 100644 --- a/node/rpcclient/rpcpool.go +++ b/node/rpcclient/rpcpool.go @@ -61,7 +61,7 @@ type RPCPool struct { // endpoints remain after filtering. func NewRPCPool(ctx types.Context, endpoints []string, logger *zap.Logger) (*RPCPool, error) { if len(endpoints) == 0 { - panic("endpoints slice cannot be empty") + return nil, errors.New("no RPC endpoints provided") } // Get timeout from context or use default @@ -331,8 +331,14 @@ func (p *RPCPool) tryAllEndpointsWithScoring(ctx context.Context, fn func(contex // Try endpoints in order of their scores (highest first) for _, client := range sortedClients { + // Take a snapshot of client fields under read lock to avoid data races + p.mu.RLock() + isHealthy := client.healthy + clientPtr := client.client + p.mu.RUnlock() + // Skip unhealthy clients - if !client.healthy || client.client == nil { + if !isHealthy || clientPtr == nil { continue } From ea3bf903d75955b8c1b77fce00c05ac5a68f7ae7 Mon Sep 17 00:00:00 2001 From: SeUkKim Date: Mon, 18 Aug 2025 21:48:57 +0900 Subject: [PATCH 21/23] refactor(rpcpool): refactor client-health and scoring methods to take an explicit target client --- node/rpcclient/rpcpool.go | 49 ++++++++++++-------------- node/rpcclient/rpcpool_test.go | 12 ++++--- node/rpcclient/scoring_test.go | 64 ++++++++++++++++++++-------------- 3 files changed, 68 insertions(+), 57 deletions(-) diff --git a/node/rpcclient/rpcpool.go b/node/rpcclient/rpcpool.go index 93a97f6b..e2d3beac 100644 --- a/node/rpcclient/rpcpool.go +++ b/node/rpcclient/rpcpool.go @@ -143,18 +143,17 @@ func (p *RPCPool) MoveToNextHealthyClient() *RPCClientInfo { return nil } -// MarkClientUnhealthy marks the current client as unhealthy -func (p *RPCPool) MarkClientUnhealthy(err error) { +// MarkClientUnhealthy marks the target client as unhealthy +func (p *RPCPool) MarkClientUnhealthy(target *RPCClientInfo, err error) { p.mu.Lock() defer p.mu.Unlock() - client := p.clients[p.currentIndex] - client.healthy = false - client.lastError = err - client.lastCheck = time.Now() + target.healthy = false + target.lastError = err + target.lastCheck = time.Now() p.logger.Warn("Marked RPC client as unhealthy", - zap.String("endpoint", client.endpoint), + zap.String("endpoint", target.endpoint), zap.Error(err)) } @@ -195,33 +194,31 @@ func (p *RPCPool) updateScore(clientInfo *RPCClientInfo, success bool, isTimeout } } -// UpdateScoreOnSuccess updates the score for the current client on successful request -func (p *RPCPool) UpdateScoreOnSuccess() { +// UpdateScoreOnSuccess updates the score for the target client on successful request +func (p *RPCPool) UpdateScoreOnSuccess(target *RPCClientInfo) { p.mu.Lock() defer p.mu.Unlock() - currentClient := p.clients[p.currentIndex] - p.updateScore(currentClient, true, false) + p.updateScore(target, true, false) p.logger.Debug("Updated endpoint score on success", - zap.String("endpoint", currentClient.endpoint), - zap.Float64("score", currentClient.score), - zap.Int64("success_count", currentClient.successCount)) + zap.String("endpoint", target.endpoint), + zap.Float64("score", target.score), + zap.Int64("success_count", target.successCount)) } -// UpdateScoreOnFailure updates the score for the current client on failed request -func (p *RPCPool) UpdateScoreOnFailure(err error, isTimeout bool) { +// UpdateScoreOnFailure updates the score for the target client on failed request +func (p *RPCPool) UpdateScoreOnFailure(target *RPCClientInfo, err error, isTimeout bool) { p.mu.Lock() defer p.mu.Unlock() - currentClient := p.clients[p.currentIndex] - p.updateScore(currentClient, false, isTimeout) + p.updateScore(target, false, isTimeout) p.logger.Debug("Updated endpoint score on failure", - zap.String("endpoint", currentClient.endpoint), - zap.Float64("score", currentClient.score), - zap.Int64("failure_count", currentClient.failureCount), - zap.Int64("timeout_count", currentClient.timeoutCount), + zap.String("endpoint", target.endpoint), + zap.Float64("score", target.score), + zap.Int64("failure_count", target.failureCount), + zap.Int64("timeout_count", target.timeoutCount), zap.Bool("is_timeout", isTimeout), zap.Error(err)) } @@ -336,7 +333,7 @@ func (p *RPCPool) tryAllEndpointsWithScoring(ctx context.Context, fn func(contex isHealthy := client.healthy clientPtr := client.client p.mu.RUnlock() - + // Skip unhealthy clients if !isHealthy || clientPtr == nil { continue @@ -364,13 +361,13 @@ func (p *RPCPool) tryAllEndpointsWithScoring(ctx context.Context, fn func(contex isTimeout = err != nil && (errors.Is(timeoutCtx.Err(), context.DeadlineExceeded)) if err == nil { - p.UpdateScoreOnSuccess() + p.UpdateScoreOnSuccess(client) return nil } // Failure - update score negatively and mark as unhealthy - p.UpdateScoreOnFailure(err, isTimeout) - p.MarkClientUnhealthy(err) + p.UpdateScoreOnFailure(client, err, isTimeout) + p.MarkClientUnhealthy(client, err) lastErr = err p.logEndpointFailure(client.endpoint, err, retryAttempt) } diff --git a/node/rpcclient/rpcpool_test.go b/node/rpcclient/rpcpool_test.go index f967478a..f7441fec 100644 --- a/node/rpcclient/rpcpool_test.go +++ b/node/rpcclient/rpcpool_test.go @@ -269,13 +269,15 @@ func TestNewRPCPool_MixedEndpoints(t *testing.T) { assert.True(t, foundWarning, "Should have logged warning for invalid endpoint") } -// TestNewRPCPool_EmptyEndpoints tests NewRPCPool with empty endpoints (should panic) +// TestNewRPCPool_EmptyEndpoints tests NewRPCPool with empty endpoints (should return error) func TestNewRPCPool_EmptyEndpoints(t *testing.T) { logger := zaptest.NewLogger(t) ctx := createTestContext(logger) - assert.Panics(t, func() { - emptyEndpoints := []string{} - _, _ = NewRPCPool(ctx, emptyEndpoints, logger) - }, "NewRPCPool should panic with empty endpoints") + emptyEndpoints := []string{} + pool, err := NewRPCPool(ctx, emptyEndpoints, logger) + + assert.Error(t, err, "NewRPCPool should return error with empty endpoints") + assert.Nil(t, pool, "Pool should be nil when no endpoints provided") + assert.Contains(t, err.Error(), "no RPC endpoints provided", "Error should indicate no endpoints provided") } diff --git a/node/rpcclient/scoring_test.go b/node/rpcclient/scoring_test.go index f8fdff32..ba6165e4 100644 --- a/node/rpcclient/scoring_test.go +++ b/node/rpcclient/scoring_test.go @@ -53,16 +53,17 @@ func TestRPCPool_UpdateScoreOnSuccess(t *testing.T) { pool.mu.RLock() initialScore := pool.clients[0].score + currentClient := pool.clients[pool.currentIndex] pool.mu.RUnlock() // Update score on success - pool.UpdateScoreOnSuccess() + pool.UpdateScoreOnSuccess(currentClient) pool.mu.RLock() - currentClient := pool.clients[pool.currentIndex] - assert.Equal(t, initialScore+ScoreIncreaseOnSuccess, currentClient.score, "Score should increase on success") - assert.Equal(t, int64(1), currentClient.successCount, "Success count should increment") - assert.Equal(t, int64(0), currentClient.failureCount, "Failure count should remain 0") + updatedClient := pool.clients[pool.currentIndex] + assert.Equal(t, initialScore+ScoreIncreaseOnSuccess, updatedClient.score, "Score should increase on success") + assert.Equal(t, int64(1), updatedClient.successCount, "Success count should increment") + assert.Equal(t, int64(0), updatedClient.failureCount, "Failure count should remain 0") pool.mu.RUnlock() } @@ -72,18 +73,19 @@ func TestRPCPool_UpdateScoreOnFailure(t *testing.T) { pool.mu.RLock() initialScore := pool.clients[0].score + currentClient := pool.clients[pool.currentIndex] pool.mu.RUnlock() testErr := fmt.Errorf("test error") // Test regular failure - pool.UpdateScoreOnFailure(testErr, false) + pool.UpdateScoreOnFailure(currentClient, testErr, false) pool.mu.RLock() - currentClient := pool.clients[pool.currentIndex] - assert.Equal(t, initialScore-ScoreDecayOnFailure, currentClient.score, "Score should decrease on failure") - assert.Equal(t, int64(0), currentClient.successCount, "Success count should remain 0") - assert.Equal(t, int64(1), currentClient.failureCount, "Failure count should increment") - assert.Equal(t, int64(0), currentClient.timeoutCount, "Timeout count should remain 0") + updatedClient := pool.clients[pool.currentIndex] + assert.Equal(t, initialScore-ScoreDecayOnFailure, updatedClient.score, "Score should decrease on failure") + assert.Equal(t, int64(0), updatedClient.successCount, "Success count should remain 0") + assert.Equal(t, int64(1), updatedClient.failureCount, "Failure count should increment") + assert.Equal(t, int64(0), updatedClient.timeoutCount, "Timeout count should remain 0") pool.mu.RUnlock() } @@ -93,18 +95,19 @@ func TestRPCPool_UpdateScoreOnTimeout(t *testing.T) { pool.mu.RLock() initialScore := pool.clients[0].score + currentClient := pool.clients[pool.currentIndex] pool.mu.RUnlock() testErr := fmt.Errorf("timeout error") // Test timeout failure - pool.UpdateScoreOnFailure(testErr, true) + pool.UpdateScoreOnFailure(currentClient, testErr, true) pool.mu.RLock() - currentClient := pool.clients[pool.currentIndex] - assert.Equal(t, initialScore-ScoreDecayOnTimeout, currentClient.score, "Score should decrease more on timeout") - assert.Equal(t, int64(0), currentClient.successCount, "Success count should remain 0") - assert.Equal(t, int64(1), currentClient.failureCount, "Failure count should increment") - assert.Equal(t, int64(1), currentClient.timeoutCount, "Timeout count should increment") + updatedClient := pool.clients[pool.currentIndex] + assert.Equal(t, initialScore-ScoreDecayOnTimeout, updatedClient.score, "Score should decrease more on timeout") + assert.Equal(t, int64(0), updatedClient.successCount, "Success count should remain 0") + assert.Equal(t, int64(1), updatedClient.failureCount, "Failure count should increment") + assert.Equal(t, int64(1), updatedClient.timeoutCount, "Timeout count should increment") pool.mu.RUnlock() } @@ -113,8 +116,12 @@ func TestRPCPool_ScoreBounds(t *testing.T) { pool := createTestRPCPool(t, endpoints) // Test maximum score bound + pool.mu.RLock() + targetClient := pool.clients[0] + pool.mu.RUnlock() + for i := 0; i < 50; i++ { - pool.UpdateScoreOnSuccess() + pool.UpdateScoreOnSuccess(targetClient) } pool.mu.RLock() @@ -129,7 +136,7 @@ func TestRPCPool_ScoreBounds(t *testing.T) { // Test minimum score bound testErr := fmt.Errorf("test error") for i := 0; i < 50; i++ { - pool.UpdateScoreOnFailure(testErr, false) + pool.UpdateScoreOnFailure(targetClient, testErr, false) } pool.mu.RLock() @@ -382,10 +389,11 @@ func TestRPCPool_ScoreInflationPrevention(t *testing.T) { pool.mu.RLock() initialScore := pool.clients[0].score + targetClient := pool.clients[0] pool.mu.RUnlock() // Test multiple successful requests - score should only increase once - pool.UpdateScoreOnSuccess() + pool.UpdateScoreOnSuccess(targetClient) pool.mu.RLock() firstSuccessScore := pool.clients[0].score firstSuccessCount := pool.clients[0].successCount @@ -396,7 +404,7 @@ func TestRPCPool_ScoreInflationPrevention(t *testing.T) { // Additional successful requests should continue to increase score for i := 0; i < 5; i++ { - pool.UpdateScoreOnSuccess() + pool.UpdateScoreOnSuccess(targetClient) } pool.mu.RLock() @@ -410,7 +418,7 @@ func TestRPCPool_ScoreInflationPrevention(t *testing.T) { // Test that failure resets the success state testErr := fmt.Errorf("test error") - pool.UpdateScoreOnFailure(testErr, false) + pool.UpdateScoreOnFailure(targetClient, testErr, false) pool.mu.RLock() afterFailureScore := pool.clients[0].score @@ -419,7 +427,7 @@ func TestRPCPool_ScoreInflationPrevention(t *testing.T) { assert.Equal(t, expectedFinalScore-ScoreDecayOnFailure, afterFailureScore, "Score should decrease on failure") // Test that score can increase again after failure - pool.UpdateScoreOnSuccess() + pool.UpdateScoreOnSuccess(targetClient) pool.mu.RLock() recoveryScore := pool.clients[0].score @@ -432,13 +440,17 @@ func TestRPCPool_ConcurrentScoreUpdates(t *testing.T) { endpoints := []string{"http://endpoint1:26657"} pool := createTestRPCPool(t, endpoints) + pool.mu.RLock() + targetClient := pool.clients[0] + pool.mu.RUnlock() + // Run concurrent score updates done := make(chan bool, 2) // Goroutine 1: Success updates go func() { for i := 0; i < 50; i++ { - pool.UpdateScoreOnSuccess() + pool.UpdateScoreOnSuccess(targetClient) } done <- true }() @@ -447,7 +459,7 @@ func TestRPCPool_ConcurrentScoreUpdates(t *testing.T) { go func() { testErr := fmt.Errorf("test error") for i := 0; i < 10; i++ { - pool.UpdateScoreOnFailure(testErr, false) + pool.UpdateScoreOnFailure(targetClient, testErr, false) } done <- true }() @@ -475,4 +487,4 @@ func TestRPCPool_ConcurrentScoreUpdates(t *testing.T) { assert.GreaterOrEqual(t, client.score, MinScore, "Score should not be below MinScore") assert.LessOrEqual(t, client.score, MaxScore, "Score should not exceed MaxScore") pool.mu.RUnlock() -} +} \ No newline at end of file From 1980ef574bd0ff4002e602677f8d7e446443646a Mon Sep 17 00:00:00 2001 From: SeUkKim Date: Mon, 18 Aug 2025 21:55:29 +0900 Subject: [PATCH 22/23] style(rpcpool): format code with gofmt --- node/rpcclient/rpcpool_test.go | 2 +- node/rpcclient/scoring_test.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/node/rpcclient/rpcpool_test.go b/node/rpcclient/rpcpool_test.go index f7441fec..6c30da32 100644 --- a/node/rpcclient/rpcpool_test.go +++ b/node/rpcclient/rpcpool_test.go @@ -276,7 +276,7 @@ func TestNewRPCPool_EmptyEndpoints(t *testing.T) { emptyEndpoints := []string{} pool, err := NewRPCPool(ctx, emptyEndpoints, logger) - + assert.Error(t, err, "NewRPCPool should return error with empty endpoints") assert.Nil(t, pool, "Pool should be nil when no endpoints provided") assert.Contains(t, err.Error(), "no RPC endpoints provided", "Error should indicate no endpoints provided") diff --git a/node/rpcclient/scoring_test.go b/node/rpcclient/scoring_test.go index ba6165e4..9d48ce63 100644 --- a/node/rpcclient/scoring_test.go +++ b/node/rpcclient/scoring_test.go @@ -119,7 +119,7 @@ func TestRPCPool_ScoreBounds(t *testing.T) { pool.mu.RLock() targetClient := pool.clients[0] pool.mu.RUnlock() - + for i := 0; i < 50; i++ { pool.UpdateScoreOnSuccess(targetClient) } @@ -487,4 +487,4 @@ func TestRPCPool_ConcurrentScoreUpdates(t *testing.T) { assert.GreaterOrEqual(t, client.score, MinScore, "Score should not be below MinScore") assert.LessOrEqual(t, client.score, MaxScore, "Score should not exceed MaxScore") pool.mu.RUnlock() -} \ No newline at end of file +} From dd3db87c9482f6491771ca2a036f7a3f2d52101a Mon Sep 17 00:00:00 2001 From: SeUkKim Date: Mon, 18 Aug 2025 22:05:26 +0900 Subject: [PATCH 23/23] test(rpcpool): use fully qualified urls in tests. --- node/rpcclient/rpcpool_test.go | 38 +++++++++++++++++----------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/node/rpcclient/rpcpool_test.go b/node/rpcclient/rpcpool_test.go index 6c30da32..de8b2bdd 100644 --- a/node/rpcclient/rpcpool_test.go +++ b/node/rpcclient/rpcpool_test.go @@ -22,42 +22,42 @@ func createTestContext(logger *zap.Logger) types.Context { func TestRPCPool_GetCurrentEndpoint(t *testing.T) { logger := zaptest.NewLogger(t) - endpoints := []string{"doi", "moro", "rene"} + endpoints := []string{"http://doi:26657", "http://moro:26657", "http://rene:26657"} pool, err := NewRPCPool(createTestContext(logger), endpoints, logger) assert.NoError(t, err) // Initial endpoint should be the first one - assert.Equal(t, "doi", pool.GetCurrentClient().endpoint) + assert.Equal(t, "http://doi:26657", pool.GetCurrentClient().endpoint) } func TestRPCPool_MoveToNextHealthyClient(t *testing.T) { logger := zaptest.NewLogger(t) - endpoints := []string{"doi", "moro", "rene"} + endpoints := []string{"http://doi:26657", "http://moro:26657", "http://rene:26657"} pool, err := NewRPCPool(createTestContext(logger), endpoints, logger) assert.NoError(t, err) // Move to next healthy client client := pool.MoveToNextHealthyClient() assert.NotNil(t, client) - assert.Equal(t, "moro", client.endpoint) - assert.Equal(t, "moro", pool.GetCurrentClient().endpoint) + assert.Equal(t, "http://moro:26657", client.endpoint) + assert.Equal(t, "http://moro:26657", pool.GetCurrentClient().endpoint) // Move to next healthy client again client = pool.MoveToNextHealthyClient() assert.NotNil(t, client) - assert.Equal(t, "rene", client.endpoint) - assert.Equal(t, "rene", pool.GetCurrentClient().endpoint) + assert.Equal(t, "http://rene:26657", client.endpoint) + assert.Equal(t, "http://rene:26657", pool.GetCurrentClient().endpoint) // Move to next healthy client should wrap around client = pool.MoveToNextHealthyClient() assert.NotNil(t, client) - assert.Equal(t, "doi", client.endpoint) - assert.Equal(t, "doi", pool.GetCurrentClient().endpoint) + assert.Equal(t, "http://doi:26657", client.endpoint) + assert.Equal(t, "http://doi:26657", pool.GetCurrentClient().endpoint) } func TestRPCPool_ExecuteWithFallback_Success(t *testing.T) { logger := zaptest.NewLogger(t) - endpoints := []string{"doi", "moro", "rene"} + endpoints := []string{"http://doi:26657", "http://moro:26657", "http://rene:26657"} pool, err := NewRPCPool(createTestContext(logger), endpoints, logger) assert.NoError(t, err) @@ -70,12 +70,12 @@ func TestRPCPool_ExecuteWithFallback_Success(t *testing.T) { assert.NoError(t, err) assert.Equal(t, 1, callCount) - assert.Equal(t, "doi", pool.GetCurrentClient().endpoint) + assert.Equal(t, "http://doi:26657", pool.GetCurrentClient().endpoint) } func TestRPCPool_ExecuteWithFallback_FallbackSuccess(t *testing.T) { logger := zaptest.NewLogger(t) - endpoints := []string{"doi", "moro", "rene"} + endpoints := []string{"http://doi:26657", "http://moro:26657", "http://rene:26657"} pool, err := NewRPCPool(createTestContext(logger), endpoints, logger) assert.NoError(t, err) @@ -91,12 +91,12 @@ func TestRPCPool_ExecuteWithFallback_FallbackSuccess(t *testing.T) { assert.NoError(t, err) assert.Equal(t, 2, callCount) - assert.Equal(t, "moro", pool.GetCurrentClient().endpoint) + assert.Equal(t, "http://moro:26657", pool.GetCurrentClient().endpoint) } func TestRPCPool_ExecuteWithFallback_AllFail(t *testing.T) { logger := zaptest.NewLogger(t) - endpoints := []string{"doi", "moro"} + endpoints := []string{"http://doi:26657", "http://moro:26657"} pool, err := NewRPCPool(createTestContext(logger), endpoints, logger) assert.NoError(t, err) pool.maxRetries = 1 // Set to 1 for faster test @@ -116,7 +116,7 @@ func TestRPCPool_ExecuteWithFallback_AllFail(t *testing.T) { func TestRPCPool_ExecuteWithFallback_Timeout(t *testing.T) { logger := zaptest.NewLogger(t) - endpoints := []string{"doi"} + endpoints := []string{"http://doi:26657"} pool, err := NewRPCPool(createTestContext(logger), endpoints, logger) assert.NoError(t, err) pool.rpcTimeout = 100 * time.Millisecond @@ -137,7 +137,7 @@ func TestRPCPool_ExecuteWithFallback_Timeout(t *testing.T) { func TestRPCPool_ExecuteWithFallback_RetrySuccess(t *testing.T) { logger := zaptest.NewLogger(t) - endpoints := []string{"doi"} + endpoints := []string{"http://doi:26657"} pool, err := NewRPCPool(createTestContext(logger), endpoints, logger) assert.NoError(t, err) pool.maxRetries = 2 @@ -159,7 +159,7 @@ func TestRPCPool_ExecuteWithFallback_RetrySuccess(t *testing.T) { func TestRPCPool_ExecuteWithFallback_ContextCancellation(t *testing.T) { logger := zaptest.NewLogger(t) - endpoints := []string{"doi", "moro"} + endpoints := []string{"http://doi:26657", "http://moro:26657"} pool, err := NewRPCPool(createTestContext(logger), endpoints, logger) assert.NoError(t, err) @@ -191,13 +191,13 @@ func TestRPCPool_Logging(t *testing.T) { core, recorded := observer.New(zap.InfoLevel) logger := zap.New(core) - endpoints := []string{"doi", "moro"} + endpoints := []string{"http://doi:26657", "http://moro:26657"} pool, err := NewRPCPool(createTestContext(logger), endpoints, logger) assert.NoError(t, err) // Function fails on first endpoint, succeeds on second _ = pool.ExecuteWithFallback(context.Background(), func(ctx context.Context) error { - if pool.GetCurrentClient().endpoint == "doi" { + if pool.GetCurrentClient().endpoint == "http://doi:26657" { return errors.New("first endpoint failed") } return nil