fix metrics and add e2e test

op-e2e/system_test.go

@@ -28,6 +28,7 @@ import (
 	"github.com/ethereum-optimism/optimism/op-bindings/predeploys"
 	"github.com/ethereum-optimism/optimism/op-node/client"
 	"github.com/ethereum-optimism/optimism/op-node/eth"
+	"github.com/ethereum-optimism/optimism/op-node/p2p"
 	"github.com/ethereum-optimism/optimism/op-node/rollup/derive"
 	"github.com/ethereum-optimism/optimism/op-node/sources"
 	"github.com/ethereum-optimism/optimism/op-node/testlog"
@@ -646,6 +647,106 @@ func TestSystemMockP2P(t *testing.T) {
 	require.Contains(t, received, receiptVerif.BlockHash)
 }
 
+// TestSystemMockPeerScoring sets up a L1 Geth node, a rollup node, and a L2 geth node and then confirms that
+// the nodes can sync L2 blocks before they are confirmed on L1.
+func TestSystemMockPeerScoring(t *testing.T) {
+	parallel(t)
+	if !verboseGethNodes {
+		log.Root().SetHandler(log.DiscardHandler())
+	}
+
+	cfg := DefaultSystemConfig(t)
+	// slow down L1 blocks so we can see the L2 blocks arrive well before the L1 blocks do.
+	// Keep the seq window small so the L2 chain is started quick
+	cfg.DeployConfig.L1BlockTime = 10
+
+	// connect the nodes
+	cfg.P2PTopology = map[string][]string{
+		"verifier":  {"sequencer", "verifier2", "verifier3"},
+		"verifier2": {"sequencer", "verifier", "verifier3"},
+		"verifier3": {"sequencer", "verifier", "verifier2"},
+	}
+
+	// Set peer scoring for each node, but without banning
+	for _, node := range cfg.Nodes {
+		params, err := p2p.GetPeerScoreParams("light", 2)
+		require.NoError(t, err)
+		node.P2P = &p2p.Config{
+			PeerScoring:    params,
+			BanningEnabled: false,
+		}
+	}
+
+	var published, received []common.Hash
+	seqTracer, verifTracer, verifTracer2, verifTracer3 := new(FnTracer), new(FnTracer), new(FnTracer), new(FnTracer)
+	seqTracer.OnPublishL2PayloadFn = func(ctx context.Context, payload *eth.ExecutionPayload) {
+		published = append(published, payload.BlockHash)
+	}
+	verifTracer.OnUnsafeL2PayloadFn = func(ctx context.Context, from peer.ID, payload *eth.ExecutionPayload) {
+		received = append(received, payload.BlockHash)
+	}
+	verifTracer2.OnUnsafeL2PayloadFn = func(ctx context.Context, from peer.ID, payload *eth.ExecutionPayload) {
+		received = append(received, payload.BlockHash)
+	}
+	verifTracer3.OnUnsafeL2PayloadFn = func(ctx context.Context, from peer.ID, payload *eth.ExecutionPayload) {
+		received = append(received, payload.BlockHash)
+	}
+	cfg.Nodes["sequencer"].Tracer = seqTracer
+	cfg.Nodes["verifier"].Tracer = verifTracer
+	cfg.Nodes["verifier2"].Tracer = verifTracer2
+	cfg.Nodes["verifier3"].Tracer = verifTracer3
+
+	sys, err := cfg.Start()
+	require.Nil(t, err, "Error starting up system")
+	defer sys.Close()
+
+	l2Seq := sys.Clients["sequencer"]
+	l2Verif := sys.Clients["verifier"]
+	l2Verif2 := sys.Clients["verifier2"]
+	l2Verif3 := sys.Clients["verifier3"]
+
+	// Transactor Account
+	ethPrivKey := cfg.Secrets.Alice
+
+	// Submit TX to L2 sequencer node
+	toAddr := common.Address{0xff, 0xff}
+	tx := types.MustSignNewTx(ethPrivKey, types.LatestSignerForChainID(cfg.L2ChainIDBig()), &types.DynamicFeeTx{
+		ChainID:   cfg.L2ChainIDBig(),
+		Nonce:     0,
+		To:        &toAddr,
+		Value:     big.NewInt(1_000_000_000),
+		GasTipCap: big.NewInt(10),
+		GasFeeCap: big.NewInt(200),
+		Gas:       21000,
+	})
+	err = l2Seq.SendTransaction(context.Background(), tx)
+	require.Nil(t, err, "Sending L2 tx to sequencer")
+
+	// Wait for tx to be mined on the L2 sequencer chain
+	receiptSeq, err := waitForTransaction(tx.Hash(), l2Seq, 6*time.Duration(sys.RollupConfig.BlockTime)*time.Second)
+	require.Nil(t, err, "Waiting for L2 tx on sequencer")
+
+	// Wait until the block it was first included in shows up in the safe chain on the verifier
+	receiptVerif, err := waitForTransaction(tx.Hash(), l2Verif, 6*time.Duration(sys.RollupConfig.BlockTime)*time.Second)
+	require.Nil(t, err, "Waiting for L2 tx on verifier")
+	require.Equal(t, receiptSeq, receiptVerif)
+
+	receiptVerif, err = waitForTransaction(tx.Hash(), l2Verif2, 6*time.Duration(sys.RollupConfig.BlockTime)*time.Second)
+	require.Nil(t, err, "Waiting for L2 tx on verifier2")
+	require.Equal(t, receiptSeq, receiptVerif)
+
+	receiptVerif, err = waitForTransaction(tx.Hash(), l2Verif3, 6*time.Duration(sys.RollupConfig.BlockTime)*time.Second)
+	require.Nil(t, err, "Waiting for L2 tx on verifier3")
+	require.Equal(t, receiptSeq, receiptVerif)
+
+	// Verify that everything that was received was published
+	require.GreaterOrEqual(t, len(published), len(received))
+	require.ElementsMatch(t, received, published[:len(received)])
+
+	// Verify that the tx was received via p2p
+	require.Contains(t, received, receiptVerif.BlockHash)
+}
+
 func TestL1InfoContract(t *testing.T) {
 	parallel(t)
 	if !verboseGethNodes {

op-node/metrics/metrics.go

@@ -119,7 +119,7 @@ type Metrics struct {
 	// P2P Metrics
 	PeerCount         prometheus.Gauge
 	StreamCount       prometheus.Gauge
-	PeerScores        *prometheus.HistogramVec
+	PeerScores        *prometheus.GaugeVec
 	GossipEventsTotal *prometheus.CounterVec
 	BandwidthTotal    *prometheus.GaugeVec
 
@@ -490,7 +490,7 @@ func (m *Metrics) RecordGossipEvent(evType int32) {
 }
 
 func (m *Metrics) RecordPeerScoring(peerID peer.ID, score float64) {
-	m.PeerScores.WithLabelValues(peerID.String()).Observe(score)
+	m.PeerScores.WithLabelValues(peerID.String()).Set(score)
 }
 
 func (m *Metrics) IncPeerCount() {

op-node/p2p/peer_gater.go

@@ -45,7 +45,6 @@ func (s *gater) Update(id peer.ID, score float64) {
 		err := s.connGater.BlockPeer(id)
 		s.log.Warn("connection gater failed to block peer", id.String(), "err", err)
 	}
-	// TODO: This will never unblock a peer since the peer will disconnect and the score update hook will not trigger this call.
 	// Unblock peers whose score has recovered to an acceptable level
 	if (score > PeerScoreThreshold) && slices.Contains(s.connGater.ListBlockedPeers(), id) {
 		err := s.connGater.UnblockPeer(id)