chore: address pr review comments

Author: silent-cipher

apps/backend/src/providers/repositories/storage-provider.repository.spec.ts

@@ -1,3 +1,4 @@
+import { In, Not } from "typeorm";
 import { beforeEach, describe, expect, it, vi } from "vitest";
 import type { StorageProvider } from "../../database/entities/storage-provider.entity.js";
 import type { PDPProviderEx } from "../../wallet-sdk/wallet-sdk.types.js";
@@ -326,12 +327,17 @@ describe("StorageProviderRepository", () => {
   });
 
   describe("upsertFromRegistry", () => {
-    let repo: { create: ReturnType<typeof vi.fn>; upsert: ReturnType<typeof vi.fn> };
+    let repo: { create: ReturnType<typeof vi.fn>; manager: { transaction: ReturnType<typeof vi.fn> } };
+    let txRepo: { upsert: ReturnType<typeof vi.fn>; update: ReturnType<typeof vi.fn> };
     let service: StorageProviderRepository;
     let loggerMock: { warn: ReturnType<typeof vi.fn>; error: ReturnType<typeof vi.fn> };
 
     beforeEach(() => {
-      repo = { create: vi.fn((data) => data), upsert: vi.fn() };
+      txRepo = { upsert: vi.fn(), update: vi.fn() };
+      repo = {
+        create: vi.fn((data) => data),
+        manager: { transaction: vi.fn((runInTransaction) => runInTransaction({ getRepository: () => txRepo })) },
+      };
       service = new StorageProviderRepository(repo as any, {} as any);
       loggerMock = { warn: vi.fn(), error: vi.fn() };
       (service as any).logger = loggerMock;
@@ -357,7 +363,7 @@ describe("StorageProviderRepository", () => {
       );
       expect(loggerMock.error).not.toHaveBeenCalled();
 
-      const [entities, options] = repo.upsert.mock.calls[0];
+      const [entities, options] = txRepo.upsert.mock.calls[0];
       expect(options).toEqual(expect.objectContaining({ conflictPaths: ["address", "network"] }));
       expect(entities).toEqual(
         expect.arrayContaining([
@@ -374,7 +380,7 @@ describe("StorageProviderRepository", () => {
       await service.upsertFromRegistry([active, inactive], "calibration");
 
       expect(loggerMock.error).not.toHaveBeenCalled();
-      const [entities] = repo.upsert.mock.calls[0];
+      const [entities] = txRepo.upsert.mock.calls[0];
       expect(entities).toEqual(
         expect.arrayContaining([
           expect.objectContaining({ address: "0xdup2", network: "calibration", providerId: 30n, name: "active" }),
@@ -391,12 +397,47 @@ describe("StorageProviderRepository", () => {
       expect(loggerMock.error).toHaveBeenCalledWith(
         expect.objectContaining({ event: "duplicate_provider_addresses_unresolved" }),
       );
-      const [entities] = repo.upsert.mock.calls[0];
+      const [entities] = txRepo.upsert.mock.calls[0];
       expect(entities).toEqual(
         expect.arrayContaining([
           expect.objectContaining({ address: "0xdup3", network: "calibration", providerId: 41n, name: "second" }),
         ]),
       );
     });
+
+    it("deactivates rows whose address is absent from the registry snapshot", async () => {
+      const stillPresent = makeProvider({ id: 50n, serviceProvider: "0xstill", isActive: true });
+
+      await service.upsertFromRegistry([stillPresent], "calibration");
+
+      expect(txRepo.update).toHaveBeenCalledWith(
+        { network: "calibration", isActive: true, address: Not(In(["0xstill"])) },
+        { isActive: false },
+      );
+    });
+
+    it("deactivates within the same transaction as the upsert, after it", async () => {
+      const provider = makeProvider({ id: 51n, serviceProvider: "0xa" });
+      const callOrder: string[] = [];
+      txRepo.upsert.mockImplementation(() => {
+        callOrder.push("upsert");
+        return Promise.resolve();
+      });
+      txRepo.update.mockImplementation(() => {
+        callOrder.push("update");
+        return Promise.resolve();
+      });
+
+      await service.upsertFromRegistry([provider], "calibration");
+
+      expect(repo.manager.transaction).toHaveBeenCalledTimes(1);
+      expect(callOrder).toEqual(["upsert", "update"]);
+    });
+
+    it("clears the address filter (deactivating every row for the network) when the registry snapshot is empty", async () => {
+      await service.upsertFromRegistry([], "calibration");
+
+      expect(txRepo.update).toHaveBeenCalledWith({ network: "calibration", isActive: true }, { isActive: false });
+    });
   });
 });

apps/backend/src/providers/repositories/storage-provider.repository.ts

@@ -1,7 +1,7 @@
 import { Injectable, Logger } from "@nestjs/common";
 import { ConfigService } from "@nestjs/config";
 import { InjectRepository } from "@nestjs/typeorm";
-import { Raw, type Repository } from "typeorm";
+import { In, Not, Raw, type Repository } from "typeorm";
 import { toJsonSafe } from "../../common/logging.js";
 import type { Network } from "../../common/types.js";
 import type { IConfig } from "../../config/index.js";
@@ -213,9 +213,27 @@ export class StorageProviderRepository {
       }),
     );
 
-    await this.repo.upsert(entities, {
-      conflictPaths: ["address", "network"],
-      skipUpdateIfNoValuesChanged: true,
+    const seenAddresses = Array.from(dedupedProviders.keys());
+
+    await this.repo.manager.transaction(async (manager) => {
+      const txRepo = manager.getRepository(StorageProvider);
+
+      await txRepo.upsert(entities, {
+        conflictPaths: ["address", "network"],
+        skipUpdateIfNoValuesChanged: true,
+      });
+
+      // Registry responses are a full snapshot each refresh, so any active row whose
+      // address wasn't in this snapshot is stale (provider deregistered/removed) and
+      // must be flipped inactive here — otherwise it stays isActive=true forever.
+      await txRepo.update(
+        {
+          network,
+          isActive: true,
+          ...(seenAddresses.length > 0 ? { address: Not(In(seenAddresses)) } : {}),
+        },
+        { isActive: false },
+      );
     });
   }
 

apps/backend/src/wallet-sdk/wallet-sdk.service.spec.ts

@@ -66,12 +66,16 @@ const makeProvider = (overrides: Partial<PDPProviderEx>): PDPProviderEx =>
 
 describe("WalletSdkService", () => {
   let service: WalletSdkService;
-  let storageProviderRepositoryMock: { countByNetwork: ReturnType<typeof vi.fn> };
+  let storageProviderRepositoryMock: {
+    countByNetwork: ReturnType<typeof vi.fn>;
+    upsertFromRegistry: ReturnType<typeof vi.fn>;
+  };
   let loggerMock: LoggerLike;
 
   beforeEach(() => {
     storageProviderRepositoryMock = {
       countByNetwork: vi.fn().mockResolvedValue(0),
+      upsertFromRegistry: vi.fn().mockResolvedValue(undefined),
     };
 
     const configService = {
@@ -135,6 +139,28 @@ describe("WalletSdkService", () => {
     expect(loadProviders).not.toHaveBeenCalled();
   });
 
+  it("returns false when the DB sync fails, so callers don't record a false success", async () => {
+    storageProviderRepositoryMock.upsertFromRegistry.mockRejectedValue(new Error("db down"));
+    const provider = makeProvider({ id: 1n });
+    const mockState = {
+      config: baseNetworkConfig,
+      warmStorageService: { getApprovedProviderIds: vi.fn().mockResolvedValue([]) },
+      spRegistry: {
+        getProviderCount: vi.fn().mockResolvedValue(1n),
+        getAllActiveProviders: vi.fn().mockResolvedValue([provider]),
+      },
+      providersLoadPromise: null,
+    };
+    (service as any).networkStates.set("calibration", mockState);
+
+    const result = await service.loadProviders("calibration");
+
+    expect(result).toBe(false);
+    expect(storageProviderRepositoryMock.upsertFromRegistry).toHaveBeenCalled();
+    expect(loggerMock.error).toHaveBeenCalledWith(expect.objectContaining({ event: "providers_sync_to_db_failed" }));
+    expect(loggerMock.error).toHaveBeenCalledWith(expect.objectContaining({ event: "providers_load_failed" }));
+  });
+
   describe("ensureWalletAllowances", () => {
     it("performs read-only check in session key mode", async () => {
       const mockState = {

apps/backend/src/wallet-sdk/wallet-sdk.service.ts

@@ -193,6 +193,7 @@ export class WalletSdkService implements OnModuleInit {
           message: "Failed to sync providers to DB",
           error: toStructuredError(err),
         });
+        throw err;
       }
 
       this.logger.log({