feat(pdp): implement ping validation for storage providers in selection process (#119)

* feat: implement ping validation for storage providers in selection process

* refactor(pdp): update PDPServer to handle null authHelper and improve provider selection logic in StorageService

* fix(storage): remove unnecessary blank line in StorageService

* fix(pdp): allow empty API and retrieval endpoints for null authHelper in PDPServer

* refactor(storage): clarify comments in StorageService regarding proof set ID and ping validation parameter

* refactor(storage): remove unused PDPServer instances and simplify provider selection logic in StorageService

* refactor(storage): streamline provider selection logic by removing unused parameters and enhancing inline sorting

* refactor(storage): update comments for clarity and enhance test coverage for provider selection logic

* refactor(pdp): simplify error handling for API and retrieval endpoints in PDPServer

* refactor(storage): remove unused parameters from selectProviderWithPing and update related tests for clarity

* fix(pdp): update ping endpoint in PDPServer to include 'pdp' in the URL

* test(pdp): update urls to avoid duplicate suffixes

---------

Co-authored-by: Rod Vagg <[email protected]>

Author: virajbhartiya

src/pdp/server.ts

@@ -117,7 +117,7 @@ export interface RootAdditionStatusResponse {
 export class PDPServer {
   private readonly _apiEndpoint: string
   private readonly _retrievalEndpoint: string
-  private readonly _authHelper: PDPAuthHelper
+  private readonly _authHelper: PDPAuthHelper | null
   private readonly _serviceName: string
 
   /**
@@ -128,7 +128,7 @@ export class PDPServer {
    * @param serviceName - Service name for uploads (defaults to 'public')
    */
   constructor (
-    authHelper: PDPAuthHelper,
+    authHelper: PDPAuthHelper | null,
     apiEndpoint: string,
     retrievalEndpoint: string,
     serviceName: string = 'public'
@@ -161,13 +161,13 @@ export class PDPServer {
     recordKeeper: string
   ): Promise<CreateProofSetResponse> {
     // Generate the EIP-712 signature for proof set creation
-    const authData = await this._authHelper.signCreateProofSet(clientDataSetId, payee, withCDN)
+    const authData = await this.getAuthHelper().signCreateProofSet(clientDataSetId, payee, withCDN)
 
     // Prepare the extra data for the contract call
     // This needs to match the ProofSetCreateData struct in Pandora contract
     const extraData = this._encodeProofSetCreateData({
       metadata: '', // Empty metadata for now
-      payer: await this._authHelper.getSignerAddress(),
+      payer: await this.getAuthHelper().getSignerAddress(),
       withCDN,
       signature: authData.signature
     })
@@ -251,7 +251,7 @@ export class PDPServer {
     }
 
     // Generate the EIP-712 signature for adding roots
-    const authData = await this._authHelper.signAddRoots(
+    const authData = await this.getAuthHelper().signAddRoots(
       clientDataSetId,
       nextRootId,
       rootDataArray // Pass RootData[] directly to auth helper
@@ -578,7 +578,7 @@ export class PDPServer {
    * @throws Error if provider is not reachable or returns non-200 status
    */
   async ping (): Promise<void> {
-    const response = await fetch(`${this._apiEndpoint}/ping`, {
+    const response = await fetch(`${this._apiEndpoint}/pdp/ping`, {
       method: 'GET',
       headers: {}
     })
@@ -594,6 +594,9 @@ export class PDPServer {
   }
 
   getAuthHelper (): PDPAuthHelper {
+    if (this._authHelper == null) {
+      throw new Error('AuthHelper is not available for an operation that requires signing')
+    }
     return this._authHelper
   }
 }

src/storage/service.ts

@@ -584,24 +584,44 @@ export class StorageService {
         if (b.currentRootCount > 0 && a.currentRootCount === 0) return 1
         return a.pdpVerifierProofSetId - b.pdpVerifierProofSetId
       })
-      const selected = sorted[0]
 
-      // Get the provider for this proof set
-      const providerId = await pandoraService.getProviderIdByAddress(selected.payee)
-      if (providerId === 0) {
-        throw createError(
-          'StorageService',
-          'smartSelectProvider',
-          `Provider ${selected.payee} for proof set ${selected.pdpVerifierProofSetId} is not currently approved`
-        )
+      // Convert sorted proof sets to providers and use shared selection logic
+      const existingProviders: ApprovedProviderInfo[] = []
+      for (const proofSet of sorted) {
+        const providerAddress = proofSet.payee.toLowerCase()
+        if (existingProviders.some(p => p.owner.toLowerCase() === providerAddress)) {
+          continue
+        }
+        const providerId = await pandoraService.getProviderIdByAddress(proofSet.payee)
+        if (providerId === 0) {
+          console.warn(`Provider ${proofSet.payee} for proof set ${proofSet.pdpVerifierProofSetId} is not currently approved, skipping`)
+          continue
+        }
+        const provider = await pandoraService.getApprovedProvider(providerId)
+        existingProviders.push(provider)
       }
 
-      const provider = await pandoraService.getApprovedProvider(providerId)
+      if (existingProviders.length > 0) {
+        const selectedProvider = await StorageService.selectProviderWithPing(existingProviders)
 
-      return {
-        provider,
-        proofSetId: selected.pdpVerifierProofSetId,
-        isExisting: true
+        // Find the first matching proof set ID for this provider
+        const matchingProofSet = sorted.find(ps =>
+          ps.payee.toLowerCase() === selectedProvider.owner.toLowerCase()
+        )
+
+        if (matchingProofSet == null) {
+          throw createError(
+            'StorageService',
+            'smartSelectProvider',
+            'Selected provider not found in proof sets'
+          )
+        }
+
+        return {
+          provider: selectedProvider,
+          proofSetId: matchingProofSet.pdpVerifierProofSetId,
+          isExisting: true
+        }
       }
     }
 
@@ -627,33 +647,90 @@ export class StorageService {
   }
 
   /**
-   * Select a random provider from the given list
+   * Select a random provider from the given list with ping validation
+   * @param providers - List of available providers
+   * @param signer - Signer for entropy generation
+   * @returns A provider that responds to ping
+   * @throws Error if no providers are reachable
    */
   private static async selectRandomProvider (
     providers: ApprovedProviderInfo[],
     signer: ethers.Signer
   ): Promise<ApprovedProviderInfo> {
-    let randomIndex: number
+    if (providers.length === 0) {
+      throw createError(
+        'StorageService',
+        'selectRandomProvider',
+        'No providers available'
+      )
+    }
 
-    // Try crypto.getRandomValues if available (HTTPS contexts)
-    if (typeof globalThis.crypto !== 'undefined' && globalThis.crypto.getRandomValues != null) {
-      const randomBytes = new Uint8Array(1)
-      globalThis.crypto.getRandomValues(randomBytes)
-      randomIndex = randomBytes[0] % providers.length
-    } else {
-      // Fallback for HTTP contexts - use multiple entropy sources
-      const timestamp = Date.now()
-      const random = Math.random()
-      // Use wallet address as additional entropy
-      const addressBytes = await signer.getAddress()
-      const addressSum = addressBytes.split('').reduce((a, c) => a + c.charCodeAt(0), 0)
+    // Randomly sort providers inline
+    const randomlySorted: ApprovedProviderInfo[] = []
+    const remaining = [...providers]
+
+    while (remaining.length > 0) {
+      let randomIndex: number
+
+      // Try crypto.getRandomValues if available (HTTPS contexts)
+      if (typeof globalThis.crypto !== 'undefined' && globalThis.crypto.getRandomValues != null) {
+        const randomBytes = new Uint8Array(1)
+        globalThis.crypto.getRandomValues(randomBytes)
+        randomIndex = randomBytes[0] % remaining.length
+      } else {
+        // Fallback for HTTP contexts - use multiple entropy sources
+        const timestamp = Date.now()
+        const random = Math.random()
+        // Use wallet address as additional entropy
+        const addressBytes = await signer.getAddress()
+        const addressSum = addressBytes.split('').reduce((a, c) => a + c.charCodeAt(0), 0)
+
+        // Combine sources for better distribution
+        const combined = (timestamp * random * addressSum) % remaining.length
+        randomIndex = Math.floor(Math.abs(combined))
+      }
+
+      randomlySorted.push(remaining.splice(randomIndex, 1)[0])
+    }
+
+    return await StorageService.selectProviderWithPing(randomlySorted)
+  }
+
+  /**
+   * Select a provider from a sorted list with ping validation.
+   * This is shared logic used by both smart selection and random selection.
+   * @param sortedProviders - List of providers to try in order
+   * @returns A provider that responds to ping
+   * @throws Error if no providers are reachable
+   */
+  private static async selectProviderWithPing (sortedProviders: ApprovedProviderInfo[]): Promise<ApprovedProviderInfo> {
+    if (sortedProviders.length === 0) {
+      throw createError(
+        'StorageService',
+        'selectProviderWithPing',
+        'No reachable storage providers available after ping validation'
+      )
+    }
 
-      // Combine sources for better distribution
-      const combined = (timestamp * random * addressSum) % providers.length
-      randomIndex = Math.floor(Math.abs(combined))
+    // Try providers in order until we find one that responds to ping
+    for (const provider of sortedProviders) {
+      try {
+        // Create a temporary PDPServer for this specific provider's endpoint
+        const providerPdpServer = new PDPServer(null, provider.pdpUrl, provider.pieceRetrievalUrl)
+        await providerPdpServer.ping()
+        return provider
+      } catch (error) {
+        console.warn(`Provider ${provider.owner} failed ping test:`, error instanceof Error ? error.message : String(error))
+        // Continue to next provider
+      }
     }
 
-    return providers[randomIndex]
+    // All providers failed ping test
+    throw createError(
+      'StorageService',
+      'selectProviderWithPing',
+      `All ${sortedProviders.length} available storage providers failed ping validation`
+    )
   }
 
   /**