FI from master

3 files changed, 80 insertions, 2 deletions

diff --git a/config/version.go b/config/version.go
index 272cff9f9..dce2bd533 100644
--- a/config/version.go
+++ b/config/version.go
@@ -33,7 +33,7 @@ const VersionMajor = 3
 
 // VersionMinor is the Minor semantic version number (x.#.z) - changed when backwards-compatible features are introduced.
 // Not enforced until after initial public release (x > 0).
-const VersionMinor = 22
+const VersionMinor = 23
 
 // Version is the type holding our full version information.
 type Version struct {
diff --git a/ledger/ledger.go b/ledger/ledger.go
index fa1be1a76..110a72fc1 100644
--- a/ledger/ledger.go
+++ b/ledger/ledger.go
@@ -462,11 +462,25 @@ func (l *Ledger) notifyCommit(r basics.Round) basics.Round {
 	if l.archival {
 		// Do not forget any blocks.
 		minToSave = 0
+	} else {
+		catchpointsMinToSave := r.SubSaturate(l.calcMinCatchpointRoundsLookback())
+		if catchpointsMinToSave < minToSave {
+			minToSave = catchpointsMinToSave
+		}
 	}
 
 	return minToSave
 }
 
+func (l *Ledger) calcMinCatchpointRoundsLookback() basics.Round {
+	// cfg.StoresCatchpoints checks that CatchpointInterval is positive
+	if !l.cfg.StoresCatchpoints() || l.cfg.CatchpointFileHistoryLength == 0 {
+		return 0
+	}
+
+	return basics.Round(2 * l.cfg.CatchpointInterval)
+}
+
 // GetLastCatchpointLabel returns the latest catchpoint label that was written to the
 // database.
 func (l *Ledger) GetLastCatchpointLabel() string {
@@ -901,7 +915,7 @@ func (l *Ledger) FlushCaches() {
 // Validate uses the ledger to validate block blk as a candidate next block.
 // It returns an error if blk is not the expected next block, or if blk is
 // not a valid block (e.g., it has duplicate transactions, overspends some
-// account, etc).
+// account, etc.).
 func (l *Ledger) Validate(ctx context.Context, blk bookkeeping.Block, executionPool execpool.BacklogPool) (*ledgercore.ValidatedBlock, error) {
 	delta, err := eval.Eval(ctx, l, blk, true, l.verifiedTxnCache, executionPool, l.tracer)
 	if err != nil {
diff --git a/ledger/ledger_test.go b/ledger/ledger_test.go
index ab5bc293a..b18428741 100644
--- a/ledger/ledger_test.go
+++ b/ledger/ledger_test.go
@@ -3287,3 +3287,67 @@ func TestLedgerMaxBlockHistoryLookback(t *testing.T) {
 	require.Error(t, err)
 	require.Empty(t, blk)
 }
+
+func TestLedgerRetainMinOffCatchpointInterval(t *testing.T) {
+	partitiontest.PartitionTest(t)
+	// This test is to ensure that the ledger retains the minimum number of blocks off the catchpoint interval.
+	blocksToMake := 2000
+
+	// Cases:
+	// 1. Base Case: Archival = false, Stores catchpoints returns true, CatchpointFileHistoryLength = >= 1 - implies catchpoint interval > 0 - min formula
+	// 2. Archival = true, stores catchpoints returns false - we keep all blocks anyway
+	// 3. Archival = false, stores catchpoints returns false - we don't modify minToSave
+	// 4. Condition: Archival = false, storesCatchpoints returns true, CatchpointFileHistoryLength is -1 - keep all catchpoint files
+	// 5. Condition: Archival = false, storesCatchpoints returns true, CatchpointFileHistoryLength is 365 - the config default setting
+
+	catchpointIntervalBlockRetentionTestCases := []struct {
+		storeCatchpoints            bool
+		archival                    bool
+		catchpointFileHistoryLength int
+	}{
+		{true, false, 1},   // should use min catchpoint formula
+		{false, true, 1},   // all blocks get retained, archival mode dictates
+		{false, false, 1},  // should not modify min blocks retained based on catchpoint interval
+		{true, false, -1},  // should use min formula, this is the keep all catchpoints setting
+		{true, false, 365}, // should use min formula, this is the default setting for catchpoint file history length
+	}
+	for _, tc := range catchpointIntervalBlockRetentionTestCases {
+		func() {
+			var genHash crypto.Digest
+			crypto.RandBytes(genHash[:])
+			cfg := config.GetDefaultLocal()
+			// set config properties based on test case
+			cfg.MaxBlockHistoryLookback = 0 // max block history lookback is not used in this test
+			if tc.storeCatchpoints {
+				cfg.CatchpointTracking = config.CatchpointTrackingModeStored
+				cfg.CatchpointInterval = 100
+			} else {
+				cfg.CatchpointInterval = 0 // sufficient for cfg.StoresCatchpoints() to return false
+			}
+			cfg.CatchpointFileHistoryLength = tc.catchpointFileHistoryLength
+			cfg.Archival = tc.archival
+
+			l := &Ledger{}
+			l.cfg = cfg
+			l.archival = cfg.Archival
+
+			for i := 1; i <= blocksToMake; i++ {
+				minBlockToKeep := l.notifyCommit(basics.Round(i))
+
+				// In archival mode, all blocks should always be kept
+				if cfg.Archival {
+					require.Equal(t, basics.Round(0), minBlockToKeep)
+				} else {
+					// This happens to work for the test case where we don't store catchpoints since mintosave is always
+					// 0 in that case.
+					expectedCatchpointLookback := 2 * cfg.CatchpointInterval
+
+					expectedMinBlockToKeep := basics.Round(uint64(i)).SubSaturate(
+						basics.Round(expectedCatchpointLookback))
+					require.Equal(t, expectedMinBlockToKeep, minBlockToKeep)
+				}
+			}
+		}()
+	}
+
+}