From f5ee98e65edf1f592e685989b895904baf470879 Mon Sep 17 00:00:00 2001 From: kant Date: Thu, 5 Mar 2026 11:08:13 -0800 Subject: [PATCH] fix: topology bidder dedup checks wrong map, dropping bidders silently The `add` method checked `t.providers` instead of `t.bidders` when deduplicating bidder connections. If a bidder's address existed in the provider map, the bidder was silently dropped and never added to the topology. This caused new provider broadcasts to reach an incomplete bidder list, requiring a bootnode restart for new providers to become visible to all bidders. --- p2p/pkg/topology/topology.go | 2 +- p2p/pkg/topology/topology_test.go | 80 +++++++++++++++++++++++++++++++ 2 files changed, 81 insertions(+), 1 deletion(-) diff --git a/p2p/pkg/topology/topology.go b/p2p/pkg/topology/topology.go index 920bf2686..adf230093 100644 --- a/p2p/pkg/topology/topology.go +++ b/p2p/pkg/topology/topology.go @@ -109,7 +109,7 @@ func (t *Topology) add(p p2p.Peer) { t.providers[p.EthAddress] = p t.metrics.ConnectedProvidersCount.Inc() case p2p.PeerTypeBidder: - if _, alreadyConnected := t.providers[p.EthAddress]; alreadyConnected { + if _, alreadyConnected := t.bidders[p.EthAddress]; alreadyConnected { return } t.bidders[p.EthAddress] = p diff --git a/p2p/pkg/topology/topology_test.go b/p2p/pkg/topology/topology_test.go index ee63fc5b9..c2079af43 100644 --- a/p2p/pkg/topology/topology_test.go +++ b/p2p/pkg/topology/topology_test.go @@ -152,3 +152,83 @@ func TestTopology(t *testing.T) { t.Fatal("wrong peer in disconnect notification") } } + +func TestBidderDedupChecksCorrectMap(t *testing.T) { + t.Parallel() + + notifier := &testNotifier{} + topo := topology.New(&testAddressbook{}, notifier, util.NewTestLogger(io.Discard)) + + // Provider and bidder with the same eth address + addr := common.HexToAddress("0xABC") + + provider := p2p.Peer{EthAddress: addr, Type: p2p.PeerTypeProvider} + bidder := p2p.Peer{EthAddress: addr, Type: p2p.PeerTypeBidder} + + topo.Connected(provider) + + // Before the fix, this bidder would be silently dropped because `add` + // checked t.providers (wrong map) for the bidder dedup. + topo.Connected(bidder) + + providers := topo.GetPeers(topology.Query{Type: p2p.PeerTypeProvider}) + bidders := topo.GetPeers(topology.Query{Type: p2p.PeerTypeBidder}) + + if len(providers) != 1 { + t.Fatalf("expected 1 provider, got %d", len(providers)) + } + if len(bidders) != 1 { + t.Fatalf("expected 1 bidder, got %d", len(bidders)) + } + + // Duplicate bidder should be deduped + topo.Connected(bidder) + bidders = topo.GetPeers(topology.Query{Type: p2p.PeerTypeBidder}) + if len(bidders) != 1 { + t.Fatalf("expected 1 bidder after duplicate connect, got %d", len(bidders)) + } +} + +func TestNewProviderBroadcastsToAllBidders(t *testing.T) { + t.Parallel() + + ann := &announcer{} + notifier := &testNotifier{} + topo := topology.New(&testAddressbook{}, notifier, util.NewTestLogger(io.Discard)) + topo.SetAnnouncer(ann) + + // Connect 3 bidders first + bidders := []p2p.Peer{ + {EthAddress: common.HexToAddress("0xB1"), Type: p2p.PeerTypeBidder}, + {EthAddress: common.HexToAddress("0xB2"), Type: p2p.PeerTypeBidder}, + {EthAddress: common.HexToAddress("0xB3"), Type: p2p.PeerTypeBidder}, + } + for _, b := range bidders { + topo.Connected(b) + } + + ann.mu.Lock() + ann.broadcasts = nil // clear broadcasts from bidder connections + ann.mu.Unlock() + + // Now connect a new provider — it should be broadcast to all 3 bidders + provider := p2p.Peer{EthAddress: common.HexToAddress("0xP1"), Type: p2p.PeerTypeProvider} + topo.Connected(provider) + + ann.mu.Lock() + defer ann.mu.Unlock() + + if len(ann.broadcasts) != 3 { + t.Fatalf("expected new provider to be broadcast to 3 bidders, got %d", len(ann.broadcasts)) + } + + broadcastAddrs := make(map[common.Address]bool) + for _, b := range ann.broadcasts { + broadcastAddrs[b.EthAddress] = true + } + for _, b := range bidders { + if !broadcastAddrs[b.EthAddress] { + t.Fatalf("bidder %s did not receive provider broadcast", b.EthAddress.Hex()) + } + } +}