Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions bitcoinspv/bootstrap.go
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,8 @@ func (r *Relayer) backfillIndexer(ctx context.Context, startHeight, endHeight in
}
Comment thread
robert-zaremba marked this conversation as resolved.
}
}
Comment on lines 248 to 276
Copy link

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the bootstrap backfill flow, blocks are sent asynchronously via SendBlocks, and then Flush is called to wait for them. However, if the context is cancelled during the backfill loop (lines 250-251), the function returns immediately without calling Flush. This means any blocks already enqueued will continue processing in the background worker even though the backfill operation has been cancelled. Consider calling Flush() or handling context cancellation more gracefully to ensure consistent behavior.

Copilot uses AI. Check for mistakes.

r.btcIndexer.Flush()
r.logger.Info().Msg("Indexer backfill completed successfully.")
return nil
}
Expand Down
75 changes: 52 additions & 23 deletions bitcoinspv/clients/btcindexer/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,61 +19,82 @@
maxRetries = 4
initialBackoff = 500 * time.Millisecond
maxBackoff = 8 * time.Second
queueSize = 100
Comment thread
robert-zaremba marked this conversation as resolved.
Outdated
)

// Client is a client for communicating with the nBTC indexer worker.
// It wraps the btcindexer API client to add retry logic.
// It wraps the btcindexer API client to add retry logic and async sending.
type Client struct {

Check failure on line 27 in bitcoinspv/clients/btcindexer/client.go

View workflow job for this annotation

GitHub Actions / Run golangci-lint

fieldalignment: struct with 224 pointer bytes could be 216 (govet)
logger zerolog.Logger
apiClient btcindexer.Client
network string

blocksChan chan []*types.IndexedBlock
done chan struct{}
}
Comment on lines +44 to 76
Copy link

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The use of custom atomicBool and atomicError types instead of the standard library's sync/atomic package is unnecessary and introduces additional complexity. Go's sync/atomic package provides atomic.Bool (since Go 1.19) and more efficient atomic operations. The custom implementations use mutex-based synchronization which is less efficient than true atomic operations.

Consider using atomic.Bool for the closed flag and either atomic.Value or atomic.Pointer for the error.

Copilot uses AI. Check for mistakes.

// NewClient creates a new client for the indexer.
func NewClient(url string, network string, authToken string, parentLogger zerolog.Logger) *Client {
return &Client{
logger: parentLogger.With().Str("module", "btcindexer_client").Logger(),
apiClient: btcindexer.NewClient(url, authToken),
network: network,
c := &Client{
logger: parentLogger.With().Str("module", "btcindexer_client").Logger(),
apiClient: btcindexer.NewClient(url, authToken),
network: network,
blocksChan: make(chan []*types.IndexedBlock, queueSize),
done: make(chan struct{}),
}
go c.worker()
return c
Comment thread
robert-zaremba marked this conversation as resolved.
}
Comment on lines +90 to +92
Copy link

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The worker goroutine is started in the constructor without any mechanism for the caller to know if it has started successfully. If the goroutine panics immediately during startup (before reaching the defer recover), the client will be in an inconsistent state.

Consider using a sync.WaitGroup or channel to signal when the worker has successfully started, or document this limitation in the function comment.

Copilot uses AI. Check for mistakes.

// worker processes blocks from the queue in a background goroutine.
func (c *Client) worker() {
defer close(c.done)
for blocks := range c.blocksChan {
if err := c.sendBlocksWithRetry(blocks); err != nil {
c.logger.Error().Err(err).Msg("Failed to send blocks to indexer")
}
}
}
Comment thread
robert-zaremba marked this conversation as resolved.

// SendBlocks sends a batch of blocks to the indexer with a retry mechanism.
// TODO: this should not block the main process
// probably we should use CF queues
func (c *Client) SendBlocks(ctx context.Context, blocks []*types.IndexedBlock) error {
// SendBlocks enqueues blocks for async sending to the indexer.
// It returns an error only if the queue is full.
func (c *Client) SendBlocks(_ context.Context, blocks []*types.IndexedBlock) error {
if c == nil {
return errors.New("btcindexer.Client is not initialized")
}
if len(blocks) == 0 {
return nil
}

select {
case c.blocksChan <- blocks:
return nil
default:
Comment thread
robert-zaremba marked this conversation as resolved.
return errors.New("indexer queue is full, dropping blocks")
Comment thread
robert-zaremba marked this conversation as resolved.
Outdated
}
Comment thread
robert-zaremba marked this conversation as resolved.
Outdated
Comment on lines +132 to +145
Copy link

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a potential deadlock if Close() is called while SendBlocks is trying to enqueue blocks. The sequence is: SendBlocks calls wg.Add(1), then tries to send to blocksChan. Meanwhile, Close() closes blocksChan and waits on done. If the channel is full and gets closed while SendBlocks is in the select statement, SendBlocks will call wg.Done() but the worker will already have exited, causing the waitgroup count to become inconsistent.

The issue is that wg.Add(1) happens before checking if the client is closed and before attempting to enqueue. Consider moving the wg.Add(1) to after successful enqueueing, or add a more robust shutdown mechanism.

Copilot uses AI. Check for mistakes.
}

// sendBlocksWithRetry sends a batch of blocks to the indexer with a retry mechanism.
func (c *Client) sendBlocksWithRetry(blocks []*types.IndexedBlock) error {
payload, err := c.preparePayload(blocks)
if err != nil {
return err
}

var lastErr error
for attempt := 0; attempt <= maxRetries; attempt++ {
if ctx.Err() != nil {
return ctx.Err()
}

shouldRetry, err := c.sendAndHandleResponse(payload)
if err != nil {
// Non-retryable
return err
}

if !shouldRetry {
// Success
return nil
}

lastErr = fmt.Errorf("attempt %d failed, retrying", attempt+1)
c.logger.Warn().Err(err).Msg("Retrying indexer call...")
c.backoff(ctx, attempt)
c.backoff(attempt)
}

return fmt.Errorf("failed to send blocks to indexer after %d attempts: %w", maxRetries+1, lastErr)
Expand Down Expand Up @@ -123,24 +144,19 @@
return putBlocksReq, nil
}

func (c *Client) backoff(ctx context.Context, attempt int) {
func (c *Client) backoff(attempt int) {
if attempt >= maxRetries {
return
}
backoff := time.Duration(1<<attempt) * initialBackoff
if backoff > maxBackoff {
backoff = maxBackoff
}
// NOTE: we dont need secure random generation here, its just retry
jitter := time.Duration(rand.Intn(1000)) * time.Millisecond //nolint:gosec
totalBackoff := backoff + jitter

c.logger.Info().Dur("wait_duration", totalBackoff).Msgf("Waiting before next attempt..")

select {
case <-time.After(totalBackoff):
case <-ctx.Done():
}
time.Sleep(totalBackoff)
Comment thread
robert-zaremba marked this conversation as resolved.
Outdated
}

// GetLatestHeight returns the latest block height known to the indexer
Expand All @@ -153,3 +169,16 @@

return height, nil
}

// Close stops the background worker and waits for it to finish.
func (c *Client) Close() {
Comment thread
robert-zaremba marked this conversation as resolved.
close(c.blocksChan)
<-c.done
Comment thread
sourcery-ai[bot] marked this conversation as resolved.
Outdated
}
Comment thread
robert-zaremba marked this conversation as resolved.

// Flush waits for all queued blocks to be sent.
func (c *Client) Flush() {
for len(c.blocksChan) > 0 {
Comment thread
sourcery-ai[bot] marked this conversation as resolved.
Outdated
time.Sleep(100 * time.Millisecond)
}
}
Comment thread
robert-zaremba marked this conversation as resolved.
Outdated
Comment thread
robert-zaremba marked this conversation as resolved.
Outdated
Comment on lines +285 to +295
Copy link

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Flush() only tracks the last error that occurred, not all errors. If multiple batches fail to send (e.g., blocks at heights 100-119 and 140-159), only the most recent error will be returned. This could result in incomplete information about what actually failed during a flush operation.

Consider either accumulating errors (e.g., using errors.Join) or maintaining a counter of failed batches along with the error.

Copilot uses AI. Check for mistakes.
2 changes: 2 additions & 0 deletions bitcoinspv/clients/btcindexer/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,6 @@ import (
type Indexer interface {
SendBlocks(ctx context.Context, blocks []*types.IndexedBlock) error
GetLatestHeight() (int64, error)
Flush()
Close()
}
2 changes: 1 addition & 1 deletion bitcoinspv/clients/mocks/mock_BTCClient.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion bitcoinspv/clients/mocks/mock_BitcoinSPV.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

66 changes: 65 additions & 1 deletion bitcoinspv/clients/mocks/mock_Indexer.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions bitcoinspv/relayer.go
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,9 @@ func (r *Relayer) Stop() {
// WaitForShutdown waits for all relayer goroutines to complete before returning
func (r *Relayer) WaitForShutdown() {
r.wg.Wait()
if r.btcIndexer != nil {
r.btcIndexer.Close()
}
Comment thread
robert-zaremba marked this conversation as resolved.
Outdated
}

// UploadToWalrus upload full BTC block to Walrus
Expand Down
Loading