feat(statement-distribution): implement active leaves update signal #4767
Conversation
There was a problem hiding this comment.
I know it's out of the scop for this PR but still i am curious to know why do we have rx_ as prefix of this file name? by any chance if you know
| if len(disableValidatorsSet) > 0 { | ||
| disabled := make([]parachaintypes.ValidatorIndex, len(disableValidatorsSet)) | ||
| for d := range disableValidatorsSet { | ||
| disabled = append(disabled, d) | ||
| } | ||
|
|
||
| logger.Debugf("disabled validators detected: %v, "+ | ||
| "session index=%v, relay parent=%s", disabled, sessionIdx, rp.String()) | ||
| } |
There was a problem hiding this comment.
Do we really need this?
| for coreIdx, paras := range schedule { | ||
| groupIdx := groupRotationInfo.GroupForCore(coreIdx, uint(numCores)) | ||
|
|
||
| assignmentsPerGroup[groupIdx] = slices.Clone(paras) |
There was a problem hiding this comment.
We are not modifying the paras so i don't think we need to clone the it.
| assignmentsPerGroup[groupIdx] = slices.Clone(paras) | |
| assignmentsPerGroup[groupIdx] = paras |
| ParachainHostMinimumBackingVotes(). | ||
| Return(uint32(3), nil) | ||
|
|
||
| featuresBitVec, err := parachaintypes.NewBitVec([]bool{true, true, false, true}) |
There was a problem hiding this comment.
It's not a good practice to have a type in a variable name.
We can rename this to nodeFeatures or features.
| wg := sync.WaitGroup{} | ||
| wg.Add(1) | ||
| go func() { | ||
| defer wg.Done() | ||
| hypotheticalMsg := <-overseerCh | ||
| msg, ok := hypotheticalMsg.(prospectiveparachainsmessages.GetHypotheticalMembership) | ||
| require.True(t, ok) | ||
|
|
||
| // just return an empty slice | ||
| msg.Response <- []*prospectiveparachainsmessages.HypotheticalMembershipResponseItem{} | ||
| }() | ||
|
|
||
| // The actual sendPeerMessagesForRelayParent will run, but we can't assert its call directly. | ||
| // Instead, we just ensure no panic and no error. | ||
| err = sd.handleActiveLeavesUpdate(activatedLeaf) | ||
| require.NoError(t, err) | ||
| wg.Wait() |
There was a problem hiding this comment.
as you have used require.True inside the go routine, the test will not fail.
you can use mock overseer implementaion which we have implemented specifically to use in test. (I prefer this)
or handle the error/fail case properly.
| require.NoError(t, err) | ||
| wg.Wait() | ||
|
|
||
| // assertions |
There was a problem hiding this comment.
let's remove unnecessary comments
Changes
Statement DistributionsubsystemTests
go test -timeout 30s github.com/ChainSafe/gossamer/dot/parachain/statement-distribution -vIssues