-
Notifications
You must be signed in to change notification settings - Fork 4
Improve event-driven reporter with cycle detection #31
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
f73dd2d
588e48f
9b12426
5e94993
38b41d6
b63bd2f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,9 +11,10 @@ | |
| "sync" | ||
| "sync/atomic" | ||
| "time" | ||
|
|
||
| "github.com/ethereum/go-ethereum/common" | ||
| "github.com/spf13/viper" | ||
| rpchttp "github.com/cometbft/cometbft/rpc/client/http" | ||
| globalfeetypes "github.com/strangelove-ventures/globalfee/x/globalfee/types" | ||
| customquery "github.com/tellor-io/layer-daemons/custom_query" | ||
| daemonflags "github.com/tellor-io/layer-daemons/flags" | ||
|
|
@@ -185,6 +186,8 @@ | |
| grpcMu sync.RWMutex | ||
| grpcConn *grpc.ClientConn | ||
| grpcClient daemontypes.GrpcClient | ||
|
|
||
| rpcClient *rpchttp.HTTP // direct reference for WebSocket subscriptions | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Adding this import in a new branch pr-31-review |
||
| grpcManager *grpcEndpointManager | ||
| rpcMu sync.RWMutex | ||
| rpcManager *rpcEndpointManager | ||
|
|
@@ -383,14 +386,13 @@ | |
| if err != nil { | ||
| return fmt.Errorf("failed to initialize RPC endpoint manager: %w", err) | ||
| } | ||
| rpcClient, rpcEndpoint, err := rpcManager.currentClient() | ||
| rpcClientVal, rpcEndpoint, err := rpcManager.currentClient() | ||
| if err != nil { | ||
| return fmt.Errorf("failed to create RPC client: %w", err) | ||
| } | ||
| c.logger.Info("CometBFT RPC client established", "endpoint", rpcEndpoint) | ||
| c.rpcManager = rpcManager | ||
| c.setRPCClient(rpcClient) | ||
|
|
||
| c.setRPCClient(rpcClientVal) | ||
| c.logger.Info("CometBFT RPC client established", "endpoint", rpcEndpoint) | ||
| encodingConfig := CreateEncodingConfig() | ||
| c.cosmosCtx = c.cosmosCtx.WithCodec(encodingConfig.Codec).WithInterfaceRegistry(encodingConfig.InterfaceRegistry).WithTxConfig(encodingConfig.TxConfig) | ||
|
|
||
|
|
@@ -535,56 +537,6 @@ | |
| return fmt.Errorf("%s failed on all gRPC endpoints: %w", operation, lastErr) | ||
| } | ||
|
|
||
| func (c *Client) RestorePrimaryEndpointsPeriodically(ctx context.Context, wg *sync.WaitGroup) { | ||
| defer wg.Done() | ||
|
|
||
| ticker := time.NewTicker(primaryEndpointCheckInterval) | ||
| defer ticker.Stop() | ||
|
|
||
| for { | ||
| select { | ||
| case <-ctx.Done(): | ||
| return | ||
| case <-ticker.C: | ||
| c.tryRestorePrimaryRPCEndpoint(ctx) | ||
| c.tryRestorePrimaryGRPCEndpoint(ctx) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| func (c *Client) tryRestorePrimaryRPCEndpoint(ctx context.Context) { | ||
| if c.rpcManager == nil || c.rpcManager.usingPrimary() { | ||
| return | ||
| } | ||
|
|
||
| probeCtx, cancel := context.WithTimeout(ctx, primaryEndpointProbeTimeout) | ||
| defer cancel() | ||
|
|
||
| rpcClient, endpoint, err := c.rpcManager.primaryClient() | ||
| if err != nil { | ||
| c.logger.Warn("Primary CometBFT RPC endpoint is not ready", "endpoint", endpoint, "error", err) | ||
| return | ||
| } | ||
| status, err := rpcClient.Status(probeCtx) | ||
| if err != nil { | ||
| c.logger.Warn("Primary CometBFT RPC endpoint health check failed", "endpoint", endpoint, "error", err) | ||
| return | ||
| } | ||
| chainID := c.chainID() | ||
| if status.NodeInfo.Network != chainID { | ||
| c.logger.Warn( | ||
| "Primary CometBFT RPC endpoint returned unexpected chain ID", | ||
| "endpoint", endpoint, | ||
| "expected_chain_id", chainID, | ||
| "actual_chain_id", status.NodeInfo.Network, | ||
| ) | ||
| return | ||
| } | ||
|
|
||
| c.setRPCClient(rpcClient) | ||
| c.rpcManager.switchToPrimary() | ||
| } | ||
|
|
||
| func (c *Client) tryRestorePrimaryGRPCEndpoint(ctx context.Context) { | ||
| if c.grpcManager == nil || c.grpcManager.usingPrimary() { | ||
| return | ||
|
|
@@ -640,25 +592,11 @@ | |
| } | ||
| } | ||
|
|
||
| func (c *Client) setRPCClient(rpcClient client.CometRPC) { | ||
| c.rpcMu.Lock() | ||
| defer c.rpcMu.Unlock() | ||
| c.cosmosCtxMu.Lock() | ||
| defer c.cosmosCtxMu.Unlock() | ||
| c.cosmosCtx = c.cosmosCtx.WithClient(rpcClient) | ||
| } | ||
|
|
||
| func (c *Client) rpcContextWithClient(rpcClient client.CometRPC) client.Context { | ||
| clientCtx := c.currentCosmosContext() | ||
| return clientCtx.WithClient(rpcClient) | ||
| } | ||
|
|
||
| func (c *Client) currentCosmosContext() client.Context { | ||
| c.cosmosCtxMu.RLock() | ||
| defer c.cosmosCtxMu.RUnlock() | ||
| return c.cosmosCtx | ||
| } | ||
|
|
||
| func (c *Client) chainID() string { | ||
| return c.currentCosmosContext().ChainID | ||
| } | ||
|
|
@@ -741,6 +679,88 @@ | |
| wg.Wait() | ||
| } | ||
|
|
||
| func (c *Client) setRPCClient(rpcClient client.CometRPC) { | ||
| c.rpcMu.Lock() | ||
| defer c.rpcMu.Unlock() | ||
| c.cosmosCtxMu.Lock() | ||
| defer c.cosmosCtxMu.Unlock() | ||
| if httpClient, ok := rpcClient.(*rpchttp.HTTP); ok { | ||
| c.rpcClient = httpClient | ||
| } | ||
| c.cosmosCtx = c.cosmosCtx.WithClient(rpcClient) | ||
| } | ||
|
|
||
| func (c *Client) currentCosmosContext() client.Context { | ||
| c.cosmosCtxMu.RLock() | ||
| defer c.cosmosCtxMu.RUnlock() | ||
| return c.cosmosCtx | ||
| } | ||
|
|
||
| func (c *Client) RestorePrimaryEndpointsPeriodically(ctx context.Context, wg *sync.WaitGroup) { | ||
| defer wg.Done() | ||
|
|
||
| if c.rpcManager == nil { | ||
| return | ||
| } | ||
|
|
||
| ticker := time.NewTicker(primaryEndpointCheckInterval) | ||
| defer ticker.Stop() | ||
|
|
||
| for { | ||
| select { | ||
| case <-ctx.Done(): | ||
| return | ||
| case <-ticker.C: | ||
| c.tryRestorePrimaryRPCEndpoint(ctx) | ||
| c.tryRestorePrimaryGRPCEndpoint(ctx) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| func (c *Client) tryRestorePrimaryRPCEndpoint(ctx context.Context) { | ||
| if c.rpcManager == nil || c.rpcManager.usingPrimary() { | ||
| return | ||
| } | ||
|
|
||
| probeCtx, cancel := context.WithTimeout(ctx, primaryEndpointProbeTimeout) | ||
| defer cancel() | ||
|
|
||
| rpcClientVal, endpoint, err := c.rpcManager.primaryClient() | ||
| if err != nil { | ||
| c.logger.Warn("Primary CometBFT RPC endpoint is not ready", "endpoint", endpoint, "error", err) | ||
| return | ||
| } | ||
| httpClient, ok := rpcClientVal.(*rpchttp.HTTP) | ||
| if !ok { | ||
| return | ||
| } | ||
| if !httpClient.IsRunning() { | ||
| if err := httpClient.Start(); err != nil { | ||
| c.logger.Warn("Primary CometBFT RPC endpoint failed to start", "endpoint", endpoint, "error", err) | ||
| return | ||
| } | ||
| } | ||
| status, err := httpClient.Status(probeCtx) | ||
| if err != nil { | ||
| c.logger.Warn("Primary CometBFT RPC endpoint health check failed", "endpoint", endpoint, "error", err) | ||
| return | ||
| } | ||
| chainID := c.currentCosmosContext().ChainID | ||
| if status.NodeInfo.Network != chainID { | ||
| c.logger.Warn( | ||
| "Primary CometBFT RPC endpoint returned unexpected chain ID", | ||
| "endpoint", endpoint, | ||
| "expected_chain_id", chainID, | ||
| "actual_chain_id", status.NodeInfo.Network, | ||
| ) | ||
| return | ||
| } | ||
|
|
||
| c.setRPCClient(rpcClientVal) | ||
| c.rpcManager.switchToPrimary() | ||
| c.logger.Info("CometBFT RPC endpoint restored to primary", "endpoint", endpoint) | ||
| } | ||
|
|
||
| func (c *Client) RefreshGasEstimatesPeriodically(ctx context.Context, wg *sync.WaitGroup) { | ||
| defer wg.Done() | ||
| ticker := time.NewTicker(c.refreshGasEstimatesInterval) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,6 +11,7 @@ import ( | |
| "sync" | ||
| "time" | ||
|
|
||
| rpchttp "github.com/cometbft/cometbft/rpc/client/http" | ||
| "github.com/ethereum/go-ethereum/accounts/abi" | ||
| "github.com/shirou/gopsutil/v3/process" | ||
| "github.com/spf13/viper" | ||
|
|
@@ -76,6 +77,177 @@ func validatorOperatorAddress(reporterAddr string) (string, string, error) { | |
|
|
||
| func (c *Client) MonitorCyclelistQuery(ctx context.Context, wg *sync.WaitGroup) { | ||
| defer wg.Done() | ||
|
|
||
| // Try to use event-driven detection (reacts within milliseconds of each new block). | ||
| // Falls back to 200ms ticker polling if the WebSocket subscription fails. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. added fallback behavior to next listed rpc in .env in this branch https://github.com/tellor-io/layer-daemons/tree/pr-31-review |
||
| blockCh, unsubscribe, err := c.subscribeNewBlocks(ctx) | ||
| if err != nil { | ||
| c.logger.Warn("block subscription failed, falling back to polling", "error", err) | ||
| c.monitorCyclelistQueryPolling(ctx) | ||
| return | ||
| } | ||
| defer unsubscribe() | ||
| c.logger.Info("MonitorCyclelistQuery: using event-driven block subscription") | ||
|
|
||
| // Fallback ticker: if no block event is received for 2s (e.g. slow node), poll anyway. | ||
| const blockEventTimeout = 2 * time.Second | ||
| fallback := time.NewTimer(blockEventTimeout) | ||
| defer fallback.Stop() | ||
|
|
||
| prevQueryData := []byte{} | ||
|
|
||
| checkCycle := func() { | ||
| queryCtx, cancel := context.WithTimeout(ctx, defaultQueryTimeout) | ||
| querydata, querymeta, err := c.CurrentQuery(queryCtx) | ||
| cancel() | ||
|
|
||
| if err != nil || querymeta == nil { | ||
| c.logger.Error("query failed", "error", err) | ||
| return | ||
| } | ||
|
|
||
| mutex.Lock() | ||
| hasCommited := commitedIds[querymeta.Id] | ||
| mutex.Unlock() | ||
| if bytes.Equal(querydata, prevQueryData) || hasCommited { | ||
| return | ||
| } | ||
|
|
||
| txCtx, cancel := context.WithTimeout(ctx, defaultTxTimeout) | ||
| done := make(chan struct{}) | ||
|
|
||
| c.logger.Info(fmt.Sprintf("starting to generate spot price report at %d", time.Now().Unix())) | ||
| go func() { | ||
| defer close(done) | ||
| if err := c.GenerateAndBroadcastSpotPriceReport(txCtx, querydata, querymeta); err != nil { | ||
| c.logger.Error("report generation failed", "error", err) | ||
| } | ||
| }() | ||
|
|
||
| select { | ||
| case <-done: | ||
| cancel() | ||
| case <-txCtx.Done(): | ||
| c.logger.Error(fmt.Sprintf("report generation timed out at %d", time.Now().Unix())) | ||
| cancel() | ||
| } | ||
|
|
||
| prevQueryData = querydata | ||
| } | ||
|
|
||
| for { | ||
| select { | ||
| case <-ctx.Done(): | ||
| return | ||
| case _, ok := <-blockCh: | ||
| if !ok { | ||
| // Channel closed; try to re-subscribe on a fallback RPC endpoint. | ||
| unsubscribe() | ||
| c.logger.Warn("block subscription channel closed, trying fallback RPC endpoint") | ||
| blockCh, unsubscribe, err = c.subscribeNewBlocks(ctx) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggest using subscribeNewBlocksFallback instead to make consistent with existing fallback behavior
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is a separate resource cleanup issue that we can solve after this is merged, or as part of this work: rpcEndpointManager creates fresh RPC clients, and the WebSocket path starts them with httpClient.Start(), but the cleanup only unsubscribes and never calls Stop(). When a subscription is replaced, a fallback endpoint is tried, Subscribe fails after Start(), or the primary RPC health check starts a client and then rejects it, the old CometBFT client workers/connections appear to be left running. Can we make the owner of each started RPC client stop it when it is no longer used? |
||
| if err != nil { | ||
| c.logger.Warn("block subscription failed on all RPC endpoints, falling back to polling", "error", err) | ||
| c.monitorCyclelistQueryPolling(ctx) | ||
| return | ||
| } | ||
| fallback.Reset(blockEventTimeout) | ||
| continue | ||
| } | ||
| fallback.Reset(blockEventTimeout) | ||
| checkCycle() | ||
| case <-fallback.C: | ||
| // No block event received within timeout; check anyway and reset. | ||
| fallback.Reset(blockEventTimeout) | ||
| checkCycle() | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // subscribeNewBlocks subscribes to CometBFT NewBlock events over WebSocket. | ||
| // Returns a channel that receives one value per block, an unsubscribe func, and any error. | ||
| // Tries each RPC endpoint in rpcManager before returning an error. | ||
| func (c *Client) subscribeNewBlocks(ctx context.Context) (<-chan struct{}, func(), error) { | ||
| if c.rpcManager == nil { | ||
| if c.rpcClient == nil { | ||
| return nil, func() {}, fmt.Errorf("rpc client not initialized") | ||
| } | ||
| return c.subscribeNewBlocksWithClient(ctx, c.rpcClient, "current") | ||
| } | ||
|
|
||
| var errs []string | ||
| rpcClientVal, endpoint, err := c.rpcManager.currentClient() | ||
| if err != nil { | ||
| errs = append(errs, fmt.Sprintf("%s: %v", endpoint, err)) | ||
| } else if blockCh, unsubscribe, err := c.subscribeNewBlocksWithClient(ctx, rpcClientVal, endpoint); err == nil { | ||
| c.setRPCClient(rpcClientVal) | ||
| return blockCh, unsubscribe, nil | ||
| } else { | ||
| errs = append(errs, fmt.Sprintf("%s: %v", endpoint, err)) | ||
| } | ||
|
|
||
| for attempt := 0; attempt < c.rpcManager.endpointCount()-1; attempt++ { | ||
| rpcClientVal, endpoint, err = c.rpcManager.nextClient() | ||
| if err != nil { | ||
| errs = append(errs, err.Error()) | ||
| break | ||
| } | ||
| blockCh, unsubscribe, err := c.subscribeNewBlocksWithClient(ctx, rpcClientVal, endpoint) | ||
| if err == nil { | ||
| c.setRPCClient(rpcClientVal) | ||
| return blockCh, unsubscribe, nil | ||
| } | ||
| errs = append(errs, fmt.Sprintf("%s: %v", endpoint, err)) | ||
| } | ||
|
|
||
| return nil, func() {}, fmt.Errorf("subscribing to NewBlock events on RPC endpoints: %s", strings.Join(errs, "; ")) | ||
| } | ||
|
|
||
| func (c *Client) subscribeNewBlocksWithClient(ctx context.Context, rpcClientVal interface{}, endpoint string) (<-chan struct{}, func(), error) { | ||
| httpClient, ok := rpcClientVal.(*rpchttp.HTTP) | ||
| if !ok { | ||
| return nil, func() {}, fmt.Errorf("RPC client for %s does not support WebSocket subscriptions", endpoint) | ||
| } | ||
| if !httpClient.IsRunning() { | ||
| if err := httpClient.Start(); err != nil { | ||
| return nil, func() {}, fmt.Errorf("starting rpc client for WebSocket: %w", err) | ||
| } | ||
| } | ||
| subscriber := fmt.Sprintf("reporter-cycle-monitor-%d", time.Now().UnixNano()) | ||
| eventCh, err := httpClient.Subscribe(ctx, subscriber, "tm.event='NewBlock'") | ||
| if err != nil { | ||
| return nil, func() {}, fmt.Errorf("subscribing to NewBlock events: %w", err) | ||
| } | ||
|
|
||
| blockCh := make(chan struct{}, 1) | ||
| go func() { | ||
| defer close(blockCh) | ||
| for { | ||
| select { | ||
| case <-ctx.Done(): | ||
| return | ||
| case _, ok := <-eventCh: | ||
| if !ok { | ||
| return | ||
| } | ||
| // Non-blocking send: if the consumer is busy processing the previous block, | ||
| // skip this event rather than queuing up a backlog. | ||
| select { | ||
| case blockCh <- struct{}{}: | ||
| default: | ||
| } | ||
| } | ||
| } | ||
| }() | ||
|
|
||
| unsubscribe := func() { | ||
| _ = httpClient.Unsubscribe(ctx, subscriber, "tm.event='NewBlock'") | ||
| } | ||
| return blockCh, unsubscribe, nil | ||
| } | ||
|
|
||
| // monitorCyclelistQueryPolling is the fallback ticker-based implementation used when | ||
| // the WebSocket block subscription is unavailable. | ||
| func (c *Client) monitorCyclelistQueryPolling(ctx context.Context) { | ||
| prevQueryData := []byte{} | ||
| retryDelay := defaultRetryDelay | ||
| ticker := time.NewTicker(defaultRetryDelay) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please address lint check