From e9e4c0b69c10b8fd8f11373e1c906eafa51914b9 Mon Sep 17 00:00:00 2001 From: bruce-riley <96066700+bruce-riley@users.noreply.github.com> Date: Mon, 6 Nov 2023 08:55:44 -0600 Subject: [PATCH] Node/EVM: Suppress unnecessary polling errors (#3495) * Node/EVM: Suppress unnecessary polling errors * Code review rework --- .../watchers/evm/connectors/batch_poller.go | 48 +++++++++++----- .../evm/connectors/batch_poller_test.go | 55 ++++++++++--------- node/pkg/watchers/evm/connectors/poller.go | 2 +- 3 files changed, 63 insertions(+), 42 deletions(-) diff --git a/node/pkg/watchers/evm/connectors/batch_poller.go b/node/pkg/watchers/evm/connectors/batch_poller.go index 77039f0c7..a8f4a45ca 100644 --- a/node/pkg/watchers/evm/connectors/batch_poller.go +++ b/node/pkg/watchers/evm/connectors/batch_poller.go @@ -143,27 +143,45 @@ func (b *BatchPollConnector) pollBlocks(ctx context.Context, logger *zap.Logger, for idx, newBlock := range newBlocks { if newBlock.Number.Cmp(prevBlocks[idx].Number) > 0 { - // If there is a gap between prev and new, we have to look up the transaction hashes for the missing ones. Do that in batches. + // If there is a gap between prev and new, we have to look up the hashes for the missing ones. Do that in batches. newBlockNum := newBlock.Number.Uint64() blockNum := prevBlocks[idx].Number.Uint64() + 1 - for blockNum < newBlockNum { + errorFound := false + lastPublishedBlock := prevBlocks[idx] + for blockNum < newBlockNum && !errorFound { batchSize := newBlockNum - blockNum if batchSize > MAX_GAP_BATCH_SIZE { batchSize = MAX_GAP_BATCH_SIZE } gapBlocks, err := b.getBlockRange(ctx, logger, blockNum, batchSize, b.batchData[idx].finality) if err != nil { - return prevBlocks, fmt.Errorf("failed to get gap blocks: %w", err) - } - for _, block := range gapBlocks { - b.blockFeed.Send(block) + // We don't return an error here because we want to go on and check the other finalities. + logger.Error("failed to get gap blocks", zap.Stringer("finality", b.batchData[idx].finality), zap.Error(err)) + errorFound = true + } else { + // Play out the blocks in this batch. If the block number is zero, that means we failed to retrieve it, so we should stop there. + for _, block := range gapBlocks { + if block.Number.Uint64() == 0 { + errorFound = true + break + } + + b.blockFeed.Send(block) + lastPublishedBlock = block + } } + blockNum += batchSize } - b.blockFeed.Send(newBlock) + if !errorFound { + // The original value of newBlocks is still good. + b.blockFeed.Send(newBlock) + } else { + newBlocks[idx] = lastPublishedBlock + } } else if newBlock.Number.Cmp(prevBlocks[idx].Number) < 0 { - logger.Warn("latest block number went backwards, ignoring it", zap.Any("newLatest", newBlock.Number), zap.Any("prevLatest", prevBlocks[idx].Number)) + logger.Debug("latest block number went backwards, ignoring it", zap.Stringer("finality", b.batchData[idx].finality), zap.Any("new", newBlock.Number), zap.Any("prev", prevBlocks[idx].Number)) newBlocks[idx] = prevBlocks[idx] } } @@ -204,12 +222,13 @@ func (b *BatchPollConnector) getBlocks(ctx context.Context, logger *zap.Logger) return nil, err } + var n big.Int m := &result.result if m.Number == nil { - logger.Error("failed to unmarshal block: Number is nil", zap.Stringer("finality", finality), zap.String("tag", b.batchData[idx].tag)) - return nil, fmt.Errorf("failed to unmarshal block: Number is nil") + logger.Debug("number is nil, treating as zero", zap.Stringer("finality", finality), zap.String("tag", b.batchData[idx].tag)) + } else { + n = big.Int(*m.Number) } - n := big.Int(*m.Number) var l1bn *big.Int if m.L1BlockNumber != nil { @@ -262,12 +281,13 @@ func (b *BatchPollConnector) getBlockRange(ctx context.Context, logger *zap.Logg return nil, err } + var n big.Int m := &result.result if m.Number == nil { - logger.Error("failed to unmarshal block: Number is nil") - return nil, fmt.Errorf("failed to unmarshal block: Number is nil") + logger.Debug("number is nil, treating as zero", zap.Stringer("finality", finality), zap.String("tag", b.batchData[idx].tag)) + } else { + n = big.Int(*m.Number) } - n := big.Int(*m.Number) var l1bn *big.Int if m.L1BlockNumber != nil { diff --git a/node/pkg/watchers/evm/connectors/batch_poller_test.go b/node/pkg/watchers/evm/connectors/batch_poller_test.go index 5545965d1..bf8e9dd97 100644 --- a/node/pkg/watchers/evm/connectors/batch_poller_test.go +++ b/node/pkg/watchers/evm/connectors/batch_poller_test.go @@ -96,6 +96,14 @@ func (e *mockConnectorForBatchPoller) RawCallContext(ctx context.Context, result func (e *mockConnectorForBatchPoller) RawBatchCallContext(ctx context.Context, b []ethRpc.BatchElem) (err error) { e.mutex.Lock() + if e.err != nil { + err := e.err + if !e.persistentError { + e.err = nil + } + e.mutex.Unlock() + return err + } for _, entry := range b { if entry.Method != "eth_getBlockByNumber" { @@ -103,36 +111,29 @@ func (e *mockConnectorForBatchPoller) RawBatchCallContext(ctx context.Context, b } // If they set the error, return that immediately. - if e.err != nil { - entry.Error = e.err - } else { - var blockNumber uint64 - if entry.Args[0] == "latest" { - blockNumber = e.prevLatest - } else if entry.Args[0] == "safe" { - blockNumber = e.prevSafe - } else if entry.Args[0] == "finalized" { - blockNumber = e.prevFinalized - } - if len(e.blockNumbers) > 0 { - blockNumber = e.blockNumbers[0] - e.blockNumbers = e.blockNumbers[1:] - } - str := fmt.Sprintf(`{"author":"0x24c275f0719fdaec6356c4eb9f39ecb9c4d37ce1","baseFeePerGas":"0x3b9aca00","difficulty":"0x0","extraData":"0x","gasLimit":"0xe4e1c0","gasUsed":"0x0","hash":"0xfc8b62a31110121c57cfcccfaf2b147cc2c13b6d01bde4737846cefd29f045cf","logsBloom":"0x00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000","miner":"0x24c275f0719fdaec6356c4eb9f39ecb9c4d37ce1","nonce":"0x0000000000000000","number":"0x%x","parentHash":"0x09d6d33a658b712f41db7fb9f775f94911ae0132123116aa4f8cf3da9f774e89","receiptsRoot":"0x56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421","sha3Uncles":"0x1dcc4de8dec75d7aab85b567b6ccd41ad312451b948a7413f0a142fd40d49347","size":"0x201","stateRoot":"0x0409ed10e03fd49424ae1489c6fbc6ff1897f45d0e214655ebdb8df94eedc3c0","timestamp":"0x6373ec24","totalDifficulty":"0x0","transactions":[],"transactionsRoot":"0x56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421","uncles":[]}`, blockNumber) - err = json.Unmarshal([]byte(str), &entry.Result) - if entry.Args[0] == "latest" { - e.prevLatest = blockNumber - } else if entry.Args[0] == "safe" { - e.prevSafe = blockNumber - } else if entry.Args[0] == "finalized" { - e.prevFinalized = blockNumber - } + var blockNumber uint64 + if entry.Args[0] == "latest" { + blockNumber = e.prevLatest + } else if entry.Args[0] == "safe" { + blockNumber = e.prevSafe + } else if entry.Args[0] == "finalized" { + blockNumber = e.prevFinalized + } + if len(e.blockNumbers) > 0 { + blockNumber = e.blockNumbers[0] + e.blockNumbers = e.blockNumbers[1:] + } + str := fmt.Sprintf(`{"author":"0x24c275f0719fdaec6356c4eb9f39ecb9c4d37ce1","baseFeePerGas":"0x3b9aca00","difficulty":"0x0","extraData":"0x","gasLimit":"0xe4e1c0","gasUsed":"0x0","hash":"0xfc8b62a31110121c57cfcccfaf2b147cc2c13b6d01bde4737846cefd29f045cf","logsBloom":"0x00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000","miner":"0x24c275f0719fdaec6356c4eb9f39ecb9c4d37ce1","nonce":"0x0000000000000000","number":"0x%x","parentHash":"0x09d6d33a658b712f41db7fb9f775f94911ae0132123116aa4f8cf3da9f774e89","receiptsRoot":"0x56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421","sha3Uncles":"0x1dcc4de8dec75d7aab85b567b6ccd41ad312451b948a7413f0a142fd40d49347","size":"0x201","stateRoot":"0x0409ed10e03fd49424ae1489c6fbc6ff1897f45d0e214655ebdb8df94eedc3c0","timestamp":"0x6373ec24","totalDifficulty":"0x0","transactions":[],"transactionsRoot":"0x56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421","uncles":[]}`, blockNumber) + err = json.Unmarshal([]byte(str), &entry.Result) + if entry.Args[0] == "latest" { + e.prevLatest = blockNumber + } else if entry.Args[0] == "safe" { + e.prevSafe = blockNumber + } else if entry.Args[0] == "finalized" { + e.prevFinalized = blockNumber } } - if !e.persistentError { - e.err = nil - } e.mutex.Unlock() return diff --git a/node/pkg/watchers/evm/connectors/poller.go b/node/pkg/watchers/evm/connectors/poller.go index f3940056f..d35ffdac6 100644 --- a/node/pkg/watchers/evm/connectors/poller.go +++ b/node/pkg/watchers/evm/connectors/poller.go @@ -149,7 +149,7 @@ func (b *FinalizerPollConnector) pollBlock(ctx context.Context, logger *zap.Logg b.blockFeed.Send(newLatest) } else if newLatest.Number.Cmp(prevLatest.Number) < 0 { - logger.Warn("latest block number went backwards, ignoring it", zap.Any("newLatest", newLatest), zap.Any("prevLatest", prevLatest)) + logger.Debug("latest block number went backwards, ignoring it", zap.Any("newLatest", newLatest), zap.Any("prevLatest", prevLatest)) newLatest = prevLatest }