From e6fba69373865e8ef0bc4549eb1dd1369052ab98 Mon Sep 17 00:00:00 2001 From: Zach Yang Date: Tue, 13 Jun 2023 10:58:56 -0400 Subject: [PATCH] feat: track nonce by chain (#219) * update repo interface * track nonce separately by chain * add unit tests --- lib/handlers/get-nonce/handler.ts | 6 +++--- lib/handlers/get-nonce/injector.ts | 2 ++ lib/handlers/get-nonce/schema/index.ts | 2 ++ lib/repositories/base.ts | 2 +- lib/repositories/orders-repository.ts | 6 +++--- test/handlers/get-nonce.test.ts | 11 +++++++---- test/repositories/dynamo-repository.test.ts | 22 ++++++++++++++++++--- 7 files changed, 37 insertions(+), 14 deletions(-) diff --git a/lib/handlers/get-nonce/handler.ts b/lib/handlers/get-nonce/handler.ts index 272d814a..1358c41a 100644 --- a/lib/handlers/get-nonce/handler.ts +++ b/lib/handlers/get-nonce/handler.ts @@ -14,13 +14,13 @@ export class GetNonceHandler extends APIGLambdaHandler< params: APIHandleRequestParams ): Promise> { const { - requestInjected: { address, log }, + requestInjected: { address, chainId, log }, containerInjected: { dbInterface }, } = params try { log.info({ address: address }, 'Getting nonce for address') - const nonce = await dbInterface.getNonceByAddress(address) + const nonce = await dbInterface.getNonceByAddressAndChain(address, chainId) return { statusCode: 200, body: { @@ -28,7 +28,7 @@ export class GetNonceHandler extends APIGLambdaHandler< }, } } catch (e: unknown) { - log.error({ e }, 'Error getting nonce') + log.error({ e }, `Error getting nonce for address ${address} on chain ${chainId}`) return { statusCode: 500, ...(e instanceof Error && { errorCode: e.message }), diff --git a/lib/handlers/get-nonce/injector.ts b/lib/handlers/get-nonce/injector.ts index 1324f550..5d7dc158 100644 --- a/lib/handlers/get-nonce/injector.ts +++ b/lib/handlers/get-nonce/injector.ts @@ -8,6 +8,7 @@ import { GetNonceQueryParams } from './schema/index' export interface RequestInjected extends ApiRInj { address: string + chainId: number } export interface ContainerInjected { @@ -41,6 +42,7 @@ export class GetNonceInjector extends ApiInjector Promise getByOfferer: (offerer: string, limit: number) => Promise getByOrderStatus: (orderStatus: string, limit: number) => Promise - getNonceByAddress: (address: string) => Promise + getNonceByAddressAndChain: (address: string, chainId: number) => Promise updateOrderStatus: ( orderHash: string, status: ORDER_STATUS, diff --git a/lib/repositories/orders-repository.ts b/lib/repositories/orders-repository.ts index 4312f07c..bb56c5ff 100644 --- a/lib/repositories/orders-repository.ts +++ b/lib/repositories/orders-repository.ts @@ -182,8 +182,8 @@ export class DynamoOrdersRepository implements BaseOrdersRepository { return res.Item as OrderEntity } - public async getNonceByAddress(address: string): Promise { - const res = await this.nonceEntity.query(address, { + public async getNonceByAddressAndChain(address: string, chainId: number): Promise { + const res = await this.nonceEntity.query(`${address}-${chainId}`, { limit: 1, reverse: true, consistent: true, @@ -220,7 +220,7 @@ export class DynamoOrdersRepository implements BaseOrdersRepository { createdAt: currentTimestampInSeconds(), }), this.nonceEntity.updateTransaction({ - offerer: order.offerer, + offerer: `${order.offerer}-${order.chainId}`, nonce: order.nonce, }), ], diff --git a/test/handlers/get-nonce.test.ts b/test/handlers/get-nonce.test.ts index 92b3f420..f7a5a458 100644 --- a/test/handlers/get-nonce.test.ts +++ b/test/handlers/get-nonce.test.ts @@ -10,13 +10,14 @@ describe('Testing get nonce handler.', () => { const requestInjectedMock = { address: MOCK_ADDRESS, + chainId: 1, log: { info: () => jest.fn(), error: () => jest.fn() }, } const injectorPromiseMock: any = { getContainerInjected: () => { return { dbInterface: { - getNonceByAddress: getNonceByAddressMock, + getNonceByAddressAndChain: getNonceByAddressMock, }, } }, @@ -25,6 +26,7 @@ describe('Testing get nonce handler.', () => { const event = { queryStringParameters: { address: MOCK_ADDRESS, + chainId: 1, }, body: null, } @@ -41,7 +43,7 @@ describe('Testing get nonce handler.', () => { it('Testing valid request and response.', async () => { const getNonceResponse = await getNonceHandler.handler(event as any, {} as any) - expect(getNonceByAddressMock).toBeCalledWith(requestInjectedMock.address) + expect(getNonceByAddressMock).toBeCalledWith(requestInjectedMock.address, 1) expect(getNonceResponse).toMatchObject({ body: JSON.stringify({ nonce: MOCK_NONCE }), statusCode: 200, @@ -58,6 +60,7 @@ describe('Testing get nonce handler.', () => { [{ address: '' }, '"address\\" is not allowed to be empty"'], [{ address: '0xF53bDa7e0337BD456cDcDab0Ab24Db43E738065' }, 'VALIDATION ERROR: Invalid address'], [{}, '"address\\" is required'], + [{ address: MOCK_ADDRESS, chainId: 'foo' }, '\\"chainId\\" must be one of [1, 137, 12341234]'], ])('Throws 400 with invalid query param %p', async (invalidQueryParam, bodyMsg) => { const invalidEvent = { ...event, @@ -77,7 +80,7 @@ describe('Testing get nonce handler.', () => { async (invalidResponseField) => { getNonceByAddressMock.mockReturnValue(invalidResponseField) const getNonceResponse = await getNonceHandler.handler(event as any, {} as any) - expect(getNonceByAddressMock).toBeCalledWith(requestInjectedMock.address) + expect(getNonceByAddressMock).toBeCalledWith(requestInjectedMock.address, 1) expect(getNonceResponse.statusCode).toEqual(500) expect(getNonceResponse.body).toEqual(expect.stringContaining('INTERNAL_ERROR')) } @@ -89,7 +92,7 @@ describe('Testing get nonce handler.', () => { throw error }) const getNonceResponse = await getNonceHandler.handler(event as any, {} as any) - expect(getNonceByAddressMock).toBeCalledWith(requestInjectedMock.address) + expect(getNonceByAddressMock).toBeCalledWith(requestInjectedMock.address, 1) expect(getNonceResponse).toMatchObject({ body: JSON.stringify({ errorCode: error.message }), statusCode: 500, diff --git a/test/repositories/dynamo-repository.test.ts b/test/repositories/dynamo-repository.test.ts index cfea77d9..4eaedad7 100644 --- a/test/repositories/dynamo-repository.test.ts +++ b/test/repositories/dynamo-repository.test.ts @@ -442,7 +442,7 @@ describe('OrdersRepository get nonce test', () => { ...MOCK_ORDER_1, nonce: '4', }) - const nonce = await ordersRepository.getNonceByAddress('hayden.eth') + const nonce = await ordersRepository.getNonceByAddressAndChain('hayden.eth', MOCK_ORDER_1.chainId) expect(nonce).toEqual('4') }) @@ -453,16 +453,32 @@ describe('OrdersRepository get nonce test', () => { orderHash: '0x4', }) // at this point, there are three orders in the DB, two with nonce 2 - const nonce = await ordersRepository.getNonceByAddress('hayden.eth') + const nonce = await ordersRepository.getNonceByAddressAndChain('hayden.eth', MOCK_ORDER_1.chainId) expect(nonce).toEqual('2') }) it('should generate random nonce for new address', async () => { const spy = jest.spyOn(nonceUtil, 'generateRandomNonce') - const res = await ordersRepository.getNonceByAddress('random.eth') + const res = await ordersRepository.getNonceByAddressAndChain('random.eth') expect(res).not.toBeUndefined() expect(spy).toHaveBeenCalled() }) + + it('should track nonce for the same address on different chains separately', async () => { + await ordersRepository.putOrderAndUpdateNonceTransaction({ + ...MOCK_ORDER_2, + nonce: '10', + }) + await ordersRepository.putOrderAndUpdateNonceTransaction({ + ...MOCK_ORDER_2, + chainId: 1, + nonce: '20', + }) + const nonce = await ordersRepository.getNonceByAddressAndChain(MOCK_ORDER_2.offerer, MOCK_ORDER_2.chainId) + expect(nonce).toEqual('10') + const nonce2 = await ordersRepository.getNonceByAddressAndChain(MOCK_ORDER_2.offerer, 1) + expect(nonce2).toEqual('20') + }) }) describe('OrdersRepository get order count by offerer test', () => {