all: post review changes

packages/indexer-agent/src/agent.ts

@@ -44,12 +44,15 @@ import isEqual from 'lodash.isequal'
 import mapValues from 'lodash.mapvalues'
 import zip from 'lodash.zip'
 
-// Temporary marker used to signal that the following contract calls have been deprecated in the
-// network it appears:
+// The new Exponential Rebates for Indexes brought changes to the protocol contracts that deprecated
+// the following methods:
 // - channelDisputeEpochs
 // - claimRebateRewards
-// Once Exponential Rebates have been deployed to all networks, these calls should be removed from
-// the code.
+// This variable acts as a signal to other parts of the code, showing that we've ignored the results
+// of these calls early in the current process.
+// This is important because the Exponential Rebates aren't active on the mainnet yet, which still
+// uses their results. Once those contract changes have been deployed to all networks, these calls
+// can be removed from the code.
 const EXPONENTIAL_REBATES_MARKER = -1
 
 type ActionReconciliationContext = [AllocationDecision[], number, number]
@@ -246,12 +249,12 @@ export class Agent {
       },
     )
 
-    this.buildEventualTree()
+    this.reconciliationLoop()
     return this
   }
 
-  buildEventualTree() {
-    const logger = this.logger.child({ context: 'reconciliation-loop' })
+  reconciliationLoop() {
+    const logger = this.logger.child({ component: 'ReconciliationLoop' })
     const currentEpochNumber: Eventual<NetworkMapped<number>> = timer(
       600_000,
     ).tryMap(
@@ -315,7 +318,6 @@ export class Agent {
           return this.multiNetworks.map(async ({ network, operator }) => {
             logger.trace('Fetching indexing rules', {
               protocolNetwork: network.specification.networkIdentifier,
-              context: 'reconciliation-loop',
             })
             let rules = await operator.indexingRules(true)
             const subgraphRuleIds = rules
@@ -1007,7 +1009,7 @@ export class Agent {
       deploymentIDSet(targetDeployments),
     )
     if (isReconciliationNeeded) {
-      // QUESTION: should we return early in here case reconciliation is not needed?
+      // TODO: Return early in here case reconciliation is not needed
       this.logger.debug('Reconcile deployments', {
         syncing: activeDeployments.map(id => id.display),
         target: targetDeployments.map(id => id.display),
@@ -1027,8 +1029,9 @@ export class Agent {
         !deploymentInList(eligibleAllocationDeployments, deployment),
     )
 
-    // QUESTION: Same as before: should we return early in here case reconciliation is
-    // not needed?
+    // TODO: Same as before: Should we return early in here case reconciliation is not needed? Also,
+    // this conditional seems the be doing the same work as the previous one, we should consolidate
+    // them.
     if (deploy.length + remove.length !== 0) {
       this.logger.info('Deployment changes', {
         deploy: deploy.map(id => id.display),
@@ -1130,7 +1133,7 @@ export class Agent {
       epoch,
     })
 
-    // QUESTION: Can we replace `filter` for `find` here? Is there such a case when we
+    // TODO: Can we replace `filter` for `find` here? Is there such a case when we
     // would have multiple allocations for the same subgraph?
     const activeDeploymentAllocations = activeAllocations.filter(
       allocation =>
@@ -1182,8 +1185,6 @@ export class Agent {
     }
   }
 
-  // QUESTION: the `activeAllocations` parameter is used only for logging. Should we
-  // remove it from this function?
   async reconcileActions(
     networkDeploymentAllocationDecisions: NetworkMapped<AllocationDecision[]>,
     epoch: NetworkMapped<number>,

packages/indexer-agent/src/commands/common-options.ts

@@ -132,7 +132,7 @@ export function injectCommonStartupOptions(argv: Argv): Argv {
     .check(argv => {
       // Unset arguments set to empty strings.
       // This can happen when users set their options as enviroment variables and don't
-      // assign any vaule to them.
+      // assign any value to them.
       //
       // For example:
       // ```sh

packages/indexer-agent/src/commands/start.ts

@@ -371,9 +371,6 @@ export async function createNetworkSpecification(
   }
 }
 
-// TODO: Split this code into two functions:
-// 1. [X] Create NetworkSpecification
-// 2. [ ] Start Agent with NetworkSpecification as input.
 export async function run(
   argv: AgentOptions,
   networkSpecifications: spec.NetworkSpecification[],
@@ -519,7 +516,9 @@ export async function run(
     multiNetworks,
   })
 
+  // --------------------------------------------------------------------------------
   // * Indexer Management Server
+  // --------------------------------------------------------------------------------
   logger.info('Launch indexer management API server', {
     port: argv.indexerManagementPort,
   })

packages/indexer-agent/src/db/migrations/11-add-protocol-network-field.ts

@@ -18,7 +18,6 @@ interface ForeignKey {
   table: string
   // This table's column that holds the FK
   columnName: string
-  // Could also be composite, but we don't have any of that kind at this moment
   refColumnName: string
 }
 

packages/indexer-cli/package.json

@@ -42,10 +42,12 @@
   },
   "devDependencies": {
     "@types/isomorphic-fetch": "0.0.36",
+    "@types/lodash.clonedeep": "^4.5.7",
     "@typescript-eslint/eslint-plugin": "5.19.0",
     "@typescript-eslint/parser": "5.19.0",
     "eslint": "8.13.0",
     "eslint-config-prettier": "8.5.0",
+    "lodash.clonedeep": "^4.5.0",
     "prettier": "2.6.2",
     "ts-jest": "27.1.4",
     "typescript": "4.6.3"

packages/indexer-cli/src/__tests__/util.ts

@@ -28,6 +28,7 @@ import {
   Logger,
   parseGRT,
 } from '@graphprotocol/common-ts'
+import cloneDeep from 'lodash.clonedeep'
 
 const INDEXER_SAVE_CLI_TEST_OUTPUT: boolean =
   !!process.env.INDEXER_SAVE_CLI_TEST_OUTPUT &&
@@ -119,8 +120,11 @@ export const setup = async () => {
     metrics,
   )
 
+  const fakeMainnetNetwork = cloneDeep(network) as Network
+  fakeMainnetNetwork.specification.networkIdentifier = 'eip155:1'
+
   const multiNetworks = new MultiNetworks(
-    [network],
+    [network, fakeMainnetNetwork],
     (n: Network) => n.specification.networkIdentifier,
   )
 

packages/indexer-common/src/indexer-management/__tests__/resolvers/cost-models.test.ts

@@ -92,7 +92,13 @@ const setupAll = async () => {
     level: __LOG_LEVEL__ ?? 'error',
   })
 
-  client = await createTestManagementClient(__DATABASE__, logger, true, metrics)
+  client = await createTestManagementClient(
+    __DATABASE__,
+    logger,
+    true,
+    metrics,
+    'eip155:1', // Override with mainnet to enable the Cost Model feature
+  )
 }
 
 const teardownAll = async () => {
@@ -718,6 +724,7 @@ describe('Feature: Inject $DAI variable', () => {
       logger,
       false,
       metrics,
+      'eip155:1', // Override with mainnet to enable the Cost Model feature
     )
 
     const initial = {

packages/indexer-common/src/indexer-management/__tests__/util.ts

@@ -60,6 +60,7 @@ export const createTestManagementClient = async (
   logger: Logger,
   injectDai: boolean,
   metrics: Metrics,
+  networkIdentifierOverride?: string,
 ): Promise<IndexerManagementClient> => {
   // Clearing the registry prevents duplicate metric registration in the default registry.
   metrics.registry.clear()
@@ -100,6 +101,10 @@ export const createTestManagementClient = async (
     metrics,
   )
 
+  if (networkIdentifierOverride) {
+    network.specification.networkIdentifier = networkIdentifierOverride
+  }
+
   const multiNetworks = new MultiNetworks(
     [network],
     (n: Network) => n.specification.networkIdentifier,

packages/indexer-common/src/indexer-management/client.ts

@@ -377,7 +377,7 @@ const SCHEMA_SDL = gql`
     indexerRegistration(protocolNetwork: String!): IndexerRegistration!
     indexerDeployments: [IndexerDeployment]!
     indexerAllocations(protocolNetwork: String!): [IndexerAllocation]!
-    indexerEndpoints(protocolNetwork: String!): [IndexerEndpoints!]!
+    indexerEndpoints(protocolNetwork: String): [IndexerEndpoints!]!
 
     costModels(deployments: [String!]): [CostModel!]!
     costModel(deployment: String!): CostModel

packages/indexer-common/src/indexer-management/resolvers/allocations.ts

@@ -398,7 +398,15 @@ export default {
         const [currentEpoch, disputeEpochs, maxAllocationEpochs, epochLength] =
           await Promise.all([
             networkMonitor.networkCurrentEpoch(),
-            contracts.staking.channelDisputeEpochs(),
+            contracts.staking.channelDisputeEpochs().catch((error) => {
+              logger.warn(
+                'Failed to fetch channel dispute epochs. Ignoring claimable allocations',
+                { error, protocolNetwork: network.specification.networkIdentifier },
+              )
+              // Remove this call to channelDisputeEpochs once Exponential Rebates are deployed to
+              // all networks. Using a default value of zero on failure for now.
+              return 0
+            }),
             contracts.staking.maxAllocationEpochs(),
             contracts.epochManager.epochLength(),
           ])

packages/indexer-common/src/indexer-management/resolvers/indexer-status.ts

@@ -4,7 +4,12 @@ import geohash from 'ngeohash'
 import gql from 'graphql-tag'
 import { IndexerManagementResolverContext } from '../client'
 import { SubgraphDeploymentID } from '@graphprotocol/common-ts'
-import { indexerError, IndexerErrorCode, Network } from '@graphprotocol/indexer-common'
+import {
+  indexerError,
+  IndexerErrorCode,
+  Network,
+  validateNetworkIdentifier,
+} from '@graphprotocol/indexer-common'
 import { extractNetwork } from './utils'
 interface Test {
   test: (url: string) => string
@@ -57,7 +62,7 @@ const URL_VALIDATION_TEST: Test = {
 
 export default {
   indexerRegistration: async (
-    { protocolNetwork: unvalidateProtocolNetwork }: { protocolNetwork: string },
+    { protocolNetwork: unvalidatedProtocolNetwork }: { protocolNetwork: string },
     { multiNetworks }: IndexerManagementResolverContext,
   ): Promise<object | null> => {
     if (!multiNetworks) {
@@ -66,7 +71,7 @@ export default {
       )
     }
 
-    const network = extractNetwork(unvalidateProtocolNetwork, multiNetworks)
+    const network = extractNetwork(unvalidatedProtocolNetwork, multiNetworks)
     const protocolNetwork = network.specification.networkIdentifier
     const address = network.specification.indexerOptions.address
     const contracts = network.contracts
@@ -163,7 +168,7 @@ export default {
   },
 
   indexerEndpoints: async (
-    { protocolNetwork }: { protocolNetwork: string },
+    { protocolNetwork: unvalidatedProtocolNetwork }: { protocolNetwork: string | null },
     { multiNetworks, logger }: IndexerManagementResolverContext,
   ): Promise<Endpoints[] | null> => {
     if (!multiNetworks) {
@@ -172,7 +177,27 @@ export default {
       )
     }
     const endpoints: Endpoints[] = []
+    let networkIdentifier: string | null = null
+
+    // Validate protocol network
+    try {
+      if (unvalidatedProtocolNetwork) {
+        networkIdentifier = validateNetworkIdentifier(unvalidatedProtocolNetwork)
+      }
+    } catch (parseError) {
+      throw new Error(
+        `Invalid protocol network identifier: '${unvalidatedProtocolNetwork}'. Error: ${parseError}`,
+      )
+    }
+
     await multiNetworks.map(async (network: Network) => {
+      // Skip if this query asks for another protocol network
+      if (
+        networkIdentifier &&
+        networkIdentifier !== network.specification.networkIdentifier
+      ) {
+        return
+      }
       try {
         const networkEndpoints = await endpointForNetwork(network)
         endpoints.push(networkEndpoints)

packages/indexer-service/src/commands/start.ts

@@ -39,7 +39,6 @@ export default {
   builder: (yargs: Argv): Argv => {
     return yargs
       .option('network-provider', {
-        //TODO:FIXME: uptrade this field as we did for Agent
         alias: 'ethereum',
         description: 'Ethereum node or provider URL',
         type: 'string',

yarn.lock

@@ -2828,6 +2828,13 @@
   resolved "https://registry.npmjs.org/@types/json5/-/json5-0.0.29.tgz"
   integrity sha512-dRLjCWHYg4oaA77cxO64oO+7JwCwnIzkZPdrrC71jQmQtlhM556pwKo5bUzqvZndkVbeFLIIi+9TC40JNF5hNQ==
 
+"@types/lodash.clonedeep@^4.5.7":
+  version "4.5.7"
+  resolved "https://registry.npmjs.org/@types/lodash.clonedeep/-/lodash.clonedeep-4.5.7.tgz#0e119f582ed6f9e6b373c04a644651763214f197"
+  integrity sha512-ccNqkPptFIXrpVqUECi60/DFxjNKsfoQxSQsgcBJCX/fuX1wgyQieojkcWH/KpE3xzLoWN/2k+ZeGqIN3paSvw==
+  dependencies:
+    "@types/lodash" "*"
+
 "@types/lodash.countby@^4.6.7":
   version "4.6.7"
   resolved "https://registry.npmjs.org/@types/lodash.countby/-/lodash.countby-4.6.7.tgz"
@@ -7582,6 +7589,11 @@ lodash.camelcase@^4.3.0:
   resolved "https://registry.npmjs.org/lodash.camelcase/-/lodash.camelcase-4.3.0.tgz"
   integrity sha1-soqmKIorn8ZRA1x3EfZathkDMaY=
 
+lodash.clonedeep@^4.5.0:
+  version "4.5.0"
+  resolved "https://registry.npmjs.org/lodash.clonedeep/-/lodash.clonedeep-4.5.0.tgz#e23f3f9c4f8fbdde872529c1071857a086e5ccef"
+  integrity sha512-H5ZhCF25riFd9uB5UCkVKo61m3S/xZk1x4wA6yp/L3RFP6Z/eHH1ymQcGLo7J3GMPfm0V/7m1tryHuGVxpqEBQ==
+
 lodash.groupby@^4.6.0:
   version "4.6.0"
   resolved "https://registry.npmjs.org/lodash.groupby/-/lodash.groupby-4.6.0.tgz"