From da441cc257e9f28c241af72698a27a36f3d2ea3c Mon Sep 17 00:00:00 2001
From: Roman <ackhtariev@gmail.com>
Date: Mon, 11 Apr 2022 23:04:59 -0400
Subject: [PATCH 1/6] progress

---
 pruning/manager.go            |  95 ++++++++------
 pruning/manager_test.go       | 241 +++++++++++++++++++---------------
 store/rootmulti/store.go      |  14 +-
 store/rootmulti/store_test.go |  15 ++-
 4 files changed, 210 insertions(+), 155 deletions(-)

diff --git a/pruning/manager.go b/pruning/manager.go
index 2b4b6309b85b..7749cffdd363 100644
--- a/pruning/manager.go
+++ b/pruning/manager.go
@@ -13,12 +13,14 @@ import (
 )
 
 type Manager struct {
+	db             dbm.DB
 	logger               log.Logger
 	opts                 types.PruningOptions
 	snapshotInterval     uint64
 	pruneHeights         []int64
+	pruneHeightsMx	   sync.Mutex
 	pruneSnapshotHeights *list.List
-	mx                   sync.Mutex
+	pruneSnapshotHeightsMx                   sync.Mutex
 }
 
 const errNegativeHeightsFmt = "failed to get pruned heights: %d"
@@ -28,15 +30,17 @@ var (
 	pruneSnapshotHeightsKey = []byte("s/pruneSnheights")
 )
 
-func NewManager(logger log.Logger) *Manager {
+func NewManager(db dbm.DB, logger log.Logger) *Manager {
 	return &Manager{
+		db: db,
 		logger:       logger,
 		opts:         types.NewPruningOptions(types.PruningNothing),
 		pruneHeights: []int64{},
+		pruneHeightsMx: sync.Mutex{},
 		// These are the heights that are multiples of snapshotInterval and kept for state sync snapshots.
 		// The heights are added to this list to be pruned when a snapshot is complete.
 		pruneSnapshotHeights: list.New(),
-		mx:                   sync.Mutex{},
+		pruneSnapshotHeightsMx:                   sync.Mutex{},
 	}
 }
 
@@ -50,15 +54,25 @@ func (m *Manager) GetOptions() types.PruningOptions {
 	return m.opts
 }
 
-// GetPruningHeights returns all heights to be pruned during the next call to Prune().
-func (m *Manager) GetPruningHeights() []int64 {
-	return m.pruneHeights
-}
+// GetFlushAndResetPruningHeights returns all heights to be pruned during the next call to Prune().
+// It also flushes and resets the pruning heights.
+func (m *Manager) GetFlushAndResetPruningHeights() ([]int64, error) {
+	if m.opts.GetPruningStrategy() == types.PruningNothing {
+		return []int64{}, nil
+	}
+	m.pruneHeightsMx.Lock()
+	defer m.pruneHeightsMx.Unlock()
 
-// ResetPruningHeights resets the heights to be pruned.
-func (m *Manager) ResetPruningHeights() {
-	// reuse previously allocated memory.
-	m.pruneHeights = m.pruneHeights[:0]
+	pruningHeights := m.pruneHeights
+	
+	// flush the update to disk so that it is not lost if crash happens.
+	if err := m.db.SetSync(pruneHeightsKey, int64SliceToBytes(pruningHeights)); err != nil {
+		return nil, err
+	}
+
+	m.pruneHeights = make([]int64, 0, m.opts.Interval)
+	
+	return pruningHeights, nil
 }
 
 // HandleHeight determines if pruneHeight height needs to be kept for pruning at the right interval prescribed by
@@ -71,9 +85,14 @@ func (m *Manager) HandleHeight(previousHeight int64) int64 {
 	}
 
 	defer func() {
-		// handle persisted snapshot heights
-		m.mx.Lock()
-		defer m.mx.Unlock()
+		m.pruneHeightsMx.Lock()
+		defer m.pruneHeightsMx.Unlock()
+
+		m.pruneSnapshotHeightsMx.Lock()
+		defer m.pruneSnapshotHeightsMx.Unlock()
+
+		// move persisted snapshot heights to pruneHeights which
+		// represent the heights to be pruned at the next pruning interval.
 		var next *list.Element
 		for e := m.pruneSnapshotHeights.Front(); e != nil; e = next {
 			snHeight := e.Value.(int64)
@@ -87,6 +106,11 @@ func (m *Manager) HandleHeight(previousHeight int64) int64 {
 				next = e.Next()
 			}
 		}
+
+		// flush the update to disk so that they are not lost if crash happens.
+		if err := m.db.SetSync(pruneHeightsKey, int64SliceToBytes(m.pruneHeights)); err != nil {
+			panic(err)
+		}
 	}()
 
 	if int64(m.opts.KeepRecent) < previousHeight {
@@ -97,7 +121,15 @@ func (m *Manager) HandleHeight(previousHeight int64) int64 {
 		// - snapshotInterval % (height - KeepRecent) != 0 as that means the height is not
 		// a 'snapshot' height.
 		if m.snapshotInterval == 0 || pruneHeight%int64(m.snapshotInterval) != 0 {
+			m.pruneHeightsMx.Lock()
+			defer m.pruneHeightsMx.Unlock()
+
 			m.pruneHeights = append(m.pruneHeights, pruneHeight)
+
+			// flush the update to disk so that they are not lost if crash happens.
+			if err := m.db.SetSync(pruneHeightsKey, int64SliceToBytes(m.pruneHeights)); err != nil {
+				panic(err)
+			}
 			return pruneHeight
 		}
 	}
@@ -108,10 +140,15 @@ func (m *Manager) HandleHeightSnapshot(height int64) {
 	if m.opts.GetPruningStrategy() == types.PruningNothing {
 		return
 	}
-	m.mx.Lock()
-	defer m.mx.Unlock()
+	m.pruneSnapshotHeightsMx.Lock()
+	defer m.pruneSnapshotHeightsMx.Unlock()
 	m.logger.Debug("HandleHeightSnapshot", "height", height)
 	m.pruneSnapshotHeights.PushBack(height)
+
+	// flush the update to disk so that they are not lost if crash happens.
+	if err := m.db.SetSync(pruneSnapshotHeightsKey, listToBytes(m.pruneSnapshotHeights)); err != nil {
+		panic(err)
+	}
 }
 
 // SetSnapshotInterval sets the interval at which the snapshots are taken.
@@ -124,15 +161,6 @@ func (m *Manager) ShouldPruneAtHeight(height int64) bool {
 	return m.opts.Interval > 0 && m.opts.GetPruningStrategy() != types.PruningNothing && height%int64(m.opts.Interval) == 0
 }
 
-// FlushPruningHeights flushes the pruning heights to the database for crash recovery.
-func (m *Manager) FlushPruningHeights(batch dbm.Batch) {
-	if m.opts.GetPruningStrategy() == types.PruningNothing {
-		return
-	}
-	m.flushPruningHeights(batch)
-	m.flushPruningSnapshotHeights(batch)
-}
-
 // LoadPruningHeights loads the pruning heights from the database as a crash recovery.
 func (m *Manager) LoadPruningHeights(db dbm.DB) error {
 	if m.opts.GetPruningStrategy() == types.PruningNothing {
@@ -167,6 +195,8 @@ func (m *Manager) loadPruningHeights(db dbm.DB) error {
 	}
 
 	if len(prunedHeights) > 0 {
+		m.pruneHeightsMx.Lock()
+		defer m.pruneHeightsMx.Unlock()
 		m.pruneHeights = prunedHeights
 	}
 
@@ -195,24 +225,13 @@ func (m *Manager) loadPruningSnapshotHeights(db dbm.DB) error {
 		offset += 8
 	}
 
-	m.mx.Lock()
-	defer m.mx.Unlock()
+	m.pruneSnapshotHeightsMx.Lock()
+	defer m.pruneSnapshotHeightsMx.Unlock()
 	m.pruneSnapshotHeights = pruneSnapshotHeights
 
 	return nil
 }
 
-func (m *Manager) flushPruningHeights(batch dbm.Batch) {
-	batch.Set(pruneHeightsKey, int64SliceToBytes(m.pruneHeights))
-}
-
-func (m *Manager) flushPruningSnapshotHeights(batch dbm.Batch) {
-	m.mx.Lock()
-	defer m.mx.Unlock()
-	batch.Set(pruneSnapshotHeightsKey, listToBytes(m.pruneSnapshotHeights))
-}
-
-// TODO: convert to a generic version with Go 1.18.
 func int64SliceToBytes(slice []int64) []byte {
 	bz := make([]byte, 0, len(slice)*8)
 	for _, ph := range slice {
diff --git a/pruning/manager_test.go b/pruning/manager_test.go
index e6a696b7f025..5634a9926d64 100644
--- a/pruning/manager_test.go
+++ b/pruning/manager_test.go
@@ -4,9 +4,7 @@ import (
 	"container/list"
 	"fmt"
 
-	"sync"
 	"testing"
-	"time"
 
 	"github.com/stretchr/testify/require"
 	"github.com/tendermint/tendermint/libs/log"
@@ -17,10 +15,12 @@ import (
 )
 
 func TestNewManager(t *testing.T) {
-	manager := pruning.NewManager(log.NewNopLogger())
+	manager := pruning.NewManager(db.NewMemDB(), log.NewNopLogger())
 
 	require.NotNil(t, manager)
-	require.NotNil(t, manager.GetPruningHeights())
+	heights, err := manager.GetFlushAndResetPruningHeights()
+	require.NoError(t, err)
+	require.NotNil(t, heights)
 	require.Equal(t, types.PruningNothing, manager.GetOptions().GetPruningStrategy())
 }
 
@@ -75,7 +75,7 @@ func TestStrategies(t *testing.T) {
 		},
 	}
 
-	manager := pruning.NewManager(log.NewNopLogger())
+	manager := pruning.NewManager(db.NewMemDB(), log.NewNopLogger())
 
 	require.NotNil(t, manager)
 
@@ -112,7 +112,8 @@ func TestStrategies(t *testing.T) {
 				handleHeightActual := manager.HandleHeight(curHeight)
 				shouldPruneAtHeightActual := manager.ShouldPruneAtHeight(curHeight)
 
-				curPruningHeihts := manager.GetPruningHeights()
+				curPruningHeihts, err := manager.GetFlushAndResetPruningHeights()
+				require.Nil(t, err)
 
 				curHeightStr := fmt.Sprintf("height: %d", curHeight)
 
@@ -121,7 +122,9 @@ func TestStrategies(t *testing.T) {
 					require.Equal(t, int64(0), handleHeightActual, curHeightStr)
 					require.False(t, shouldPruneAtHeightActual, curHeightStr)
 
-					require.Equal(t, 0, len(manager.GetPruningHeights()))
+					heights, err := manager.GetFlushAndResetPruningHeights()
+					require.NoError(t, err)
+					require.Equal(t, 0, len(heights))
 				default:
 					if curHeight > int64(curKeepRecent) && (tc.snapshotInterval != 0 && (curHeight-int64(curKeepRecent))%int64(tc.snapshotInterval) != 0 || tc.snapshotInterval == 0) {
 						expectedHeight := curHeight - int64(curKeepRecent)
@@ -131,12 +134,15 @@ func TestStrategies(t *testing.T) {
 					} else {
 						require.Equal(t, int64(0), handleHeightActual, curHeightStr)
 
-						require.Equal(t, 0, len(manager.GetPruningHeights()))
+						heights, err := manager.GetFlushAndResetPruningHeights()
+						require.NoError(t, err)
+						require.Equal(t, 0, len(heights))
 					}
 					require.Equal(t, curHeight%int64(curInterval) == 0, shouldPruneAtHeightActual, curHeightStr)
 				}
-				manager.ResetPruningHeights()
-				require.Equal(t, 0, len(manager.GetPruningHeights()))
+				heights, err := manager.GetFlushAndResetPruningHeights()
+				require.NoError(t, err)
+				require.Equal(t, 0, len(heights))
 			}
 		})
 	}
@@ -185,23 +191,120 @@ func TestHandleHeight_Inputs(t *testing.T) {
 
 	for name, tc := range testcases {
 		t.Run(name, func(t *testing.T) {
-			manager := pruning.NewManager(log.NewNopLogger())
+			manager := pruning.NewManager(db.NewMemDB(), log.NewNopLogger())
 			require.NotNil(t, manager)
 			manager.SetOptions(types.NewPruningOptions(tc.strategy))
 
 			handleHeightActual := manager.HandleHeight(tc.height)
 			require.Equal(t, tc.expectedResult, handleHeightActual)
 
-			require.Equal(t, tc.expectedHeights, manager.GetPruningHeights())
+			actualHeights, err := manager.GetFlushAndResetPruningHeights()
+			require.NoError(t, err)
+			require.Equal(t, len(tc.expectedHeights), len(actualHeights))
+			require.Equal(t, tc.expectedHeights, actualHeights)
 		})
 	}
 }
 
-func TestFlushLoad(t *testing.T) {
-	manager := pruning.NewManager(log.NewNopLogger())
-	require.NotNil(t, manager)
+func TestHandleHeight_FlushToDisk(t *testing.T) {
+	testcases := map[string]struct {
+		previousHeight int64
+		keepRecent uint64
+		snapshotInterval uint64
+		movedSnapshotHeights []int64
+		expectedHandleHeightResult int64
+		expectedLoadPruningHeightsResult error
+		expectedLoadedHeights []int64
+	}{
+		"simple flush occurs": {
+			previousHeight: 11,
+			keepRecent: 10,
+			snapshotInterval: 0,
+			movedSnapshotHeights: []int64{},
+			expectedHandleHeightResult: 11 - 10,
+			expectedLoadPruningHeightsResult: nil,
+			expectedLoadedHeights: []int64{11 - 10},
+		},
+		"previous height <= keep recent - no update and no flush": {
+			previousHeight: 9,
+			keepRecent: 10,
+			snapshotInterval: 0,
+			movedSnapshotHeights: []int64{},
+			expectedHandleHeightResult: 0,
+			expectedLoadPruningHeightsResult: nil,
+			expectedLoadedHeights: []int64{},
+		},
+		"previous height alligns with snapshot interval - no update and no flush": {
+			previousHeight: 12,
+			keepRecent: 10,
+			snapshotInterval: 2,
+			movedSnapshotHeights: []int64{},
+			expectedHandleHeightResult: 0,
+			expectedLoadPruningHeightsResult: nil,
+			expectedLoadedHeights: []int64{},
+		},
+		"previous height does not align with snapshot interval - flush": {
+			previousHeight: 12,
+			keepRecent: 10,
+			snapshotInterval: 3,
+			movedSnapshotHeights: []int64{},
+			expectedHandleHeightResult: 2,
+			expectedLoadPruningHeightsResult: nil,
+			expectedLoadedHeights: []int64{2},
+		},
+		"moved snapshot heights - flushed": {
+			previousHeight: 32,
+			keepRecent: 10,
+			snapshotInterval: 5,
+			movedSnapshotHeights: []int64{15, 20, 25},
+			expectedHandleHeightResult: 22,
+			expectedLoadPruningHeightsResult: nil,
+			expectedLoadedHeights: []int64{15, 20, 22},
+		},
+		"previous height alligns with snapshot interval - no update but flush snapshot heights": {
+			previousHeight: 30,
+			keepRecent: 10,
+			snapshotInterval: 5,
+			movedSnapshotHeights: []int64{15, 20, 25},
+			expectedHandleHeightResult: 0,
+			expectedLoadPruningHeightsResult: nil,
+			expectedLoadedHeights: []int64{15},
+		},
+	}
+
+	for name, tc := range testcases {
+		t.Run(name, func(t *testing.T) {
+			// Setup
+			db := db.NewMemDB()
+			manager := pruning.NewManager(db, log.NewNopLogger())
+			require.NotNil(t, manager)
 
+			manager.SetSnapshotInterval(tc.snapshotInterval)
+			manager.SetOptions(types.NewCustomPruningOptions(uint64(tc.keepRecent), uint64(10)))
+
+			for _, snapshotHeight := range tc.movedSnapshotHeights {
+				manager.HandleHeightSnapshot(snapshotHeight)
+			}
+
+			// Test
+			handleHeightActual := manager.HandleHeight(tc.previousHeight)
+			require.Equal(t, tc.expectedHandleHeightResult, handleHeightActual)
+
+			err := manager.LoadPruningHeights(db)
+			require.NoError(t, err)
+
+			heights, err := manager.GetFlushAndResetPruningHeights()
+			require.NoError(t, err)
+			require.Equal(t, len(tc.expectedLoadedHeights), len(heights))
+			require.ElementsMatch(t, tc.expectedLoadedHeights, heights)
+		})
+	}
+}
+
+func TestFlushLoad(t *testing.T) {
 	db := db.NewMemDB()
+	manager := pruning.NewManager(db, log.NewNopLogger())
+	require.NotNil(t, manager)
 
 	curStrategy := types.NewCustomPruningOptions(100, 15)
 
@@ -228,32 +331,28 @@ func TestFlushLoad(t *testing.T) {
 			require.Equal(t, int64(0), handleHeightActual, curHeightStr)
 		}
 
-		if manager.ShouldPruneAtHeight(curHeight) {
-			manager.ResetPruningHeights()
-			heightsToPruneMirror = make([]int64, 0)
-		}
-
-		// N.B.: There is no reason behind the choice of 3.
-		if curHeight%3 == 0 {
-			require.Equal(t, heightsToPruneMirror, manager.GetPruningHeights(), curHeightStr)
-			batch := db.NewBatch()
-			manager.FlushPruningHeights(batch)
-			require.NoError(t, batch.Write())
-			require.NoError(t, batch.Close())
+		if manager.ShouldPruneAtHeight(curHeight) && curHeight > int64(keepRecent) {
+			actualHeights, err := manager.GetFlushAndResetPruningHeights()
+			require.NoError(t, err)
+			require.Equal(t, len(heightsToPruneMirror), len(actualHeights))
+			require.Equal(t, heightsToPruneMirror, actualHeights)
 
-			manager.ResetPruningHeights()
-			require.Equal(t, make([]int64, 0), manager.GetPruningHeights(), curHeightStr)
+			err = manager.LoadPruningHeights(db)
+			require.NoError(t, err)
 
-			err := manager.LoadPruningHeights(db)
+			actualHeights, err = manager.GetFlushAndResetPruningHeights()
 			require.NoError(t, err)
-			require.Equal(t, heightsToPruneMirror, manager.GetPruningHeights(), curHeightStr)
+			require.Equal(t, len(heightsToPruneMirror), len(actualHeights))
+			require.Equal(t, heightsToPruneMirror, actualHeights)
+
+			heightsToPruneMirror = make([]int64, 0)
 		}
 	}
 }
 
 func TestLoadPruningHeights(t *testing.T) {
 	var (
-		manager = pruning.NewManager(log.NewNopLogger())
+		manager = pruning.NewManager(db.NewMemDB(), log.NewNopLogger())
 		err     error
 	)
 	require.NotNil(t, manager)
@@ -323,86 +422,10 @@ func TestLoadPruningHeights(t *testing.T) {
 }
 
 func TestLoadPruningHeights_PruneNothing(t *testing.T) {
-	var manager = pruning.NewManager(log.NewNopLogger())
+	var manager = pruning.NewManager(db.NewMemDB(), log.NewNopLogger())
 	require.NotNil(t, manager)
 
 	manager.SetOptions(types.NewPruningOptions(types.PruningNothing))
 
 	require.Nil(t, manager.LoadPruningHeights(db.NewMemDB()))
 }
-
-func TestWithSnapshot(t *testing.T) {
-	manager := pruning.NewManager(log.NewNopLogger())
-	require.NotNil(t, manager)
-
-	curStrategy := types.NewCustomPruningOptions(10, 10)
-
-	snapshotInterval := uint64(15)
-	manager.SetSnapshotInterval(snapshotInterval)
-
-	manager.SetOptions(curStrategy)
-	require.Equal(t, curStrategy, manager.GetOptions())
-
-	keepRecent := curStrategy.KeepRecent
-
-	heightsToPruneMirror := make([]int64, 0)
-
-	mx := sync.Mutex{}
-	snapshotHeightsToPruneMirror := list.New()
-
-	wg := sync.WaitGroup{}
-	defer wg.Wait()
-
-	for curHeight := int64(1); curHeight < 100000; curHeight++ {
-		mx.Lock()
-		handleHeightActual := manager.HandleHeight(curHeight)
-
-		curHeightStr := fmt.Sprintf("height: %d", curHeight)
-
-		if curHeight > int64(keepRecent) && (curHeight-int64(keepRecent))%int64(snapshotInterval) != 0 {
-			expectedHandleHeight := curHeight - int64(keepRecent)
-			require.Equal(t, expectedHandleHeight, handleHeightActual, curHeightStr)
-			heightsToPruneMirror = append(heightsToPruneMirror, expectedHandleHeight)
-		} else {
-			require.Equal(t, int64(0), handleHeightActual, curHeightStr)
-		}
-
-		actualHeightsToPrune := manager.GetPruningHeights()
-
-		var next *list.Element
-		for e := snapshotHeightsToPruneMirror.Front(); e != nil; e = next {
-			snapshotHeight := e.Value.(int64)
-			if snapshotHeight < curHeight-int64(keepRecent) {
-				heightsToPruneMirror = append(heightsToPruneMirror, snapshotHeight)
-
-				// We must get next before removing to be able to continue iterating.
-				next = e.Next()
-				snapshotHeightsToPruneMirror.Remove(e)
-			} else {
-				next = e.Next()
-			}
-		}
-
-		require.Equal(t, heightsToPruneMirror, actualHeightsToPrune, curHeightStr)
-		mx.Unlock()
-
-		if manager.ShouldPruneAtHeight(curHeight) {
-			manager.ResetPruningHeights()
-			heightsToPruneMirror = make([]int64, 0)
-		}
-
-		// Mimic taking snapshots in the background
-		if curHeight%int64(snapshotInterval) == 0 {
-			wg.Add(1)
-			go func(curHeightCp int64) {
-				time.Sleep(time.Nanosecond * 500)
-
-				mx.Lock()
-				manager.HandleHeightSnapshot(curHeightCp)
-				snapshotHeightsToPruneMirror.PushBack(curHeightCp)
-				mx.Unlock()
-				wg.Done()
-			}(curHeight)
-		}
-	}
-}
diff --git a/store/rootmulti/store.go b/store/rootmulti/store.go
index 3ccc1cd9feca..8a5dadf349b4 100644
--- a/store/rootmulti/store.go
+++ b/store/rootmulti/store.go
@@ -79,7 +79,7 @@ func NewStore(db dbm.DB, logger log.Logger) *Store {
 		keysByName:     make(map[string]types.StoreKey),
 		listeners:      make(map[types.StoreKey][]types.WriteListener),
 		removalMap:     make(map[types.StoreKey]bool),
-		pruningManager: pruning.NewManager(logger),
+		pruningManager: pruning.NewManager(db, logger),
 	}
 }
 
@@ -550,13 +550,18 @@ func (rs *Store) handlePruning(version int64) error {
 }
 
 func (rs *Store) pruneStores() error {
-	pruningHeights := rs.pruningManager.GetPruningHeights()
-	rs.logger.Debug(fmt.Sprintf("pruning the following heights: %v\n", pruningHeights))
+	pruningHeights, err := rs.pruningManager.GetFlushAndResetPruningHeights()
+	if err != nil {
+		return err
+	}
 
 	if len(pruningHeights) == 0 {
+		rs.logger.Debug("pruning skipped, no heights to prune")
 		return nil
 	}
 
+	rs.logger.Debug(fmt.Sprintf("pruning the following heights: %v\n", pruningHeights))
+
 	for key, store := range rs.stores {
 		// If the store is wrapped with an inter-block cache, we must first unwrap
 		// it to get the underlying IAVL store.
@@ -575,7 +580,6 @@ func (rs *Store) pruneStores() error {
 			return err
 		}
 	}
-	rs.pruningManager.ResetPruningHeights()
 	return nil
 }
 
@@ -964,7 +968,7 @@ func (rs *Store) flushMetadata(db dbm.DB, version int64, cInfo *types.CommitInfo
 	}
 
 	flushLatestVersion(batch, version)
-	rs.pruningManager.FlushPruningHeights(batch)
+	// rs.pruningManager.FlushPruningHeights(batch)
 
 	if err := batch.WriteSync(); err != nil {
 		panic(fmt.Errorf("error on batch write %w", err))
diff --git a/store/rootmulti/store_test.go b/store/rootmulti/store_test.go
index 930361600dca..6881aa6067e5 100644
--- a/store/rootmulti/store_test.go
+++ b/store/rootmulti/store_test.go
@@ -525,18 +525,27 @@ func TestMultiStore_PruningRestart(t *testing.T) {
 	// ensure we've persisted the current batch of heights to prune to the store's DB
 	err := ms.pruningManager.LoadPruningHeights(ms.db)
 	require.NoError(t, err)
-	require.Equal(t, pruneHeights, ms.pruningManager.GetPruningHeights())
+
+	actualHeightsToPrune, err := ms.pruningManager.GetFlushAndResetPruningHeights()
+	require.NoError(t, err)
+	require.Equal(t, pruneHeights, len(actualHeightsToPrune))
 
 	// "restart"
 	ms = newMultiStoreWithMounts(db, pruningtypes.NewCustomPruningOptions(2, 11))
 	ms.SetSnapshotInterval(3)
 	err = ms.LoadLatestVersion()
 	require.NoError(t, err)
-	require.Equal(t, pruneHeights, ms.pruningManager.GetPruningHeights())
+
+	actualHeightsToPrune, err = ms.pruningManager.GetFlushAndResetPruningHeights()
+	require.NoError(t, err)
+	require.Equal(t, pruneHeights, len(actualHeightsToPrune))
 
 	// commit one more block and ensure the heights have been pruned
 	ms.Commit()
-	require.Empty(t, ms.pruningManager.GetPruningHeights())
+
+	actualHeightsToPrune, err = ms.pruningManager.GetFlushAndResetPruningHeights()
+	require.NoError(t, err)
+	require.Empty(t, actualHeightsToPrune)
 
 	for _, v := range pruneHeights {
 		_, err := ms.CacheMultiStoreWithVersion(v)

From 762b802f9cab39f589f2996586107c2d58461e64 Mon Sep 17 00:00:00 2001
From: Roman <ackhtariev@gmail.com>
Date: Tue, 12 Apr 2022 00:42:08 -0400
Subject: [PATCH 2/6] refactor pruning manager to have no data races; flush
 heights immediately when updated in memory; unit tests

---
 pruning/export_test.go        |  2 ++
 pruning/manager.go            | 61 +++++++++++++++++++++--------------
 pruning/manager_test.go       | 44 +++++++++++++++++++++++--
 store/rootmulti/store.go      |  1 -
 store/rootmulti/store_test.go | 57 +++++++++++++++++++++++++++++++-
 5 files changed, 136 insertions(+), 29 deletions(-)

diff --git a/pruning/export_test.go b/pruning/export_test.go
index 6a270b46eb82..d993200ce67a 100644
--- a/pruning/export_test.go
+++ b/pruning/export_test.go
@@ -9,4 +9,6 @@ var (
 	// functions
 	Int64SliceToBytes = int64SliceToBytes
 	ListToBytes       = listToBytes
+	LoadPruningHeights = loadPruningHeights
+	LoadPruningSnapshotHeights = loadPruningSnapshotHeights
 )
diff --git a/pruning/manager.go b/pruning/manager.go
index 7749cffdd363..33a5c8acb265 100644
--- a/pruning/manager.go
+++ b/pruning/manager.go
@@ -136,8 +136,12 @@ func (m *Manager) HandleHeight(previousHeight int64) int64 {
 	return 0
 }
 
+// HandleHeightSnapshot persists the snapshot height to be pruned at the next appropriate
+// height defined by the pruning strategy. Flushes the update to disk and panics if the flush fails
+// height must be greater than 0 and pruning strategy any but pruning nothing. If one of these conditions
+// is not met, this function does nothing.
 func (m *Manager) HandleHeightSnapshot(height int64) {
-	if m.opts.GetPruningStrategy() == types.PruningNothing {
+	if m.opts.GetPruningStrategy() == types.PruningNothing || height <= 0 {
 		return
 	}
 	m.pruneSnapshotHeightsMx.Lock()
@@ -166,19 +170,38 @@ func (m *Manager) LoadPruningHeights(db dbm.DB) error {
 	if m.opts.GetPruningStrategy() == types.PruningNothing {
 		return nil
 	}
-	if err := m.loadPruningHeights(db); err != nil {
+	loadedPruneHeights, err := loadPruningHeights(db)
+	if err != nil {
 		return err
 	}
-	return m.loadPruningSnapshotHeights(db)
+
+	if len(loadedPruneHeights) > 0 {
+		m.pruneHeightsMx.Lock()
+		defer m.pruneHeightsMx.Unlock()
+		m.pruneHeights = loadedPruneHeights
+	}
+
+	loadedPruneSnapshotHeights, err := loadPruningSnapshotHeights(db)
+	if err != nil {
+		return err
+	}
+
+	if loadedPruneSnapshotHeights.Len() > 0 {
+		m.pruneSnapshotHeightsMx.Lock()
+		defer m.pruneSnapshotHeightsMx.Unlock()
+		m.pruneSnapshotHeights = loadedPruneSnapshotHeights
+	}
+
+	return nil
 }
 
-func (m *Manager) loadPruningHeights(db dbm.DB) error {
+func loadPruningHeights(db dbm.DB) ([]int64, error) {
 	bz, err := db.Get(pruneHeightsKey)
 	if err != nil {
-		return fmt.Errorf("failed to get pruned heights: %w", err)
+		return []int64{}, fmt.Errorf("failed to get pruned heights: %w", err)
 	}
 	if len(bz) == 0 {
-		return nil
+		return []int64{}, nil
 	}
 
 	prunedHeights := make([]int64, len(bz)/8)
@@ -186,7 +209,7 @@ func (m *Manager) loadPruningHeights(db dbm.DB) error {
 	for offset < len(bz) {
 		h := int64(binary.BigEndian.Uint64(bz[offset : offset+8]))
 		if h < 0 {
-			return fmt.Errorf(errNegativeHeightsFmt, h)
+			return []int64{}, fmt.Errorf(errNegativeHeightsFmt, h)
 		}
 
 		prunedHeights[i] = h
@@ -194,30 +217,24 @@ func (m *Manager) loadPruningHeights(db dbm.DB) error {
 		offset += 8
 	}
 
-	if len(prunedHeights) > 0 {
-		m.pruneHeightsMx.Lock()
-		defer m.pruneHeightsMx.Unlock()
-		m.pruneHeights = prunedHeights
-	}
-
-	return nil
+	return prunedHeights, nil
 }
 
-func (m *Manager) loadPruningSnapshotHeights(db dbm.DB) error {
+func loadPruningSnapshotHeights(db dbm.DB) (*list.List, error) {
 	bz, err := db.Get(pruneSnapshotHeightsKey)
+	pruneSnapshotHeights := list.New()
 	if err != nil {
-		return fmt.Errorf("failed to get post-snapshot pruned heights: %w", err)
+		return pruneSnapshotHeights, fmt.Errorf("failed to get post-snapshot pruned heights: %w", err)
 	}
 	if len(bz) == 0 {
-		return nil
+		return pruneSnapshotHeights, nil
 	}
 
-	pruneSnapshotHeights := list.New()
 	i, offset := 0, 0
 	for offset < len(bz) {
 		h := int64(binary.BigEndian.Uint64(bz[offset : offset+8]))
 		if h < 0 {
-			return fmt.Errorf(errNegativeHeightsFmt, h)
+			return pruneSnapshotHeights, fmt.Errorf(errNegativeHeightsFmt, h)
 		}
 
 		pruneSnapshotHeights.PushBack(h)
@@ -225,11 +242,7 @@ func (m *Manager) loadPruningSnapshotHeights(db dbm.DB) error {
 		offset += 8
 	}
 
-	m.pruneSnapshotHeightsMx.Lock()
-	defer m.pruneSnapshotHeightsMx.Unlock()
-	m.pruneSnapshotHeights = pruneSnapshotHeights
-
-	return nil
+	return pruneSnapshotHeights, nil
 }
 
 func int64SliceToBytes(slice []int64) []byte {
diff --git a/pruning/manager_test.go b/pruning/manager_test.go
index 5634a9926d64..84303ce3ed85 100644
--- a/pruning/manager_test.go
+++ b/pruning/manager_test.go
@@ -206,7 +206,7 @@ func TestHandleHeight_Inputs(t *testing.T) {
 	}
 }
 
-func TestHandleHeight_FlushToDisk(t *testing.T) {
+func TestHandleHeight_FlushLoadFromDisk(t *testing.T) {
 	testcases := map[string]struct {
 		previousHeight int64
 		keepRecent uint64
@@ -286,11 +286,16 @@ func TestHandleHeight_FlushToDisk(t *testing.T) {
 				manager.HandleHeightSnapshot(snapshotHeight)
 			}
 
-			// Test
+			// Test HandleHeight and flush
 			handleHeightActual := manager.HandleHeight(tc.previousHeight)
 			require.Equal(t, tc.expectedHandleHeightResult, handleHeightActual)
 
-			err := manager.LoadPruningHeights(db)
+			loadedPruneHeights, err := pruning.LoadPruningHeights(db)
+			require.NoError(t, err)
+			require.Equal(t, len(loadedPruneHeights), len(loadedPruneHeights))
+
+			// Test load back
+			err = manager.LoadPruningHeights(db)
 			require.NoError(t, err)
 
 			heights, err := manager.GetFlushAndResetPruningHeights()
@@ -301,6 +306,39 @@ func TestHandleHeight_FlushToDisk(t *testing.T) {
 	}
 }
 
+func TestHandleHeightSnapshot_FlushLoadFromDisk(t *testing.T) {
+	loadedHeightsMirror := []int64{}
+
+	// Setup
+	db := db.NewMemDB()
+	manager := pruning.NewManager(db, log.NewNopLogger())
+	require.NotNil(t, manager)
+
+	manager.SetOptions(types.NewPruningOptions(types.PruningEverything))
+
+	for snapshotHeight := int64(-1); snapshotHeight < 100; snapshotHeight++ {
+		// Test flush
+		manager.HandleHeightSnapshot(snapshotHeight)
+		
+		// Post test
+		if snapshotHeight > 0 {
+			loadedHeightsMirror = append(loadedHeightsMirror, snapshotHeight)
+		}
+
+		loadedSnapshotHeights, err := pruning.LoadPruningSnapshotHeights(db)
+		require.NoError(t, err)
+		require.Equal(t, len(loadedHeightsMirror), loadedSnapshotHeights.Len())
+
+		// Test load back
+		err = manager.LoadPruningHeights(db)
+		require.NoError(t, err)
+
+		loadedSnapshotHeights, err = pruning.LoadPruningSnapshotHeights(db)
+		require.NoError(t, err)
+		require.Equal(t, len(loadedHeightsMirror), loadedSnapshotHeights.Len())
+	}
+}
+
 func TestFlushLoad(t *testing.T) {
 	db := db.NewMemDB()
 	manager := pruning.NewManager(db, log.NewNopLogger())
diff --git a/store/rootmulti/store.go b/store/rootmulti/store.go
index 8a5dadf349b4..0e4a1d6d11aa 100644
--- a/store/rootmulti/store.go
+++ b/store/rootmulti/store.go
@@ -968,7 +968,6 @@ func (rs *Store) flushMetadata(db dbm.DB, version int64, cInfo *types.CommitInfo
 	}
 
 	flushLatestVersion(batch, version)
-	// rs.pruningManager.FlushPruningHeights(batch)
 
 	if err := batch.WriteSync(); err != nil {
 		panic(fmt.Errorf("error on batch write %w", err))
diff --git a/store/rootmulti/store_test.go b/store/rootmulti/store_test.go
index 6881aa6067e5..f97b9263f81e 100644
--- a/store/rootmulti/store_test.go
+++ b/store/rootmulti/store_test.go
@@ -508,6 +508,60 @@ func TestMultiStore_Pruning(t *testing.T) {
 	}
 }
 
+func TestMultiStore_Pruning_SameHeightsTwice(t *testing.T) {
+	const (
+		numVersions int64 = 10
+		keepRecent uint64 = 2
+		interval uint64 = 10
+	)
+
+	expectedHeights := []int64{}
+	for i := int64(1); i < numVersions - int64(keepRecent); i++ {
+		expectedHeights = append(expectedHeights, i)
+	}
+
+	db := dbm.NewMemDB()
+
+	ms := newMultiStoreWithMounts(db, pruningtypes.NewCustomPruningOptions(keepRecent, interval))
+	require.NoError(t, ms.LoadLatestVersion())
+
+	var lastCommitInfo types.CommitID
+	for i := int64(0); i < numVersions; i++ {
+		lastCommitInfo = ms.Commit()
+	}
+
+	require.Equal(t, numVersions, lastCommitInfo.Version)
+
+	for v := int64(1); v < numVersions - int64(keepRecent); v++ {
+		err := ms.LoadVersion(v)
+		require.Error(t, err, "expected error when loading pruned height: %d", v)
+	}
+
+	for v := int64(numVersions - int64(keepRecent)); v < numVersions; v++ {
+		err := ms.LoadVersion(v)
+		require.NoError(t, err, "expected no error when loading height: %d", v)
+	}
+
+	// Get latest
+	err := ms.LoadVersion(numVersions - 1)
+	require.NoError(t, err)
+
+	// Ensure already pruned heights were loaded
+	heights, err := ms.pruningManager.GetFlushAndResetPruningHeights()
+	require.NoError(t, err)
+	require.Equal(t, expectedHeights, heights)
+
+	require.NoError(t, ms.pruningManager.LoadPruningHeights(db))
+
+	// Test pruning the same heights again
+	lastCommitInfo = ms.Commit()
+	require.Equal(t, numVersions, lastCommitInfo.Version)
+
+	// Ensure that can commit one more height with no panic
+	lastCommitInfo = ms.Commit()
+	require.Equal(t, numVersions + 1, lastCommitInfo.Version)
+}
+
 func TestMultiStore_PruningRestart(t *testing.T) {
 	db := dbm.NewMemDB()
 	ms := newMultiStoreWithMounts(db, pruningtypes.NewCustomPruningOptions(2, 11))
@@ -528,7 +582,8 @@ func TestMultiStore_PruningRestart(t *testing.T) {
 
 	actualHeightsToPrune, err := ms.pruningManager.GetFlushAndResetPruningHeights()
 	require.NoError(t, err)
-	require.Equal(t, pruneHeights, len(actualHeightsToPrune))
+	require.Equal(t, len(pruneHeights), len(actualHeightsToPrune))
+	require.Equal(t, pruneHeights, actualHeightsToPrune)
 
 	// "restart"
 	ms = newMultiStoreWithMounts(db, pruningtypes.NewCustomPruningOptions(2, 11))

From 1339d36c47e8bc82e9395ef81b487c931a875caf Mon Sep 17 00:00:00 2001
From: Roman <ackhtariev@gmail.com>
Date: Tue, 12 Apr 2022 00:55:14 -0400
Subject: [PATCH 3/6] typo in TestMultiStore_PruningRestart

---
 store/rootmulti/store_test.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/store/rootmulti/store_test.go b/store/rootmulti/store_test.go
index f97b9263f81e..fc4c266bbe2b 100644
--- a/store/rootmulti/store_test.go
+++ b/store/rootmulti/store_test.go
@@ -593,7 +593,7 @@ func TestMultiStore_PruningRestart(t *testing.T) {
 
 	actualHeightsToPrune, err = ms.pruningManager.GetFlushAndResetPruningHeights()
 	require.NoError(t, err)
-	require.Equal(t, pruneHeights, len(actualHeightsToPrune))
+	require.Equal(t, pruneHeights, actualHeightsToPrune)
 
 	// commit one more block and ensure the heights have been pruned
 	ms.Commit()

From 05fedfcd6b2b9e8860f70b7ce0aa79e22ee76a88 Mon Sep 17 00:00:00 2001
From: Roman <ackhtariev@gmail.com>
Date: Tue, 12 Apr 2022 00:57:00 -0400
Subject: [PATCH 4/6] fmt

---
 pruning/export_test.go        |  6 +--
 pruning/manager.go            | 32 ++++++------
 pruning/manager_test.go       | 92 +++++++++++++++++------------------
 store/rootmulti/store_test.go | 12 ++---
 4 files changed, 71 insertions(+), 71 deletions(-)

diff --git a/pruning/export_test.go b/pruning/export_test.go
index d993200ce67a..51b1c10462a9 100644
--- a/pruning/export_test.go
+++ b/pruning/export_test.go
@@ -7,8 +7,8 @@ var (
 	PruneSnapshotHeightsKey = pruneSnapshotHeightsKey
 
 	// functions
-	Int64SliceToBytes = int64SliceToBytes
-	ListToBytes       = listToBytes
-	LoadPruningHeights = loadPruningHeights
+	Int64SliceToBytes          = int64SliceToBytes
+	ListToBytes                = listToBytes
+	LoadPruningHeights         = loadPruningHeights
 	LoadPruningSnapshotHeights = loadPruningSnapshotHeights
 )
diff --git a/pruning/manager.go b/pruning/manager.go
index 33a5c8acb265..9c717bbaceb3 100644
--- a/pruning/manager.go
+++ b/pruning/manager.go
@@ -13,14 +13,14 @@ import (
 )
 
 type Manager struct {
-	db             dbm.DB
-	logger               log.Logger
-	opts                 types.PruningOptions
-	snapshotInterval     uint64
-	pruneHeights         []int64
-	pruneHeightsMx	   sync.Mutex
-	pruneSnapshotHeights *list.List
-	pruneSnapshotHeightsMx                   sync.Mutex
+	db                     dbm.DB
+	logger                 log.Logger
+	opts                   types.PruningOptions
+	snapshotInterval       uint64
+	pruneHeights           []int64
+	pruneHeightsMx         sync.Mutex
+	pruneSnapshotHeights   *list.List
+	pruneSnapshotHeightsMx sync.Mutex
 }
 
 const errNegativeHeightsFmt = "failed to get pruned heights: %d"
@@ -32,15 +32,15 @@ var (
 
 func NewManager(db dbm.DB, logger log.Logger) *Manager {
 	return &Manager{
-		db: db,
-		logger:       logger,
-		opts:         types.NewPruningOptions(types.PruningNothing),
-		pruneHeights: []int64{},
+		db:             db,
+		logger:         logger,
+		opts:           types.NewPruningOptions(types.PruningNothing),
+		pruneHeights:   []int64{},
 		pruneHeightsMx: sync.Mutex{},
 		// These are the heights that are multiples of snapshotInterval and kept for state sync snapshots.
 		// The heights are added to this list to be pruned when a snapshot is complete.
-		pruneSnapshotHeights: list.New(),
-		pruneSnapshotHeightsMx:                   sync.Mutex{},
+		pruneSnapshotHeights:   list.New(),
+		pruneSnapshotHeightsMx: sync.Mutex{},
 	}
 }
 
@@ -64,14 +64,14 @@ func (m *Manager) GetFlushAndResetPruningHeights() ([]int64, error) {
 	defer m.pruneHeightsMx.Unlock()
 
 	pruningHeights := m.pruneHeights
-	
+
 	// flush the update to disk so that it is not lost if crash happens.
 	if err := m.db.SetSync(pruneHeightsKey, int64SliceToBytes(pruningHeights)); err != nil {
 		return nil, err
 	}
 
 	m.pruneHeights = make([]int64, 0, m.opts.Interval)
-	
+
 	return pruningHeights, nil
 }
 
diff --git a/pruning/manager_test.go b/pruning/manager_test.go
index 84303ce3ed85..f00171c673f4 100644
--- a/pruning/manager_test.go
+++ b/pruning/manager_test.go
@@ -152,9 +152,9 @@ func TestHandleHeight_Inputs(t *testing.T) {
 	var keepRecent int64 = int64(types.NewPruningOptions(types.PruningEverything).KeepRecent)
 
 	testcases := map[string]struct {
-		height int64
-		expectedResult int64
-		strategy types.PruningStrategy
+		height          int64
+		expectedResult  int64
+		strategy        types.PruningStrategy
 		expectedHeights []int64
 	}{
 		"previousHeight is negative - prune everything - invalid previousHeight": {
@@ -208,67 +208,67 @@ func TestHandleHeight_Inputs(t *testing.T) {
 
 func TestHandleHeight_FlushLoadFromDisk(t *testing.T) {
 	testcases := map[string]struct {
-		previousHeight int64
-		keepRecent uint64
-		snapshotInterval uint64
-		movedSnapshotHeights []int64
-		expectedHandleHeightResult int64
+		previousHeight                   int64
+		keepRecent                       uint64
+		snapshotInterval                 uint64
+		movedSnapshotHeights             []int64
+		expectedHandleHeightResult       int64
 		expectedLoadPruningHeightsResult error
-		expectedLoadedHeights []int64
+		expectedLoadedHeights            []int64
 	}{
 		"simple flush occurs": {
-			previousHeight: 11,
-			keepRecent: 10,
-			snapshotInterval: 0,
-			movedSnapshotHeights: []int64{},
-			expectedHandleHeightResult: 11 - 10,
+			previousHeight:                   11,
+			keepRecent:                       10,
+			snapshotInterval:                 0,
+			movedSnapshotHeights:             []int64{},
+			expectedHandleHeightResult:       11 - 10,
 			expectedLoadPruningHeightsResult: nil,
-			expectedLoadedHeights: []int64{11 - 10},
+			expectedLoadedHeights:            []int64{11 - 10},
 		},
 		"previous height <= keep recent - no update and no flush": {
-			previousHeight: 9,
-			keepRecent: 10,
-			snapshotInterval: 0,
-			movedSnapshotHeights: []int64{},
-			expectedHandleHeightResult: 0,
+			previousHeight:                   9,
+			keepRecent:                       10,
+			snapshotInterval:                 0,
+			movedSnapshotHeights:             []int64{},
+			expectedHandleHeightResult:       0,
 			expectedLoadPruningHeightsResult: nil,
-			expectedLoadedHeights: []int64{},
+			expectedLoadedHeights:            []int64{},
 		},
 		"previous height alligns with snapshot interval - no update and no flush": {
-			previousHeight: 12,
-			keepRecent: 10,
-			snapshotInterval: 2,
-			movedSnapshotHeights: []int64{},
-			expectedHandleHeightResult: 0,
+			previousHeight:                   12,
+			keepRecent:                       10,
+			snapshotInterval:                 2,
+			movedSnapshotHeights:             []int64{},
+			expectedHandleHeightResult:       0,
 			expectedLoadPruningHeightsResult: nil,
-			expectedLoadedHeights: []int64{},
+			expectedLoadedHeights:            []int64{},
 		},
 		"previous height does not align with snapshot interval - flush": {
-			previousHeight: 12,
-			keepRecent: 10,
-			snapshotInterval: 3,
-			movedSnapshotHeights: []int64{},
-			expectedHandleHeightResult: 2,
+			previousHeight:                   12,
+			keepRecent:                       10,
+			snapshotInterval:                 3,
+			movedSnapshotHeights:             []int64{},
+			expectedHandleHeightResult:       2,
 			expectedLoadPruningHeightsResult: nil,
-			expectedLoadedHeights: []int64{2},
+			expectedLoadedHeights:            []int64{2},
 		},
 		"moved snapshot heights - flushed": {
-			previousHeight: 32,
-			keepRecent: 10,
-			snapshotInterval: 5,
-			movedSnapshotHeights: []int64{15, 20, 25},
-			expectedHandleHeightResult: 22,
+			previousHeight:                   32,
+			keepRecent:                       10,
+			snapshotInterval:                 5,
+			movedSnapshotHeights:             []int64{15, 20, 25},
+			expectedHandleHeightResult:       22,
 			expectedLoadPruningHeightsResult: nil,
-			expectedLoadedHeights: []int64{15, 20, 22},
+			expectedLoadedHeights:            []int64{15, 20, 22},
 		},
 		"previous height alligns with snapshot interval - no update but flush snapshot heights": {
-			previousHeight: 30,
-			keepRecent: 10,
-			snapshotInterval: 5,
-			movedSnapshotHeights: []int64{15, 20, 25},
-			expectedHandleHeightResult: 0,
+			previousHeight:                   30,
+			keepRecent:                       10,
+			snapshotInterval:                 5,
+			movedSnapshotHeights:             []int64{15, 20, 25},
+			expectedHandleHeightResult:       0,
 			expectedLoadPruningHeightsResult: nil,
-			expectedLoadedHeights: []int64{15},
+			expectedLoadedHeights:            []int64{15},
 		},
 	}
 
@@ -319,7 +319,7 @@ func TestHandleHeightSnapshot_FlushLoadFromDisk(t *testing.T) {
 	for snapshotHeight := int64(-1); snapshotHeight < 100; snapshotHeight++ {
 		// Test flush
 		manager.HandleHeightSnapshot(snapshotHeight)
-		
+
 		// Post test
 		if snapshotHeight > 0 {
 			loadedHeightsMirror = append(loadedHeightsMirror, snapshotHeight)
diff --git a/store/rootmulti/store_test.go b/store/rootmulti/store_test.go
index fc4c266bbe2b..f2b1bc82ed0a 100644
--- a/store/rootmulti/store_test.go
+++ b/store/rootmulti/store_test.go
@@ -510,13 +510,13 @@ func TestMultiStore_Pruning(t *testing.T) {
 
 func TestMultiStore_Pruning_SameHeightsTwice(t *testing.T) {
 	const (
-		numVersions int64 = 10
-		keepRecent uint64 = 2
-		interval uint64 = 10
+		numVersions int64  = 10
+		keepRecent  uint64 = 2
+		interval    uint64 = 10
 	)
 
 	expectedHeights := []int64{}
-	for i := int64(1); i < numVersions - int64(keepRecent); i++ {
+	for i := int64(1); i < numVersions-int64(keepRecent); i++ {
 		expectedHeights = append(expectedHeights, i)
 	}
 
@@ -532,7 +532,7 @@ func TestMultiStore_Pruning_SameHeightsTwice(t *testing.T) {
 
 	require.Equal(t, numVersions, lastCommitInfo.Version)
 
-	for v := int64(1); v < numVersions - int64(keepRecent); v++ {
+	for v := int64(1); v < numVersions-int64(keepRecent); v++ {
 		err := ms.LoadVersion(v)
 		require.Error(t, err, "expected error when loading pruned height: %d", v)
 	}
@@ -559,7 +559,7 @@ func TestMultiStore_Pruning_SameHeightsTwice(t *testing.T) {
 
 	// Ensure that can commit one more height with no panic
 	lastCommitInfo = ms.Commit()
-	require.Equal(t, numVersions + 1, lastCommitInfo.Version)
+	require.Equal(t, numVersions+1, lastCommitInfo.Version)
 }
 
 func TestMultiStore_PruningRestart(t *testing.T) {

From ff87a2379bdc957087151eace4162aab76f43504 Mon Sep 17 00:00:00 2001
From: Roman <ackhtariev@gmail.com>
Date: Tue, 12 Apr 2022 01:06:28 -0400
Subject: [PATCH 5/6] improve comments

---
 pruning/manager.go | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/pruning/manager.go b/pruning/manager.go
index 9c717bbaceb3..cb33a31c4edf 100644
--- a/pruning/manager.go
+++ b/pruning/manager.go
@@ -36,10 +36,15 @@ func NewManager(db dbm.DB, logger log.Logger) *Manager {
 		logger:         logger,
 		opts:           types.NewPruningOptions(types.PruningNothing),
 		pruneHeights:   []int64{},
+		// Although pruneHeights happen in the same goroutine with the normal execution,
+		// we sync access to them to avoid soundness issues in the future if concurrency pattern changes.
 		pruneHeightsMx: sync.Mutex{},
 		// These are the heights that are multiples of snapshotInterval and kept for state sync snapshots.
 		// The heights are added to this list to be pruned when a snapshot is complete.
 		pruneSnapshotHeights:   list.New(),
+		// Snapshots are taken in a separate goroutine fromt the regular execution
+		// and can be delivered asynchrounously via HandleHeightSnapshot.
+		// Therefore, we sync access to pruneSnapshotHeights with this mutex. 
 		pruneSnapshotHeightsMx: sync.Mutex{},
 	}
 }
@@ -65,7 +70,7 @@ func (m *Manager) GetFlushAndResetPruningHeights() ([]int64, error) {
 
 	pruningHeights := m.pruneHeights
 
-	// flush the update to disk so that it is not lost if crash happens.
+	// flush the updates to disk so that it is not lost if crash happens.
 	if err := m.db.SetSync(pruneHeightsKey, int64SliceToBytes(pruningHeights)); err != nil {
 		return nil, err
 	}
@@ -107,7 +112,7 @@ func (m *Manager) HandleHeight(previousHeight int64) int64 {
 			}
 		}
 
-		// flush the update to disk so that they are not lost if crash happens.
+		// flush the updates to disk so that they are not lost if crash happens.
 		if err := m.db.SetSync(pruneHeightsKey, int64SliceToBytes(m.pruneHeights)); err != nil {
 			panic(err)
 		}
@@ -126,7 +131,7 @@ func (m *Manager) HandleHeight(previousHeight int64) int64 {
 
 			m.pruneHeights = append(m.pruneHeights, pruneHeight)
 
-			// flush the update to disk so that they are not lost if crash happens.
+			// flush the updates to disk so that they are not lost if crash happens.
 			if err := m.db.SetSync(pruneHeightsKey, int64SliceToBytes(m.pruneHeights)); err != nil {
 				panic(err)
 			}
@@ -138,8 +143,8 @@ func (m *Manager) HandleHeight(previousHeight int64) int64 {
 
 // HandleHeightSnapshot persists the snapshot height to be pruned at the next appropriate
 // height defined by the pruning strategy. Flushes the update to disk and panics if the flush fails
-// height must be greater than 0 and pruning strategy any but pruning nothing. If one of these conditions
-// is not met, this function does nothing.
+// The input height must be greater than 0 and pruning strategy any but pruning nothing. 
+// If one of these conditions is not met, this function does nothing.
 func (m *Manager) HandleHeightSnapshot(height int64) {
 	if m.opts.GetPruningStrategy() == types.PruningNothing || height <= 0 {
 		return
@@ -149,7 +154,7 @@ func (m *Manager) HandleHeightSnapshot(height int64) {
 	m.logger.Debug("HandleHeightSnapshot", "height", height)
 	m.pruneSnapshotHeights.PushBack(height)
 
-	// flush the update to disk so that they are not lost if crash happens.
+	// flush the updates to disk so that they are not lost if crash happens.
 	if err := m.db.SetSync(pruneSnapshotHeightsKey, listToBytes(m.pruneSnapshotHeights)); err != nil {
 		panic(err)
 	}

From dbbde10fa326790b41b6ab80c0076eae8228d0ff Mon Sep 17 00:00:00 2001
From: Roman <ackhtariev@gmail.com>
Date: Tue, 12 Apr 2022 10:04:25 -0400
Subject: [PATCH 6/6] avoid mutex init, move comments to struct declaration,
 return nil with error, fix logs

---
 pruning/manager.go       | 20 +++++++++-----------
 store/rootmulti/store.go |  4 ++--
 2 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/pruning/manager.go b/pruning/manager.go
index cb33a31c4edf..cd13365f1962 100644
--- a/pruning/manager.go
+++ b/pruning/manager.go
@@ -18,8 +18,15 @@ type Manager struct {
 	opts                   types.PruningOptions
 	snapshotInterval       uint64
 	pruneHeights           []int64
+	// Although pruneHeights happen in the same goroutine with the normal execution,
+	// we sync access to them to avoid soundness issues in the future if concurrency pattern changes.
 	pruneHeightsMx         sync.Mutex
+	// These are the heights that are multiples of snapshotInterval and kept for state sync snapshots.
+	// The heights are added to this list to be pruned when a snapshot is complete.
 	pruneSnapshotHeights   *list.List
+	// Snapshots are taken in a separate goroutine fromt the regular execution
+	// and can be delivered asynchrounously via HandleHeightSnapshot.
+	// Therefore, we sync access to pruneSnapshotHeights with this mutex. 
 	pruneSnapshotHeightsMx sync.Mutex
 }
 
@@ -36,16 +43,7 @@ func NewManager(db dbm.DB, logger log.Logger) *Manager {
 		logger:         logger,
 		opts:           types.NewPruningOptions(types.PruningNothing),
 		pruneHeights:   []int64{},
-		// Although pruneHeights happen in the same goroutine with the normal execution,
-		// we sync access to them to avoid soundness issues in the future if concurrency pattern changes.
-		pruneHeightsMx: sync.Mutex{},
-		// These are the heights that are multiples of snapshotInterval and kept for state sync snapshots.
-		// The heights are added to this list to be pruned when a snapshot is complete.
 		pruneSnapshotHeights:   list.New(),
-		// Snapshots are taken in a separate goroutine fromt the regular execution
-		// and can be delivered asynchrounously via HandleHeightSnapshot.
-		// Therefore, we sync access to pruneSnapshotHeights with this mutex. 
-		pruneSnapshotHeightsMx: sync.Mutex{},
 	}
 }
 
@@ -203,7 +201,7 @@ func (m *Manager) LoadPruningHeights(db dbm.DB) error {
 func loadPruningHeights(db dbm.DB) ([]int64, error) {
 	bz, err := db.Get(pruneHeightsKey)
 	if err != nil {
-		return []int64{}, fmt.Errorf("failed to get pruned heights: %w", err)
+		return nil, fmt.Errorf("failed to get pruned heights: %w", err)
 	}
 	if len(bz) == 0 {
 		return []int64{}, nil
@@ -229,7 +227,7 @@ func loadPruningSnapshotHeights(db dbm.DB) (*list.List, error) {
 	bz, err := db.Get(pruneSnapshotHeightsKey)
 	pruneSnapshotHeights := list.New()
 	if err != nil {
-		return pruneSnapshotHeights, fmt.Errorf("failed to get post-snapshot pruned heights: %w", err)
+		return nil, fmt.Errorf("failed to get post-snapshot pruned heights: %w", err)
 	}
 	if len(bz) == 0 {
 		return pruneSnapshotHeights, nil
diff --git a/store/rootmulti/store.go b/store/rootmulti/store.go
index 0e4a1d6d11aa..9b6c60504f82 100644
--- a/store/rootmulti/store.go
+++ b/store/rootmulti/store.go
@@ -556,11 +556,11 @@ func (rs *Store) pruneStores() error {
 	}
 
 	if len(pruningHeights) == 0 {
-		rs.logger.Debug("pruning skipped, no heights to prune")
+		rs.logger.Debug("pruning skipped; no heights to prune")
 		return nil
 	}
 
-	rs.logger.Debug(fmt.Sprintf("pruning the following heights: %v\n", pruningHeights))
+	rs.logger.Debug("pruning heights", "heights", pruningHeights)
 
 	for key, store := range rs.stores {
 		// If the store is wrapped with an inter-block cache, we must first unwrap