From 8e4c931eeb1b5793cc44a221aa54bb3029db0bf0 Mon Sep 17 00:00:00 2001 From: Lee Bousfield Date: Sat, 9 Sep 2023 22:14:20 -0600 Subject: [PATCH 1/3] Handle block "not found" case in batch revert polling --- arbnode/batch_poster.go | 40 ++++++++++++++++++++++++---------------- 1 file changed, 24 insertions(+), 16 deletions(-) diff --git a/arbnode/batch_poster.go b/arbnode/batch_poster.go index 42b983f0fb..8144c9b7be 100644 --- a/arbnode/batch_poster.go +++ b/arbnode/batch_poster.go @@ -11,6 +11,7 @@ import ( "fmt" "math" "math/big" + "strings" "sync/atomic" "time" @@ -262,12 +263,13 @@ func NewBatchPoster(dataPosterDB ethdb.Database, l1Reader *headerreader.HeaderRe // contain reverted batch_poster transaction. // It returns true if it finds batch posting needs to halt, which is true if a batch reverts // unless the data poster is configured with noop storage which can tolerate reverts. -func (b *BatchPoster) checkReverts(ctx context.Context, from, to int64) (bool, error) { - if from > to { - return false, fmt.Errorf("wrong range, from: %d is more to: %d", from, to) +// From must be a pointer to the starting block, which is updated after each block is checked for reverts +func (b *BatchPoster) checkReverts(ctx context.Context, from *int64, to int64) (bool, error) { + if *from > to { + return false, fmt.Errorf("wrong range, from: %d > to: %d", from, to) } - for idx := from; idx <= to; idx++ { - number := big.NewInt(idx) + for ; *from <= to; *from++ { + number := big.NewInt(*from) block, err := b.l1Reader.Client().BlockByNumber(ctx, number) if err != nil { return false, fmt.Errorf("getting block: %v by number: %w", number, err) @@ -277,7 +279,7 @@ func (b *BatchPoster) checkReverts(ctx context.Context, from, to int64) (bool, e if err != nil { return false, fmt.Errorf("getting sender of transaction tx: %v, %w", tx.Hash(), err) } - if bytes.Equal(from.Bytes(), b.dataPoster.Sender().Bytes()) { + if from == b.dataPoster.Sender() { r, err := b.l1Reader.Client().TransactionReceipt(ctx, tx.Hash()) if err != nil { return false, fmt.Errorf("getting a receipt for transaction: %v, %w", tx.Hash(), err) @@ -303,7 +305,7 @@ func (b *BatchPoster) pollForReverts(ctx context.Context) { headerCh, unsubscribe := b.l1Reader.Subscribe(false) defer unsubscribe() - last := int64(0) // number of last seen block + nextToCheck := int64(0) // the first unchecked block for { // Poll until: // - L1 headers reader channel is closed, or @@ -312,31 +314,37 @@ func (b *BatchPoster) pollForReverts(ctx context.Context) { select { case h, ok := <-headerCh: if !ok { - log.Info("L1 headers channel has been closed") + log.Info("L1 headers channel checking for batch poster reverts has been closed") return } // If this is the first block header, set last seen as number-1. // We may see same block number again if there is L1 reorg, in that // case we check the block again. - if last == 0 || last == h.Number.Int64() { - last = h.Number.Int64() - 1 + if nextToCheck == 0 || nextToCheck == h.Number.Int64() { + nextToCheck = h.Number.Int64() } - if h.Number.Int64()-last > 100 { - log.Warn("Large gap between last seen and current block number, skipping check for reverts", "last", last, "current", h.Number) - last = h.Number.Int64() + if h.Number.Int64()-nextToCheck > 100 { + log.Warn("Large gap between last seen and current block number, skipping check for reverts", "last", nextToCheck, "current", h.Number) + nextToCheck = h.Number.Int64() continue } - reverted, err := b.checkReverts(ctx, last+1, h.Number.Int64()) + reverted, err := b.checkReverts(ctx, &nextToCheck, h.Number.Int64()) if err != nil { - log.Error("Checking batch reverts", "error", err) + logLevel := log.Error + if strings.Contains(err.Error(), "not found") { + // Just parent chain node inconsistency + // One node sent us a block, but another didn't have it + // We'll try to check this block again next loop + logLevel = log.Debug + } + logLevel("Error checking batch reverts", "err", err) continue } if reverted { b.batchReverted.Store(true) return } - last = h.Number.Int64() case <-ctx.Done(): return } From e9dd36a900545be953f38449bf57aa8317808e58 Mon Sep 17 00:00:00 2001 From: Lee Bousfield Date: Tue, 12 Sep 2023 12:18:13 -0600 Subject: [PATCH 2/3] Don't confirmDataPosterIsReady if watchtower --- staker/staker.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/staker/staker.go b/staker/staker.go index 8fdbbd648f..1b6538b161 100644 --- a/staker/staker.go +++ b/staker/staker.go @@ -552,11 +552,11 @@ func (s *Staker) confirmDataPosterIsReady(ctx context.Context) error { } func (s *Staker) Act(ctx context.Context) (*types.Transaction, error) { - err := s.confirmDataPosterIsReady(ctx) - if err != nil { - return nil, err - } if s.config.strategy != WatchtowerStrategy { + err := s.confirmDataPosterIsReady(ctx) + if err != nil { + return nil, err + } whitelisted, err := s.IsWhitelisted(ctx) if err != nil { return nil, fmt.Errorf("error checking if whitelisted: %w", err) From 39480a0425ff76b4b743273004eb029f5d86aa51 Mon Sep 17 00:00:00 2001 From: Lee Bousfield Date: Wed, 13 Sep 2023 11:14:11 -0600 Subject: [PATCH 3/3] Reorder prechecker balance and conditional options checks --- arbnode/execution/tx_pre_checker.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/arbnode/execution/tx_pre_checker.go b/arbnode/execution/tx_pre_checker.go index dc069a6d18..968a1f266b 100644 --- a/arbnode/execution/tx_pre_checker.go +++ b/arbnode/execution/tx_pre_checker.go @@ -145,11 +145,6 @@ func PreCheckTx(bc *core.BlockChain, chainConfig *params.ChainConfig, header *ty if config.Strictness < TxPreCheckerStrictnessLikelyCompatible { return nil } - balance := statedb.GetBalance(sender) - cost := tx.Cost() - if arbmath.BigLessThan(balance, cost) { - return fmt.Errorf("%w: address %v have %v want %v", core.ErrInsufficientFunds, sender, balance, cost) - } if options != nil { if err := options.Check(extraInfo.L1BlockNumber, header.Time, statedb); err != nil { conditionalTxRejectedByTxPreCheckerCurrentStateCounter.Inc(1) @@ -185,6 +180,11 @@ func PreCheckTx(bc *core.BlockChain, chainConfig *params.ChainConfig, header *ty conditionalTxAcceptedByTxPreCheckerOldStateCounter.Inc(1) } } + balance := statedb.GetBalance(sender) + cost := tx.Cost() + if arbmath.BigLessThan(balance, cost) { + return fmt.Errorf("%w: address %v have %v want %v", core.ErrInsufficientFunds, sender, balance, cost) + } if config.Strictness >= TxPreCheckerStrictnessFullValidation && tx.Nonce() > stateNonce { return MakeNonceError(sender, tx.Nonce(), stateNonce) }