Sort signers in decided message #484
Conversation
ssv/runner_validations.go
Outdated
| @@ -50,7 +50,7 @@ func (b *BaseRunner) ValidatePostConsensusMsg(runner Runner, psigMsgs *types.Par | |||
| } | |||
|
|
|||
| // TODO https://github.com/ssvlabs/ssv-spec/issues/142 need to fix with this issue solution instead. | |||
| if b.State.DecidedValue == nil || len(b.State.DecidedValue) == 0 { | |||
There was a problem hiding this comment.
that's from another PR no?
you can resolve when solving conflicts
There was a problem hiding this comment.
Due to the linter update, I have been fixing this lint issue in all the recent PRs so that actions would pass
| ) | ||
|
|
||
| // SortedDecided tests the creation of the decided message that should have sorted signers | ||
| func SortedDecided() tests.SpecTest { |
There was a problem hiding this comment.
Notice that ControllerSpecTest.OutputMessages is unused. For this test, we actually use ControllerSpecTest.RunInstanceData.ExpectedDecidedState.BroadcastedDecided to test the broadcasted decided message.
There was a problem hiding this comment.
Notice that not only does none of the implemented tests define it but the spec test itself doesn't use it.
As I see it, the reason is that the controller spec test wants to test decided values and the decided messages for a list of instances (and not the instance's broadcasted messages).
I think that we should either drop this field or implement it in the tests if wanted.
* Sort signers in decided message * Add test for sorted signers in decided msg * Generate JSON tests * Fix lint issue
Overview
This PR adds the feature of sorting the signers in the decided message.
It adds a "sorted decided" test to test the sorting upon receiving commit messages in an unsorted order.
Closes #472