common,cli,agent: remove multi-network-mode #1090
Conversation
dwerner
force-pushed
the
fix/remove-multi-network-mode
branch
from
b236134 to
29c901e
Compare
dwerner
force-pushed
the
fix/remove-multi-network-mode
branch
from
1070468 to
531a773
Compare
dwerner
force-pushed
the
fix/remove-multi-network-mode
branch
from
531a773 to
5e72ec7
Compare
| logger.warn(`Failed to fetch max allocation epochs`, { error }), | ||
| }, | ||
| ) | ||
| const { network } = this.networkAndOperator |
There was a problem hiding this comment.
nit: could const { network, operator } = this.networkAndOperator be done once at the beginning of the function? then you don't need to get the network/operator again in each of these eventuals
| const { network } = this.networkAndOperator | ||
| if ( | ||
| this.deploymentManagement === DeploymentManagementMode.AUTO || | ||
| network.networkMonitor.poiDisputeMonitoringEnabled() |
There was a problem hiding this comment.
No so this was an optimization I made for the upgrade indexer which was querying all (some 25k) deployments from the graph node in this eventual, despite not needing to. This was causing some 30 mins (!) being spent on that request in some cases, blocking all other work.
Neither DeploymentManagementMode.AUTO or PoiDisputeMonitoring were enabled there so it made sense to block the call behind that. IIRC In a later testing session I found that when DeploymentManagementMode.AUTO was enabled, this still wasn't being hit due to the eventual in multiNetwork and networkMapped not having a value at the right time. So this is re-enabling the optimization to the narrower case, but now without the networkMapped hoop to jump through.
| logger.warn( | ||
| `Failed to merge L2 transfer decisions, trying again later`, | ||
| { | ||
| const networkDeploymentAllocationDecisions: Eventual<AllocationDecision[]> = |
There was a problem hiding this comment.
Love all the deleted code here. One thing that comes to mind, however: is it possible that the Upgrade Indexer is currently allocating to some subgraphs based on the L2 transfer conditions? Not sure how we could validate that, but it would be good to make sure this won't make us drop any subgraphs
There was a problem hiding this comment.
(Maybe this can be inferred from Upgrade Indexer logs?)
| }, | ||
| ) | ||
| const { network } = this.networkAndOperator | ||
| const allocationsByNetwork = |
There was a problem hiding this comment.
nit: allocationsByNetwork is probably not the best name for this now
| } | ||
| return allocationDecisions | ||
| const { network, operator } = this.networkAndOperator | ||
| let validatedAllocationDecisions = allocationDecisions |
There was a problem hiding this comment.
Should we clone to avoid mutation here?
| decision.deployment.bytes32 == networkSubgraphDeployment.id.bytes32, | ||
| ) | ||
| if (networkSubgraphIndex >= 0) { | ||
| allocationDecisions[networkSubgraphIndex].toAllocate = false |
There was a problem hiding this comment.
this is mutating allocationDecisions instead of validatedAllocationDecisions - it works because they're the same array by reference, but this looks risky
| operator: network.transactionManager.wallet.address, | ||
| const networkLogger = logger.child({ | ||
| protocolNetwork: this.network.specification.networkIdentifier, | ||
| indexer: this.network.specification.indexerOptions.address, |
There was a problem hiding this comment.
might it still be useful to log the operator address here?
|
|
||
| return queryAllocations(logger, networkSubgraph, variables, context) | ||
| }, | ||
| const allocationsByNetwork = await queryAllocations( |
There was a problem hiding this comment.
nit: same as above, allocationsByNetwork might be a confusing name? maybe just allocationsResult?
This PR removes multi-network mode while preserving the network config parsing logic that it was tied to. This enables a significant reduction in complexity.
Changes:
network-specifications-dirnetwork-specifications-diris provided then the values are used, otherwise single-network mode's style of pulling values from argv/env are used.