Address PR review comments

arbnode/batch_poster.go

@@ -259,20 +259,23 @@ func NewBatchPoster(ctx context.Context, opts *BatchPosterOpts) (*BatchPoster, e
 		return nil, err
 	}
 	b := &BatchPoster{
-		l1Reader:         opts.L1Reader,
-		inbox:            opts.Inbox,
-		streamer:         opts.Streamer,
-		syncMonitor:      opts.SyncMonitor,
-		config:           opts.Config,
-		bridge:           bridge,
-		seqInbox:         seqInbox,
-		seqInboxABI:      seqInboxABI,
-		seqInboxAddr:     opts.DeployInfo.SequencerInbox,
-		gasRefunderAddr:  opts.Config().gasRefunder,
-		bridgeAddr:       opts.DeployInfo.Bridge,
-		daWriter:         opts.DAWriter,
-		redisLock:        redisLock,
-		messagesPerBatch: arbmath.NewMovingAverage[uint64](20),
+		l1Reader:        opts.L1Reader,
+		inbox:           opts.Inbox,
+		streamer:        opts.Streamer,
+		syncMonitor:     opts.SyncMonitor,
+		config:          opts.Config,
+		bridge:          bridge,
+		seqInbox:        seqInbox,
+		seqInboxABI:     seqInboxABI,
+		seqInboxAddr:    opts.DeployInfo.SequencerInbox,
+		gasRefunderAddr: opts.Config().gasRefunder,
+		bridgeAddr:      opts.DeployInfo.Bridge,
+		daWriter:        opts.DAWriter,
+		redisLock:       redisLock,
+	}
+	b.messagesPerBatch, err = arbmath.NewMovingAverage[uint64](20)
+	if err != nil {
+		return nil, err
 	}
 	dataPosterConfigFetcher := func() *dataposter.DataPosterConfig {
 		return &(opts.Config().DataPoster)
@@ -1070,6 +1073,12 @@ func (b *BatchPoster) maybePostSequencerBatch(ctx context.Context) (bool, error)
 	if messagesPerBatch == 0 {
 		// This should be impossible because we always post at least one message in a batch.
 		// That said, better safe than sorry, as we would panic if this remained at 0.
+		log.Warn(
+			"messagesPerBatch is somehow zero",
+			"postedMessages", postedMessages,
+			"buildingFrom", batchPosition.MessageCount,
+			"buildingTo", b.building.msgCount,
+		)
 		messagesPerBatch = 1
 	}
 	backlog := uint64(unpostedMessages) / messagesPerBatch

util/arbmath/moving_average.go

@@ -3,36 +3,37 @@
 
 package arbmath
 
+import "fmt"
+
 // A simple moving average of a generic number type.
 type MovingAverage[T Number] struct {
-	period         int
 	buffer         []T
 	bufferPosition int
 	sum            T
 }
 
-func NewMovingAverage[T Number](period int) *MovingAverage[T] {
+func NewMovingAverage[T Number](period int) (*MovingAverage[T], error) {
 	if period <= 0 {
-		panic("MovingAverage period must be positive")
+		return nil, fmt.Errorf("MovingAverage period specified as %v but it must be positive", period)
 	}
 	return &MovingAverage[T]{
-		period: period,
 		buffer: make([]T, 0, period),
-	}
+	}, nil
 }
 
 func (a *MovingAverage[T]) Update(value T) {
-	if a.period <= 0 {
+	period := cap(a.buffer)
+	if period == 0 {
 		return
 	}
-	if len(a.buffer) < a.period {
+	if len(a.buffer) < period {
 		a.buffer = append(a.buffer, value)
 		a.sum += value
 	} else {
 		a.sum += value
 		a.sum -= a.buffer[a.bufferPosition]
 		a.buffer[a.bufferPosition] = value
-		a.bufferPosition = (a.bufferPosition + 1) % a.period
+		a.bufferPosition = (a.bufferPosition + 1) % period
 	}
 }
 

util/arbmath/moving_average_test.go

@@ -6,30 +6,42 @@ package arbmath
 import "testing"
 
 func TestMovingAverage(t *testing.T) {
-	ma := NewMovingAverage[int](5)
+	_, err := NewMovingAverage[int](0)
+	if err == nil {
+		t.Error("Expected error when creating moving average of period 0")
+	}
+	_, err = NewMovingAverage[int](-1)
+	if err == nil {
+		t.Error("Expected error when creating moving average of period -1")
+	}
+
+	ma, err := NewMovingAverage[int](5)
+	if err != nil {
+		t.Fatalf("Error creating moving average of period 5: %v", err)
+	}
 	if ma.Average() != 0 {
-		t.Errorf("moving average should be 0 at start, got %v", ma.Average())
+		t.Errorf("Average() = %v, want 0", ma.Average())
 	}
 	ma.Update(2)
 	if ma.Average() != 2 {
-		t.Errorf("moving average should be 2, got %v", ma.Average())
+		t.Errorf("Average() = %v, want 2", ma.Average())
 	}
 	ma.Update(4)
 	if ma.Average() != 3 {
-		t.Errorf("moving average should be 3, got %v", ma.Average())
+		t.Errorf("Average() = %v, want 3", ma.Average())
 	}
 
 	for i := 0; i < 5; i++ {
 		ma.Update(10)
 	}
 	if ma.Average() != 10 {
-		t.Errorf("moving average should be 10, got %v", ma.Average())
+		t.Errorf("Average() = %v, want 10", ma.Average())
 	}
 
 	for i := 0; i < 5; i++ {
 		ma.Update(0)
 	}
 	if ma.Average() != 0 {
-		t.Errorf("moving average should be 0, got %v", ma.Average())
+		t.Errorf("Average() = %v, want 0", ma.Average())
 	}
 }