diff --git a/pkg/core/consensus/user/README.md b/pkg/core/consensus/user/README.md index 5cb94907b..14fbd0368 100644 --- a/pkg/core/consensus/user/README.md +++ b/pkg/core/consensus/user/README.md @@ -63,11 +63,11 @@ Parameters: Procedure: ``` -i = 0 // Start with provisioner at index 0 +i = 0 // Start with the first provisioner in the list loop: 1. provisioner = Provisioners[i] // Get provisioner at index `i` 2. if provisioner.stake >= score // If the provisioner's stake is higher than `score`, return provisioner // extract the provisioner 4. score = score - provisioner.stake // Otherwise, decrement the score by the provisioner's stake - 5. i++ // And move to the next provisioner + 5. i++ % Provisioners.size() // And move to the next provisioner (loop over when reaching the last index) ``` diff --git a/pkg/core/consensus/user/sortition.go b/pkg/core/consensus/user/sortition.go index fa5e45c1d..d26705c67 100644 --- a/pkg/core/consensus/user/sortition.go +++ b/pkg/core/consensus/user/sortition.go @@ -159,41 +159,39 @@ func (p Provisioners) CreateVotingCommittee(seed []byte, round uint64, step uint return *votingCommittee } -// extractCommitteeMember walks through the provisioners set, while deducting each stake +// extractCommitteeMember loops over the provisioners set, deducting each stake // from the sortition 'score', until this is lower than the current stake. -// When this occurs, it returns the BLS key of the provisioner on which it stops (i.e. the extracted member). +// When this occurs, it returns the BLS key of the provisioner on which the loop stops (i.e. the extracted member). func (p Provisioners) extractCommitteeMember(score uint64) []byte { - var m *Member - var e error - - for i := 0; ; i++ { - // If a provisioner is missing, we use the provisioner at position 0 - if m, e = p.MemberAt(i); e != nil { - m, e = p.MemberAt(0) - - // If provisioner 0 is also missing, panic - if e != nil { - // FIXME: shall this panic? - log.Panic(e) + // Loop over provisioners + for { + // NB: We tried to simplify this loop, using `for _, m := range p.Members` + // However, this was not deterministic due to the insertion mechanisms of the map. + // This indeterminism results in a "newblock msg is not signed by a block generator" error + // Hence we switch back to previous implementation using `i` as index + for i := 0; i < p.Set.Len(); i++ { + m, err := p.MemberAt(i) + if err != nil { + // This should never happen, but we check, just in case. + log.Panic(fmt.Errorf("no member: #%d err: %v", i, err)) } - i = 0 - } + stake, err := p.GetStake(m.PublicKeyBLS) + if err != nil { + // This should never happen, but we check, just in case. + // If we get an error from GetStake, it means we either got a public key of a + // provisioner who is no longer in the set, or we got a malformed public key. + // We can't repair our committee on the fly, so we have to panic. + log.Panic(fmt.Errorf("pk: %s err: %v", util.StringifyBytes(m.PublicKeyBLS), err)) + } - stake, err := p.GetStake(m.PublicKeyBLS) - if err != nil { - // If we get an error from GetStake, it means we either got a public key of a - // provisioner who is no longer in the set, or we got a malformed public key. - // We can't repair our committee on the fly, so we have to panic. - log.Panic(fmt.Errorf("pk: %s err: %v", util.StringifyBytes(m.PublicKeyBLS), err)) - } + // If the current stake is higher than the score, return the current provisioner's BLS key + if stake >= score { + return m.PublicKeyBLS + } - // If the current stake is higher than the score, return the current provisioner's BLS key - if stake >= score { - return m.PublicKeyBLS + score -= stake } - - score -= stake } }