Skip to content

Commit

Permalink
respond to feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
@jimjbrettj
jimjbrettj committed Aug 15, 2023
1 parent 04441f2 commit 66a31fa
Show file tree
Hide file tree
Showing 6 changed files with 90 additions and 85 deletions.
36 changes: 18 additions & 18 deletions client/consensus/grandpa/authorities.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ type IsDescendentOf[H comparable] func(base, target H) (bool, error)

// setIDNumber represents the set id and block number of an authority set hashNumber
type setIDNumber[N constraints.Unsigned] struct {
SetId uint64
SetID uint64
BlockNumber N
}

Expand Down Expand Up @@ -199,7 +199,7 @@ type AuthoritySet[H comparable, N constraints.Unsigned] struct {
// The current active authorities.
CurrentAuthorities AuthorityList
// The current set id.
SetId uint64
SetID uint64
// Tree of pending standard changes across forks. Standard changes are
// enacted on finality and must be enacted (i.e. finalized) in-order across
// a given branch
Expand Down Expand Up @@ -247,7 +247,7 @@ func NewGenesisAuthoritySet[H comparable, N constraints.Unsigned](initial Author

// NewAuthoritySet creates a new AuthoritySet
func NewAuthoritySet[H comparable, N constraints.Unsigned](authorities AuthorityList,
setId uint64,
setID uint64,
pendingStandardChanges ForkTree[H, N],
pendingForcedChanges []PendingChange[H, N],
authoritySetChanges AuthoritySetChanges[N],
Expand All @@ -258,7 +258,7 @@ func NewAuthoritySet[H comparable, N constraints.Unsigned](authorities Authority

return &AuthoritySet[H, N]{
CurrentAuthorities: authorities,
SetId: setId,
SetID: setID,
PendingStandardChanges: pendingStandardChanges,
PendingForcedChanges: pendingForcedChanges,
AuthoritySetChanges: authoritySetChanges,
Expand All @@ -267,7 +267,7 @@ func NewAuthoritySet[H comparable, N constraints.Unsigned](authorities Authority

// current Get the current set id and a reference to the current authority set.
func (authSet *AuthoritySet[H, N]) current() (uint64, *AuthorityList) {
return authSet.SetId, &authSet.CurrentAuthorities
return authSet.SetID, &authSet.CurrentAuthorities
}

func (authSet *AuthoritySet[H, N]) revert() {
Expand Down Expand Up @@ -526,12 +526,12 @@ func (authSet *AuthoritySet[H, N]) applyForcedChanges(bestHash H,
// TODO telemetry

authSetChanges := authSet.AuthoritySetChanges
authSetChanges.append(authSet.SetId, medianLastFinalized)
authSetChanges.append(authSet.SetID, medianLastFinalized)
newSet = &appliedChanges[H, N]{
medianLastFinalized,
AuthoritySet[H, N]{
CurrentAuthorities: change.NextAuthorities,
SetId: authSet.SetId + 1,
SetID: authSet.SetID + 1,
PendingStandardChanges: NewChangeTree[H, N](), // new set, new changes
PendingForcedChanges: []PendingChange[H, N]{},
AuthoritySetChanges: authSetChanges,
Expand Down Expand Up @@ -602,9 +602,9 @@ func (authSet *AuthoritySet[H, N]) applyStandardChanges(
// TODO add telemetry

// Store the set_id together with the last block_number for the set
authSet.AuthoritySetChanges.append(authSet.SetId, finalizedNumber)
authSet.AuthoritySetChanges.append(authSet.SetID, finalizedNumber)
authSet.CurrentAuthorities = finalizationResult.Value.NextAuthorities
authSet.SetId++
authSet.SetID++

status.NewSetBlock = &hashNumber[H, N]{
hash: finalizedHash,
Expand Down Expand Up @@ -665,9 +665,9 @@ func (pc *PendingChange[H, N]) EffectiveNumber() N {
type AuthoritySetChanges[N constraints.Unsigned] []setIDNumber[N]

// append an setIDNumber to AuthoritySetChanges
func (asc *AuthoritySetChanges[N]) append(setId uint64, blockNumber N) {
func (asc *AuthoritySetChanges[N]) append(setID uint64, blockNumber N) {
*asc = append(*asc, setIDNumber[N]{
SetId: setId,
SetID: setID,
BlockNumber: blockNumber,
})
}
Expand Down Expand Up @@ -707,7 +707,7 @@ func (asc *AuthoritySetChanges[N]) getSetID(blockNumber N) (latest bool, set *se
authChange := authSet[idx]

// if this is the first index but not the first set id then we are missing data.
if idx == 0 && authChange.SetId != 0 {
if idx == 0 && authChange.SetID != 0 {
return false, nil, nil // Unknown case
}

Expand Down Expand Up @@ -742,19 +742,19 @@ func (asc *AuthoritySetChanges[N]) insert(blockNumber N) {

set := *asc

var setId uint64
var setID uint64
if idx == 0 {
setId = 0
setID = 0
} else {
setId = set[idx-1].SetId + 1
setID = set[idx-1].SetID + 1
}

if idx != len(set) && set[idx].SetId == setId {
if idx != len(set) && set[idx].SetID == setID {
panic("inserting authority set hashNumber")
}

change := setIDNumber[N]{
SetId: setId,
SetID: setID,
BlockNumber: blockNumber,
}

Expand Down Expand Up @@ -801,7 +801,7 @@ func (asc *AuthoritySetChanges[N]) IterFrom(blockNumber N) *AuthoritySetChanges[
authChange := authSet[idx]

// if this is the first index but not the first set id then we are missing data.
if idx == 0 && authChange.SetId != 0 {
if idx == 0 && authChange.SetID != 0 {
return nil
}
}
Expand Down
66 changes: 33 additions & 33 deletions client/consensus/grandpa/authorities_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ func TestCurrentLimitFiltersMin(t *testing.T) {

authorities := AuthoritySet[Hash, uint]{
CurrentAuthorities: currentAuthorities,
SetId: 0,
SetID: 0,
PendingStandardChanges: NewChangeTree[Hash, uint](),
PendingForcedChanges: []PendingChange[Hash, uint]{},
AuthoritySetChanges: AuthoritySetChanges[uint]{},
Expand Down Expand Up @@ -96,7 +96,7 @@ func TestChangesIteratedInPreOrder(t *testing.T) {

authorities := AuthoritySet[Hash, uint]{
CurrentAuthorities: currentAuthorities,
SetId: 0,
SetID: 0,
PendingStandardChanges: NewChangeTree[Hash, uint](),
PendingForcedChanges: []PendingChange[Hash, uint]{},
AuthoritySetChanges: AuthoritySetChanges[uint]{},
Expand Down Expand Up @@ -175,7 +175,7 @@ func TestChangesIteratedInPreOrder(t *testing.T) {
func TestApplyChange(t *testing.T) {
authorities := AuthoritySet[Hash, uint]{
CurrentAuthorities: AuthorityList{},
SetId: 0,
SetID: 0,
PendingStandardChanges: NewChangeTree[Hash, uint](),
PendingForcedChanges: []PendingChange[Hash, uint]{},
AuthoritySetChanges: AuthoritySetChanges[uint]{},
Expand Down Expand Up @@ -277,12 +277,12 @@ func TestApplyChange(t *testing.T) {
require.True(t, status.Changed)
require.Equal(t, status.NewSetBlock, expectedBlockInfo)
require.Equal(t, authorities.CurrentAuthorities, setA)
require.Equal(t, authorities.SetId, uint64(1))
require.Equal(t, authorities.SetID, uint64(1))

pendingChanges = authorities.pendingChanges()
require.Equal(t, 0, len(pendingChanges))
expChange := setIDNumber[uint]{
SetId: 0,
SetID: 0,
BlockNumber: 15,
}
require.Equal(t, authorities.AuthoritySetChanges, AuthoritySetChanges[uint]{expChange})
Expand All @@ -291,7 +291,7 @@ func TestApplyChange(t *testing.T) {
func TestDisallowMultipleChangesBeingFinalizedAtOnce(t *testing.T) {
authorities := AuthoritySet[Hash, uint]{
CurrentAuthorities: AuthorityList{},
SetId: 0,
SetID: 0,
PendingStandardChanges: NewChangeTree[Hash, uint](),
PendingForcedChanges: []PendingChange[Hash, uint]{},
AuthoritySetChanges: AuthoritySetChanges[uint]{},
Expand Down Expand Up @@ -376,12 +376,12 @@ func TestDisallowMultipleChangesBeingFinalizedAtOnce(t *testing.T) {
number: 15,
}
expAuthSetChange := AuthoritySetChanges[uint]{setIDNumber[uint]{
SetId: 0,
SetID: 0,
BlockNumber: 15,
}}
require.Equal(t, expectedBlockInfo, status.NewSetBlock)
require.Equal(t, setA, authorities.CurrentAuthorities)
require.Equal(t, uint64(1), authorities.SetId)
require.Equal(t, uint64(1), authorities.SetID)
require.Equal(t, expAuthSetChange, authorities.AuthoritySetChanges)

status, err = authorities.applyStandardChanges(
Expand All @@ -398,25 +398,25 @@ func TestDisallowMultipleChangesBeingFinalizedAtOnce(t *testing.T) {
}
expAuthSetChange = AuthoritySetChanges[uint]{
setIDNumber[uint]{
SetId: 0,
SetID: 0,
BlockNumber: 15,
},
setIDNumber[uint]{
SetId: 1,
SetID: 1,
BlockNumber: 40,
},
}

require.Equal(t, expectedBlockInfo, status.NewSetBlock)
require.Equal(t, setC, authorities.CurrentAuthorities)
require.Equal(t, uint64(2), authorities.SetId)
require.Equal(t, uint64(2), authorities.SetID)
require.Equal(t, expAuthSetChange, authorities.AuthoritySetChanges)
}

func TestEnactsStandardChangeWorks(t *testing.T) {
authorities := AuthoritySet[Hash, uint]{
CurrentAuthorities: AuthorityList{},
SetId: 0,
SetID: 0,
PendingStandardChanges: NewChangeTree[Hash, uint](),
PendingForcedChanges: []PendingChange[Hash, uint]{},
AuthoritySetChanges: AuthoritySetChanges[uint]{},
Expand Down Expand Up @@ -494,7 +494,7 @@ func TestEnactsStandardChangeWorks(t *testing.T) {
func TestForceChanges(t *testing.T) {
authorities := AuthoritySet[Hash, uint]{
CurrentAuthorities: AuthorityList{},
SetId: 0,
SetID: 0,
PendingStandardChanges: NewChangeTree[Hash, uint](),
PendingForcedChanges: []PendingChange[Hash, uint]{},
AuthoritySetChanges: AuthoritySetChanges[uint]{},
Expand Down Expand Up @@ -583,12 +583,12 @@ func TestForceChanges(t *testing.T) {
median: 42,
set: AuthoritySet[Hash, uint]{
CurrentAuthorities: setA,
SetId: 1,
SetID: 1,
PendingStandardChanges: NewChangeTree[Hash, uint](),
PendingForcedChanges: []PendingChange[Hash, uint]{},
AuthoritySetChanges: AuthoritySetChanges[uint]{
setIDNumber[uint]{
SetId: 0,
SetID: 0,
BlockNumber: 42,
},
},
Expand All @@ -604,7 +604,7 @@ func TestForceChangesWithNoDelay(t *testing.T) {
// NOTE: this is a regression test
authorities := AuthoritySet[Hash, uint]{
CurrentAuthorities: AuthorityList{},
SetId: 0,
SetID: 0,
PendingStandardChanges: NewChangeTree[Hash, uint](),
PendingForcedChanges: []PendingChange[Hash, uint]{},
AuthoritySetChanges: AuthoritySetChanges[uint]{},
Expand Down Expand Up @@ -643,7 +643,7 @@ func TestForceChangesWithNoDelay(t *testing.T) {
func TestForceChangesBlockedByStandardChanges(t *testing.T) {
authorities := AuthoritySet[Hash, uint]{
CurrentAuthorities: AuthorityList{},
SetId: 0,
SetID: 0,
PendingStandardChanges: NewChangeTree[Hash, uint](),
PendingForcedChanges: []PendingChange[Hash, uint]{},
AuthoritySetChanges: AuthoritySetChanges[uint]{},
Expand Down Expand Up @@ -721,7 +721,7 @@ func TestForceChangesBlockedByStandardChanges(t *testing.T) {
// we apply the first pending standard hashNumber at #15
expChanges := AuthoritySetChanges[uint]{
setIDNumber[uint]{
SetId: 0,
SetID: 0,
BlockNumber: 15,
},
}
Expand All @@ -735,7 +735,7 @@ func TestForceChangesBlockedByStandardChanges(t *testing.T) {

// we apply the pending standard hashNumber at #20
expChanges = append(expChanges, setIDNumber[uint]{
SetId: 1,
SetID: 1,
BlockNumber: 20,
})
_, err = authorities.applyStandardChanges(bytesToHash([]byte("hash_b")), 20, staticIsDescendentOf[Hash](true), nil)
Expand All @@ -745,14 +745,14 @@ func TestForceChangesBlockedByStandardChanges(t *testing.T) {
// that finality stalled at #31, and the next pending standard hashNumber is effective
// at #35. subsequent forced changes on the same branch must be kept
expChanges = append(expChanges, setIDNumber[uint]{
SetId: 2,
SetID: 2,
BlockNumber: 31,
})
exp := appliedChanges[Hash, uint]{
median: 31,
set: AuthoritySet[Hash, uint]{
CurrentAuthorities: setA,
SetId: 3,
SetID: 3,
PendingStandardChanges: NewChangeTree[Hash, uint](),
PendingForcedChanges: []PendingChange[Hash, uint]{},
AuthoritySetChanges: expChanges,
Expand All @@ -775,7 +775,7 @@ func TestNextChangeWorks(t *testing.T) {

authorities := AuthoritySet[Hash, uint]{
CurrentAuthorities: currentAuthorities,
SetId: 0,
SetID: 0,
PendingStandardChanges: NewChangeTree[Hash, uint](),
PendingForcedChanges: []PendingChange[Hash, uint]{},
AuthoritySetChanges: AuthoritySetChanges[uint]{},
Expand Down Expand Up @@ -967,7 +967,7 @@ func TestCleanUpStaleForcedChangesWhenApplyingStandardChange(t *testing.T) {

authorities := AuthoritySet[Hash, uint]{
CurrentAuthorities: currentAuthorities,
SetId: 0,
SetID: 0,
PendingStandardChanges: NewChangeTree[Hash, uint](),
PendingForcedChanges: []PendingChange[Hash, uint]{},
AuthoritySetChanges: AuthoritySetChanges[uint]{},
Expand Down Expand Up @@ -1074,7 +1074,7 @@ func TestCleanUpStaleForcedChangesWhenApplyingStandardChangeAlternateCase(t *tes

authorities := AuthoritySet[Hash, uint]{
CurrentAuthorities: currentAuthorities,
SetId: 0,
SetID: 0,
PendingStandardChanges: NewChangeTree[Hash, uint](),
PendingForcedChanges: []PendingChange[Hash, uint]{},
AuthoritySetChanges: AuthoritySetChanges[uint]{},
Expand Down Expand Up @@ -1180,7 +1180,7 @@ func TestAuthoritySetChangesInsert(t *testing.T) {
authoritySetChanges.insert(101)

expChange := setIDNumber[uint]{
SetId: 2,
SetID: 2,
BlockNumber: 101,
}
_, set, err := authoritySetChanges.getSetID(100)
Expand All @@ -1199,12 +1199,12 @@ func TestAuthoritySetChangesForCompleteData(t *testing.T) {
authoritySetChanges.append(2, 121)

expChange0 := setIDNumber[uint]{
SetId: 0,
SetID: 0,
BlockNumber: 41,
}

expChange1 := setIDNumber[uint]{
SetId: 1,
SetID: 1,
BlockNumber: 81,
}

Expand Down Expand Up @@ -1236,7 +1236,7 @@ func TestAuthoritySetChangesForIncompleteData(t *testing.T) {
authoritySetChanges.append(4, 121)

expChange := setIDNumber[uint]{
SetId: 3,
SetID: 3,
BlockNumber: 81,
}

Expand Down Expand Up @@ -1279,15 +1279,15 @@ func TestIterFromWorks(t *testing.T) {

expectedChanges := &AuthoritySetChanges[uint]{
setIDNumber[uint]{
SetId: 1,
SetID: 1,
BlockNumber: 41,
},
setIDNumber[uint]{
SetId: 2,
SetID: 2,
BlockNumber: 81,
},
setIDNumber[uint]{
SetId: 3,
SetID: 3,
BlockNumber: 121,
},
}
Expand All @@ -1297,11 +1297,11 @@ func TestIterFromWorks(t *testing.T) {

expectedChanges = &AuthoritySetChanges[uint]{
setIDNumber[uint]{
SetId: 2,
SetID: 2,
BlockNumber: 81,
},
setIDNumber[uint]{
SetId: 3,
SetID: 3,
BlockNumber: 121,
},
}
Expand Down
Loading

0 comments on commit 66a31fa

Please sign in to comment.