refactor: snapshot manager is created independently from snapshot-int… (#236) (#255)

(cherry picked from commit 0041954)

Co-authored-by: Roman <roman@osmosis.team>

Author: mergify[bot]

baseapp/abci.go

@@ -691,12 +691,12 @@ func (app *BaseApp) GetBlockRetentionHeight(commitHeight int64) int64 {
 	}
 
 	if app.snapshotManager != nil {
+		snapshotInterval := int64(app.snapshotManager.GetInterval())
 		// Define the state pruning offset, i.e. the block offset at which the
 		// underlying logical database is persisted to disk.
-		statePruningOffset := int64(app.snapshotManager.GetInterval())
-		if statePruningOffset > 0 {
-			if commitHeight > statePruningOffset {
-				v := commitHeight - (commitHeight % statePruningOffset)
+		if snapshotInterval > 0 {
+			if commitHeight > snapshotInterval {
+				v := commitHeight - (commitHeight % snapshotInterval)
 				retentionHeight = minNonZero(retentionHeight, v)
 			} else {
 				// Hitting this case means we have persisting enabled but have yet to reach
@@ -705,11 +705,10 @@ func (app *BaseApp) GetBlockRetentionHeight(commitHeight int64) int64 {
 				// any state committed to disk.
 				return 0
 			}
-		}
-
-		snapshotRetentionHeights := app.snapshotManager.GetSnapshotBlockRetentionHeights()
-		if snapshotRetentionHeights > 0 {
-			retentionHeight = minNonZero(retentionHeight, commitHeight-snapshotRetentionHeights)
+			snapshotRetentionHeights := app.snapshotManager.GetSnapshotBlockRetentionHeights()
+			if snapshotRetentionHeights > 0 {
+				retentionHeight = minNonZero(retentionHeight, commitHeight-snapshotRetentionHeights)
+			}
 		}
 	}
 

baseapp/baseapp_test.go

@@ -2265,10 +2265,11 @@ func TestBaseApp_Init_PruningAndSnapshot(t *testing.T) {
 	require.NoError(t, err)
 
 	testCases := map[string]struct {
-		bapp             *BaseApp
-		expectedPruning  pruningtypes.PruningOptions
-		expectedSnapshot snapshottypes.SnapshotOptions
-		expectedErr      error
+		bapp                 *BaseApp
+		expectedPruning      pruningtypes.PruningOptions
+		expectedSnapshot     snapshottypes.SnapshotOptions
+		expectedErr          error
+		isSnapshotManagerNil bool
 	}{
 		"snapshot but no pruning": {
 			NewBaseApp(name, logger, db, nil,
@@ -2278,14 +2279,26 @@ func TestBaseApp_Init_PruningAndSnapshot(t *testing.T) {
 			snapshottypes.NewSnapshotOptions(1500, 2),
 			// if no pruning is set, the default is PruneNothing
 			nil,
+			false,
+		},
+		"nil snapshot store": {
+			NewBaseApp(name, logger, db, nil,
+				SetPruning(pruningtypes.NewPruningOptions(pruningtypes.PruningNothing)),
+				SetSnapshot(nil, snapshottypes.NewSnapshotOptions(1500, 2)),
+			),
+			pruningtypes.NewPruningOptions(pruningtypes.PruningNothing),
+			snapshottypes.SnapshotOptions{},
+			nil,
+			true,
 		},
 		"pruning everything only": {
 			NewBaseApp(name, logger, db, nil,
 				SetPruning(pruningtypes.NewPruningOptions(pruningtypes.PruningEverything)),
 			),
 			pruningtypes.NewPruningOptions(pruningtypes.PruningEverything),
-			snapshottypes.NewSnapshotOptions(snapshottypes.SnapshotIntervalOff, 0),
+			snapshottypes.SnapshotOptions{},
 			nil,
+			true,
 		},
 		"pruning nothing only": {
 			NewBaseApp(name, logger, db, nil,
@@ -2294,6 +2307,7 @@ func TestBaseApp_Init_PruningAndSnapshot(t *testing.T) {
 			pruningtypes.NewPruningOptions(pruningtypes.PruningNothing),
 			snapshottypes.NewSnapshotOptions(snapshottypes.SnapshotIntervalOff, 0),
 			nil,
+			true,
 		},
 		"pruning default only": {
 			NewBaseApp(name, logger, db, nil,
@@ -2302,6 +2316,7 @@ func TestBaseApp_Init_PruningAndSnapshot(t *testing.T) {
 			pruningtypes.NewPruningOptions(pruningtypes.PruningDefault),
 			snapshottypes.NewSnapshotOptions(snapshottypes.SnapshotIntervalOff, 0),
 			nil,
+			true,
 		},
 		"pruning custom only": {
 			NewBaseApp(name, logger, db, nil,
@@ -2310,6 +2325,7 @@ func TestBaseApp_Init_PruningAndSnapshot(t *testing.T) {
 			pruningtypes.NewCustomPruningOptions(10, 10),
 			snapshottypes.NewSnapshotOptions(snapshottypes.SnapshotIntervalOff, 0),
 			nil,
+			true,
 		},
 		"pruning everything and snapshots": {
 			NewBaseApp(name, logger, db, nil,
@@ -2319,6 +2335,7 @@ func TestBaseApp_Init_PruningAndSnapshot(t *testing.T) {
 			pruningtypes.NewPruningOptions(pruningtypes.PruningEverything),
 			snapshottypes.NewSnapshotOptions(1500, 2),
 			nil,
+			false,
 		},
 		"pruning nothing and snapshots": {
 			NewBaseApp(name, logger, db, nil,
@@ -2328,6 +2345,7 @@ func TestBaseApp_Init_PruningAndSnapshot(t *testing.T) {
 			pruningtypes.NewPruningOptions(pruningtypes.PruningNothing),
 			snapshottypes.NewSnapshotOptions(1500, 2),
 			nil,
+			false,
 		},
 		"pruning default and snapshots": {
 			NewBaseApp(name, logger, db, nil,
@@ -2337,6 +2355,7 @@ func TestBaseApp_Init_PruningAndSnapshot(t *testing.T) {
 			pruningtypes.NewPruningOptions(pruningtypes.PruningDefault),
 			snapshottypes.NewSnapshotOptions(1500, 2),
 			nil,
+			false,
 		},
 		"pruning custom and snapshots": {
 			NewBaseApp(name, logger, db, nil,
@@ -2346,6 +2365,7 @@ func TestBaseApp_Init_PruningAndSnapshot(t *testing.T) {
 			pruningtypes.NewCustomPruningOptions(10, 10),
 			snapshottypes.NewSnapshotOptions(1500, 2),
 			nil,
+			false,
 		},
 		"error custom pruning 0 interval": {
 			NewBaseApp(name, logger, db, nil,
@@ -2355,6 +2375,7 @@ func TestBaseApp_Init_PruningAndSnapshot(t *testing.T) {
 			pruningtypes.NewCustomPruningOptions(10, 0),
 			snapshottypes.NewSnapshotOptions(1500, 2),
 			pruningtypes.ErrPruningIntervalZero,
+			false,
 		},
 		"error custom pruning too small interval": {
 			NewBaseApp(name, logger, db, nil,
@@ -2364,6 +2385,7 @@ func TestBaseApp_Init_PruningAndSnapshot(t *testing.T) {
 			pruningtypes.NewCustomPruningOptions(10, 9),
 			snapshottypes.NewSnapshotOptions(1500, 2),
 			pruningtypes.ErrPruningIntervalTooSmall,
+			false,
 		},
 		"error custom pruning too small keep recent": {
 			NewBaseApp(name, logger, db, nil,
@@ -2373,15 +2395,17 @@ func TestBaseApp_Init_PruningAndSnapshot(t *testing.T) {
 			pruningtypes.NewCustomPruningOptions(9, 10),
 			snapshottypes.NewSnapshotOptions(1500, 2),
 			pruningtypes.ErrPruningKeepRecentTooSmall,
+			false,
 		},
-		"snapshot zero interval - manager not set": {
+		"snapshot zero interval - manager is set": {
 			NewBaseApp(name, logger, db, nil,
 				SetPruning(pruningtypes.NewCustomPruningOptions(10, 10)),
 				SetSnapshot(snapshotStore, snapshottypes.NewSnapshotOptions(0, 2)),
 			),
 			pruningtypes.NewCustomPruningOptions(10, 10),
-			snapshottypes.NewSnapshotOptions(snapshottypes.SnapshotIntervalOff, 0),
+			snapshottypes.NewSnapshotOptions(snapshottypes.SnapshotIntervalOff, 2),
 			nil,
+			false,
 		},
 		"snapshot zero keep recent - allowed": {
 			NewBaseApp(name, logger, db, nil,
@@ -2391,6 +2415,7 @@ func TestBaseApp_Init_PruningAndSnapshot(t *testing.T) {
 			pruningtypes.NewCustomPruningOptions(10, 10),
 			snapshottypes.NewSnapshotOptions(1500, 0), // 0 snapshot-keep-recent means keep all
 			nil,
+			false,
 		},
 	}
 
@@ -2407,7 +2432,7 @@ func TestBaseApp_Init_PruningAndSnapshot(t *testing.T) {
 		actualPruning := tc.bapp.cms.GetPruning()
 		require.Equal(t, tc.expectedPruning, actualPruning)
 
-		if tc.expectedSnapshot.Interval == snapshottypes.SnapshotIntervalOff {
+		if tc.isSnapshotManagerNil {
 			require.Nil(t, tc.bapp.snapshotManager)
 			continue
 		}

baseapp/options.go

@@ -234,7 +234,7 @@ func (app *BaseApp) SetSnapshot(snapshotStore *snapshots.Store, opts snapshottyp
 	if app.sealed {
 		panic("SetSnapshot() on sealed BaseApp")
 	}
-	if snapshotStore == nil || opts.Interval == snapshottypes.SnapshotIntervalOff {
+	if snapshotStore == nil {
 		app.snapshotManager = nil
 		return
 	}