From 6bb2ef923894a3572c3fa824c3bf1a69759eb43a Mon Sep 17 00:00:00 2001 From: kao Date: Sat, 15 Dec 2018 01:23:08 -0800 Subject: Respond to CR --- packages/order-watcher/README.md | 37 +++++--- .../src/order_watcher/order_watcher_websocket.ts | 69 ++++++++------ .../order-watcher/src/schemas/websocket_schemas.ts | 53 ++++++++--- packages/order-watcher/src/types.ts | 50 ++++++++-- .../test/order_watcher_websocket_test.ts | 101 ++++++++++++++------- 5 files changed, 209 insertions(+), 101 deletions(-) (limited to 'packages') diff --git a/packages/order-watcher/README.md b/packages/order-watcher/README.md index 7eae0ae16..aad90a59a 100644 --- a/packages/order-watcher/README.md +++ b/packages/order-watcher/README.md @@ -5,7 +5,7 @@ An order watcher daemon that watches for order validity. #### Read the wiki [article](https://0xproject.com/wiki#0x-OrderWatcher). OrderWatcher also comes with a WebSocket server to provide language-agnostic access -to order watching functionality. We used the [WebSocket Client and Server Implementation for Node](https://www.npmjs.com/package/websocket). +to order watching functionality. We used the [WebSocket Client and Server Implementation for Node](https://www.npmjs.com/package/websocket). The server sends and receives messages that conform to the [JSON RPC specifications](https://www.jsonrpc.org/specification). ## Installation @@ -46,7 +46,7 @@ The first step for making a request is establishing a connection with the server ``` var W3CWebSocket = require('websocket').w3cwebsocket; -wsClient = new WebSocket.w3cwebsocket('ws://127.0.0.1:8080'); +wsClient = new W3CWebSocket('ws://127.0.0.1:8080'); ``` In Python, you could use the [websocket-client library](http://pypi.python.org/pypi/websocket-client/) and run: @@ -56,19 +56,22 @@ from websocket import create_connection wsClient = create_connection("ws://127.0.0.1:8080") ``` -With the connection established, you prepare the payload for your request. The payload is a json object with the following structure: +With the connection established, you prepare the payload for your request. The payload is a json object with a format established by the [JSON RPC specification](https://www.jsonrpc.org/specification): -* For `GET_STATE`, the payload is `{ action: 'GET_STATS }`. -* For `ADD_ORDER`, use `{ action: 'ADD_ORDER', signedOrder: }`. -* For `REMOVE_ORDER`, use `{ action: 'REMOVE_ORDER', orderHash: }`. +* `id`: All requests require you to specify a string as an id. When the server responds to the request, it provides an id as well to allow you to determine which request it is responding to. +* `jsonrpc`: This is always the string `'2.0'`. +* `method`: This specifies the OrderWatcher method you want to call. I.e., `'ADD_ORDER'`, `'REMOVE_ORDER'`, and `'GET_STATS'`. +* `params`: These contain the parameters needed by OrderWatcher to execute the method you called. For `ADD_ORDER`, provide `{ signedOrder: }`. For `REMOVE_ORDER`, provide `{ orderHash: }`. For `GET_STATS`, no parameters are needed, so you may leave this empty. Next, convert the payload to a string and send it through the connection. In Javascript: ``` const addOrderPayload = { - action: 'ADD_ORDER', - signedOrder: , + id: 'order32', + jsonrpc: '2.0', + method: 'ADD_ORDER', + params: { signedOrder: }, }; wsClient.send(JSON.stringify(addOrderPayload)); ``` @@ -78,8 +81,10 @@ In Python: ``` import json remove_order_payload = { - 'action': 'REMOVE_ORDER', - 'orderHash': '0x6edc16bf37fde79f5012088c33784c730e2f103d9ab1caf73060c386ad107b7e', + 'id': 'order33', + 'jsonrpc': '2.0', + 'method': 'REMOVE_ORDER', + 'params': {'orderHash': '0x6edc16bf37fde79f5012088c33784c730e2f103d9ab1caf73060c386ad107b7e'}, } wsClient.send(json.dumps(remove_order_payload)); ``` @@ -87,16 +92,18 @@ wsClient.send(json.dumps(remove_order_payload)); **Response** The server responds to all requests in a similar format. In the data field, you'll find another json object that has been converted into a string. This json object contains the following fields: -* `action`: The action the server is responding to. Eg. `ADD_ORDER`. When order states change the server may also initiate a response. In this case, action will be listed as `UPDATE`. -* `success`: `true` or `false`; Indicates whether the server handled the request without problems. -* `result`: This field varies based on the action. `UPDATE` responses contained the new order state. `GET_STATS` responses contain the current order count. When there are errors, the error messages are stored in here. +* `id`: The id corresponding to the request that the server is responding to. `UPDATE` responses are not based on any requests so the `id` field is `null`. +* `jsonrpc`: Always `'2.0'`. +* `method`: The method the server is responding to. Eg. `ADD_ORDER`. When order states change the server may also initiate a response. In this case, method will be listed as `UPDATE`. +* `result`: This field varies based on the method. `UPDATE` responses contained the new order state. `GET_STATS` responses contain the current order count. When there are errors, this field is `null`. +* `error`: When there is an error executing a request, the error message is listed here. When the server responds successfully, this field is `null`. In Javascript, the responses can be parsed using the `onmessage` callback: ``` wsClient.onmessage = (msg) => { const responseData = JSON.parse(msg.data); - const action = responseData.action + const method = responseData.method }; ``` @@ -104,7 +111,7 @@ In Python, `recv` is a lightweight way to receive a response: ``` result = wsClient.recv() -action = result.action +method = result.method ``` ## Contributing diff --git a/packages/order-watcher/src/order_watcher/order_watcher_websocket.ts b/packages/order-watcher/src/order_watcher/order_watcher_websocket.ts index 806c7c6a5..7a88597ef 100644 --- a/packages/order-watcher/src/order_watcher/order_watcher_websocket.ts +++ b/packages/order-watcher/src/order_watcher/order_watcher_websocket.ts @@ -7,9 +7,10 @@ import * as WebSocket from 'websocket'; import { webSocketRequestSchema, webSocketUtf8MessageSchema } from '../schemas/websocket_schemas'; import { + GetStatsResult, OnOrderStateChangeCallback, - OrderWatcherAction, OrderWatcherConfig, + OrderWatcherMethod, WebSocketRequest, WebSocketResponse, } from '../types'; @@ -18,12 +19,13 @@ import { assert } from '../utils/assert'; import { OrderWatcher } from './order_watcher'; const DEFAULT_HTTP_PORT = 8080; +const JSONRPC_VERSION = '2.0'; // Wraps the OrderWatcher functionality in a WebSocket server. Motivations: // 1) Users can watch orders via non-typescript programs. // 2) Better encapsulation so that users can work export class OrderWatcherWebSocketServer { - public readonly _orderWatcher: OrderWatcher; // public for testing + private readonly _orderWatcher: OrderWatcher; private readonly _httpServer: http.Server; private readonly _connectionStore: Set; private readonly _wsServer: WebSocket.server; @@ -66,7 +68,9 @@ export class OrderWatcherWebSocketServer { httpServer: this._httpServer, // Avoid setting autoAcceptConnections to true as it defeats all // standard cross-origin protection facilities built into the protocol - // and the browser. Also ensures that a request event is emitted by + // and the browser. + // Source: https://www.npmjs.com/package/websocket#server-example + // Also ensures that a request event is emitted by // the server whenever a new WebSocket request is made. autoAcceptConnections: false, }); @@ -76,7 +80,7 @@ export class OrderWatcherWebSocketServer { // machine by the same user. As such, no security checks are in place. const connection: WebSocket.connection = request.accept(null, request.origin); logUtils.log(`${new Date()} [Server] Accepted connection from origin ${request.origin}.`); - connection.on('message', await this._onMessageCallbackAsync.bind(this, connection)); + connection.on('message', this._onMessageCallbackAsync.bind(this, connection)); connection.on('close', this._onCloseCallback.bind(this, connection)); this._connectionStore.add(connection); }); @@ -106,20 +110,25 @@ export class OrderWatcherWebSocketServer { } private async _onMessageCallbackAsync(connection: WebSocket.connection, message: any): Promise { - const response: WebSocketResponse = { - action: null, - success: false, - result: null, - }; + let response: WebSocketResponse; try { assert.doesConformToSchema('message', message, webSocketUtf8MessageSchema); const request: WebSocketRequest = JSON.parse(message.utf8Data); assert.doesConformToSchema('request', request, webSocketRequestSchema); - response.action = request.action; - response.success = true; - response.result = await this._routeRequestAsync(request); + assert.isString(request.jsonrpc, JSONRPC_VERSION); + response = { + id: request.id, + jsonrpc: JSONRPC_VERSION, + method: request.method, + result: await this._routeRequestAsync(request), + }; } catch (err) { - response.result = err.toString(); + response = { + id: null, + jsonrpc: JSONRPC_VERSION, + method: null, + error: err.toString(), + }; } logUtils.log(`${new Date()} [Server] OrderWatcher output: ${JSON.stringify(response)}`); connection.sendUTF(JSON.stringify(response)); @@ -130,29 +139,28 @@ export class OrderWatcherWebSocketServer { logUtils.log(`${new Date()} [Server] Client ${connection.remoteAddress} disconnected.`); } - private async _routeRequestAsync(request: WebSocketRequest): Promise { - logUtils.log(`${new Date()} [Server] Request received: ${request.action}`); - let result = null; - switch (request.action) { - case OrderWatcherAction.AddOrder: { - const signedOrder: SignedOrder = OrderWatcherWebSocketServer._parseSignedOrder(request.signedOrder); + private async _routeRequestAsync(request: WebSocketRequest): Promise { + logUtils.log(`${new Date()} [Server] Request received: ${request.method}`); + switch (request.method) { + case OrderWatcherMethod.AddOrder: { + const signedOrder: SignedOrder = OrderWatcherWebSocketServer._parseSignedOrder( + request.params.signedOrder, + ); await this._orderWatcher.addOrderAsync(signedOrder); break; } - case OrderWatcherAction.RemoveOrder: { - const orderHash = request.orderHash || '_'; - this._orderWatcher.removeOrder(orderHash); + case OrderWatcherMethod.RemoveOrder: { + this._orderWatcher.removeOrder(request.params.orderHash || 'undefined'); break; } - case OrderWatcherAction.GetStats: { - result = this._orderWatcher.getStats(); + case OrderWatcherMethod.GetStats: { + return this._orderWatcher.getStats(); break; } default: - // Should never reach here. Should be caught by JSON schema check. - throw new Error(`[Server] Invalid request action: ${request.action}`); + // Should never reach here. Should be caught by JSON schema check. } - return result; + return undefined; } /** @@ -163,9 +171,10 @@ export class OrderWatcherWebSocketServer { private _broadcastCallback(err: Error | null, orderState?: OrderState): void { this._connectionStore.forEach((connection: WebSocket.connection) => { const response: WebSocketResponse = { - action: OrderWatcherAction.Update, - success: true, - result: orderState || err, + id: null, + jsonrpc: JSONRPC_VERSION, + method: OrderWatcherMethod.Update, + result: orderState, }; connection.sendUTF(JSON.stringify(response)); }); diff --git a/packages/order-watcher/src/schemas/websocket_schemas.ts b/packages/order-watcher/src/schemas/websocket_schemas.ts index c250d12f1..5e4e1ab74 100644 --- a/packages/order-watcher/src/schemas/websocket_schemas.ts +++ b/packages/order-watcher/src/schemas/websocket_schemas.ts @@ -9,24 +9,53 @@ export const webSocketUtf8MessageSchema = { export const webSocketRequestSchema = { id: '/webSocketRequestSchema', - properties: { - action: { enum: ['GET_STATS', 'ADD_ORDER', 'REMOVE_ORDER'] }, - signedOrder: { $ref: '/signedOrderSchema' }, - orderHash: { type: 'string' }, + type: 'object', + definitions: { + signedOrderParam: { + type: 'object', + properties: { + signedOrder: { $ref: '/signedOrderSchema' }, + }, + required: ['signedOrder'], + }, + orderHashParam: { + type: 'object', + properties: { + orderHash: { $ref: '/hexSchema' }, + }, + required: ['orderHash'], + }, }, - anyOf: [ + oneOf: [ { - properties: { action: { enum: ['ADD_ORDER'] } }, - required: ['signedOrder'], + type: 'object', + properties: { + id: { type: 'string' }, + jsonrpc: { type: 'string' }, + method: { enum: ['ADD_ORDER'] }, + params: { $ref: '#/definitions/signedOrderParam' }, + }, + required: ['id', 'jsonrpc', 'method', 'params'], }, { - properties: { action: { enum: ['REMOVE_ORDER'] } }, - required: ['orderHash'], + type: 'object', + properties: { + id: { type: 'string' }, + jsonrpc: { type: 'string' }, + method: { enum: ['REMOVE_ORDER'] }, + params: { $ref: '#/definitions/orderHashParam' }, + }, + required: ['id', 'jsonrpc', 'method', 'params'], }, { - properties: { action: { enum: ['GET_STATS'] } }, - required: [], + type: 'object', + properties: { + id: { type: 'string' }, + jsonrpc: { type: 'string' }, + method: { enum: ['GET_STATS'] }, + params: {}, + }, + required: ['id', 'jsonrpc', 'method'], }, ], - type: 'object', }; diff --git a/packages/order-watcher/src/types.ts b/packages/order-watcher/src/types.ts index 7f6219732..90d383660 100644 --- a/packages/order-watcher/src/types.ts +++ b/packages/order-watcher/src/types.ts @@ -32,8 +32,8 @@ export enum InternalOrderWatcherError { WethNotInTokenRegistry = 'WETH_NOT_IN_TOKEN_REGISTRY', } -export enum OrderWatcherAction { - // Actions initiated by the user. +export enum OrderWatcherMethod { + // Methods initiated by the user. GetStats = 'GET_STATS', AddOrder = 'ADD_ORDER', RemoveOrder = 'REMOVE_ORDER', @@ -46,16 +46,46 @@ export enum OrderWatcherAction { // Users have to create a json object of this format and attach it to // the data field of their WebSocket message to interact with the server. -export interface WebSocketRequest { - action: OrderWatcherAction; - signedOrder?: SignedOrder; - orderHash?: string; +export type WebSocketRequest = AddOrderRequest | RemoveOrderRequest | GetStatsRequest; + +interface AddOrderRequest { + id: string; + jsonrpc: string; + method: OrderWatcherMethod.AddOrder; + params: { signedOrder: SignedOrder }; +} + +interface RemoveOrderRequest { + id: string; + jsonrpc: string; + method: OrderWatcherMethod.RemoveOrder; + params: { orderHash: string }; +} + +interface GetStatsRequest { + id: string; + jsonrpc: string; + method: OrderWatcherMethod.GetStats; } // Users should expect a json object of this format in the data field // of the WebSocket messages that the server sends out. -export interface WebSocketResponse { - action: OrderWatcherAction | null; - success: boolean; - result: any; +export type WebSocketResponse = SuccessfulWebSocketResponse | ErrorWebSocketResponse; + +interface SuccessfulWebSocketResponse { + id: string | null; // id is null for UPDATE + jsonrpc: string; + method: OrderWatcherMethod; + result: OrderState | GetStatsResult | undefined; // result is undefined for ADD_ORDER and REMOVE_ORDER +} + +interface ErrorWebSocketResponse { + id: null; + jsonrpc: string; + method: null; + error: string; +} + +export interface GetStatsResult { + orderCount: number; } diff --git a/packages/order-watcher/test/order_watcher_websocket_test.ts b/packages/order-watcher/test/order_watcher_websocket_test.ts index a9e72ce21..c4d1ede45 100644 --- a/packages/order-watcher/test/order_watcher_websocket_test.ts +++ b/packages/order-watcher/test/order_watcher_websocket_test.ts @@ -41,8 +41,10 @@ describe.only('OrderWatcherWebSocket', async () => { let zrxTokenAddress: string; let signedOrder: SignedOrder; let orderHash: string; - let addOrderPayload: { action: string; signedOrder: SignedOrder }; - let removeOrderPayload: { action: string; orderHash: string }; + // Manually encode types rather than use /src/types to mimick real data that user + // would input. Otherwise we would be forced to use enums, which hide problems. + let addOrderPayload: { id: string; jsonrpc: string; method: string; params: { signedOrder: SignedOrder } }; + let removeOrderPayload: { id: string; jsonrpc: string; method: string; params: { orderHash: string } }; const decimals = constants.ZRX_DECIMALS; const fillableAmount = Web3Wrapper.toBaseUnitAmount(new BigNumber(5), decimals); // HACK: createFillableSignedOrderAsync is Promise-based, which forces us @@ -88,12 +90,16 @@ describe.only('OrderWatcherWebSocket', async () => { ); orderHash = orderHashUtils.getOrderHashHex(signedOrder); addOrderPayload = { - action: 'ADD_ORDER', - signedOrder, + id: 'addOrderPayload', + jsonrpc: '2.0', + method: 'ADD_ORDER', + params: { signedOrder }, }; removeOrderPayload = { - action: 'REMOVE_ORDER', - orderHash, + id: 'removeOrderPayload', + jsonrpc: '2.0', + method: 'REMOVE_ORDER', + params: { orderHash }, }; // Prepare OrderWatcher WebSocket server @@ -118,48 +124,75 @@ describe.only('OrderWatcherWebSocket', async () => { it('responds to getStats requests correctly', (done: any) => { const payload = { - action: 'GET_STATS', + id: 'getStats', + jsonrpc: '2.0', + method: 'GET_STATS', }; wsClient.onopen = () => wsClient.send(JSON.stringify(payload)); wsClient.onmessage = (msg: any) => { const responseData = JSON.parse(msg.data); - expect(responseData.action).to.be.eq('GET_STATS'); - expect(responseData.success).to.be.true(); + expect(responseData.id).to.be.eq('getStats'); + expect(responseData.jsonrpc).to.be.eq('2.0'); + expect(responseData.method).to.be.eq('GET_STATS'); expect(responseData.result.orderCount).to.be.eq(0); done(); }; }); - it('throws an error when an invalid action is attempted', async () => { - const invalidActionPayload = { - action: 'BAD_ACTION', + it('throws an error when an invalid method is attempted', async () => { + const invalidMethodPayload = { + id: 'invalidMethodPayload', + jsonrpc: '2.0', + method: 'BAD_METHOD', }; - wsClient.onopen = () => wsClient.send(JSON.stringify(invalidActionPayload)); + wsClient.onopen = () => wsClient.send(JSON.stringify(invalidMethodPayload)); const errorMsg = await _onMessageAsync(wsClient); const errorData = JSON.parse(errorMsg.data); // tslint:disable-next-line:no-unused-expression - expect(errorData.action).to.be.null; - expect(errorData.success).to.be.false(); - expect(errorData.result).to.match(/^Error: Expected request to conform to schema/); + expect(errorData.id).to.be.null; + // tslint:disable-next-line:no-unused-expression + expect(errorData.method).to.be.null; + expect(errorData.jsonrpc).to.be.eq('2.0'); + expect(errorData.error).to.match(/^Error: Expected request to conform to schema/); + }); + + it('throws an error when jsonrpc field missing from request', async () => { + const noJsonRpcPayload = { + id: 'noJsonRpcPayload', + method: 'GET_STATS', + }; + wsClient.onopen = () => wsClient.send(JSON.stringify(noJsonRpcPayload)); + const errorMsg = await _onMessageAsync(wsClient); + const errorData = JSON.parse(errorMsg.data); + // tslint:disable-next-line:no-unused-expression + expect(errorData.method).to.be.null; + expect(errorData.jsonrpc).to.be.eq('2.0'); + expect(errorData.error).to.match(/^Error: Expected request to conform to schema/); }); it('throws an error when we try to add an order without a signedOrder', async () => { const noSignedOrderAddOrderPayload = { - action: 'ADD_ORDER', - orderHash: '0x0', + id: 'noSignedOrderAddOrderPayload', + jsonrpc: '2.0', + method: 'ADD_ORDER', + orderHash: '0x7337e2f2a9aa2ed6afe26edc2df7ad79c3ffa9cf9b81a964f707ea63f5272355', }; wsClient.onopen = () => wsClient.send(JSON.stringify(noSignedOrderAddOrderPayload)); const errorMsg = await _onMessageAsync(wsClient); const errorData = JSON.parse(errorMsg.data); // tslint:disable-next-line:no-unused-expression - expect(errorData.action).to.be.null; - expect(errorData.success).to.be.false(); - expect(errorData.result).to.match(/^Error: Expected request to conform to schema/); + expect(errorData.id).to.be.null; + // tslint:disable-next-line:no-unused-expression + expect(errorData.method).to.be.null; + expect(errorData.jsonrpc).to.be.eq('2.0'); + expect(errorData.error).to.match(/^Error: Expected request to conform to schema/); }); it('throws an error when we try to add a bad signedOrder', async () => { const invalidAddOrderPayload = { - action: 'ADD_ORDER', + id: 'invalidAddOrderPayload', + jsonrpc: '2.0', + method: 'ADD_ORDER', signedOrder: { makerAddress: '0x0', }, @@ -168,27 +201,26 @@ describe.only('OrderWatcherWebSocket', async () => { const errorMsg = await _onMessageAsync(wsClient); const errorData = JSON.parse(errorMsg.data); // tslint:disable-next-line:no-unused-expression - expect(errorData.action).to.be.null; - expect(errorData.success).to.be.false(); - expect(errorData.result).to.match(/^Error: Expected request to conform to schema/); + expect(errorData.id).to.be.null; + // tslint:disable-next-line:no-unused-expression + expect(errorData.method).to.be.null; + expect(errorData.error).to.match(/^Error: Expected request to conform to schema/); }); it('executes addOrder and removeOrder requests correctly', async () => { wsClient.onopen = () => wsClient.send(JSON.stringify(addOrderPayload)); const addOrderMsg = await _onMessageAsync(wsClient); const addOrderData = JSON.parse(addOrderMsg.data); - expect(addOrderData.action).to.be.eq('ADD_ORDER'); - expect(addOrderData.success).to.be.true(); - expect((wsServer._orderWatcher as any)._orderByOrderHash).to.deep.include({ + expect(addOrderData.method).to.be.eq('ADD_ORDER'); + expect((wsServer as any)._orderWatcher._orderByOrderHash).to.deep.include({ [orderHash]: signedOrder, }); wsClient.send(JSON.stringify(removeOrderPayload)); const removeOrderMsg = await _onMessageAsync(wsClient); const removeOrderData = JSON.parse(removeOrderMsg.data); - expect(removeOrderData.action).to.be.eq('REMOVE_ORDER'); - expect(removeOrderData.success).to.be.true(); - expect((wsServer._orderWatcher as any)._orderByOrderHash).to.not.deep.include({ + expect(removeOrderData.method).to.be.eq('REMOVE_ORDER'); + expect((wsServer as any)._orderWatcher._orderByOrderHash).to.not.deep.include({ [orderHash]: signedOrder, }); }); @@ -204,8 +236,7 @@ describe.only('OrderWatcherWebSocket', async () => { // Ensure that orderStateInvalid message is received. const orderWatcherUpdateMsg = await _onMessageAsync(wsClient); const orderWatcherUpdateData = JSON.parse(orderWatcherUpdateMsg.data); - expect(orderWatcherUpdateData.action).to.be.eq('UPDATE'); - expect(orderWatcherUpdateData.success).to.be.true(); + expect(orderWatcherUpdateData.method).to.be.eq('UPDATE'); const invalidOrderState = orderWatcherUpdateData.result as OrderStateInvalid; expect(invalidOrderState.isValid).to.be.false(); expect(invalidOrderState.orderHash).to.be.eq(orderHash); @@ -227,7 +258,9 @@ describe.only('OrderWatcherWebSocket', async () => { takerAddress, ); const nonZeroMakerFeeOrderPayload = { - action: 'ADD_ORDER', + id: 'nonZeroMakerFeeOrderPayload', + jsonrpc: '2.0', + method: 'ADD_ORDER', signedOrder: nonZeroMakerFeeSignedOrder, }; -- cgit v1.2.3