From ede1fa89c2cbfd2832d1ba9a893df28b232a07dc Mon Sep 17 00:00:00 2001 From: Eric Badiere Date: Sun, 10 Nov 2024 16:04:43 -0700 Subject: [PATCH 1/2] fix: Added gas deviation, resolving promises to avoid waiting and connection handling to address flaky tests. Signed-off-by: Eric Badiere --- packages/server/tests/helpers/utils.ts | 6 +- .../ws-server/tests/acceptance/call.spec.ts | 2 +- .../tests/acceptance/getBlockByNumber.spec.ts | 8 +- .../acceptance/getTransactionReceipt.spec.ts | 2 +- .../tests/acceptance/newFilter.spec.ts | 2 +- .../tests/acceptance/subscribe.spec.ts | 31 +++++--- .../acceptance/subscribeNewHeads.spec.ts | 5 +- .../tests/acceptance/validations.spec.ts | 4 +- packages/ws-server/tests/helper/index.ts | 73 ++++++++++++++++++- 9 files changed, 108 insertions(+), 25 deletions(-) diff --git a/packages/server/tests/helpers/utils.ts b/packages/server/tests/helpers/utils.ts index f40a4f063c..b3506a22e2 100644 --- a/packages/server/tests/helpers/utils.ts +++ b/packages/server/tests/helpers/utils.ts @@ -256,13 +256,17 @@ export class Utils { requestId: any, mirrorNodeServer: any, ) => { + const gasPriceDeviation = parseFloat(ConfigService.get('TEST_GAS_PRICE_DEVIATION') ?? '0.2'); + const gasPrice = await rpcServer.gasPrice(requestId); + const gasPriceWithDeviation = gasPrice * (1 + gasPriceDeviation); + const transaction = { value: ONE_TINYBAR, gasLimit: numberTo0x(30000), chainId: Number(CHAIN_ID), to: accounts[1].address, nonce: await rpcServer.getAccountNonce(accounts[0].address, requestId), - maxFeePerGas: await rpcServer.gasPrice(requestId), + maxFeePerGas: gasPriceWithDeviation, }; const signedTx = await accounts[0].wallet.signTransaction(transaction); diff --git a/packages/ws-server/tests/acceptance/call.spec.ts b/packages/ws-server/tests/acceptance/call.spec.ts index a1f6a17948..cc97f8cdcd 100644 --- a/packages/ws-server/tests/acceptance/call.spec.ts +++ b/packages/ws-server/tests/acceptance/call.spec.ts @@ -100,7 +100,7 @@ describe('@web-socket-batch-1 eth_call', async function () { }); afterEach(async () => { - if (ethersWsProvider) await ethersWsProvider.destroy(); + ethersWsProvider = await WsTestHelper.closeWebsocketConnections(ethersWsProvider); }); after(async () => { diff --git a/packages/ws-server/tests/acceptance/getBlockByNumber.spec.ts b/packages/ws-server/tests/acceptance/getBlockByNumber.spec.ts index bac77b65bc..852970e93d 100644 --- a/packages/ws-server/tests/acceptance/getBlockByNumber.spec.ts +++ b/packages/ws-server/tests/acceptance/getBlockByNumber.spec.ts @@ -44,7 +44,7 @@ describe('@web-socket-batch-1 eth_getBlockByNumber', async function () { }); afterEach(async () => { - if (ethersWsProvider) await ethersWsProvider.destroy(); + ethersWsProvider = await WsTestHelper.closeWebsocketConnections(ethersWsProvider); }); after(async () => { @@ -63,8 +63,10 @@ describe('@web-socket-batch-1 eth_getBlockByNumber', async function () { it(`@release Should execute eth_getBlockByNumber on Standard Web Socket and handle valid requests correctly`, async () => { const expectedResult = await global.relay.call(METHOD_NAME, ['latest', false]); - const response = await WsTestHelper.sendRequestToStandardWebSocket(METHOD_NAME, [expectedResult.number, true]); - await Utils.wait(1000); + const response = await WsTestHelper.sendRequestToStandardWebSocketWithResolve(METHOD_NAME, [ + expectedResult.number, + true, + ]); WsTestHelper.assertJsonRpcObject(response); expect(response.result).to.deep.eq(expectedResult); }); diff --git a/packages/ws-server/tests/acceptance/getTransactionReceipt.spec.ts b/packages/ws-server/tests/acceptance/getTransactionReceipt.spec.ts index be26f3061b..1f92908686 100644 --- a/packages/ws-server/tests/acceptance/getTransactionReceipt.spec.ts +++ b/packages/ws-server/tests/acceptance/getTransactionReceipt.spec.ts @@ -92,7 +92,7 @@ describe('@web-socket-batch-2 eth_getTransactionReceipt', async function () { }); afterEach(async () => { - if (ethersWsProvider) await ethersWsProvider.destroy(); + ethersWsProvider = await WsTestHelper.closeWebsocketConnections(ethersWsProvider); }); after(async () => { diff --git a/packages/ws-server/tests/acceptance/newFilter.spec.ts b/packages/ws-server/tests/acceptance/newFilter.spec.ts index 2d3984119d..d8801919ae 100644 --- a/packages/ws-server/tests/acceptance/newFilter.spec.ts +++ b/packages/ws-server/tests/acceptance/newFilter.spec.ts @@ -83,7 +83,7 @@ describe('@web-socket-batch-2 eth_newFilter', async function () { } it(`@release Should execute eth_newFilter on Standard Web Socket and handle valid requests correctly`, async () => { - const response = await WsTestHelper.sendRequestToStandardWebSocket(METHOD_NAME, [wsFilterObj]); + const response = await WsTestHelper.sendRequestToStandardWebSocketWithResolve(METHOD_NAME, [wsFilterObj]); WsTestHelper.assertJsonRpcObject(response); const filterId = response.result; diff --git a/packages/ws-server/tests/acceptance/subscribe.spec.ts b/packages/ws-server/tests/acceptance/subscribe.spec.ts index 479705e632..2112759173 100644 --- a/packages/ws-server/tests/acceptance/subscribe.spec.ts +++ b/packages/ws-server/tests/acceptance/subscribe.spec.ts @@ -50,7 +50,7 @@ const unsubscribeAndCloseConnections = async (provider: ethers.WebSocketProvider return result; }; -const createLogs = async (contract: ethers.Contract, requestId) => { +const createLogs = async (wsProvider: ethers.WebSocketProvider, contract: ethers.Contract, requestId) => { const gasOptions = await Utils.gasOptions(requestId); const tx1 = await contract.log0(10, gasOptions); @@ -68,9 +68,22 @@ const createLogs = async (contract: ethers.Contract, requestId) => { const tx5 = await contract.log4(11, 22, 33, 44, gasOptions); await tx5.wait(); - await new Promise((resolve) => setTimeout(resolve, 2000)); + fetchTransactionWithRetry(wsProvider, tx5.hash); }; +async function fetchTransactionWithRetry(txHash, maxRetries = 5, delay = 1000) { + for (let attempt = 1; attempt <= maxRetries; attempt++) { + const transaction = await WsTestHelper.sendRequestToStandardWebSocket('eth_getTransactionByHash', [txHash]); + if (transaction && transaction.result.blockNumber) { + // Transaction is fully populated + return transaction; + } + // Wait for a specified delay before retrying + await new Promise((resolve) => setTimeout(resolve, delay)); + } + throw new Error(`Transaction ${txHash} not fully populated after ${maxRetries} attempts.`); +} + describe('@web-socket-batch-3 eth_subscribe', async function () { this.timeout(240 * 1000); // 240 seconds const CHAIN_ID = ConfigService.get('CHAIN_ID') || 0; @@ -123,11 +136,7 @@ describe('@web-socket-batch-3 eth_subscribe', async function () { }); afterEach(async () => { - if (wsProvider) { - await wsProvider.destroy(); - await new Promise((resolve) => setTimeout(resolve, 1000)); - } - if (server) expect(server._connections).to.equal(0); + wsProvider = await WsTestHelper.closeWebsocketConnections(wsProvider); }); describe('Connection', async function () { @@ -561,7 +570,7 @@ describe('@web-socket-batch-3 eth_subscribe', async function () { describe('Subscribes to log events', async function () { let logContractSigner2, logContractSigner3, wsLogsProvider, contracts, cLen; let ANONYMOUS_LOG_DATA, topic1, topic2; - let eventsReceivedGlobal: any[] = []; + const eventsReceivedGlobal: any[] = []; // Deploy several contracts before(async function () { @@ -573,7 +582,7 @@ describe('@web-socket-batch-3 eth_subscribe', async function () { logContractSigner2 = await Utils.deployContractWithEthersV2([], LogContractJson, accounts[0].wallet); logContractSigner3 = await Utils.deployContractWithEthersV2([], LogContractJson, accounts[0].wallet); - await createLogs(logContractSigner2, requestId); + await createLogs(wsLogsProvider, logContractSigner2, requestId); const mirrorLogs = await mirrorNode.get(`/contracts/${logContractSigner2.target}/results/logs`, requestId); expect(mirrorLogs).to.exist; @@ -633,7 +642,7 @@ describe('@web-socket-batch-3 eth_subscribe', async function () { // Create logs from all deployed contracts for (let i = 0; i < cLen; i++) { - await createLogs(contracts[i], requestId); + await createLogs(wsLogsProvider, contracts[i], requestId); } await wsLogsProvider.websocket.close(); @@ -652,7 +661,7 @@ describe('@web-socket-batch-3 eth_subscribe', async function () { }); it('@release Subscribes for contract logs for a specific contract address (using evmAddress)', async function () { - let eventsReceived = eventsReceivedGlobal[1]; + const eventsReceived = eventsReceivedGlobal[1]; // Only the logs from logContractSigner.target are captured expect(eventsReceived.length).to.eq(5); diff --git a/packages/ws-server/tests/acceptance/subscribeNewHeads.spec.ts b/packages/ws-server/tests/acceptance/subscribeNewHeads.spec.ts index d60baf698c..6aebcb1011 100644 --- a/packages/ws-server/tests/acceptance/subscribeNewHeads.spec.ts +++ b/packages/ws-server/tests/acceptance/subscribeNewHeads.spec.ts @@ -136,10 +136,7 @@ describe('@web-socket-batch-3 eth_subscribe newHeads', async function () { }); afterEach(async () => { - if (wsProvider) { - await wsProvider.destroy(); - await new Promise((resolve) => setTimeout(resolve, 1000)); - } + wsProvider = await WsTestHelper.closeWebsocketConnections(wsProvider); }); describe('Configuration', async function () { diff --git a/packages/ws-server/tests/acceptance/validations.spec.ts b/packages/ws-server/tests/acceptance/validations.spec.ts index 9f23a7909b..1efbb8d3f9 100644 --- a/packages/ws-server/tests/acceptance/validations.spec.ts +++ b/packages/ws-server/tests/acceptance/validations.spec.ts @@ -51,14 +51,14 @@ describe('@release @web-socket-batch-1 JSON-RPC requests validation', async func WsTestHelper.overrideEnvsInMochaDescribe({ REQUEST_ID_IS_OPTIONAL: true }); - let ethersWsProvider: WebSocketProvider; + let ethersWsProvider: WebSocketProvider | null; beforeEach(async () => { ethersWsProvider = new ethers.WebSocketProvider(WsTestConstant.WS_RELAY_URL); }); afterEach(async () => { - if (ethersWsProvider) await ethersWsProvider.destroy(); + ethersWsProvider = await WsTestHelper.closeWebsocketConnections(ethersWsProvider); }); after(async () => { diff --git a/packages/ws-server/tests/helper/index.ts b/packages/ws-server/tests/helper/index.ts index 77d03e00e7..7e77ebe068 100644 --- a/packages/ws-server/tests/helper/index.ts +++ b/packages/ws-server/tests/helper/index.ts @@ -20,7 +20,7 @@ import WebSocket from 'ws'; import { expect } from 'chai'; -import { WebSocketProvider } from 'ethers'; +import { ethers, WebSocketProvider } from 'ethers'; import { ConfigService } from '@hashgraph/json-rpc-config-service/dist/services'; import { ConfigServiceTestHelper } from '../../../config-service/tests/configServiceTestHelper'; @@ -68,6 +68,52 @@ export class WsTestHelper { return response; } + static sendRequestToStandardWebSocketWithResolve(method: string, params: any, timeout = 5000): Promise { + return new Promise((resolve, reject) => { + const BATCH_REQUEST_METHOD_NAME = 'batch_request'; + const webSocket = new WebSocket(WsTestConstant.WS_RELAY_URL); + let isResolved = false; + + const timer = setTimeout(() => { + if (!isResolved) { + webSocket.close(); + reject(new Error('WebSocket response timeout')); + } + }, timeout); + + webSocket.on('error', (error) => { + if (!isResolved) { + isResolved = true; + clearTimeout(timer); + webSocket.close(); + reject(error); + } + }); + + webSocket.on('open', () => { + let request; + + if (method === BATCH_REQUEST_METHOD_NAME) { + request = JSON.stringify(params); + } else { + request = JSON.stringify(WsTestHelper.prepareJsonRpcObject(method, params)); + } + + webSocket.send(request); + }); + + webSocket.on('message', (data: string) => { + if (!isResolved) { + isResolved = true; + clearTimeout(timer); + const response = JSON.parse(data); + webSocket.close(); + resolve(response); + } + }); + }); + } + static async assertFailInvalidParamsStandardWebSocket(method: string, params: any[]) { const response = await WsTestHelper.sendRequestToStandardWebSocket(method, params); WsTestHelper.assertJsonRpcObject(response); @@ -162,6 +208,31 @@ export class WsTestHelper { tests(); }); } + + static async closeWebsocketConnections(ethersWsProvider: ethers.WebSocketProvider | null) { + if (ethersWsProvider) { + const ws = ethersWsProvider._websocket; + if (ws) { + if (ws.readyState === WebSocket.CONNECTING) { + // Wait for the WebSocket to either open or close + await new Promise((resolve) => { + const handleOpenOrClose = () => { + ws.removeEventListener('open', handleOpenOrClose); + ws.removeEventListener('close', handleOpenOrClose); + resolve(); + }; + ws.addEventListener('open', handleOpenOrClose); + ws.addEventListener('close', handleOpenOrClose); + }); + } + // Now it's safe to destroy the provider + await ethersWsProvider.destroy(); + } + ethersWsProvider = null; + await new Promise((resolve) => setTimeout(resolve, 1000)); + } + return ethersWsProvider; + } } export class WsTestConstant { From 8a9d0c445501b61c1786ee642910c618887165d4 Mon Sep 17 00:00:00 2001 From: Eric Badiere Date: Tue, 12 Nov 2024 11:10:53 -0700 Subject: [PATCH 2/2] fix: Added the SUBSCRIPTIONS_ENABLED=true option for tests running against servers running locally. Signed-off-by: Eric Badiere --- packages/server/tests/localAcceptance.env | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/server/tests/localAcceptance.env b/packages/server/tests/localAcceptance.env index 1b926182c1..deccd65d0b 100644 --- a/packages/server/tests/localAcceptance.env +++ b/packages/server/tests/localAcceptance.env @@ -32,3 +32,4 @@ HBAR_RATE_LIMIT_TINYBAR=9000000000# 90 HBARs HBAR_RATE_LIMIT_DURATION=86400000# 24 hours HBAR_RATE_LIMIT_BASIC=6000000000# 60 HBARs HBAR_SPENDING_PLANS_CONFIG_FILE=./packages/server/tests/testSpendingPlansConfig.json +SUBSCRIPTIONS_ENABLED=true