diff --git a/.changeset/fifty-ducks-beg.md b/.changeset/fifty-ducks-beg.md new file mode 100644 index 0000000000..cac80f60d2 --- /dev/null +++ b/.changeset/fifty-ducks-beg.md @@ -0,0 +1,5 @@ +--- +"ccip": patch +--- + +use deterministic sorting in LM plugin diff --git a/core/services/ocr2/plugins/liquiditymanager/rebalcalc/consensus.go b/core/services/ocr2/plugins/liquiditymanager/rebalcalc/consensus.go index f2cd506ffb..2483c21100 100644 --- a/core/services/ocr2/plugins/liquiditymanager/rebalcalc/consensus.go +++ b/core/services/ocr2/plugins/liquiditymanager/rebalcalc/consensus.go @@ -82,9 +82,9 @@ func PendingTransfersConsensus(observations []models.Observation, f int) ([]mode } } - // sort by network id for deterministic results + // sort by ID for deterministic results sort.Slice(quorumEvents, func(i, j int) bool { - return quorumEvents[i].From < quorumEvents[j].From + return quorumEvents[i].ID < quorumEvents[j].ID }) return quorumEvents, nil @@ -187,8 +187,11 @@ func GraphEdgesConsensus(observations []models.Observation, f int) ([]models.Edg } } - // sort by network id for deterministic results + // sort for deterministic results sort.Slice(quorumEdges, func(i, j int) bool { + if quorumEdges[i].Source == quorumEdges[j].Source { + return quorumEdges[i].Dest < quorumEdges[j].Dest + } return quorumEdges[i].Source < quorumEdges[j].Source }) diff --git a/core/services/ocr2/plugins/liquiditymanager/rebalcalc/consensus_test.go b/core/services/ocr2/plugins/liquiditymanager/rebalcalc/consensus_test.go index 360cfce4b4..0a932f83e2 100644 --- a/core/services/ocr2/plugins/liquiditymanager/rebalcalc/consensus_test.go +++ b/core/services/ocr2/plugins/liquiditymanager/rebalcalc/consensus_test.go @@ -110,15 +110,15 @@ func TestPendingTransfersConsensus(t *testing.T) { f int } tests := []struct { - name string - args args - want []models.PendingTransfer - wantErr bool + name string + args args + numExpected int + wantErr bool }{ { "no observations", args{[]models.Observation{}, 1}, - []models.PendingTransfer{}, + 0, true, }, { @@ -126,7 +126,7 @@ func TestPendingTransfersConsensus(t *testing.T) { args{[]models.Observation{ {}, {}, {}, }, 2}, - []models.PendingTransfer{}, + 0, true, }, { @@ -152,10 +152,7 @@ func TestPendingTransfersConsensus(t *testing.T) { }, }, }, 1}, - []models.PendingTransfer{ - {Transfer: models.Transfer{From: 1, To: 2, Amount: ubig.NewI(1)}}, - {Transfer: models.Transfer{From: 2, To: 3, Amount: ubig.NewI(2)}}, - }, + 2, false, }, } @@ -166,7 +163,7 @@ func TestPendingTransfersConsensus(t *testing.T) { require.Error(t, err) } else { require.NoError(t, err) - require.Equal(t, tt.want, got) + require.Equal(t, tt.numExpected, len(got)) } }) } @@ -296,6 +293,45 @@ func TestGraphEdgesConsensus(t *testing.T) { {Source: 2, Dest: 3}, }, false, + }, { + "differently ordered edges", + args{[]models.Observation{ + { + Edges: []models.Edge{ + {Source: 1, Dest: 4}, + {Source: 1, Dest: 1}, + {Source: 1, Dest: 3}, + {Source: 2, Dest: 1}, + {Source: 1, Dest: 2}, + }, + }, + { + Edges: []models.Edge{ + {Source: 2, Dest: 1}, + {Source: 1, Dest: 4}, + {Source: 1, Dest: 3}, + {Source: 1, Dest: 2}, + {Source: 1, Dest: 1}, + }, + }, + { + Edges: []models.Edge{ + {Source: 1, Dest: 2}, + {Source: 1, Dest: 4}, + {Source: 2, Dest: 1}, + {Source: 1, Dest: 1}, + {Source: 1, Dest: 3}, + }, + }, + }, 1}, + []models.Edge{ + {Source: 1, Dest: 1}, + {Source: 1, Dest: 2}, + {Source: 1, Dest: 3}, + {Source: 1, Dest: 4}, + {Source: 2, Dest: 1}, + }, + false, }, } for _, tt := range tests {