Merge branch 'master' into mel-batchpostingreport-bugfix

Author: ganeshvanahalli

.github/workflows/_validation-tests.yml

@@ -70,12 +70,12 @@ jobs:
             # 3. Send 2 validation requests
             RESPONSE1=$(jq -n --slurpfile input ${{ env.INPUT_FILE }} \
               '{jsonrpc: "2.0", id: 1, method: "validation_validate", params: [$input[0]]}' | \
-              curl -s -X POST http://localhost:4141/validation_validate \
+              curl -s -X POST http://localhost:4141/ \
                 -H "Content-Type: application/json" -d @-)
 
             RESPONSE2=$(jq -n --slurpfile input ${{ env.INPUT_FILE }} \
               '{jsonrpc: "2.0", id: 2, method: "validation_validate", params: [$input[0]]}' | \
-              curl -s -X POST http://localhost:4141/validation_validate \
+              curl -s -X POST http://localhost:4141/ \
                 -H "Content-Type: application/json" -d @-)
 
             # 4. Stop the server

Cargo.lock

@@ -55,7 +55,7 @@ enum-iterator = { version = "2.0.1" }
 eyre = { version = "0.6.5" }
 fnv = { version = "1.0.7" }
 gperftools = { version = "0.2.0" }
-hex = { version = "0.4.3" }
+hex = { version = "0.4.3", default-features = false }
 k256 = { version = "0.13.4", default-features = false}
 lazy_static = { version = "1.4.0" }
 libc = { version = "0.2.132" }

Cargo.toml

@@ -181,6 +181,7 @@ build-node-deps: $(go_source) build-prover-header build-prover-lib build-jit .ma
 .PHONY: test-go-deps
 test-go-deps: \
 	build-replay-env \
+	build-validation-server \
 	$(stylus_test_wasms) \
 	$(arbitrator_stylus_lib) \
 	$(arbitrator_generated_header) \

Makefile

@@ -200,7 +200,19 @@ func (msg *L1IncomingMessage) FillInBatchGasFieldsWithParentBlock(batchFetcher F
 			if msg.LegacyBatchGasCost == nil {
 				return fmt.Errorf("failed to fetch batch mentioned by batch posting report: %w", err)
 			}
+			// Pre-arbos50, LegacyBatchGasCost and BatchDataStats are interchangeable.
+			// Post-arbos50, BatchDataStats is required. However, this code doesn't
+			// know which arbos version applies to the current block.
+			//
+			// Since we already have LegacyBatchGasCost, we don't return an error here.
+			// Instead, we assume this is a pre-arbos50 block and proceed without
+			// BatchDataStats. If that assumption is wrong (i.e. this is actually
+			// post-arbos50), createBatchPostingReportTransaction will detect the
+			// missing BatchDataStats and fail there, since it does know the arbos
+			// version. In practice, any node that supports arbos50 populates both
+			// fields together, so this fallback path should not be reached.
 			log.Warn("Failed reading batch data for filling message - leaving BatchDataStats empty")
+			return nil
 		} else {
 			gotHash := crypto.Keccak256Hash(batchData)
 			if gotHash != batchHash {

arbos/arbostypes/incomingmessage.go

@@ -592,6 +592,131 @@ func TestServerMissingFeedServerVersion(t *testing.T) {
 	}
 }
 
+type accumulatingTransactionStreamer struct {
+	mu       sync.Mutex
+	messages []*message.BroadcastFeedMessage
+}
+
+func (ts *accumulatingTransactionStreamer) AddBroadcastMessages(feedMessages []*message.BroadcastFeedMessage) error {
+	ts.mu.Lock()
+	defer ts.mu.Unlock()
+	ts.messages = append(ts.messages, feedMessages...)
+	return nil
+}
+
+func (ts *accumulatingTransactionStreamer) getMessages() []*message.BroadcastFeedMessage {
+	ts.mu.Lock()
+	defer ts.mu.Unlock()
+	result := make([]*message.BroadcastFeedMessage, len(ts.messages))
+	copy(result, ts.messages)
+	return result
+}
+
+// awaitCount waits until at least count messages have been received.
+// The timeout is a safety net to prevent the test from hanging.
+func (ts *accumulatingTransactionStreamer) awaitCount(t *testing.T, count int, timeout time.Duration) {
+	t.Helper()
+	deadline := time.After(timeout)
+	for {
+		if len(ts.getMessages()) >= count {
+			return
+		}
+		select {
+		case <-deadline:
+			t.Fatalf("timed out waiting for %d messages, got %d", count, len(ts.getMessages()))
+		case <-time.After(10 * time.Millisecond):
+		}
+	}
+}
+
+func TestInvalidSignatureMessagesAreSkipped(t *testing.T) {
+	t.Parallel()
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+
+	chainId := uint64(9742)
+
+	// Trusted key: broadcaster signs with this, client trusts this
+	trustedKey, err := crypto.GenerateKey()
+	Require(t, err)
+	trustedAddr := crypto.PubkeyToAddress(trustedKey.PublicKey)
+	trustedSigner := signature.DataSignerFromPrivateKey(trustedKey)
+
+	// Untrusted key: used to create messages with invalid signatures
+	untrustedKey, err := crypto.GenerateKey()
+	Require(t, err)
+	untrustedSigner := signature.DataSignerFromPrivateKey(untrustedKey)
+
+	feedErrChan := make(chan error, 10)
+	trustedBroadcaster := broadcaster.NewBroadcaster(func() *wsbroadcastserver.BroadcasterConfig { return &wsbroadcastserver.DefaultTestBroadcasterConfig }, chainId, feedErrChan, trustedSigner)
+
+	Require(t, trustedBroadcaster.Initialize())
+	Require(t, trustedBroadcaster.Start(ctx))
+	defer trustedBroadcaster.StopAndWait()
+
+	// Second broadcaster (not started) used only to create messages signed with the untrusted key
+	untrustedBroadcaster := broadcaster.NewBroadcaster(func() *wsbroadcastserver.BroadcasterConfig { return &wsbroadcastserver.DefaultTestBroadcasterConfig }, chainId, make(chan error, 1), untrustedSigner)
+
+	ts := &accumulatingTransactionStreamer{}
+
+	clientFeedErrChan := make(chan error, 10)
+	broadcastClient, err := newTestBroadcastClient(
+		DefaultTestConfig,
+		trustedBroadcaster.ListenerAddr(),
+		chainId,
+		0,
+		ts,
+		nil,
+		clientFeedErrChan,
+		&trustedAddr,
+		t,
+	)
+	Require(t, err)
+	broadcastClient.Start(ctx)
+	defer broadcastClient.StopAndWait()
+
+	// Batch 1: valid messages (seq 0, 1) - should be delivered.
+	Require(t, trustedBroadcaster.BroadcastFeedMessages(feedMessage(t, trustedBroadcaster, 0)))
+	Require(t, trustedBroadcaster.BroadcastFeedMessages(feedMessage(t, trustedBroadcaster, 1)))
+	ts.awaitCount(t, 2, 10*time.Second)
+
+	// Batch 2: invalid messages (seq 2, 3) signed with untrusted key - should be skipped.
+	Require(t, trustedBroadcaster.BroadcastFeedMessages(feedMessage(t, untrustedBroadcaster, 2)))
+	Require(t, trustedBroadcaster.BroadcastFeedMessages(feedMessage(t, untrustedBroadcaster, 3)))
+
+	// Sentinel (seq 2): a valid message that deterministically proves the client has
+	// processed and skipped the invalid messages. WebSocket messages are ordered, so the
+	// sentinel can only arrive after the invalid ones have been processed (and skipped).
+	// Invalid messages don't advance nextSeqNum (stays at 2), so the sentinel is seq 2.
+	Require(t, trustedBroadcaster.BroadcastFeedMessages(feedMessage(t, trustedBroadcaster, 2)))
+	ts.awaitCount(t, 3, 10*time.Second)
+
+	// Verify: only valid messages were delivered, and all have trusted signatures.
+	got := ts.getMessages()
+	if len(got) != 3 {
+		t.Fatalf("expected 3 messages, got %d", len(got))
+	}
+	for i, msg := range got {
+		if msg.SequenceNumber != arbutil.MessageIndex(i) { // nolint: gosec
+			t.Fatalf("message %d: unexpected seq number: %d", i, msg.SequenceNumber)
+		}
+		hash := msg.SignatureHash(chainId)
+		sigPub, err := crypto.SigToPub(hash.Bytes(), msg.Signature)
+		Require(t, err)
+		signerAddr := crypto.PubkeyToAddress(*sigPub)
+		if signerAddr != trustedAddr {
+			t.Fatalf("message %d (seq %d): signed by %s, expected trusted signer %s", i, msg.SequenceNumber, signerAddr, trustedAddr)
+		}
+	}
+
+	// Verify no fatal errors occurred (invalid signatures are non-fatal since NIT-4017)
+	select {
+	case err := <-clientFeedErrChan:
+		t.Fatalf("unexpected fatal feed error: %v", err)
+	default:
+	}
+}
+
 func TestBroadcastClientReconnectsOnServerDisconnect(t *testing.T) {
 	t.Parallel()
 	ctx, cancel := context.WithCancel(context.Background())

broadcastclient/broadcastclient_test.go

@@ -0,0 +1,2 @@
+### Fixed
+ - If batchFetcher returns error use existing LegacyBatchGasCost value

changelog/bragaigor-nit-4598.md

@@ -0,0 +1,2 @@
+### Fixed
+- Fix `rlp: expected List` error when fetching transaction receipts for blocks with Arbitrum legacy receipt encoding

changelog/jco-fix-legacy-receipt-rlp.md

@@ -0,0 +1,3 @@
+### Ignored
+- Add support to timeboost auctioneer for BidFloorAgent
+

changelog/jcolvin-bidfloorvalidator.md

@@ -0,0 +1,5 @@
+### Fixed
+ - Block validator no longer crashes on timeout errors during validation. Timeout errors are retried separately from other validation failures, up to a configurable limit.
+
+### Configuration
+ - Added `--node.block-validator.validation-spawning-allowed-timeouts` (default `3`): maximum number of timeout errors allowed per validation before treating it as fatal. Timeout errors have their own counter, separate from `--node.block-validator.validation-spawning-allowed-attempts`.