bug(storage): ski cleaner service should apply only to owned tokens LFDT-Panurus#1902 (LFDT-Panurus#1903)

Signed-off-by: Angelo De Caro <adc@zurich.ibm.com>

Author: adecaro

docs/services/storage/keystore_cleanup.md

@@ -83,7 +83,9 @@ type Storage interface {
     // AcquireCleanupLeadership obtains distributed lock for leader election
     AcquireCleanupLeadership(ctx context.Context, lockID int64) (Leadership, bool, error)
 
-    // GetDeletedTokensPendingSKICleanup queries deleted tokens older than TTL that haven't been cleaned
+    // GetDeletedTokensPendingSKICleanup queries deleted, owned tokens older than TTL that haven't been cleaned.
+    // Tokens for which this node is only an auditor or issuer are excluded, since this node
+    // never holds the secret keys for those tokens.
     GetDeletedTokensPendingSKICleanup(ctx context.Context, olderThan time.Duration, limit int) ([]DeletedToken, error)
 
     // MarkTokenCleaned records successful key cleanup to prevent reprocessing
@@ -303,10 +305,12 @@ A token transitions through the following states related to cleanup:
 
 - **Active**: Token is unspent and in use
 - **Deleted**: Token marked as deleted (`is_deleted=true`, `spent_at` set)
-- **Eligible for Cleanup**: Deleted token older than TTL without a cleanup record in `token_ski_cleanups`
+- **Eligible for Cleanup**: Deleted, **owned** token (`owner=true`) older than TTL without a cleanup record in `token_ski_cleanups`
 - **Cleaned**: Keys deleted from keystore (record exists in `token_ski_cleanups`)
 
-The cleanup service only processes tokens in the "Eligible for Cleanup" state.
+The cleanup service only processes tokens in the "Eligible for Cleanup" state. Tokens for which
+this node is only an auditor or issuer (`owner=false`) are never selected, since this node
+never holds the secret keys for tokens it does not own.
 
 ## Database Schema
 

token/services/storage/db/dbtest/tokens.go

@@ -1546,4 +1546,45 @@ func TGetDeletedTokensPendingSKICleanup(t *testing.T, db TestTokenDB) {
 
 		assert.Equal(t, 3, count, "should return all indices for the same transaction")
 	})
+
+	// Test 12: Non-owned tokens (auditor-only, issuer-only) must be excluded,
+	// since this node never holds the secret keys for tokens it does not own.
+	t.Run("ExcludeNonOwnedTokens", func(t *testing.T) {
+		createAndDeleteNonOwnedToken := func(txID string, index uint64, ownerType string, auditor, issuer bool) {
+			tr := driver2.TokenRecord{
+				TxID:           txID,
+				Index:          index,
+				OwnerRaw:       []byte{1, 2, 3},
+				OwnerType:      ownerType,
+				OwnerIdentity:  fmt.Appendf(nil, "owner_%s_%d", txID, index),
+				Ledger:         []byte("ledger"),
+				LedgerMetadata: []byte{},
+				Quantity:       "0x01",
+				Type:           ABC,
+				Amount:         1,
+				Owner:          false,
+				Auditor:        auditor,
+				Issuer:         issuer,
+			}
+			require.NoError(t, db.StoreToken(ctx, tr, nil))
+			require.NoError(t, db.DeleteTokens(ctx, "deleter_tx", &token.ID{TxId: txID, Index: index}))
+		}
+
+		createAndDeleteNonOwnedToken("auditor_only", 0, "idemix", true, false)
+		createAndDeleteNonOwnedToken("issuer_only", 0, "idemix", false, true)
+		createAndDeleteToken("owned_control", 0, "idemix")
+
+		tokens, err := db.GetDeletedTokensPendingSKICleanup(ctx, 0, 100)
+		require.NoError(t, err, "query should not error")
+
+		foundOwnedControl := false
+		for _, tok := range tokens {
+			assert.NotEqual(t, "auditor_only", tok.TxID, "auditor-only token should not be returned")
+			assert.NotEqual(t, "issuer_only", tok.TxID, "issuer-only token should not be returned")
+			if tok.TxID == "owned_control" {
+				foundOwnedControl = true
+			}
+		}
+		assert.True(t, foundOwnedControl, "owned token should still be returned")
+	})
 }

token/services/storage/db/driver/token.go

@@ -269,7 +269,8 @@ type TokenStore interface {
 	Notifier() (TokenNotifier, error)
 	// GetDeletedTokensPendingSKICleanup returns deleted tokens older than the specified duration that haven't had their SKI keys cleaned yet.
 	// This is used by the keystore cleanup service to identify tokens whose cryptographic keys can be safely removed.
-	// Only tokens without a record in the token_ski_cleanups table are returned.
+	// Only tokens without a record in the token_ski_cleanups table are returned, and only tokens owned by this node,
+	// since this node only holds the secret keys for tokens it owns, not for tokens it merely audited or issued.
 	GetDeletedTokensPendingSKICleanup(ctx context.Context, olderThan time.Duration, limit int) ([]DeletedToken, error)
 	// MarkTokenCleaned marks a token as having its cryptographic keys cleaned up.
 	// This prevents the cleanup service from processing the same token multiple times.

token/services/storage/db/sql/common/tokens.go

@@ -1183,8 +1183,10 @@ func (db *TokenStore) GetCertifications(ctx context.Context, ids []*token.ID) ([
 	return certifications, nil
 }
 
-// GetDeletedTokensPendingSKICleanup returns deleted tokens older than the specified duration
+// GetDeletedTokensPendingSKICleanup returns deleted, owned tokens older than the specified duration
 // that haven't had their SKI keys cleaned up yet (no record in token_ski_cleanups table).
+// Only tokens owned by this node are considered, since this node only holds the secret keys
+// for tokens it owns, not for tokens it merely audited or issued.
 func (db *TokenStore) GetDeletedTokensPendingSKICleanup(ctx context.Context, olderThan time.Duration, limit int) ([]driver.DeletedToken, error) {
 	cutoffTime := time.Now().UTC().Add(-olderThan)
 
@@ -1210,6 +1212,7 @@ func (db *TokenStore) GetDeletedTokensPendingSKICleanup(ctx context.Context, old
 		))).
 		Where(cond.And(
 			cond.CmpVal(tokenTable.Field("is_deleted"), "=", true),
+			cond.CmpVal(tokenTable.Field("owner"), "=", true),
 			cond.CmpVal(tokenTable.Field("spent_at"), "<", cutoffTime),
 			cond.IsNil(cleanupTable.Field("tx_id")),
 		)).

token/services/storage/services/cleanup/manager.go

@@ -32,7 +32,8 @@ type DeletedToken struct {
 type Storage interface {
 	// AcquireCleanupLeadership acquires an advisory lock for cleanup leadership
 	AcquireCleanupLeadership(ctx context.Context, lockID int64) (Leadership, bool, error)
-	// GetDeletedTokensPendingSKICleanup returns deleted tokens older than the specified duration that haven't had their SKI keys cleaned
+	// GetDeletedTokensPendingSKICleanup returns deleted tokens older than the specified duration that haven't had their SKI keys cleaned.
+	// Only tokens owned by this node are returned, since this node only holds the secret keys for tokens it owns.
 	GetDeletedTokensPendingSKICleanup(ctx context.Context, olderThan time.Duration, limit int) ([]DeletedToken, error)
 	// MarkTokenCleaned marks a token as having its SKI keys cleaned up
 	MarkTokenCleaned(ctx context.Context, txID string, index uint64, cleanedBy string) error