Skip to content

Commit

Permalink
fix(dot/state): clean up scheduled changes once a forced change is ap…
Browse files Browse the repository at this point in the history
…plied (#3219)
  • Loading branch information
EclesioMeloJunior authored Apr 24, 2023
1 parent a7a9815 commit 5ebec46
Show file tree
Hide file tree
Showing 3 changed files with 76 additions and 11 deletions.
13 changes: 7 additions & 6 deletions dot/state/grandpa.go
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,8 @@ func (s *GrandpaState) ApplyScheduledChanges(finalizedHeader *types.Header) erro
logger.Debugf("Applying authority set change scheduled at block #%d",
changeToApply.change.announcingHeader.Number)

// TODO: add afg.applying_scheduled_authority_set_change telemetry info here
// TODO(#3218): add afg.applying_scheduled_authority_set_change telemetry info here

return nil
}

Expand Down Expand Up @@ -226,10 +227,9 @@ func (s *GrandpaState) ApplyForcedChanges(importedBlockHeader *types.Header) err
return fmt.Errorf("%w: %s", errPendingScheduledChanges, dependant.change)
}

logger.Debugf("applying forced change: %s", forcedChange)
logger.Debugf("Applying authority set forced change: %s", forcedChange)

// TODO: send the telemetry messages here
// afg.applying_forced_authority_set_change
// TODO(#3218) afg.applying_forced_authority_set_change

currentSetID, err := s.GetCurrentSetID()
if err != nil {
Expand Down Expand Up @@ -257,9 +257,10 @@ func (s *GrandpaState) ApplyForcedChanges(importedBlockHeader *types.Header) err
return fmt.Errorf("cannot set change set id at block")
}

logger.Debugf("Applying authority set forced change at block #%d",
forcedChange.announcingHeader.Number)
logger.Debugf("Applied authority set forced change: %s", forcedChange)

s.forcedChanges.pruneAll()
s.scheduledChangeRoots.pruneAll()
return nil
}

Expand Down
12 changes: 10 additions & 2 deletions dot/state/grandpa_changes.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ type pendingChange struct {
}

func (p pendingChange) String() string {
return fmt.Sprintf("announcing header: %s (%d), delay: %d, next authorities: %d",
p.announcingHeader.Hash(), p.announcingHeader.Number, p.delay, len(p.nextAuthorities))
return fmt.Sprintf("announcing header: %s (#%d), delay: %d, effective block number: %d, next authorities: %d",
p.announcingHeader.Hash().Short(), p.announcingHeader.Number, p.delay, p.effectiveNumber(), len(p.nextAuthorities))
}

func (p *pendingChange) effectiveNumber() uint {
Expand Down Expand Up @@ -132,6 +132,10 @@ func (oc *orderedPendingChanges) pruneChanges(hash common.Hash, isDescendantOf i
return nil
}

func (oc *orderedPendingChanges) pruneAll() {
*oc = make([]pendingChange, 0, oc.Len())
}

type pendingChangeNode struct {
change *pendingChange
nodes []*pendingChangeNode
Expand Down Expand Up @@ -314,3 +318,7 @@ func (ct *changeTree) pruneChanges(hash common.Hash, isDescendantOf isDescendant
*ct = onBranchChanges
return nil
}

func (ct *changeTree) pruneAll() {
*ct = []*pendingChangeNode{}
}
62 changes: 59 additions & 3 deletions dot/state/grandpa_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -600,10 +600,11 @@ func TestApplyForcedChanges(t *testing.T) {

tests := map[string]struct {
wantErr error
// 2 index array where the 0 index describes the fork and the 1 index describes the header
// 2 indexed array where the 0 index describes the fork and the 1 index describes the header
importedHeader [2]int
expectedGRANDPAAuthoritySet []types.GrandpaAuthoritiesRaw
expectedSetID uint64
expectedPruning bool

generateForks func(t *testing.T, blockState *BlockState) [][]*types.Header
changes func(*GrandpaState, [][]*types.Header)
Expand All @@ -613,6 +614,7 @@ func TestApplyForcedChanges(t *testing.T) {
importedHeader: [2]int{0, 3}, // chain A from and header number 4
expectedSetID: 0,
expectedGRANDPAAuthoritySet: genesisGrandpaVoters,
expectedPruning: false,
},
"apply_forced_change_without_pending_scheduled_changes": {
generateForks: genericForks,
Expand All @@ -639,8 +641,9 @@ func TestApplyForcedChanges(t *testing.T) {
},
})
},
importedHeader: [2]int{0, 9}, // import block number 10 from fork A
expectedSetID: 1,
importedHeader: [2]int{0, 9}, // import block number 10 from fork A
expectedSetID: 1,
expectedPruning: true,
expectedGRANDPAAuthoritySet: []types.GrandpaAuthoritiesRaw{
{Key: keyring.KeyCharlie.Public().(*sr25519.PublicKey).AsBytes()},
{Key: keyring.KeyBob.Public().(*sr25519.PublicKey).AsBytes()},
Expand All @@ -663,6 +666,7 @@ func TestApplyForcedChanges(t *testing.T) {
},
importedHeader: [2]int{2, 1}, // import block number 7 from chain C
expectedSetID: 0,
expectedPruning: false,
expectedGRANDPAAuthoritySet: genesisGrandpaVoters,
},
"import_block_from_another_fork_should_do_nothing": {
Expand All @@ -681,6 +685,7 @@ func TestApplyForcedChanges(t *testing.T) {
},
importedHeader: [2]int{1, 9}, // import block number 12 from chain B
expectedSetID: 0,
expectedPruning: false,
expectedGRANDPAAuthoritySet: genesisGrandpaVoters,
},
"apply_forced_change_with_pending_scheduled_changes_should_fail": {
Expand Down Expand Up @@ -720,6 +725,44 @@ func TestApplyForcedChanges(t *testing.T) {
wantErr: errPendingScheduledChanges,
expectedGRANDPAAuthoritySet: genesisGrandpaVoters,
expectedSetID: 0,
expectedPruning: false,
},

"apply_forced_change_should_prune_scheduled_changes": {
generateForks: genericForks,
changes: func(gs *GrandpaState, headers [][]*types.Header) {
chainBBlock12 := headers[1][9]
chainBBlock11 := headers[1][8]
chainBBlock10 := headers[1][7]

// add scheduled changes for block 10, 11 and 12 from for B
addScheduledChanges := []*types.Header{chainBBlock10, chainBBlock11, chainBBlock12}
for _, blockHeader := range addScheduledChanges {
gs.addScheduledChange(blockHeader, types.GrandpaScheduledChange{
Delay: 0,
Auths: []types.GrandpaAuthoritiesRaw{
{Key: keyring.KeyDave.Public().(*sr25519.PublicKey).AsBytes()},
},
})
}

// add a forced change for block 9 from chain C with delay of 3 blocks
// once block 11 got imported Gossamer should clean up all the scheduled + forced changes
chainCBlock9 := headers[2][2]
gs.addForcedChange(chainCBlock9, types.GrandpaForcedChange{
Delay: 3,
BestFinalizedBlock: 2,
Auths: []types.GrandpaAuthoritiesRaw{
{Key: keyring.KeyCharlie.Public().(*sr25519.PublicKey).AsBytes()},
{Key: keyring.KeyBob.Public().(*sr25519.PublicKey).AsBytes()},
{Key: keyring.KeyDave.Public().(*sr25519.PublicKey).AsBytes()},
},
})
},
importedHeader: [2]int{2, 7}, // import block 12 from chain C
expectedGRANDPAAuthoritySet: genesisGrandpaVoters,
expectedSetID: 0,
expectedPruning: false,
},
}

Expand All @@ -744,6 +787,9 @@ func TestApplyForcedChanges(t *testing.T) {
selectedFork := forks[tt.importedHeader[0]]
selectedImportedHeader := selectedFork[tt.importedHeader[1]]

amountOfForced := gs.forcedChanges.Len()
amountOfScheduled := gs.scheduledChangeRoots.Len()

err = gs.ApplyForcedChanges(selectedImportedHeader)
if tt.wantErr != nil {
require.Error(t, err)
Expand All @@ -752,6 +798,16 @@ func TestApplyForcedChanges(t *testing.T) {
require.NoError(t, err)
}

if tt.expectedPruning {
// we should reset the changes set once a forced change is applied
const expectedLen = 0
require.Equal(t, expectedLen, gs.forcedChanges.Len())
require.Equal(t, expectedLen, gs.scheduledChangeRoots.Len())
} else {
require.Equal(t, amountOfForced, gs.forcedChanges.Len())
require.Equal(t, amountOfScheduled, gs.scheduledChangeRoots.Len())
}

currentSetID, err := gs.GetCurrentSetID()
require.NoError(t, err)
require.Equal(t, tt.expectedSetID, currentSetID)
Expand Down

0 comments on commit 5ebec46

Please sign in to comment.