Skip to content

Commit

Permalink
Use deterministic sorts in LM consensus functions (#814)
Browse files Browse the repository at this point in the history
## Motivation

Outcomes should be deterministic from the observations, regardless the
order or order of fields

## Solution

Use deterministic sorting to always yield the same result for outcomes
  • Loading branch information
RyanRHall authored May 8, 2024
1 parent 22668bd commit e06e2c6
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 14 deletions.
5 changes: 5 additions & 0 deletions .changeset/fifty-ducks-beg.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"ccip": patch
---

use deterministic sorting in LM plugin
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
})

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,23 +110,23 @@ 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,
},
{
"not enough observations",
args{[]models.Observation{
{}, {}, {},
}, 2},
[]models.PendingTransfer{},
0,
true,
},
{
Expand All @@ -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,
},
}
Expand All @@ -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))
}
})
}
Expand Down Expand Up @@ -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 {
Expand Down

0 comments on commit e06e2c6

Please sign in to comment.