From 837b39a54c435e57657990ffc337d2498f9d1022 Mon Sep 17 00:00:00 2001 From: Will Meister Date: Thu, 19 Mar 2020 13:00:53 -0500 Subject: [PATCH 1/3] handling batch rpc requests --- .../src/app/fullnode-rpc-server.ts | 97 +++++++++++-------- 1 file changed, 54 insertions(+), 43 deletions(-) diff --git a/packages/rollup-full-node/src/app/fullnode-rpc-server.ts b/packages/rollup-full-node/src/app/fullnode-rpc-server.ts index 3a48e2f47f60..5a1189cc608e 100644 --- a/packages/rollup-full-node/src/app/fullnode-rpc-server.ts +++ b/packages/rollup-full-node/src/app/fullnode-rpc-server.ts @@ -49,51 +49,62 @@ export class FullnodeRpcServer extends ExpressHttpServer { */ protected initRoutes(): void { this.app.post('/', async (req, res) => { - let request: JsonRpcRequest - try { - request = req.body - if (!isJsonRpcRequest(request)) { - log.debug(`Received request of unsupported format: [${request}]`) - return res.json(buildJsonRpcError('INVALID_REQUEST', null)) - } + return res.json(await this.handleRequest(req)) + }) + } - const result = await this.fullnodeHandler.handleRequest( - request.method, - request.params + protected async handleRequest(req: any): Promise { + let request: JsonRpcRequest + try { + request = req.body + if (Array.isArray(request)) { + const requestArray: any[] = request as any[] + return Promise.all( + requestArray.map((x) => this.handleRequest({ body: x })) ) - return res.json({ - id: request.id, - jsonrpc: request.jsonrpc, - result, - }) - } catch (err) { - if (err instanceof RevertError) { - log.debug(`Request reverted. Request: ${JSON.stringify(request)}`) - const errorResponse: JsonRpcErrorResponse = buildJsonRpcError( - 'REVERT_ERROR', - request.id - ) - errorResponse.error.message = err.message - return res.json(errorResponse) - } - if (err instanceof UnsupportedMethodError) { - log.debug( - `Received request with unsupported method: [${JSON.stringify( - request - )}]` - ) - return res.json(buildJsonRpcError('METHOD_NOT_FOUND', request.id)) - } else if (err instanceof InvalidParametersError) { - log.debug( - `Received request with valid method but invalid parameters: [${JSON.stringify( - request - )}]` - ) - return res.json(buildJsonRpcError('INVALID_PARAMS', request.id)) - } - logError(log, `Uncaught exception at endpoint-level`, err) - return res.json(buildJsonRpcError('INTERNAL_ERROR', request.id)) } - }) + + if (!isJsonRpcRequest(request)) { + log.debug(`Received request of unsupported format: [${request}]`) + return buildJsonRpcError('INVALID_REQUEST', null) + } + + const result = await this.fullnodeHandler.handleRequest( + request.method, + request.params + ) + return { + id: request.id, + jsonrpc: request.jsonrpc, + result, + } + } catch (err) { + if (err instanceof RevertError) { + log.debug(`Request reverted. Request: ${JSON.stringify(request)}`) + const errorResponse: JsonRpcErrorResponse = buildJsonRpcError( + 'REVERT_ERROR', + request.id + ) + errorResponse.error.message = err.message + return errorResponse + } + if (err instanceof UnsupportedMethodError) { + log.debug( + `Received request with unsupported method: [${JSON.stringify( + request + )}]` + ) + return buildJsonRpcError('METHOD_NOT_FOUND', request.id) + } else if (err instanceof InvalidParametersError) { + log.debug( + `Received request with valid method but invalid parameters: [${JSON.stringify( + request + )}]` + ) + return buildJsonRpcError('INVALID_PARAMS', request.id) + } + logError(log, `Uncaught exception at endpoint-level`, err) + return buildJsonRpcError('INTERNAL_ERROR', request.id) + } } } From 4d58c8ab193e9fe562752f9e652fefd030cd01a8 Mon Sep 17 00:00:00 2001 From: Will Meister Date: Thu, 19 Mar 2020 13:02:39 -0500 Subject: [PATCH 2/3] test updates --- .../test/app/fullnode-rpc-server.spec.ts | 158 ++++++++++++------ 1 file changed, 105 insertions(+), 53 deletions(-) diff --git a/packages/rollup-full-node/test/app/fullnode-rpc-server.spec.ts b/packages/rollup-full-node/test/app/fullnode-rpc-server.spec.ts index d29255f47a4b..335b16bc6285 100644 --- a/packages/rollup-full-node/test/app/fullnode-rpc-server.spec.ts +++ b/packages/rollup-full-node/test/app/fullnode-rpc-server.spec.ts @@ -53,6 +53,31 @@ const request = async ( }) } +const getBadPayloads = () => { + return [ + { + jsonrpc: '2.0', + method: defaultSupportedMethods[0], + }, + { + id: '1', + jsonrpc: '2.0', + method: defaultSupportedMethods[0], + }, + { + id: 1, + method: defaultSupportedMethods[0], + }, + { + id: 1, + jsonrpc: 2.0, + method: defaultSupportedMethods[0], + }, + { id: 1, jsonrpc: '2.0' }, + { id: 1, jsonrpc: '2.0', method: unsupportedMethod }, + ] +} + const host = '0.0.0.0' const port = 9999 @@ -77,75 +102,102 @@ describe('FullnodeHandler RPC Server', () => { } }) - it('should work for valid requests & methods', async () => { - const results: AxiosResponse[] = await Promise.all( - Array.from(defaultSupportedMethods).map((x) => - request(client, { id: 1, jsonrpc: '2.0', method: x }) + describe('single requests', () => { + it('should work for valid requests & methods', async () => { + const results: AxiosResponse[] = await Promise.all( + Array.from(defaultSupportedMethods).map((x) => + request(client, { id: 1, jsonrpc: '2.0', method: x }) + ) ) - ) - results.forEach((r) => { - r.status.should.equal(200) + results.forEach((r) => { + r.status.should.equal(200) - r.data.should.haveOwnProperty('id') - r.data['id'].should.equal(1) + r.data.should.haveOwnProperty('id') + r.data['id'].should.equal(1) - r.data.should.haveOwnProperty('jsonrpc') - r.data['jsonrpc'].should.equal('2.0') + r.data.should.haveOwnProperty('jsonrpc') + r.data['jsonrpc'].should.equal('2.0') - r.data.should.haveOwnProperty('result') - r.data['result'].should.equal(dummyResponse) + r.data.should.haveOwnProperty('result') + r.data['result'].should.equal(dummyResponse) + }) }) - }) - it('fails on bad format or method', async () => { - const results: AxiosResponse[] = await Promise.all([ - request(client, { jsonrpc: '2.0', method: defaultSupportedMethods[0] }), - request(client, { - id: '1', - jsonrpc: '2.0', - method: defaultSupportedMethods[0], - }), - request(client, { id: 1, method: defaultSupportedMethods[0] }), - request(client, { - id: 1, - jsonrpc: 2.0, - method: defaultSupportedMethods[0], - }), - request(client, { id: 1, jsonrpc: '2.0' }), - request(client, { id: 1, jsonrpc: '2.0', method: unsupportedMethod }), - ]) + it('fails on bad format or method', async () => { + const results: AxiosResponse[] = await Promise.all( + getBadPayloads().map((x) => request(client, x)) + ) + results.forEach((r) => { + r.status.should.equal(200) - results.forEach((r) => { - r.status.should.equal(200) + r.data.should.haveOwnProperty('id') + r.data.should.haveOwnProperty('jsonrpc') + r.data.should.haveOwnProperty('error') - r.data.should.haveOwnProperty('id') - r.data.should.haveOwnProperty('jsonrpc') - r.data.should.haveOwnProperty('error') + r.data['jsonrpc'].should.equal('2.0') + }) + }) - r.data['jsonrpc'].should.equal('2.0') + it('reverts properly', async () => { + const result: AxiosResponse = await request(client, { + id: 1, + jsonrpc: '2.0', + method: revertMethod, + }) + + result.status.should.equal(200) + + result.data.should.haveOwnProperty('id') + result.data.should.haveOwnProperty('jsonrpc') + result.data.should.haveOwnProperty('error') + result.data['error'].should.haveOwnProperty('message') + result.data['error'].should.haveOwnProperty('code') + result.data['error']['message'].should.equal( + JSONRPC_ERRORS.REVERT_ERROR.message + ) + result.data['error']['code'].should.equal( + JSONRPC_ERRORS.REVERT_ERROR.code + ) + + result.data['jsonrpc'].should.equal('2.0') }) }) - it('reverts properly', async () => { - const result: AxiosResponse = await request(client, { - id: 1, - jsonrpc: '2.0', - method: revertMethod, + describe('batch requests', () => { + it('should work for valid requests & methods', async () => { + const batchRequest = Array.from( + defaultSupportedMethods + ).map((method, id) => ({ jsonrpc: '2.0', id, method })) + const result: AxiosResponse = await request(client, batchRequest) + + result.status.should.equal(200) + const results = result.data + + results.forEach((r, id) => { + r.should.haveOwnProperty('id') + r['id'].should.equal(id) + + r.should.haveOwnProperty('jsonrpc') + r['jsonrpc'].should.equal('2.0') + + r.should.haveOwnProperty('result') + r['result'].should.equal(dummyResponse) + }) }) + it('should fail on bad format or for valid requests & methods', async () => { + const result: AxiosResponse = await request(client, getBadPayloads()) - result.status.should.equal(200) + result.status.should.equal(200) + const results = result.data - result.data.should.haveOwnProperty('id') - result.data.should.haveOwnProperty('jsonrpc') - result.data.should.haveOwnProperty('error') - result.data['error'].should.haveOwnProperty('message') - result.data['error'].should.haveOwnProperty('code') - result.data['error']['message'].should.equal( - JSONRPC_ERRORS.REVERT_ERROR.message - ) - result.data['error']['code'].should.equal(JSONRPC_ERRORS.REVERT_ERROR.code) + results.forEach((r) => { + r.should.haveOwnProperty('id') + r.should.haveOwnProperty('jsonrpc') + r.should.haveOwnProperty('error') - result.data['jsonrpc'].should.equal('2.0') + r['jsonrpc'].should.equal('2.0') + }) + }) }) }) From e2ceb8daab4ba2cf2356afbcd07cd6c63fd00efd Mon Sep 17 00:00:00 2001 From: Will Meister Date: Thu, 19 Mar 2020 13:22:04 -0500 Subject: [PATCH 3/3] Adding logic and test to disallow batch requests of batch requests --- .../src/app/fullnode-rpc-server.ts | 29 +++++++++++++++---- .../test/app/fullnode-rpc-server.spec.ts | 23 +++++++++++++++ 2 files changed, 47 insertions(+), 5 deletions(-) diff --git a/packages/rollup-full-node/src/app/fullnode-rpc-server.ts b/packages/rollup-full-node/src/app/fullnode-rpc-server.ts index 5a1189cc608e..7d8ff4d079dc 100644 --- a/packages/rollup-full-node/src/app/fullnode-rpc-server.ts +++ b/packages/rollup-full-node/src/app/fullnode-rpc-server.ts @@ -8,6 +8,7 @@ import { Logger, JsonRpcRequest, JsonRpcErrorResponse, + JsonRpcResponse, } from '@eth-optimism/core-utils' /* Internal Imports */ @@ -53,19 +54,34 @@ export class FullnodeRpcServer extends ExpressHttpServer { }) } - protected async handleRequest(req: any): Promise { + /** + * Handles the provided request, returning the appropriate response object + * @param req The request to handle + * @param inBatchRequest Whether or not this is being called within the context of handling a batch requset + * @returns The JSON-stringifiable response object. + */ + protected async handleRequest( + req: any, + inBatchRequest: boolean = false + ): Promise { let request: JsonRpcRequest try { request = req.body - if (Array.isArray(request)) { + if (Array.isArray(request) && !inBatchRequest) { + log.debug(`Received batch request: [${JSON.stringify(request)}]`) const requestArray: any[] = request as any[] return Promise.all( - requestArray.map((x) => this.handleRequest({ body: x })) + requestArray.map( + (x) => + this.handleRequest({ body: x }, true) as Promise + ) ) } if (!isJsonRpcRequest(request)) { - log.debug(`Received request of unsupported format: [${request}]`) + log.debug( + `Received request of unsupported format: [${JSON.stringify(request)}]` + ) return buildJsonRpcError('INVALID_REQUEST', null) } @@ -104,7 +120,10 @@ export class FullnodeRpcServer extends ExpressHttpServer { return buildJsonRpcError('INVALID_PARAMS', request.id) } logError(log, `Uncaught exception at endpoint-level`, err) - return buildJsonRpcError('INTERNAL_ERROR', request.id) + return buildJsonRpcError( + 'INTERNAL_ERROR', + request && request.id ? request.id : null + ) } } } diff --git a/packages/rollup-full-node/test/app/fullnode-rpc-server.spec.ts b/packages/rollup-full-node/test/app/fullnode-rpc-server.spec.ts index 335b16bc6285..791cb473901f 100644 --- a/packages/rollup-full-node/test/app/fullnode-rpc-server.spec.ts +++ b/packages/rollup-full-node/test/app/fullnode-rpc-server.spec.ts @@ -196,6 +196,29 @@ describe('FullnodeHandler RPC Server', () => { r.should.haveOwnProperty('jsonrpc') r.should.haveOwnProperty('error') + r['jsonrpc'].should.equal('2.0') + }) + }) + it('should not allow batches of batches', async () => { + const batchOfBatches = Array.from( + defaultSupportedMethods + ).map((method, id) => [{ jsonrpc: '2.0', id, method }]) + const result: AxiosResponse = await request(client, batchOfBatches) + + result.status.should.equal(200) + const results = result.data + + results.forEach((r) => { + r.should.haveOwnProperty('id') + r.should.haveOwnProperty('jsonrpc') + r.should.haveOwnProperty('error') + r['error'].should.haveOwnProperty('message') + r['error'].should.haveOwnProperty('code') + r['error']['message'].should.equal( + JSONRPC_ERRORS.INVALID_REQUEST.message + ) + r['error']['code'].should.equal(JSONRPC_ERRORS.INVALID_REQUEST.code) + r['jsonrpc'].should.equal('2.0') }) })