fix(dot/state): clean up scheduled changes once a forced change is ap…

…plied (#3219)

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
 }
 

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{}
+}

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)