Skip to content

feat: make blockNumberOrTagOrHash a required parameter for eth_getStorageAt (#3839) #3879

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion packages/relay/src/lib/decorators/cache.decorator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
6 changes: 3 additions & 3 deletions packages/relay/src/lib/eth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -671,15 +671,15 @@ 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<string>} A promise that resolves to the storage value as a hexadecimal string
*/
@rpcMethod
@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), {
Expand All @@ -688,7 +688,7 @@ export class EthImpl implements Eth {
async getStorageAt(
address: string,
slot: string,
blockNumberOrTagOrHash: string | null,
blockNumberOrTagOrHash: string,
requestDetails: RequestDetails,
): Promise<string> {
return this.contractService.getStorageAt(address, slot, blockNumberOrTagOrHash, requestDetails);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<string>} The value at the given storage position
*/
public async getStorageAt(
address: string,
slot: string,
blockNumberOrTagOrHash: string | null,
blockNumberOrTagOrHash: string,
requestDetails: RequestDetails,
): Promise<string> {
const requestIdPrefix = requestDetails.formattedRequestId;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export interface IContractService {
getStorageAt: (
address: string,
slot: string,
blockNumberOrTagOrHash: string | null,
blockNumberOrTagOrHash: string,
requestDetails: RequestDetails,
) => Promise<string>;

Expand Down
21 changes: 0 additions & 21 deletions packages/relay/tests/lib/eth/eth_getStorageAt.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 <argumentIndex> 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));

Expand Down
40 changes: 0 additions & 40 deletions packages/server/tests/acceptance/rpc_batch2.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down
10 changes: 3 additions & 7 deletions packages/server/tests/integration/server.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
}
});
});
Expand Down