From b077298c1256dd0ebcbbd790aa12f834c877fee5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Walczak?= Date: Wed, 25 Jun 2025 16:16:05 +0200 Subject: [PATCH] feat: make `blockNumberOrTagOrHash` a required parameter for `eth_getStorageAt` (#3839) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Michał Walczak --- .../src/lib/decorators/cache.decorator.ts | 3 +- packages/relay/src/lib/eth.ts | 6 +-- .../contractService/ContractService.ts | 4 +- .../contractService/IContractService.ts | 2 +- .../tests/lib/eth/eth_getStorageAt.spec.ts | 21 ---------- .../tests/acceptance/rpc_batch2.spec.ts | 40 ------------------- .../server/tests/integration/server.spec.ts | 10 ++--- 7 files changed, 11 insertions(+), 75 deletions(-) diff --git a/packages/relay/src/lib/decorators/cache.decorator.ts b/packages/relay/src/lib/decorators/cache.decorator.ts index 9ccec1b15f..4278db81ce 100644 --- a/packages/relay/src/lib/decorators/cache.decorator.ts +++ b/packages/relay/src/lib/decorators/cache.decorator.ts @@ -49,7 +49,8 @@ const shouldSkipCachingForSingleParams = (args: IArgument[], params: CacheSingle return true; } - // do not cache optional parameters like 'blockNumber' on 'eth_getStorageAt' + // do not cache when a parameter is missing or undefined + // this handles cases where optional parameters are not provided if (!Object.prototype.hasOwnProperty.call(args, item.index) || args[item.index] === undefined) { return true; } diff --git a/packages/relay/src/lib/eth.ts b/packages/relay/src/lib/eth.ts index 122f9e0d60..a990e4305b 100644 --- a/packages/relay/src/lib/eth.ts +++ b/packages/relay/src/lib/eth.ts @@ -671,7 +671,7 @@ export class EthImpl implements Eth { * * @param {string} address - The Ethereum address to get the storage value from * @param {string} slot - The storage slot to get the value from - * @param {string | null} blockNumberOrTagOrHash - The block number or tag or hash to get the storage value from + * @param {string} blockNumberOrTagOrHash - The block number or tag or hash to get the storage value from * @param {RequestDetails} requestDetails - The request details for logging and tracking * @returns {Promise} A promise that resolves to the storage value as a hexadecimal string */ @@ -679,7 +679,7 @@ export class EthImpl implements Eth { @rpcParamValidationRules({ 0: { type: ParamType.ADDRESS, required: true }, 1: { type: ParamType.HEX64, required: true }, - 2: { type: ParamType.BLOCK_NUMBER_OR_HASH, required: false }, + 2: { type: ParamType.BLOCK_NUMBER_OR_HASH, required: true }, }) @rpcParamLayoutConfig(RPC_LAYOUT.custom((params) => [params[0], params[1], params[2]])) @cache(CacheService.getInstance(CACHE_LEVEL.L1), { @@ -688,7 +688,7 @@ export class EthImpl implements Eth { async getStorageAt( address: string, slot: string, - blockNumberOrTagOrHash: string | null, + blockNumberOrTagOrHash: string, requestDetails: RequestDetails, ): Promise { return this.contractService.getStorageAt(address, slot, blockNumberOrTagOrHash, requestDetails); diff --git a/packages/relay/src/lib/services/ethService/contractService/ContractService.ts b/packages/relay/src/lib/services/ethService/contractService/ContractService.ts index 66da4a0962..55ebc18605 100644 --- a/packages/relay/src/lib/services/ethService/contractService/ContractService.ts +++ b/packages/relay/src/lib/services/ethService/contractService/ContractService.ts @@ -288,14 +288,14 @@ export class ContractService implements IContractService { * * @param {string} address - The address of the storage * @param {string} slot - The slot index (hex string) - * @param {string | null} blockNumberOrTagOrHash - Block number, tag, or hash + * @param {string} blockNumberOrTagOrHash - Block number, tag, or hash * @param {RequestDetails} requestDetails - The request details for logging and tracking * @returns {Promise} The value at the given storage position */ public async getStorageAt( address: string, slot: string, - blockNumberOrTagOrHash: string | null, + blockNumberOrTagOrHash: string, requestDetails: RequestDetails, ): Promise { const requestIdPrefix = requestDetails.formattedRequestId; diff --git a/packages/relay/src/lib/services/ethService/contractService/IContractService.ts b/packages/relay/src/lib/services/ethService/contractService/IContractService.ts index decc86fa0d..a8d6ad54b2 100644 --- a/packages/relay/src/lib/services/ethService/contractService/IContractService.ts +++ b/packages/relay/src/lib/services/ethService/contractService/IContractService.ts @@ -35,7 +35,7 @@ export interface IContractService { getStorageAt: ( address: string, slot: string, - blockNumberOrTagOrHash: string | null, + blockNumberOrTagOrHash: string, requestDetails: RequestDetails, ) => Promise; diff --git a/packages/relay/tests/lib/eth/eth_getStorageAt.spec.ts b/packages/relay/tests/lib/eth/eth_getStorageAt.spec.ts index 3cb8856b74..cee6d5d654 100644 --- a/packages/relay/tests/lib/eth/eth_getStorageAt.spec.ts +++ b/packages/relay/tests/lib/eth/eth_getStorageAt.spec.ts @@ -205,27 +205,6 @@ describe('@ethGetStorageAt eth_getStorageAt spec', async function () { // verify slot value }); - // Block number is a required param, this should not work and should be removed when/if validations are added. - // Instead, the relay should return `missing value for required argument error`. - it('eth_getStorageAt with match null block', async function () { - // mirror node request mocks - restMock - .onGet( - `contracts/${CONTRACT_ADDRESS_1}/state?slot=${DEFAULT_CURRENT_CONTRACT_STATE.state[0].slot}&limit=100&order=desc`, - ) - .reply(200, JSON.stringify(DEFAULT_CURRENT_CONTRACT_STATE)); - - const result = await ethImpl.getStorageAt( - CONTRACT_ADDRESS_1, - defaultDetailedContractResults.state_changes[0].slot, - null, - requestDetails, - ); - confirmResult(result); - - // verify slot value - }); - it('eth_getStorageAt should throw a predefined RESOURCE_NOT_FOUND when block not found', async function () { restMock.onGet(`blocks/${BLOCK_NUMBER}`).reply(200, JSON.stringify(null)); diff --git a/packages/server/tests/acceptance/rpc_batch2.spec.ts b/packages/server/tests/acceptance/rpc_batch2.spec.ts index faff6b72f9..f0d7ae8776 100644 --- a/packages/server/tests/acceptance/rpc_batch2.spec.ts +++ b/packages/server/tests/acceptance/rpc_batch2.spec.ts @@ -971,46 +971,6 @@ describe('@api-batch-2 RPC Server Acceptance Tests', function () { expect(storageValBeforeChange).to.eq(beginStorageVal); }); - it('should execute "eth_getStorageAt" request to get current state changes without passing block', async function () { - const BEGIN_EXPECTED_STORAGE_VAL = '0x000000000000000000000000000000000000000000000000000000000000000f'; - const END_EXPECTED_STORAGE_VAL = '0x0000000000000000000000000000000000000000000000000000000000000008'; - - const beginStorageVal = await relay.call( - RelayCalls.ETH_ENDPOINTS.ETH_GET_STORAGE_AT, - [storageContractAddress, '0x0000000000000000000000000000000000000000000000000000000000000000'], - requestId, - ); - expect(beginStorageVal).to.eq(BEGIN_EXPECTED_STORAGE_VAL); - - const gasPrice = await relay.gasPrice(requestId); - const transaction = { - value: 0, - gasLimit: 50000, - chainId: Number(CHAIN_ID), - to: storageContractAddress, - nonce: await relay.getAccountNonce(accounts[1].address), - gasPrice: gasPrice, - data: STORAGE_CONTRACT_UPDATE, - maxPriorityFeePerGas: gasPrice, - maxFeePerGas: gasPrice, - type: 2, - }; - - const signedTx = await accounts[1].wallet.signTransaction(transaction); - const transactionHash = await relay.sendRawTransaction(signedTx, requestId); - await relay.pollForValidTransactionReceipt(transactionHash); - - // wait for the transaction to propogate to mirror node - await new Promise((r) => setTimeout(r, 4000)); - - const storageVal = await relay.call( - RelayCalls.ETH_ENDPOINTS.ETH_GET_STORAGE_AT, - [storageContractAddress, '0x0000000000000000000000000000000000000000000000000000000000000000'], - requestId, - ); - expect(storageVal).to.eq(END_EXPECTED_STORAGE_VAL); - }); - it('should execute "eth_getStorageAt" request to get current state changes with passing specific block', async function () { const EXPECTED_STORAGE_VAL = '0x0000000000000000000000000000000000000000000000000000000000000008'; diff --git a/packages/server/tests/integration/server.spec.ts b/packages/server/tests/integration/server.spec.ts index ad8e6b35de..b253038d94 100644 --- a/packages/server/tests/integration/server.spec.ts +++ b/packages/server/tests/integration/server.spec.ts @@ -2007,22 +2007,18 @@ describe('RPC Server', function () { } }); - it('validates parameter 2 is valid block tag', async function () { + it('validates parameter 2 exists', async function () { try { await testClient.post('/', { id: '2', jsonrpc: '2.0', method: RelayCalls.ETH_ENDPOINTS.ETH_GET_STORAGE_AT, - params: ['0x0000000000000000000000000000000000000001', '0x1', 'newest'], + params: ['0x0000000000000000000000000000000000000001', '0x1'], }); Assertions.expectedError(); } catch (error: any) { - BaseTest.invalidParamError( - error.response, - Validator.ERROR_CODE, - `Invalid parameter 2: The value passed is not valid: newest. ${Constants.BLOCK_NUMBER_ERROR} OR ${Constants.BLOCK_HASH_ERROR}`, - ); + BaseTest.invalidParamError(error.response, Validator.ERROR_CODE, MISSING_PARAM_ERROR + ' 2'); } }); });