diff --git a/.changeset/stable-nodes-pagination.md b/.changeset/stable-nodes-pagination.md new file mode 100644 index 00000000000..e7b58f644ca --- /dev/null +++ b/.changeset/stable-nodes-pagination.md @@ -0,0 +1,7 @@ +--- +"chainlink": patch +--- + +Sort aggregated NodeStatuses by (ChainID, Name) so the /nodes dashboard paginates deterministically across reloads when more than 100 nodes are configured. + +#bugfix diff --git a/core/services/chainlink/relayer_chain_interoperators.go b/core/services/chainlink/relayer_chain_interoperators.go index ce4ea40e818..b66cefcfdd6 100644 --- a/core/services/chainlink/relayer_chain_interoperators.go +++ b/core/services/chainlink/relayer_chain_interoperators.go @@ -408,6 +408,12 @@ func (rs *CoreRelayerChainInteroperators) NodeStatuses(ctx context.Context, offs if totalErr != nil { return nil, 0, totalErr } + // Per-relayer node ordering is whatever the underlying chain implementation + // returns and is not guaranteed to be stable across calls (#18263). Sort + // the aggregated slice by (ChainID, Name) so the API surface is paginated + // deterministically and operators can find the same node on the same page + // across reloads. + sortNodeStatuses(result) if len(result) < offset { return nil, 0, nil } @@ -418,6 +424,20 @@ func (rs *CoreRelayerChainInteroperators) NodeStatuses(ctx context.Context, offs return result, count, nil } +// sortNodeStatuses orders node statuses by (ChainID, Name) in place so the +// /nodes dashboard surface paginates deterministically. SortStableFunc keeps +// the input order on equal (ChainID, Name) pairs so the aggregate is fully +// reproducible. Extracted for unit testing without standing up a full +// relayer; see #18263. +func sortNodeStatuses(nodes []types.NodeStatus) { + slices.SortStableFunc(nodes, func(a, b types.NodeStatus) int { + if c := strings.Compare(a.ChainID, b.ChainID); c != 0 { + return c + } + return strings.Compare(a.Name, b.Name) + }) +} + type FilterFn func(id types.RelayID) bool var AllRelayers = func(id types.RelayID) bool { diff --git a/core/services/chainlink/relayer_chain_interoperators_internal_test.go b/core/services/chainlink/relayer_chain_interoperators_internal_test.go new file mode 100644 index 00000000000..7da91b4000e --- /dev/null +++ b/core/services/chainlink/relayer_chain_interoperators_internal_test.go @@ -0,0 +1,112 @@ +package chainlink + +import ( + "testing" + + "github.com/smartcontractkit/chainlink-common/pkg/types" + "github.com/stretchr/testify/assert" +) + +// TestSortNodeStatuses is a regression test for #18263. The /nodes dashboard +// page was reordering every render because per-relayer node iteration is not +// guaranteed to be stable. The aggregator now sorts by (ChainID, Name) so +// repeated calls with the same inputs return the same page. +func TestSortNodeStatuses(t *testing.T) { + t.Parallel() + + cases := []struct { + name string + in []types.NodeStatus + want []types.NodeStatus + }{ + { + name: "shuffled within a chain sorts by name", + in: []types.NodeStatus{ + {ChainID: "1", Name: "zebra"}, + {ChainID: "1", Name: "alpha"}, + {ChainID: "1", Name: "mango"}, + }, + want: []types.NodeStatus{ + {ChainID: "1", Name: "alpha"}, + {ChainID: "1", Name: "mango"}, + {ChainID: "1", Name: "zebra"}, + }, + }, + { + name: "groups by chain id then sorts by name", + in: []types.NodeStatus{ + {ChainID: "2", Name: "bear"}, + {ChainID: "1", Name: "zebra"}, + {ChainID: "2", Name: "ant"}, + {ChainID: "1", Name: "alpha"}, + }, + want: []types.NodeStatus{ + {ChainID: "1", Name: "alpha"}, + {ChainID: "1", Name: "zebra"}, + {ChainID: "2", Name: "ant"}, + {ChainID: "2", Name: "bear"}, + }, + }, + { + name: "lexicographic chain ids (string compare, not numeric)", + in: []types.NodeStatus{ + {ChainID: "10", Name: "n1"}, + {ChainID: "2", Name: "n2"}, + {ChainID: "1", Name: "n3"}, + }, + // ChainID is a string in NodeStatus, so "10" sorts before "2" + // lexicographically. The sort intentionally mirrors that — chain + // id space is opaque (EVM, Solana, dummy, ...) so a numeric sort + // would be wrong for non-EVM chains. + want: []types.NodeStatus{ + {ChainID: "1", Name: "n3"}, + {ChainID: "10", Name: "n1"}, + {ChainID: "2", Name: "n2"}, + }, + }, + { + name: "already sorted is a no-op", + in: []types.NodeStatus{ + {ChainID: "1", Name: "a"}, + {ChainID: "1", Name: "b"}, + {ChainID: "2", Name: "c"}, + }, + want: []types.NodeStatus{ + {ChainID: "1", Name: "a"}, + {ChainID: "1", Name: "b"}, + {ChainID: "2", Name: "c"}, + }, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := append([]types.NodeStatus(nil), tc.in...) + sortNodeStatuses(got) + assert.Equal(t, tc.want, got) + }) + } + + t.Run("empty and nil slices are no-ops", func(t *testing.T) { + empty := []types.NodeStatus{} + sortNodeStatuses(empty) + assert.Empty(t, empty) + sortNodeStatuses(nil) + }) +} + +// TestSortNodeStatusesStable asserts equal keys preserve input order so the +// aggregator output stays predictable even when two relayers happen to use +// the same (ChainID, Name) pair (e.g. dev fixtures, multi-relayer dummy +// chains). +func TestSortNodeStatusesStable(t *testing.T) { + t.Parallel() + + in := []types.NodeStatus{ + {ChainID: "1", Name: "same", Config: "first"}, + {ChainID: "1", Name: "same", Config: "second"}, + {ChainID: "1", Name: "same", Config: "third"}, + } + got := append([]types.NodeStatus(nil), in...) + sortNodeStatuses(got) + assert.Equal(t, in, got, "equal-key entries must keep their original order") +}