Skip to content

Commit

Permalink
fix: do not update orders with no status change
Browse files Browse the repository at this point in the history
  • Loading branch information
zzmp committed Jan 22, 2024
1 parent beaabbd commit cb9e000
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 42 deletions.
25 changes: 22 additions & 3 deletions lib/handlers/check-order-status/handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ export class CheckOrderStatusHandler extends SfnLambdaHandler<ContainerInjected,
provider,
orderWatcher,
orderQuoter,
orderStatus,
} = input.requestInjected

const order = checkDefined(
Expand Down Expand Up @@ -96,7 +97,9 @@ export class CheckOrderStatusHandler extends SfnLambdaHandler<ContainerInjected,
gasCostInETH,
receipt.effectiveGasPrice.toString(),
receipt.gasUsed.toString(),
settledAmounts.reduce((prev, cur) => (prev && BigNumber.from(prev.amountOut).gt(cur.amountOut) ? prev : cur))
settledAmounts.reduce((prev, cur) =>
prev && BigNumber.from(prev.amountOut).gt(cur.amountOut) ? prev : cur
)
)

const percentDecayed = (timestamp - order.decayStartTime) / (order.decayEndTime - order.decayStartTime)
Expand All @@ -114,6 +117,7 @@ export class CheckOrderStatusHandler extends SfnLambdaHandler<ContainerInjected,
retryCount,
startingBlockNumber: fromBlock,
chainId,
lastStatus: orderStatus,
orderStatus: ORDER_STATUS.FILLED,
txHash: fillEvent.txHash,
settledAmounts,
Expand All @@ -140,6 +144,7 @@ export class CheckOrderStatusHandler extends SfnLambdaHandler<ContainerInjected,
retryCount,
startingBlockNumber: fromBlock,
chainId,
lastStatus: orderStatus,
// if there are no fill logs, retry one more time in case of node syncing issues
orderStatus: getFillLogAttempts == 0 ? ORDER_STATUS.OPEN : ORDER_STATUS.EXPIRED,
getFillLogAttempts: getFillLogAttempts + 1,
Expand All @@ -158,6 +163,7 @@ export class CheckOrderStatusHandler extends SfnLambdaHandler<ContainerInjected,
retryCount,
startingBlockNumber: fromBlock,
chainId,
lastStatus: orderStatus,
orderStatus: ORDER_STATUS.INSUFFICIENT_FUNDS,
validation,
},
Expand All @@ -174,6 +180,7 @@ export class CheckOrderStatusHandler extends SfnLambdaHandler<ContainerInjected,
retryCount,
startingBlockNumber: fromBlock,
chainId,
lastStatus: orderStatus,
orderStatus: ORDER_STATUS.ERROR,
validation,
},
Expand All @@ -198,7 +205,9 @@ export class CheckOrderStatusHandler extends SfnLambdaHandler<ContainerInjected,
gasCostInETH,
receipt.effectiveGasPrice.toString(),
receipt.gasUsed.toString(),
settledAmounts.reduce((prev, cur) => (prev && BigNumber.from(prev.amountOut).gt(cur.amountOut) ? prev : cur))
settledAmounts.reduce((prev, cur) =>
prev && BigNumber.from(prev.amountOut).gt(cur.amountOut) ? prev : cur
)
)

const percentDecayed =
Expand All @@ -219,6 +228,7 @@ export class CheckOrderStatusHandler extends SfnLambdaHandler<ContainerInjected,
retryCount,
startingBlockNumber: fromBlock,
chainId,
lastStatus: orderStatus,
orderStatus: ORDER_STATUS.FILLED,
txHash: fillEvent.txHash,
settledAmounts,
Expand All @@ -243,6 +253,7 @@ export class CheckOrderStatusHandler extends SfnLambdaHandler<ContainerInjected,
retryCount,
startingBlockNumber: fromBlock,
chainId,
lastStatus: orderStatus,
// if there are no fill logs, retry one more time in case of node syncing issues
orderStatus: getFillLogAttempts == 0 ? ORDER_STATUS.OPEN : ORDER_STATUS.CANCELLED,
getFillLogAttempts: getFillLogAttempts + 1,
Expand All @@ -261,6 +272,7 @@ export class CheckOrderStatusHandler extends SfnLambdaHandler<ContainerInjected,
retryCount,
startingBlockNumber: fromBlock,
chainId,
lastStatus: orderStatus,
orderStatus: ORDER_STATUS.OPEN,
validation,
},
Expand All @@ -277,6 +289,7 @@ export class CheckOrderStatusHandler extends SfnLambdaHandler<ContainerInjected,
retryCount: number
startingBlockNumber: number
chainId: number
lastStatus: ORDER_STATUS
orderStatus: ORDER_STATUS
validation: OrderValidation
txHash?: string
Expand All @@ -292,6 +305,7 @@ export class CheckOrderStatusHandler extends SfnLambdaHandler<ContainerInjected,
retryCount,
startingBlockNumber,
chainId,
lastStatus,
orderStatus,
txHash,
settledAmounts,
Expand All @@ -306,14 +320,19 @@ export class CheckOrderStatusHandler extends SfnLambdaHandler<ContainerInjected,
retryCount,
startingBlockNumber,
chainId,
lastStatus,
orderStatus,
txHash,
settledAmounts,
getFillLogAttempts,
},
'updating order status'
)
await dbInterface.updateOrderStatus(orderHash, orderStatus, txHash, settledAmounts)
// Avoid updating the order if the status is unchanged.
// This also avoids unnecessarily triggering downstream events from dynamodb changes.
if (orderStatus !== lastStatus) {
await dbInterface.updateOrderStatus(orderHash, orderStatus, txHash, settledAmounts)
}

if (IS_TERMINAL_STATE(orderStatus)) {
metrics.putMetric(`OrderSfn-${orderStatus}`, 1)
Expand Down
7 changes: 4 additions & 3 deletions lib/handlers/check-order-status/injector.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,19 @@ import { MetricsLogger } from 'aws-embedded-metrics'
import { DynamoDB } from 'aws-sdk'
import { default as bunyan, default as Logger } from 'bunyan'
import { ethers } from 'ethers'
import { ORDER_STATUS } from '../../entities'
import { checkDefined } from '../../preconditions/preconditions'
import { BaseOrdersRepository } from '../../repositories/base'
import { DynamoOrdersRepository } from '../../repositories/orders-repository'
import { setGlobalMetrics } from '../../util/metrics'
import { BaseRInj, SfnInjector, SfnStateInputOutput } from '../base/index'
import { checkDefined } from '../../preconditions/preconditions';

export interface RequestInjected extends BaseRInj {
chainId: number
quoteId: string
orderHash: string
startingBlockNumber: number
orderStatus: string
orderStatus: ORDER_STATUS
getFillLogAttempts: number
retryCount: number
provider: ethers.providers.StaticJsonRpcProvider
Expand Down Expand Up @@ -59,7 +60,7 @@ export class CheckOrderStatusInjector extends SfnInjector<ContainerInjected, Req
orderHash: event.orderHash as string,
quoteId: event.quoteId as string,
startingBlockNumber: event.startingBlockNumber ? (event.startingBlockNumber as number) : 0,
orderStatus: event.orderStatus as string,
orderStatus: event.orderStatus as ORDER_STATUS,
getFillLogAttempts: event.getFillLogAttempts ? (event.getFillLogAttempts as number) : 0,
retryCount: event.retryCount ? (event.retryCount as number) : 0,
provider: provider,
Expand Down
79 changes: 43 additions & 36 deletions test/handlers/check-order-status.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,10 @@ describe('Testing check order status handler', () => {
provider: {
getBlockNumber: providerMock,
getTransaction: getTransactionMock,
getBlock: () => Promise.resolve({
timestamp: 123456
})
getBlock: () =>
Promise.resolve({
timestamp: 123456,
}),
},
}
},
Expand Down Expand Up @@ -163,22 +164,25 @@ describe('Testing check order status handler', () => {
const checkorderStatusHandler = new CheckOrderStatusHandler('check-order-status', initialInjectorPromiseMock)
validateMock.mockReturnValue(OrderValidation.Expired)
getTransactionMock.mockReturnValueOnce({
wait: () => Promise.resolve({
effectiveGasPrice: BigNumber.from(1),
gasUsed: 100
})
wait: () =>
Promise.resolve({
effectiveGasPrice: BigNumber.from(1),
gasUsed: 100,
}),
})
getFillInfoMock.mockReturnValue([{
orderHash: MOCK_ORDER_HASH,
filler: '0x123',
nonce: BigNumber.from(1),
swapper: '0x123',
blockNumber: 12321312313,
txHash: '0x1244345323',
inputs: [{token: 'USDC', amount: BigNumber.from(100)}],
outputs: [{token: 'WETH', amount: BigNumber.from(1)}],
}])

getFillInfoMock.mockReturnValue([
{
orderHash: MOCK_ORDER_HASH,
filler: '0x123',
nonce: BigNumber.from(1),
swapper: '0x123',
blockNumber: 12321312313,
txHash: '0x1244345323',
inputs: [{ token: 'USDC', amount: BigNumber.from(100) }],
outputs: [{ token: 'WETH', amount: BigNumber.from(1) }],
},
])

expect(await checkorderStatusHandler.handler(handlerEventMock)).toMatchObject({
orderStatus: ORDER_STATUS.FILLED,
})
Expand All @@ -189,22 +193,25 @@ describe('Testing check order status handler', () => {
const checkorderStatusHandler = new CheckOrderStatusHandler('check-order-status', initialInjectorPromiseMock)
validateMock.mockReturnValue(OrderValidation.NonceUsed)
getTransactionMock.mockReturnValueOnce({
wait: () => Promise.resolve({
effectiveGasPrice: BigNumber.from(1),
gasUsed: 100
})
wait: () =>
Promise.resolve({
effectiveGasPrice: BigNumber.from(1),
gasUsed: 100,
}),
})
getFillInfoMock.mockReturnValue([{
orderHash: MOCK_ORDER_HASH,
filler: '0x123',
nonce: BigNumber.from(1),
swapper: '0x123',
blockNumber: 12321312313,
txHash: '0x1244345323',
inputs: [{token: 'USDC', amount: BigNumber.from(100)}],
outputs: [{token: 'WETH', amount: BigNumber.from(1)}],
}])

getFillInfoMock.mockReturnValue([
{
orderHash: MOCK_ORDER_HASH,
filler: '0x123',
nonce: BigNumber.from(1),
swapper: '0x123',
blockNumber: 12321312313,
txHash: '0x1244345323',
inputs: [{ token: 'USDC', amount: BigNumber.from(100) }],
outputs: [{ token: 'WETH', amount: BigNumber.from(1) }],
},
])

expect(await checkorderStatusHandler.handler(handlerEventMock)).toMatchObject({
orderStatus: ORDER_STATUS.FILLED,
})
Expand Down Expand Up @@ -239,7 +246,7 @@ describe('Testing check order status handler', () => {
const response = await checkOrderStatusHandler.handler(handlerEventMock)
expect(getByHashMock).toBeCalledWith(MOCK_ORDER_HASH)
expect(validateMock).toBeCalled()
expect(updateOrderStatusMock).toBeCalledWith(MOCK_ORDER_HASH, ORDER_STATUS.OPEN, undefined, undefined)
expect(updateOrderStatusMock).not.toBeCalled() // there is no update
expect(response).toEqual({
orderHash: MOCK_ORDER_HASH,
orderStatus: 'open',
Expand All @@ -257,7 +264,7 @@ describe('Testing check order status handler', () => {
const response = await checkOrderStatusHandler.handler(handlerEventMock)
expect(getByHashMock).toBeCalledWith(MOCK_ORDER_HASH)
expect(validateMock).toBeCalled()
expect(updateOrderStatusMock).toBeCalledWith(MOCK_ORDER_HASH, ORDER_STATUS.OPEN, undefined, undefined)
expect(updateOrderStatusMock).not.toBeCalled() // there is no update
expect(response).toEqual({
orderHash: MOCK_ORDER_HASH,
orderStatus: 'open',
Expand All @@ -275,7 +282,7 @@ describe('Testing check order status handler', () => {
const response = await checkOrderStatusHandler.handler(handlerEventMock)
expect(getByHashMock).toBeCalledWith(MOCK_ORDER_HASH)
expect(validateMock).toBeCalled()
expect(updateOrderStatusMock).toBeCalledWith(MOCK_ORDER_HASH, ORDER_STATUS.OPEN, undefined, undefined)
expect(updateOrderStatusMock).not.toBeCalled() // there is no update
expect(response).toEqual({
orderHash: MOCK_ORDER_HASH,
orderStatus: 'open',
Expand Down

0 comments on commit cb9e000

Please sign in to comment.