feat: reject no indexer_result during preprocessing (#439)

Move the validation step earlier to the preprocess/evaluate pipeline. Measurements missing the indexer result are dropped as invalid. Signed-off-by: Miroslav Bajtoš <[email protected]>
filecoin-station · Jan 9, 2025 · 6709840 · 6709840
1 parent 3ac022a
commit 6709840
Show file tree

Hide file tree

Showing 5 changed files with 27 additions and 75 deletions.
diff --git a/lib/evaluate.js b/lib/evaluate.js
@@ -267,15 +267,6 @@ export const runFraudDetection = async ({
     tasksAllowedForStations.size
   )
 
-  //
-  // 0. Filter out measurements missing required fields.
-  //
-  for (const m of measurements) {
-    if (!m.indexerResult) {
-      m.fraudAssessment = 'IPNI_NOT_QUERIED'
-    }
-  }
-
   //
   // 1. Filter out measurements not belonging to any valid task in this round
   //    or missing some of the required fields like `inet_group`

diff --git a/lib/preprocess.js b/lib/preprocess.js
@@ -230,14 +230,15 @@ export const parseParticipantAddress = filWalletAddress => {
 /**
  * @param {Measurement} measurement
  */
-const assertValidMeasurement = measurement => {
+export const assertValidMeasurement = measurement => {
   assert(
     typeof measurement === 'object' && measurement !== null,
     'object required'
   )
   assert(ethers.isAddress(measurement.participantAddress), 'valid participant address required')
   assert(typeof measurement.inet_group === 'string', 'valid inet group required')
   assert(typeof measurement.finished_at === 'number', 'field `finished_at` must be set to a number')
+  assert(measurement.indexerResult, 'field `indexerResult` must be set')
   if (measurement.stationId) {
     assert(
       typeof measurement.stationId === 'string' &&
@@ -256,9 +257,6 @@ export const getRetrievalResult = (measurement) => {
     case 'OK':
     case 'HTTP_NOT_ADVERTISED':
       break
-    case undefined:
-    case null:
-      return 'IPNI_NOT_QUERIED'
     default:
       return `IPNI_${measurement.indexer_result}`
   }

diff --git a/lib/typings.d.ts b/lib/typings.d.ts
@@ -41,7 +41,6 @@ export type FraudAssesment =
   | 'TASK_WRONG_NODE'
   | 'DUP_INET_GROUP'
   | 'TOO_MANY_TASKS'
-  | 'IPNI_NOT_QUERIED'
   | CommitteeCheckError
 
 
@@ -58,7 +57,6 @@ export type RetrievalResult =
   | 'CONTENT_VERIFICATION_FAILED'
   | 'UNEXPECTED_CAR_BLOCK'
   | 'CANNOT_PARSE_CAR_FILE'
-  | 'IPNI_NOT_QUERIED'
   | 'IPNI_NO_VALID_ADVERTISEMENT'
   | 'IPNI_ERROR_FETCH'
   | `IPNI_ERROR_${number}`
@@ -97,9 +95,7 @@ export interface RawMeasurement {
     | 'HTTP_NOT_ADVERTISED'
     | 'NO_VALID_ADVERTISEMENT'
     | 'ERROR_FETCH'
-    | `ERROR_${number}`
-    | undefined
-    | null;
+    | `ERROR_${number}`;
 }
 
 export type CreatePgClient = () => Promise<import('pg').Client>;
diff --git a/test/evaluate.js b/test/evaluate.js
@@ -699,45 +699,6 @@ describe('fraud detection', function () {
     )
   })
 
-  it('rejects measurements missing indexer result', async () => {
-    /** @type {RoundDetails} */
-    const sparkRoundDetails = {
-      ...SPARK_ROUND_DETAILS,
-      retrievalTasks: [
-        {
-          cid: VALID_MEASUREMENT.cid,
-          minerId: 'f1test'
-        }
-      ]
-    }
-
-    const measurements = [
-      {
-        ...VALID_MEASUREMENT,
-        inet_group: 'group1',
-        indexerResult: /** @type {const} */('OK')
-      },
-      {
-        ...VALID_MEASUREMENT,
-        inet_group: 'group2',
-        indexerResult: undefined
-      }
-    ]
-
-    await runFraudDetection({
-      roundIndex: 1n,
-      measurements,
-      sparkRoundDetails,
-      requiredCommitteeSize: 1,
-      logger
-    })
-
-    assert.deepStrictEqual(
-      measurements.map(m => m.fraudAssessment),
-      ['OK', 'IPNI_NOT_QUERIED']
-    )
-  })
-
   it('rejects tasks not allowed by the tasking algorithm', async () => {
     /** @type {RoundDetails} */
     const sparkRoundDetails = {

diff --git a/test/preprocess.js b/test/preprocess.js
@@ -1,9 +1,16 @@
-import { getRetrievalResult, parseParticipantAddress, preprocess, Measurement, parseMeasurements } from '../lib/preprocess.js'
+import {
+  getRetrievalResult,
+  parseParticipantAddress,
+  preprocess,
+  Measurement,
+  parseMeasurements,
+  assertValidMeasurement
+} from '../lib/preprocess.js'
 import { Point } from '../lib/telemetry.js'
 import assert from 'node:assert'
 import createDebug from 'debug'
 import { assertPointFieldValue, assertRecordedTelemetryPoint } from './helpers/assertions.js'
-import { VALID_STATION_ID } from './helpers/test-data.js'
+import { VALID_MEASUREMENT, VALID_STATION_ID } from './helpers/test-data.js'
 import { RoundData } from '../lib/round.js'
 
 const debug = createDebug('test')
@@ -27,6 +34,7 @@ describe('preprocess', () => {
       station_id: VALID_STATION_ID,
       spark_version: '1.2.3',
       inet_group: 'ig1',
+      indexer_result: 'OK',
       finished_at: '2023-11-01T09:00:00.000Z',
       first_byte_at: '2023-11-01T09:00:01.000Z',
       start_at: '2023-11-01T09:00:02.000Z',
@@ -46,6 +54,7 @@ describe('preprocess', () => {
         station_id: VALID_STATION_ID,
         spark_version: '1.2.3',
         inet_group: 'ig1',
+        indexer_result: 'OK',
         finished_at: '2023-11-01T09:00:00.000Z',
         first_byte_at: '2023-11-01T09:00:01.000Z',
         start_at: '2023-11-01T09:00:02.000Z',
@@ -207,22 +216,6 @@ describe('getRetrievalResult', () => {
     assert.strictEqual(result, 'UNKNOWN_ERROR')
   })
 
-  it('missing indexer result -> IPNI_NOT_QUERIED', () => {
-    const result = getRetrievalResult({
-      ...SUCCESSFUL_RETRIEVAL,
-      indexer_result: undefined
-    })
-    assert.strictEqual(result, 'IPNI_NOT_QUERIED')
-  })
-
-  it('indexer result is null -> IPNI_NOT_QUERIED', () => {
-    const result = getRetrievalResult({
-      ...SUCCESSFUL_RETRIEVAL,
-      indexer_result: null
-    })
-    assert.strictEqual(result, 'IPNI_NOT_QUERIED')
-  })
-
   it('IPNI HTTP_NOT_ADVERTISED -> OK', () => {
     const result = getRetrievalResult({
       ...SUCCESSFUL_RETRIEVAL,
@@ -305,3 +298,16 @@ describe('getRetrievalResult', () => {
     assert.strictEqual(result, 'CANNOT_PARSE_CAR_FILE')
   })
 })
+
+describe('assertValidMeasurement', () => {
+  it('rejects measurements where indexer_result is null', () => {
+    const m = {
+      ...VALID_MEASUREMENT,
+      indexerResult: null
+    }
+    assert.throws(
+      () => assertValidMeasurement(m),
+      /field `indexerResult` must be set/
+    )
+  })
+})