chore: Refactor catchpoint tracking logic into config (#5725)

config/config.go

@@ -89,6 +89,21 @@ const MaxGenesisIDLen = 128
 // MaxEvalDeltaTotalLogSize is the maximum size of the sum of all log sizes in a single eval delta.
 const MaxEvalDeltaTotalLogSize = 1024
 
+// CatchpointTrackingModeUntracked defines the CatchpointTracking mode that does _not_ track catchpoints
+const CatchpointTrackingModeUntracked = -1
+
+// CatchpointTrackingModeAutomatic defines the CatchpointTracking mode that automatically determines catchpoint tracking
+// and storage based on the Archival property and CatchpointInterval.
+const CatchpointTrackingModeAutomatic = 0
+
+// CatchpointTrackingModeTracked defines the CatchpointTracking mode that tracks catchpoint
+// as long as CatchpointInterval > 0
+const CatchpointTrackingModeTracked = 1
+
+// CatchpointTrackingModeStored defines the CatchpointTracking mode that tracks and stores catchpoints
+// as long as CatchpointInterval > 0
+const CatchpointTrackingModeStored = 2
+
 // LoadConfigFromDisk returns a Local config structure based on merging the defaults
 // with settings loaded from the config file from the custom dir.  If the custom file
 // cannot be loaded, the default config is returned (with the error from loading the

config/config_test.go

@@ -803,3 +803,129 @@ func TestResolveLogPaths(t *testing.T) {
 	require.Equal(t, "mycoolLogDir/node.log", log)
 	require.Equal(t, "myCoolLogArchive/node.archive.log", archive)
 }
+
+func TestStoresCatchpoints(t *testing.T) {
+	partitiontest.PartitionTest(t)
+
+	var tests = []struct {
+		name               string
+		catchpointTracking int64
+		catchpointInterval uint64
+		archival           bool
+		expected           bool
+	}{
+		{
+			name:               "-1 w/ no catchpoint interval expects false",
+			catchpointTracking: CatchpointTrackingModeUntracked,
+			catchpointInterval: 0,
+			expected:           false,
+		},
+		{
+			name:               "-1 expects false",
+			catchpointTracking: CatchpointTrackingModeUntracked,
+			catchpointInterval: GetDefaultLocal().CatchpointInterval,
+			archival:           GetDefaultLocal().Archival,
+			expected:           false,
+		},
+		{
+			name:               "0 expects false",
+			catchpointTracking: CatchpointTrackingModeAutomatic,
+			catchpointInterval: GetDefaultLocal().CatchpointInterval,
+			archival:           GetDefaultLocal().Archival,
+			expected:           false,
+		},
+		{
+			name:               "0 w/ archival expects true",
+			catchpointTracking: CatchpointTrackingModeAutomatic,
+			catchpointInterval: GetDefaultLocal().CatchpointInterval,
+			archival:           true,
+			expected:           true,
+		},
+		{
+			name:               "0 w/ archival & catchpointInterval=0 expects false",
+			catchpointTracking: CatchpointTrackingModeAutomatic,
+			catchpointInterval: 0,
+			archival:           true,
+			expected:           false,
+		},
+		{
+			name:               "1 expects false",
+			catchpointTracking: CatchpointTrackingModeTracked,
+			catchpointInterval: GetDefaultLocal().CatchpointInterval,
+			archival:           GetDefaultLocal().Archival,
+			expected:           false,
+		},
+		{
+			name:               "1 w/ archival expects true",
+			catchpointTracking: CatchpointTrackingModeTracked,
+			catchpointInterval: GetDefaultLocal().CatchpointInterval,
+			archival:           true,
+			expected:           true,
+		},
+		{
+			name:               "1 w/ archival & catchpointInterval=0 expects false",
+			catchpointTracking: CatchpointTrackingModeTracked,
+			catchpointInterval: 0,
+			archival:           true,
+			expected:           false,
+		},
+		{
+			name:               "2 w/ catchpointInterval=0 expects false",
+			catchpointTracking: CatchpointTrackingModeStored,
+			catchpointInterval: 0,
+			archival:           GetDefaultLocal().Archival,
+			expected:           false,
+		},
+		{
+			name:               "2 expects true",
+			catchpointTracking: CatchpointTrackingModeStored,
+			catchpointInterval: GetDefaultLocal().CatchpointInterval,
+			archival:           GetDefaultLocal().Archival,
+			expected:           true,
+		},
+		{
+			name:               "99 expects false",
+			catchpointTracking: 99,
+			catchpointInterval: GetDefaultLocal().CatchpointInterval,
+			archival:           GetDefaultLocal().Archival,
+			expected:           false,
+		},
+		{
+			name:               "99 w/ catchpointInterval=0 expects false",
+			catchpointTracking: 99,
+			catchpointInterval: 0,
+			archival:           GetDefaultLocal().Archival,
+			expected:           false,
+		},
+		{
+			name:               "27 expects false",
+			catchpointTracking: 27,
+			catchpointInterval: GetDefaultLocal().CatchpointInterval,
+			archival:           GetDefaultLocal().Archival,
+			expected:           false,
+		},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			cfg := GetDefaultLocal()
+			cfg.CatchpointTracking = test.catchpointTracking
+			cfg.CatchpointInterval = test.catchpointInterval
+			cfg.Archival = test.archival
+			require.Equal(t, test.expected, cfg.StoresCatchpoints())
+			if cfg.StoresCatchpoints() {
+				require.Equal(t, true, cfg.TracksCatchpoints())
+			}
+		})
+	}
+}
+
+func TestTracksCatchpointsWithoutStoring(t *testing.T) {
+	partitiontest.PartitionTest(t)
+
+	cfg := GetDefaultLocal()
+	cfg.CatchpointTracking = CatchpointTrackingModeTracked
+	cfg.CatchpointInterval = 10000
+	cfg.Archival = false
+	require.Equal(t, true, cfg.TracksCatchpoints())
+	require.Equal(t, false, cfg.StoresCatchpoints())
+}

config/localTemplate.go

@@ -388,7 +388,7 @@ type Local struct {
 	// A value of 1 means "track catchpoints as long as CatchpointInterval > 0".
 	// A value of 2 means "track catchpoints and always generate catchpoint files as long as CatchpointInterval > 0".
 	// A value of 0 means automatic, which is the default value. In this mode, a non archival node would not track the catchpoints, and an archival node would track the catchpoints as long as CatchpointInterval > 0.
-	// Other values of CatchpointTracking would give a warning in the log file, and would behave as if the default value was provided.
+	// Other values of CatchpointTracking would behave as if the default value was provided.
 	CatchpointTracking int64 `version[11]:"0"`
 
 	// LedgerSynchronousMode defines the synchronous mode used by the ledger database. The supported options are:
@@ -862,3 +862,34 @@ func (cfg *Local) AdjustConnectionLimits(requiredFDs, maxFDs uint64) bool {
 
 	return true
 }
+
+// StoresCatchpoints returns true if the node is configured to store catchpoints
+func (cfg *Local) StoresCatchpoints() bool {
+	if cfg.CatchpointInterval <= 0 {
+		return false
+	}
+	switch cfg.CatchpointTracking {
+	case CatchpointTrackingModeUntracked:
+		// No catchpoints.
+	default:
+		fallthrough
+	case CatchpointTrackingModeAutomatic, CatchpointTrackingModeTracked:
+		if cfg.Archival {
+			return true
+		}
+	case CatchpointTrackingModeStored:
+		return true
+	}
+	return false
+}
+
+// TracksCatchpoints returns true if the node is configured to track catchpoints
+func (cfg *Local) TracksCatchpoints() bool {
+	if cfg.StoresCatchpoints() {
+		return true
+	}
+	if cfg.CatchpointTracking == CatchpointTrackingModeTracked && cfg.CatchpointInterval > 0 {
+		return true
+	}
+	return false
+}

ledger/acctupdates.go

@@ -93,7 +93,7 @@ const initializingAccountCachesMessageTimeout = 3 * time.Second
 // where we end up batching up to 1000 rounds in a single update.
 const accountsUpdatePerRoundHighWatermark = 1 * time.Second
 
-// forceCatchpointFileGeneration defines the CatchpointTracking mode that would be used to
+// forceCatchpointFileGenerationTrackingMode defines the CatchpointTracking mode that would be used to
 // force a node to generate catchpoint files.
 const forceCatchpointFileGenerationTrackingMode = 99
 

ledger/catchpointtracker.go

@@ -163,34 +163,16 @@ func (ct *catchpointTracker) initialize(cfg config.Local, paths DirsAndPrefix) {
 	// the temp file uses the hot data directories
 	ct.tmpDir = paths.HotGenesisDir
 
-	switch cfg.CatchpointTracking {
-	case -1:
-		// No catchpoints.
-	default:
-		// Give a warning, then fall through to case 0.
-		logging.Base().Warnf("catchpointTracker: the CatchpointTracking field in the config.json file contains an invalid value (%d). The default value of 0 would be used instead.", cfg.CatchpointTracking)
-		fallthrough
-	case 0:
-		if cfg.Archival && (cfg.CatchpointInterval > 0) {
-			ct.catchpointInterval = cfg.CatchpointInterval
-			ct.enableGeneratingCatchpointFiles = true
-		}
-	case 1:
-		if cfg.CatchpointInterval > 0 {
-			ct.catchpointInterval = cfg.CatchpointInterval
-			ct.enableGeneratingCatchpointFiles = cfg.Archival
-		}
-	case 2:
-		if cfg.CatchpointInterval > 0 {
-			ct.catchpointInterval = cfg.CatchpointInterval
-			ct.enableGeneratingCatchpointFiles = true
-		}
-	case forceCatchpointFileGenerationTrackingMode:
-		if cfg.CatchpointInterval > 0 {
-			ct.catchpointInterval = cfg.CatchpointInterval
-			ct.enableGeneratingCatchpointFiles = true
-			ct.forceCatchpointFileWriting = true
-		}
+	if cfg.TracksCatchpoints() {
+		ct.catchpointInterval = cfg.CatchpointInterval
+	}
+	ct.enableGeneratingCatchpointFiles = cfg.StoresCatchpoints()
+
+	// Overwrite previous options if forceCatchpointFileGenerationTrackingMode
+	if cfg.CatchpointTracking == forceCatchpointFileGenerationTrackingMode && cfg.CatchpointInterval > 0 {
+		ct.catchpointInterval = cfg.CatchpointInterval
+		ct.forceCatchpointFileWriting = true
+		ct.enableGeneratingCatchpointFiles = true
 	}
 
 	ct.catchpointFileHistoryLength = cfg.CatchpointFileHistoryLength