Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

common: Reduce stringency of action validation when adding to queue #833

Merged
merged 1 commit into from
Dec 12, 2023
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
common: Reduce stringency of action validation when adding to queue
- Specifically: remove the check that the deployment is published to the
network to avoid race conditions waiting for the network subgraph to
update
  • Loading branch information
fordN committed Dec 12, 2023
commit 3dc19f88d0a9e5345c7dae4ae0a4768f7b6d889c
4 changes: 3 additions & 1 deletion packages/indexer-common/src/actions.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { NetworkMonitor } from './indexer-management'
import { AllocationStatus } from './allocations'
import { Logger } from '@graphprotocol/common-ts'
import { WhereOperators, WhereOptions } from 'sequelize'
import { Op } from 'sequelize'
import { WhereAttributeHashValue } from 'sequelize/types/model'
@@ -81,6 +82,7 @@ export const isValidActionInput = (
export const validateActionInputs = async (
actions: ActionInput[],
networkMonitor: NetworkMonitor,
logger: Logger,
): Promise<void> => {
// Validate actions before adding to queue
// TODO: Perform all checks simultaneously and throw combined error if 1 or more fail
@@ -125,7 +127,7 @@ export const validateActionInputs = async (
action.deploymentID,
)
if (!subgraphDeployment) {
throw new Error(
logger.warn(
`No subgraphDeployment with ipfsHash = '${action.deploymentID}' found on the network`,
)
}
Original file line number Diff line number Diff line change
@@ -28,15 +28,13 @@ import {
import { CombinedError } from '@urql/core'
import { GraphQLError } from 'graphql'
import {
allocateToNotPublishedDeployment,
createTestManagementClient,
invalidReallocateAction,
invalidUnallocateAction,
queuedAllocateAction,
subgraphDeployment1,
subgraphDeployment2,
subgraphDeployment3,
notPublishedSubgraphDeployment,
} from '../util'

const QUEUE_ACTIONS_MUTATION = gql`
@@ -632,23 +630,6 @@ describe('Actions', () => {
).resolves.toHaveProperty('data.actions', [expectedUpdated])
})

test('Reject action with deployment not on network', async () => {
const inputActions = [allocateToNotPublishedDeployment]

await expect(
client.mutation(QUEUE_ACTIONS_MUTATION, { actions: inputActions }).toPromise(),
).resolves.toHaveProperty(
'error',
new CombinedError({
graphQLErrors: [
new GraphQLError(
`No subgraphDeployment with ipfsHash = '${notPublishedSubgraphDeployment}' found on the network`,
),
],
}),
)
})

test('Reject unallocate action with inactive allocationID', async () => {
// This allocation has been closed on chain
const closedAllocation = '0x0001572b5fde192fc1c65630fabb5e13d3ad173e'
14 changes: 0 additions & 14 deletions packages/indexer-common/src/indexer-management/__tests__/util.ts
Original file line number Diff line number Diff line change
@@ -132,8 +132,6 @@ export const defaults: IndexerManagementDefaults = {
export const subgraphDeployment1 = 'Qmew9PZUJCoDzXqqU6vGyTENTKHrrN4dy5h94kertfudqy'
export const subgraphDeployment2 = 'QmWq1pmnhEvx25qxpYYj9Yp6E1xMKMVoUjXVQBxUJmreSe'
export const subgraphDeployment3 = 'QmRhH2nhNibDVPZmYqq3TUZZARZ77vgjYCvPNiGBCogtgM'
export const notPublishedSubgraphDeployment =
'QmeqJ6hsdyk9dVbo1tvRgAxWrVS3rkERiEMsxzPShKLco6'

export const queuedAllocateAction = {
status: ActionStatus.QUEUED,
@@ -147,18 +145,6 @@ export const queuedAllocateAction = {
protocolNetwork: 'goerli',
} as ActionInput

export const allocateToNotPublishedDeployment = {
status: ActionStatus.QUEUED,
type: ActionType.ALLOCATE,
deploymentID: notPublishedSubgraphDeployment,
amount: '10000',
force: false,
source: 'indexerAgent',
reason: 'indexingRule',
priority: 0,
protocolNetwork: 'goerli',
} as ActionInput

export const invalidUnallocateAction = {
status: ActionStatus.QUEUED,
type: ActionType.UNALLOCATE,
Original file line number Diff line number Diff line change
@@ -173,7 +173,7 @@ export default {
await multiNetworks.mapNetworkMapped(
groupBy(actions, (action) => action.protocolNetwork),
(network: Network, actions: ActionInput[]) =>
validateActionInputs(actions, network.networkMonitor),
validateActionInputs(actions, network.networkMonitor, logger),
)

const alreadyQueuedActions = await ActionManager.fetchActions(models, {