satellite/reputation: check node age in addition to number of audits to determine vetted status

Change-Id: I139a0093922cf80c19281e34059313eddc476fc2

Author: Jennifer Johnson

satellite/reputation/config.go

@@ -33,6 +33,7 @@ type Config struct {
 	SuspensionGracePeriod time.Duration `help:"the time period that must pass before suspended nodes will be disqualified" releaseDefault:"168h" devDefault:"1h"`
 	SuspensionDQEnabled   bool          `help:"whether nodes will be disqualified if they have been suspended for longer than the suspended grace period" releaseDefault:"false" devDefault:"true"`
 	AuditCount            int64         `help:"the number of times a node has been audited to not be considered a New Node" releaseDefault:"100" devDefault:"0"`
+	MinimumNodeAge        time.Duration `help:"the minimum age a node must have since creation before being vetted" releaseDefault:"504h" devDefault:"0h"`
 	AuditHistory          AuditHistoryConfig
 	FlushInterval         time.Duration `help:"the maximum amount of time that should elapse before cached reputation writes are flushed to the database (if 0, no reputation cache is used)" releaseDefault:"2h" devDefault:"2m"`
 	ErrorRetryInterval    time.Duration `help:"the amount of time that should elapse before the cache retries failed database operations" releaseDefault:"1m" devDefault:"5s"`

satellite/reputation/db_test.go

@@ -69,6 +69,63 @@ func TestUpdate(t *testing.T) {
 	})
 }
 
+func TestUpdateWithMinimumNodeAge(t *testing.T) {
+	testplanet.Run(t, testplanet.Config{
+		SatelliteCount: 1, StorageNodeCount: 1,
+		Reconfigure: testplanet.Reconfigure{
+			Satellite: func(log *zap.Logger, index int, config *satellite.Config) {
+				config.Reputation.AuditCount = 2
+				config.Reputation.MinimumNodeAge = 24 * time.Hour // 1 day minimum age
+			},
+		},
+	}, func(t *testing.T, ctx *testcontext.Context, planet *testplanet.Planet) {
+		node := planet.StorageNodes[0]
+		node.Contact.Chore.Pause(ctx)
+
+		db := planet.Satellites[0].DB.Reputation()
+
+		// Test 1: Node meets audit count but is too young -> should not vet
+		updateReq := reputation.UpdateRequest{
+			NodeID:       node.ID(),
+			AuditOutcome: reputation.AuditOffline,
+			Config: reputation.Config{
+				AuditCount:     planet.Satellites[0].Config.Reputation.AuditCount,
+				MinimumNodeAge: planet.Satellites[0].Config.Reputation.MinimumNodeAge,
+				AuditHistory:   testAuditHistoryConfig(),
+			},
+		}
+
+		// First audit - this creates the reputation record
+		now := time.Now()
+		nodeStats, err := db.Update(ctx, updateReq, now)
+		require.NoError(t, err)
+		assert.Nil(t, nodeStats.VettedAt, "node should not be vetted after 1 audit")
+
+		// Get the actual CreatedAt from the database
+		startTime := nodeStats.CreatedAt
+		require.NotNil(t, startTime)
+
+		// Second audit 1 hr (meets audit count but still too young, needs 24 hours)
+		secondAuditTime := startTime.Add(time.Hour)
+		nodeStats, err = db.Update(ctx, updateReq, secondAuditTime)
+		require.NoError(t, err)
+		assert.Nil(t, nodeStats.VettedAt, "node should not be vetted yet - too young despite meeting audit count")
+
+		// Test 2: Node is now old enough and meets audit count -> should vet
+		// Simulate time passing by doing an audit 25 hours after node creation
+		oldEnoughTime := startTime.Add(25 * time.Hour)
+		nodeStats, err = db.Update(ctx, updateReq, oldEnoughTime)
+		require.NoError(t, err)
+		assert.NotNil(t, nodeStats.VettedAt, "node should be vetted now - old enough and meets audit count")
+
+		// Test 3: Verify vetted_at timestamp is not overwritten on subsequent audits
+		nodeStats2, err := db.Update(ctx, updateReq, oldEnoughTime.Add(time.Hour))
+		require.NoError(t, err)
+		assert.NotNil(t, nodeStats2.VettedAt)
+		assert.Equal(t, nodeStats.VettedAt, nodeStats2.VettedAt, "vetted_at should not change on subsequent audits")
+	})
+}
+
 // testApplyUpdatesEquivalentToMultipleUpdates checks that the ApplyUpdates call
 // is equivalent to making multiple separate Update() calls (modulo some details
 // like exact-time-of-disqualification).
@@ -91,7 +148,7 @@ func testApplyUpdatesEquivalentToMultipleUpdates(ctx context.Context, t *testing
 		t.Run(testDef.name, func(t *testing.T) {
 			node1 := testrand.NodeID()
 			node2 := testrand.NodeID()
-			startTime := time.Now().Add(-time.Hour)
+			startTime := time.Now()
 			var (
 				info1, info2 *reputation.Info
 				err          error
@@ -190,6 +247,7 @@ func TestApplyUpdatesEquivalentToMultipleUpdates(t *testing.T) {
 		SuspensionGracePeriod: 20 * time.Minute,
 		SuspensionDQEnabled:   true,
 		AuditCount:            3,
+		MinimumNodeAge:        0,
 		AuditHistory: reputation.AuditHistoryConfig{
 			WindowSize:               10 * time.Minute,
 			TrackingPeriod:           1 * time.Hour,
@@ -220,6 +278,7 @@ func TestApplyUpdatesEquivalentToMultipleUpdatesCached(t *testing.T) {
 		SuspensionGracePeriod: 20 * time.Minute,
 		SuspensionDQEnabled:   true,
 		AuditCount:            3,
+		MinimumNodeAge:        0,
 		AuditHistory: reputation.AuditHistoryConfig{
 			WindowSize:               10 * time.Minute,
 			TrackingPeriod:           1 * time.Hour,
@@ -353,7 +412,7 @@ func TestDBDisqualificationNodeOffline(t *testing.T) {
 				SuspensionDQEnabled:   false,
 				AuditCount:            0,
 				AuditHistory: reputation.AuditHistoryConfig{
-					WindowSize:               0,
+					WindowSize:               1 * time.Second,
 					TrackingPeriod:           1 * time.Second,
 					GracePeriod:              0,
 					OfflineThreshold:         1,

satellite/reputation/service.go

@@ -36,6 +36,7 @@ type DB interface {
 type Info struct {
 	AuditSuccessCount           int64
 	TotalAuditCount             int64
+	CreatedAt                   *time.Time
 	VettedAt                    *time.Time
 	UnknownAuditSuspended       *time.Time
 	OfflineSuspended            *time.Time
@@ -53,6 +54,7 @@ type Info struct {
 // Copy creates a deep copy of the Info object.
 func (i *Info) Copy() *Info {
 	i2 := *i
+	i2.CreatedAt = cloneTime(i.CreatedAt)
 	i2.VettedAt = cloneTime(i.VettedAt)
 	i2.UnknownAuditSuspended = cloneTime(i.UnknownAuditSuspended)
 	i2.OfflineSuspended = cloneTime(i.OfflineSuspended)

satellite/reputation/writecache.go

@@ -185,12 +185,15 @@ func (cdb *CachingDB) ApplyUpdates(ctx context.Context, nodeID storj.NodeID, upd
 		cachedInfo.TotalAuditCount += int64(updates.PositiveResults + updates.FailureResults + updates.OfflineResults + updates.UnknownResults)
 		cachedInfo.OnlineScore = cachedInfo.AuditHistory.Score
 
-		if cachedInfo.VettedAt == nil && cachedInfo.TotalAuditCount >= config.AuditCount {
-			cachedInfo.VettedAt = &now
-			// if we think the node is newly vetted, perform a sync to
-			// have the best chance of propagating that information to
-			// other satellite services.
-			doRequestSync = true
+		if cachedInfo.CreatedAt != nil {
+			timeSinceCreation := now.Sub(*cachedInfo.CreatedAt)
+			if cachedInfo.VettedAt == nil && timeSinceCreation >= config.MinimumNodeAge && cachedInfo.TotalAuditCount >= config.AuditCount {
+				cachedInfo.VettedAt = &now
+				// if we think the node is newly vetted, perform a sync to
+				// have the best chance of propagating that information to
+				// other satellite services.
+				doRequestSync = true
+			}
 		}
 
 		// for audit failure, only update normal alpha/beta

satellite/satellite-config.yaml.lock

@@ -1829,6 +1829,9 @@ identity.key-path: /root/.local/share/storj/identity/satellite/identity.key
 # the value to which a beta reputation value should be initialized
 # reputation.initial-beta: 0
 
+# the minimum age a node must have since creation before being vetted
+# reputation.minimum-node-age: 504h0m0s
+
 # whether nodes will be disqualified if they have been suspended for longer than the suspended grace period
 # reputation.suspension-dq-enabled: false
 

satellite/satellitedb/reputations.go

@@ -170,6 +170,7 @@ func (reputations *reputations) Get(ctx context.Context, nodeID storj.NodeID) (*
 	return &reputation.Info{
 		AuditSuccessCount:           res.AuditSuccessCount,
 		TotalAuditCount:             res.TotalAuditCount,
+		CreatedAt:                   &res.CreatedAt,
 		VettedAt:                    res.VettedAt,
 		UnknownAuditSuspended:       res.UnknownAuditSuspended,
 		OfflineSuspended:            res.OfflineSuspended,
@@ -546,7 +547,8 @@ func (reputations *reputations) populateUpdateNodeStats(dbNode *dbx.Reputation,
 		OnlineScore: float64Field{set: true, value: historyResponse.NewScore},
 	}
 
-	if vettedAt == nil && updatedTotalAuditCount >= config.AuditCount {
+	timeSinceCreation := now.Sub(dbNode.CreatedAt)
+	if vettedAt == nil && timeSinceCreation >= config.MinimumNodeAge && updatedTotalAuditCount >= config.AuditCount {
 		updateFields.VettedAt = timeField{set: true, value: now}
 	}
 
@@ -696,6 +698,7 @@ func dbxToReputationInfo(dbNode *dbx.Reputation) (reputation.Info, error) {
 	info := reputation.Info{
 		AuditSuccessCount:           dbNode.AuditSuccessCount,
 		TotalAuditCount:             dbNode.TotalAuditCount,
+		CreatedAt:                   &dbNode.CreatedAt,
 		VettedAt:                    dbNode.VettedAt,
 		UnknownAuditSuspended:       dbNode.UnknownAuditSuspended,
 		OfflineSuspended:            dbNode.OfflineSuspended,