From c6cfd6ccffd600d98e2acab0073e8d73c1b8de74 Mon Sep 17 00:00:00 2001 From: Logan Nguyen Date: Wed, 22 May 2024 16:17:48 -0400 Subject: [PATCH] feat: added constructValidLogSubscriptionFilter() to discard any unexpected parameters for logs subcription filter object (#2521) * fix: removed param validator for eth_subscribe:logs Signed-off-by: Logan Nguyen * fix: added unexpectedParams validator back Signed-off-by: Logan Nguyen * feat: added constructValidLogSubscriptionFilter() to strip off unexpected params Signed-off-by: Logan Nguyen * fix: fixed multiple address check Signed-off-by: Logan Nguyen --------- Signed-off-by: Logan Nguyen --- .../src/controllers/eth_subscribe.ts | 16 +++-- packages/ws-server/src/utils/utils.ts | 11 +++ .../tests/acceptance/subscribe.spec.ts | 16 +++++ packages/ws-server/tests/unit/utils.spec.ts | 70 +++++++++++++++++++ 4 files changed, 108 insertions(+), 5 deletions(-) create mode 100644 packages/ws-server/tests/unit/utils.spec.ts diff --git a/packages/ws-server/src/controllers/eth_subscribe.ts b/packages/ws-server/src/controllers/eth_subscribe.ts index f21fc292e8..e35370820a 100644 --- a/packages/ws-server/src/controllers/eth_subscribe.ts +++ b/packages/ws-server/src/controllers/eth_subscribe.ts @@ -18,12 +18,12 @@ * */ -import { getMultipleAddressesEnabled } from '../utils/utils'; import { predefined, Relay } from '@hashgraph/json-rpc-relay'; import { validateSubscribeEthLogsParams } from '../utils/validators'; import constants from '@hashgraph/json-rpc-relay/dist/lib/constants'; -import jsonResp from '@hashgraph/json-rpc-server/dist/koaJsonRpc/lib/RpcResponse'; import { MirrorNodeClient } from '@hashgraph/json-rpc-relay/dist/lib/clients'; +import jsonResp from '@hashgraph/json-rpc-server/dist/koaJsonRpc/lib/RpcResponse'; +import { constructValidLogSubscriptionFilter, getMultipleAddressesEnabled } from '../utils/utils'; /** * Subscribes to new block headers (newHeads) events and returns the response and subscription ID. @@ -115,15 +115,21 @@ const handleEthSubscribeLogs = async ( relay: Relay, mirrorNodeClient: MirrorNodeClient, ): Promise<{ response: any; subscriptionId: any }> => { - await validateSubscribeEthLogsParams(filters, requestIdPrefix, mirrorNodeClient); - if (!getMultipleAddressesEnabled() && Array.isArray(filters.address) && filters.address.length > 1) { + const validFiltersObject = constructValidLogSubscriptionFilter(filters); + + await validateSubscribeEthLogsParams(validFiltersObject, requestIdPrefix, mirrorNodeClient); + if ( + !getMultipleAddressesEnabled() && + Array.isArray(validFiltersObject['address']) && + validFiltersObject['address'].length > 1 + ) { response = jsonResp( request.id, predefined.INVALID_PARAMETER('filters.address', 'Only one contract address is allowed'), undefined, ); } else { - subscriptionId = relay.subs()?.subscribe(ctx.websocket, event, filters); + subscriptionId = relay.subs()?.subscribe(ctx.websocket, event, validFiltersObject); } return { response, subscriptionId }; }; diff --git a/packages/ws-server/src/utils/utils.ts b/packages/ws-server/src/utils/utils.ts index d40d132440..20f2ccf680 100644 --- a/packages/ws-server/src/utils/utils.ts +++ b/packages/ws-server/src/utils/utils.ts @@ -195,3 +195,14 @@ const hasInvalidReqestId = ( return !hasId; }; + +/** + * Constructs a valid log subscription filter from the provided filters, retaining only the 'address' and 'topics' fields while discarding any unexpected parameters. + * @param {any} filters - The filters to construct the subscription filter from. + * @returns {Object} A valid log subscription filter object. + */ +export const constructValidLogSubscriptionFilter = (filters: any): object => { + return Object.fromEntries( + Object.entries(filters).filter(([key, value]) => value !== undefined && ['address', 'topics'].includes(key)), + ); +}; diff --git a/packages/ws-server/tests/acceptance/subscribe.spec.ts b/packages/ws-server/tests/acceptance/subscribe.spec.ts index 122e566d33..71037965ac 100644 --- a/packages/ws-server/tests/acceptance/subscribe.spec.ts +++ b/packages/ws-server/tests/acceptance/subscribe.spec.ts @@ -22,6 +22,7 @@ import WebSocket from 'ws'; import { ethers } from 'ethers'; import chai, { expect } from 'chai'; +import { WsTestHelper } from '../helper'; import { solidity } from 'ethereum-waffle'; import { Utils } from '@hashgraph/json-rpc-server/tests/helpers/utils'; import Constants from '@hashgraph/json-rpc-server/tests/helpers/constants'; @@ -889,6 +890,21 @@ describe('@release @web-socket-batch-3 eth_subscribe', async function () { ['logs', { address: logContractSigner.target, topics: ['0x000'] }], ]); }); + + it('Should ignore invalid params in filter object and still successfully call eth_subscribe Logs ', async function () { + const randomTopic = '0x1d29d0f04057864b829c60f025fdba344f1623eb30b90820f5a6c39ffbd1c512'; + + // @notice: the only two valid params a filter object can have is `address` and `topics`. + // However, WS server should ignore any invalid params passed in the request. + const response = await WsTestHelper.sendRequestToStandardWebSocket( + 'eth_subscribe', + ['logs', { address: logContractSigner.target, topics: [randomTopic], fromBlock: '0x0', toBlock: 'latest' }], + 1000, + ); + WsTestHelper.assertJsonRpcObject(response); + expect(response.result).to.exist; + expect(ethers.isHexString(response.result)).to.be.true; + }); }); // skip this test if using a remote relay since updating the env vars would not affect it diff --git a/packages/ws-server/tests/unit/utils.spec.ts b/packages/ws-server/tests/unit/utils.spec.ts new file mode 100644 index 0000000000..c1479d3004 --- /dev/null +++ b/packages/ws-server/tests/unit/utils.spec.ts @@ -0,0 +1,70 @@ +/*- + * + * Hedera JSON RPC Relay + * + * Copyright (C) 2024 Hedera Hashgraph, LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +import { expect } from 'chai'; +import { constructValidLogSubscriptionFilter } from '../../src/utils/utils'; + +describe('Utilities unit tests', async function () { + describe('constructValidLogSubscriptionFilter tests', () => { + it('Should ignore all the unexpected params and return a new filter object with valid params (address, topics)', () => { + const originalFilter = { + address: '0x23f5e49569A835d7bf9AefD30e4f60CdD570f225', + topics: ['0x1d29d0f04057864b829c60f025fdba344f1623eb30b90820f5a6c39ffbd1c512'], + fromBlock: '0x0', + toBlock: 'latest', + hedera: '0xhbar', + }; + const originalFilterKeys = Object.keys(originalFilter); + + const validFilter = constructValidLogSubscriptionFilter(originalFilter); + const validFilterKeys = Object.keys(validFilter); + + expect(validFilterKeys).to.not.deep.eq(originalFilterKeys); + expect(validFilterKeys.length).to.eq(2); // address & topics + expect(validFilter['address']).to.eq(originalFilter.address); + expect(validFilter['topics']).to.eq(originalFilter.topics); + expect(validFilter['fromBlock']).to.not.exist; + expect(validFilter['toBlock']).to.not.exist; + expect(validFilter['hedera']).to.not.exist; + }); + + it('Should only add valid params if presented in original filter object', () => { + // original missing `address` param + const originalFilter1 = { + topics: ['0x1d29d0f04057864b829c60f025fdba344f1623eb30b90820f5a6c39ffbd1c512'], + }; + const validFilter1 = constructValidLogSubscriptionFilter(originalFilter1); + const validFilter1Keys = Object.keys(validFilter1); + expect(validFilter1Keys.length).to.eq(1); + expect(validFilter1['address']).to.not.exist; + expect(validFilter1['topics']).to.eq(originalFilter1.topics); + + // original missing `topics` param + const originalFilter2 = { + address: '0x23f5e49569A835d7bf9AefD30e4f60CdD570f225', + }; + const validFilter2 = constructValidLogSubscriptionFilter(originalFilter2); + const validFilter2Keys = Object.keys(validFilter2); + expect(validFilter2Keys.length).to.eq(1); + expect(validFilter2['topics']).to.not.exist; + expect(validFilter2['address']).to.eq(originalFilter2.address); + }); + }); +});