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

fix(order-tracking): sfn max attemmpts increase; sfn execution arn naming tweak #501

Merged
merged 3 commits into from
Dec 20, 2024
Merged
Show file tree
Hide file tree
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
2 changes: 1 addition & 1 deletion bin/definitions/order-tracking-sfn.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
"Lambda.SdkClientException"
],
"IntervalSeconds": 2,
"MaxAttempts": 4,
"MaxAttempts": 100,
"BackoffRate": 2
}
],
Expand Down
2 changes: 1 addition & 1 deletion bin/stacks/step-function-stack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ export class StepFunctionStack extends cdk.NestedStack {
},
timeout: Duration.seconds(60),
environment: {
VERSION: '2',
VERSION: '3',
NODE_OPTIONS: '--enable-source-maps',
...props.envVars,
stage: stage,
Expand Down
6 changes: 4 additions & 2 deletions lib/handlers/check-order-status/handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ export class CheckOrderStatusHandler extends SfnLambdaHandler<ContainerInjected,
requestInjected: RequestInjected
}): Promise<SfnStateInputOutput> {
//make sure to change "Variable": "$.retryCount", in order-tracking-sfn.json to be 1+retryCount
if (input.requestInjected?.retryCount > 300) {
const retryCount = input.requestInjected?.retryCount ?? 0
if (retryCount > 300) {
const stateMachineArn = input.requestInjected.stateMachineArn
await kickoffOrderTrackingSfn(
{
Expand All @@ -36,7 +37,8 @@ export class CheckOrderStatusHandler extends SfnLambdaHandler<ContainerInjected,
orderType: input.requestInjected.orderType,
stateMachineArn: input.requestInjected.stateMachineArn,
},
stateMachineArn
stateMachineArn,
retryCount
)
powertoolsMetric
.singleMetric()
Expand Down
13 changes: 6 additions & 7 deletions lib/handlers/shared/sfn.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,19 +13,18 @@ export type OrderTrackingSfnInput = {
stateMachineArn: string
}

//TODO: remove random for sfn name
//this is to stop collisions in the running step function name
const BIG_NUMBER = 1000000000000

export async function kickoffOrderTrackingSfn(sfnInput: OrderTrackingSfnInput, stateMachineArn: string) {
export async function kickoffOrderTrackingSfn(
sfnInput: OrderTrackingSfnInput,
stateMachineArn: string,
retryCount = 0
) {
log.info('starting state machine')
const region = checkDefined(process.env['REGION'], 'REGION is undefined')
const sfnClient = new SFNClient({ region: region })
const rand = Math.floor(Math.random() * BIG_NUMBER)
const startExecutionCommand = new StartExecutionCommand({
stateMachineArn: stateMachineArn,
input: JSON.stringify(sfnInput),
name: sfnInput.orderHash + '_' + rand,
name: sfnInput.orderHash + '_' + retryCount,
})
log.info('Starting state machine execution', { startExecutionCommand })
await sfnClient.send(startExecutionCommand)
Expand Down
7 changes: 4 additions & 3 deletions lib/services/UniswapXOrderService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@ import { KmsSigner } from '@uniswap/signer'
import {
CosignedPriorityOrder,
CosignedV2DutchOrder,
CosignedV3DutchOrder,
DutchOrder,
OrderType,
OrderValidation,
OrderValidator as OnChainOrderValidator,
CosignedV3DutchOrder,
} from '@uniswap/uniswapx-sdk'
import { ethers } from 'ethers'
import { ORDER_STATUS, UniswapXOrderEntity } from '../entities'
Expand All @@ -17,6 +17,7 @@ import { OrderValidationFailedError } from '../errors/OrderValidationFailedError
import { TooManyOpenOrdersError } from '../errors/TooManyOpenOrdersError'
import { GetOrdersQueryParams } from '../handlers/get-orders/schema'
import { GetDutchV2OrderResponse } from '../handlers/get-orders/schema/GetDutchV2OrderResponse'
import { GetDutchV3OrderResponse } from '../handlers/get-orders/schema/GetDutchV3OrderResponse'
import { GetOrdersResponse } from '../handlers/get-orders/schema/GetOrdersResponse'
import { GetPriorityOrderResponse } from '../handlers/get-orders/schema/GetPriorityOrderResponse'
import { OnChainValidatorMap } from '../handlers/OnChainValidatorMap'
Expand All @@ -32,7 +33,6 @@ import { BaseOrdersRepository } from '../repositories/base'
import { OffChainUniswapXOrderValidator } from '../util/OffChainUniswapXOrderValidator'
import { DUTCH_LIMIT, formatOrderEntity } from '../util/order'
import { AnalyticsServiceInterface } from './analytics-service'
import { GetDutchV3OrderResponse } from '../handlers/get-orders/schema/GetDutchV3OrderResponse'
const MAX_QUERY_RETRY = 10

export class UniswapXOrderService {
Expand Down Expand Up @@ -174,7 +174,8 @@ export class UniswapXOrderService {
orderType,
stateMachineArn,
},
stateMachineArn
stateMachineArn,
0
)
}

Expand Down
3 changes: 2 additions & 1 deletion test/unit/handlers/post-order/post-limit-order.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,8 @@ describe('Testing post limit order handler.', () => {
expect(validatorMock).toBeCalledWith(order)
expect(kickoffOrderTrackingSfn).toHaveBeenCalledWith(
expect.objectContaining({ orderType: 'Limit' }),
expect.anything()
expect.anything(),
0
)
expect(postOrderResponse).toEqual({
body: JSON.stringify({ hash: expectedOrderEntity.orderHash }),
Expand Down
42 changes: 31 additions & 11 deletions test/unit/services/UniswapXOrderService.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,21 +6,21 @@ import { ORDER_STATUS, UniswapXOrderEntity } from '../../../lib/entities'
import { OnChainValidatorMap } from '../../../lib/handlers/OnChainValidatorMap'
import { kickoffOrderTrackingSfn } from '../../../lib/handlers/shared/sfn'
import { DutchV1Order, DutchV2Order } from '../../../lib/models'
import { DutchV3Order } from '../../../lib/models/DutchV3Order'
import { LimitOrder } from '../../../lib/models/LimitOrder'
import { PriorityOrder } from '../../../lib/models/PriorityOrder'
import { BaseOrdersRepository } from '../../../lib/repositories/base'
import { AnalyticsService } from '../../../lib/services/analytics-service'
import { UniswapXOrderService } from '../../../lib/services/UniswapXOrderService'
import { ChainId } from '../../../lib/util/chain'
import { OffChainUniswapXOrderValidator } from '../../../lib/util/OffChainUniswapXOrderValidator'
import { DUTCH_LIMIT } from '../../../lib/util/order'
import { SDKDutchOrderFactory } from '../../factories/SDKDutchOrderV1Factory'
import { SDKDutchOrderV2Factory } from '../../factories/SDKDutchOrderV2Factory'
import { SDKDutchOrderV3Factory } from '../../factories/SDKDutchOrderV3Factory'
import { SDKPriorityOrderFactory } from '../../factories/SDKPriorityOrderFactory'
import { QueryParamsBuilder } from '../builders/QueryParamsBuilder'
import { COSIGNATURE, MOCK_PROVIDER_MAP } from '../fixtures'
import { DutchV3Order } from '../../../lib/models/DutchV3Order'
import { ChainId } from '../../../lib/util/chain'
import { SDKDutchOrderV3Factory } from '../../factories/SDKDutchOrderV3Factory'

jest.mock('../../../lib/handlers/shared/sfn', () => {
return { kickoffOrderTrackingSfn: jest.fn() }
Expand Down Expand Up @@ -81,7 +81,8 @@ describe('UniswapXOrderService', () => {
quoteId: '',
stateMachineArn: undefined,
},
undefined
undefined,
0
)
})

Expand Down Expand Up @@ -131,7 +132,8 @@ describe('UniswapXOrderService', () => {
quoteId: '',
stateMachineArn: undefined,
},
undefined
undefined,
0
)
})

Expand Down Expand Up @@ -615,9 +617,10 @@ describe('UniswapXOrderService', () => {
)
})


test('getDutchV3Orders calls db with Dutch_V3', async () => {
const dutchV3Orders = [1, 2, 3].map(() => new DutchV3Order(SDKDutchOrderV3Factory.buildDutchV3Order(ChainId.ARBITRUM_ONE), '', ChainId.ARBITRUM_ONE))
const dutchV3Orders = [1, 2, 3].map(
() => new DutchV3Order(SDKDutchOrderV3Factory.buildDutchV3Order(ChainId.ARBITRUM_ONE), '', ChainId.ARBITRUM_ONE)
)
const mockOrder = dutchV3Orders.map((o) => o.toEntity(ORDER_STATUS.OPEN))
const repository = mock<BaseOrdersRepository<UniswapXOrderEntity>>()
repository.getOrdersFilteredByType.mockResolvedValue({ orders: mockOrder })
Expand All @@ -636,7 +639,12 @@ describe('UniswapXOrderService', () => {
)

const limit = 50
const params = new QueryParamsBuilder().withDesc().withSort().withSortKey().withChainId(ChainId.ARBITRUM_ONE).build()
const params = new QueryParamsBuilder()
.withDesc()
.withSort()
.withSortKey()
.withChainId(ChainId.ARBITRUM_ONE)
.build()
const response = await service.getDutchV3Orders(limit, params, undefined)
const expectedResponse = {
orders: mockOrder.map((o) => DutchV3Order.fromEntity(o).toGetResponse()),
Expand All @@ -655,7 +663,9 @@ describe('UniswapXOrderService', () => {
})

test('getDutchV3Orders loops with empty response', async () => {
const dutchV3Orders = [1, 2, 3].map(() => new DutchV3Order(SDKDutchOrderV3Factory.buildDutchV3Order(ChainId.ARBITRUM_ONE), '', ChainId.ARBITRUM_ONE))
const dutchV3Orders = [1, 2, 3].map(
() => new DutchV3Order(SDKDutchOrderV3Factory.buildDutchV3Order(ChainId.ARBITRUM_ONE), '', ChainId.ARBITRUM_ONE)
)
const mockOrder = dutchV3Orders.map((o) => o.toEntity(ORDER_STATUS.OPEN))
const repository = mock<BaseOrdersRepository<UniswapXOrderEntity>>()
repository.getOrdersFilteredByType.mockResolvedValueOnce({ orders: [], cursor: 'cursor' })
Expand All @@ -675,7 +685,12 @@ describe('UniswapXOrderService', () => {
)

const limit = 50
const params = new QueryParamsBuilder().withDesc().withSort().withSortKey().withChainId(ChainId.ARBITRUM_ONE).build()
const params = new QueryParamsBuilder()
.withDesc()
.withSort()
.withSortKey()
.withChainId(ChainId.ARBITRUM_ONE)
.build()
const response = await service.getDutchV3Orders(limit, params, undefined)
const expectedResponse = {
orders: mockOrder.map((o) => DutchV3Order.fromEntity(o).toGetResponse()),
Expand Down Expand Up @@ -717,7 +732,12 @@ describe('UniswapXOrderService', () => {
)

const limit = 50
const params = new QueryParamsBuilder().withDesc().withSort().withSortKey().withChainId(ChainId.ARBITRUM_ONE).build()
const params = new QueryParamsBuilder()
.withDesc()
.withSort()
.withSortKey()
.withChainId(ChainId.ARBITRUM_ONE)
.build()
const response = await service.getDutchV3Orders(limit, params, undefined)

expect(response).toEqual({ orders: [], cursor: 'cursor' })
Expand Down
14 changes: 7 additions & 7 deletions test/unit/util/order-validator.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -348,18 +348,18 @@ describe('Testing off chain validation', () => {
describe('Testing v3 order validation', () => {
it('Should return valid', () => {
const order = SDKDutchOrderV3Factory.buildDutchV3Order(ChainId.ARBITRUM_ONE, {
cosigner: process.env.LABS_COSIGNER
cosigner: process.env.LABS_COSIGNER,
})
order.info.deadline = CURRENT_TIME + ONE_DAY
const validationResp = validationProvider.validate(order)
expect(validationResp).toEqual({
valid: true
valid: true,
})
})

it('Should throw invalid deadline', () => {
const order = SDKDutchOrderV3Factory.buildDutchV3Order(ChainId.ARBITRUM_ONE, {
cosigner: process.env.LABS_COSIGNER
cosigner: process.env.LABS_COSIGNER,
})
const validationResp = validationProvider.validate(order)
expect(validationResp).toEqual({
Expand All @@ -376,7 +376,7 @@ describe('Testing off chain validation', () => {
const validationResp = validationProvider.validate(order)
expect(validationResp).toEqual({
valid: false,
errorString: 'Invalid cosigner: ValidationError: "value" must be [0x0000000000000000000000000000000000000000]',
errorString: expect.stringContaining('Invalid cosigner: ValidationError: "value" must be'),
})
})

Expand Down Expand Up @@ -449,7 +449,7 @@ describe('Testing off chain validation', () => {
relativeAmounts: [BigInt(1000000000000000000)],
},
},
]
],
})
order.info.deadline = CURRENT_TIME + ONE_DAY
// Set to be empty
Expand Down Expand Up @@ -529,9 +529,9 @@ describe('Testing off chain validation', () => {
outputs: [
{
startAmount: BigNumber.from(100),
minAmount: BigNumber.from(100),
minAmount: BigNumber.from(100),
},
]
],
})
// Override with less than the startAmount
order.info.cosignerData.outputOverrides = [BigNumber.from('99')]
Expand Down
Loading