Merge branch 'andrius/fix-relayed-key-lookup' into 'main'

[MINOR] fix(finalizer): use dedicated key type for already-relayed lookup

See merge request flarenetwork/FSP/flare-system-client!82

Author: adg-flare

client/finalizer/finalizer_queue.go

@@ -212,7 +212,7 @@ func (p *finalizerQueueProcessor) processDelayedQueue(ctx context.Context, items
 	}
 
 	for _, item := range items {
-		if relayedItems[*item] {
+		if relayedItems[relayedKey{protocolID: item.protocolID, votingRoundID: item.votingRoundID}] {
 			continue
 		}
 		logger.Infof("Finalizer processes delayed queue item for round %v for protocol %v", item.votingRoundID, item.protocolID)

client/finalizer/relay_client.go

@@ -198,20 +198,34 @@ func (r *relayContractClient) SubmitPayloads(ctx context.Context, input []byte,
 	}
 }
 
-// ProtocolMessageRelayed returns a set of pairs of protocol and round that have been finalized.
-func (r *relayContractClient) ProtocolMessageRelayed(ctx context.Context, db finalizerDB, from time.Time, to time.Time) (map[queueItem]bool, error) {
+// relayedKey is the lookup key for ProtocolMessageRelayed events.
+//
+// It deliberately excludes the seed and msgHash fields of queueItem:
+// ProtocolMessageRelayed events identify a finalization uniquely by
+// (protocolID, votingRoundID), and using queueItem directly as a map
+// key would compare *big.Int by pointer identity — guaranteeing the
+// lookup in processDelayedQueue never matches.
+type relayedKey struct {
+	protocolID    uint8
+	votingRoundID uint32
+}
+
+// ProtocolMessageRelayed returns a set of (protocolID, votingRoundID)
+// pairs that have already been finalized on chain in the given time
+// range.
+func (r *relayContractClient) ProtocolMessageRelayed(ctx context.Context, db finalizerDB, from time.Time, to time.Time) (map[relayedKey]bool, error) {
 	logs, err := db.FetchLogsByAddressAndTopic0(ctx, r.address, r.topic0PMR, from.Unix(), to.Unix())
 	if err != nil {
 		return nil, err
 	}
 
-	result := make(map[queueItem]bool)
+	result := make(map[relayedKey]bool)
 	for _, log := range logs {
 		data, err := shared.ParseProtocolMessageRelayedEvent(r.relay, log)
 		if err != nil {
 			return nil, err
 		}
-		result[queueItem{
+		result[relayedKey{
 			protocolID:    data.ProtocolId,
 			votingRoundID: data.VotingRoundId,
 		}] = true

client/finalizer/relay_client_test.go

@@ -0,0 +1,72 @@
+package finalizer
+
+import (
+	"math/big"
+	"testing"
+
+	"github.com/ethereum/go-ethereum/common"
+	"github.com/stretchr/testify/require"
+)
+
+// TestRelayedKey_LookupMatchesWithUnrelatedQueueItemFields pins the
+// fix for the duplicate-suppression bug in processDelayedQueue: the
+// lookup key built from a ProtocolMessageRelayed event must match the
+// lookup performed against a queueItem regardless of that item's seed
+// (*big.Int, pointer-compared by maps) and msgHash.
+//
+// Pre-fix, ProtocolMessageRelayed used queueItem as the map key with
+// only protocolID + votingRoundID populated, so the lookup with a
+// fully-populated queueItem never matched and every delayed item was
+// re-sent (the relay contract returns the non-fatal "Already relayed"
+// error, so the bug only burned gas).
+func TestRelayedKey_LookupMatchesWithUnrelatedQueueItemFields(t *testing.T) {
+	relayed := map[relayedKey]bool{
+		{protocolID: 100, votingRoundID: 42}: true,
+		{protocolID: 200, votingRoundID: 99}: true,
+	}
+
+	item := &queueItem{
+		seed:          big.NewInt(0xdeadbeef), // non-nil pointer — would have broken the old lookup
+		votingRoundID: 42,
+		protocolID:    100,
+		msgHash:       common.HexToHash("0xabc"), // non-zero — would have broken the old lookup
+	}
+
+	require.True(
+		t,
+		relayed[relayedKey{protocolID: item.protocolID, votingRoundID: item.votingRoundID}],
+		"already-relayed lookup must match by (protocolID, votingRoundID) regardless of seed/msgHash",
+	)
+
+	miss := &queueItem{
+		seed:          big.NewInt(1),
+		votingRoundID: 43, // different round
+		protocolID:    100,
+		msgHash:       common.HexToHash("0xabc"),
+	}
+	require.False(
+		t,
+		relayed[relayedKey{protocolID: miss.protocolID, votingRoundID: miss.votingRoundID}],
+		"different (protocolID, votingRoundID) must not match",
+	)
+}
+
+// TestRelayedKey_TwoNonNilSeedsDoNotCollide makes the failure mode of
+// the pre-fix code explicit: under the old shape, two queueItems
+// holding distinct *big.Int seeds compared as DIFFERENT map keys even
+// when their (protocolID, votingRoundID) were equal — that's exactly
+// why the producer-built map (seed=nil) never matched the
+// consumer-side lookup (seed != nil).
+func TestRelayedKey_TwoNonNilSeedsDoNotCollide(t *testing.T) {
+	a := queueItem{seed: big.NewInt(1), votingRoundID: 1, protocolID: 1}
+	b := queueItem{seed: big.NewInt(1), votingRoundID: 1, protocolID: 1}
+
+	m := map[queueItem]bool{a: true}
+	require.False(t, m[b], "*big.Int is pointer-compared in map keys — distinct allocations of equal values DO NOT match")
+
+	// And the fix: relayedKey excludes the pointer field entirely.
+	ra := relayedKey{protocolID: 1, votingRoundID: 1}
+	rb := relayedKey{protocolID: 1, votingRoundID: 1}
+	rm := map[relayedKey]bool{ra: true}
+	require.True(t, rm[rb], "relayedKey is pure value type — equal values match")
+}