diff --git a/dot/state/grandpa.go b/dot/state/grandpa.go index f88050d53d..a4ae7bce33 100644 --- a/dot/state/grandpa.go +++ b/dot/state/grandpa.go @@ -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 } @@ -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 { @@ -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 } diff --git a/dot/state/grandpa_changes.go b/dot/state/grandpa_changes.go index e06f1fe0fa..0ca04857d0 100644 --- a/dot/state/grandpa_changes.go +++ b/dot/state/grandpa_changes.go @@ -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 { @@ -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 @@ -314,3 +318,7 @@ func (ct *changeTree) pruneChanges(hash common.Hash, isDescendantOf isDescendant *ct = onBranchChanges return nil } + +func (ct *changeTree) pruneAll() { + *ct = []*pendingChangeNode{} +} diff --git a/dot/state/grandpa_test.go b/dot/state/grandpa_test.go index 33989f69ba..f13f7900f4 100644 --- a/dot/state/grandpa_test.go +++ b/dot/state/grandpa_test.go @@ -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) @@ -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, @@ -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()}, @@ -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": { @@ -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": { @@ -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, }, } @@ -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) @@ -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)