Skip to content

Commit 22a61a5

Browse files
Nana-ECgeorgi-l95
Nana-EC
and
georgi-l95
authored
Add nullable check to estimateGas validation (#849)
MM seems to send a null data param in the transaction object. We should not be validating in that case. - Expand trace logs to `eth.estimateGas()` to highlight param - Add nullable concept to validation object params and set data in transaction object to nullable - Expand `predefined.INVALID_PARAMETER` logs to make failures easier to troubleshoot - Updated and expanded tests to confirm `isValidAndNonNullableParam` logic - Updated RPC tests impacted by addition of value to the log Signed-off-by: georgi-l95 <[email protected]> Signed-off-by: Nana Essilfie-Conduah <[email protected]> Signed-off-by: georgi-l95 <[email protected]> Co-authored-by: georgi-l95 <[email protected]>
1 parent 57f73c0 commit 22a61a5

File tree

6 files changed

+258
-180
lines changed

6 files changed

+258
-180
lines changed

‎packages/relay/src/lib/eth.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -322,7 +322,7 @@ export class EthImpl implements Eth {
322322
// eslint-disable-next-line @typescript-eslint/no-unused-vars
323323
async estimateGas(transaction: any, _blockParam: string | null, requestId?: string) {
324324
const requestIdPrefix = formatRequestIdMessage(requestId);
325-
this.logger.trace(`${requestIdPrefix} estimateGas()`);
325+
this.logger.trace(`${requestIdPrefix} estimateGas(transaction=${JSON.stringify(transaction)}, _blockParam=${_blockParam})`);
326326
if (!transaction || !transaction.data || transaction.data === '0x') {
327327
return EthImpl.gasTxBaseCost;
328328
} else {

‎packages/server/src/validator/objectTypes.ts‎

Lines changed: 40 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -4,70 +4,90 @@ import { predefined } from '@hashgraph/json-rpc-relay';
44
export const OBJECTS_VALIDATIONS = {
55
"blockHashObject": {
66
"blockHash": {
7-
type: "blockHash"
7+
type: "blockHash",
8+
nullable: false
89
}
910
},
1011
"blockNumberObject": {
1112
"blockNumber": {
12-
type: "blockNumber"
13+
type: "blockNumber",
14+
nullable: false
1315
}
1416
},
1517
"filter": {
1618
"blockHash": {
17-
type: "blockHash"
19+
type: "blockHash",
20+
nullable: false
1821
},
1922
"fromBlock": {
20-
type: "blockNumber"
23+
type: "blockNumber",
24+
nullable: false
2125
},
2226
"toBlock": {
23-
type: "blockNumber"
27+
type: "blockNumber",
28+
nullable: false
2429
},
2530
"address": {
26-
type: "addressFilter"
31+
type: "addressFilter",
32+
nullable: false
2733
},
2834
"topics": {
29-
type: "topics"
35+
type: "topics",
36+
nullable: false
3037
}
3138
},
3239
"transaction": {
3340
"from": {
34-
type: "address"
41+
type: "address",
42+
nullable: false
3543
},
3644
"to": {
37-
type: "address"
45+
type: "address",
46+
nullable: false
3847
},
3948
"gas": {
40-
type: "hex"
49+
type: "hex",
50+
nullable: false
4151
},
4252
"gasPrice": {
43-
type: "hex"
53+
type: "hex",
54+
nullable: false
4455
},
4556
"maxPriorityFeePerGas": {
46-
type: "hex"
57+
type: "hex",
58+
nullable: false
4759
},
4860
"maxFeePerGas": {
49-
type: "hex"
61+
type: "hex",
62+
nullable: false
5063
},
5164
"value": {
52-
type: "hex"
65+
type: "hex",
66+
nullable: false
5367
},
5468
"data": {
55-
type: "hex"
69+
type: "hex",
70+
nullable: true
5671
},
5772
"type": {
58-
type: "hex"
73+
type: "hex",
74+
nullable: false
5975
},
6076
"chainId": {
61-
type: "hex"
77+
type: "hex",
78+
nullable: false
6279
},
6380
"nonce": {
64-
type: "hex"
81+
type: "hex",
82+
nullable: false
6583
},
6684
"input": {
67-
type: "hex"
85+
type: "hex",
86+
nullable: false
6887
},
6988
"accessList": {
70-
type: "array"
89+
type: "array",
90+
nullable: false
7191
}
7292
}
7393
};

‎packages/server/src/validator/utils.ts‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ export function validateParam(index: number | string, param: any, validation: an
1616
if (param != null) {
1717
const result = isArray? paramType.test(index, param, validation.type[1]) : paramType.test(param);
1818
if(result === false) {
19-
throw predefined.INVALID_PARAMETER(index, paramType.error);
19+
throw predefined.INVALID_PARAMETER(index, `${paramType.error}, value: ${param}`);
2020
}
2121
}
2222
}
@@ -31,16 +31,16 @@ export function validateObject(object: any, filters: any) {
3131
throw predefined.MISSING_REQUIRED_PARAMETER(`'${property}' for ${object.name()}`);
3232
}
3333

34-
if (param !== undefined) {
34+
if (isValidAndNonNullableParam(param, validation.nullable)) {
3535
try {
3636
result = Validator.TYPES[validation.type].test(param);
3737

3838
if(!result) {
39-
throw predefined.INVALID_PARAMETER(`'${property}' for ${object.name()}`, Validator.TYPES[validation.type].error);
39+
throw predefined.INVALID_PARAMETER(`'${property}' for ${object.name()}`, `${Validator.TYPES[validation.type].error}, value: ${param}`);
4040
}
4141
} catch(error: any) {
4242
if (error instanceof JsonRpcError) {
43-
throw predefined.INVALID_PARAMETER(`'${property}' for ${object.name()}`, Validator.TYPES[validation.type].error);
43+
throw predefined.INVALID_PARAMETER(`'${property}' for ${object.name()}`, `${Validator.TYPES[validation.type].error}, value: ${param}`);
4444
}
4545

4646
throw error;
@@ -71,3 +71,7 @@ export function hasUnexpectedParams(actual: any, expected: any, object: string)
7171
export function requiredIsMissing(param: any, required: boolean) {
7272
return required && param === undefined;
7373
}
74+
75+
export function isValidAndNonNullableParam(param: any, nullable: boolean) {
76+
return param !== undefined && (param !== null || !nullable);
77+
}

‎packages/server/tests/acceptance/rpc.spec.ts‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -961,8 +961,8 @@ describe('@api RPC Server Acceptance Tests', function () {
961961
it('should not be able to execute "eth_estimateGas" with wrong from field', async function () {
962962
try {
963963
await relay.call('eth_estimateGas', [{
964-
from: '0x114f60009ee6b84861c0cdae8829751e517bc4d7',
965-
to: '0xae410f34f7487e2cd03396499cebb09b79f45',
964+
from: '0x114f60009ee6b84861c0cdae8829751e517b',
965+
to: '0xae410f34f7487e2cd03396499cebb09b79f45d6e',
966966
value: '0xa688906bd8b00000',
967967
gas: '0xd97010',
968968
accessList: []
@@ -972,7 +972,7 @@ describe('@api RPC Server Acceptance Tests', function () {
972972
const err = JSON.parse(error.body);
973973
expect(error).to.not.be.null;
974974
expect(err.error.name).to.be.equal('Invalid parameter');
975-
expect(err.error.message.endsWith(`Invalid parameter 'to' for TransactionObject: Expected 0x prefixed string representing the address (20 bytes)`)).to.be.true;
975+
expect(err.error.message.endsWith(`Invalid parameter 'from' for TransactionObject: Expected 0x prefixed string representing the address (20 bytes), value: 0x114f60009ee6b84861c0cdae8829751e517b`)).to.be.true;
976976
}
977977
});
978978

@@ -990,7 +990,7 @@ describe('@api RPC Server Acceptance Tests', function () {
990990
const err = JSON.parse(error.body);
991991
expect(error).to.not.be.null;
992992
expect(err.error.name).to.be.equal('Invalid parameter');
993-
expect(err.error.message.endsWith(`Invalid parameter 'to' for TransactionObject: Expected 0x prefixed string representing the address (20 bytes)`)).to.be.true;
993+
expect(err.error.message.endsWith(`Invalid parameter 'to' for TransactionObject: Expected 0x prefixed string representing the address (20 bytes), value: 0xae410f34f7487e2cd03396499cebb09b79f45`)).to.be.true;
994994
}
995995
});
996996

@@ -1008,7 +1008,7 @@ describe('@api RPC Server Acceptance Tests', function () {
10081008
const err = JSON.parse(error.body);
10091009
expect(error).to.not.be.null;
10101010
expect(err.error.name).to.be.equal('Invalid parameter');
1011-
expect(err.error.message.endsWith(`Invalid parameter 'value' for TransactionObject: Expected 0x prefixed hexadecimal value`)).to.be.true;
1011+
expect(err.error.message.endsWith(`Invalid parameter 'value' for TransactionObject: Expected 0x prefixed hexadecimal value, value: 123`)).to.be.true;
10121012
}
10131013
});
10141014

@@ -1026,7 +1026,7 @@ describe('@api RPC Server Acceptance Tests', function () {
10261026
const err = JSON.parse(error.body);
10271027
expect(error).to.not.be.null;
10281028
expect(err.error.name).to.be.equal('Invalid parameter');
1029-
expect(err.error.message.endsWith(`Invalid parameter 'gas' for TransactionObject: Expected 0x prefixed hexadecimal value`)).to.be.true;
1029+
expect(err.error.message.endsWith(`Invalid parameter 'gas' for TransactionObject: Expected 0x prefixed hexadecimal value, value: 123`)).to.be.true;
10301030
}
10311031
});
10321032
});
@@ -1454,7 +1454,7 @@ describe('@api RPC Server Acceptance Tests', function () {
14541454
await relay.call('eth_call', [callData, 'newest'], requestId);
14551455
Assertions.expectedError();
14561456
} catch (error) {
1457-
Assertions.jsonRpcError(error,predefined.INVALID_PARAMETER(1, 'Expected 0x prefixed string representing the hash (32 bytes) in object, 0x prefixed hexadecimal block number, or the string "latest", "earliest" or "pending'));
1457+
Assertions.jsonRpcError(error,predefined.INVALID_PARAMETER(1, 'Expected 0x prefixed string representing the hash (32 bytes) in object, 0x prefixed hexadecimal block number, or the string "latest", "earliest" or "pending, value: newest'));
14581458
}
14591459
});
14601460

@@ -1469,7 +1469,7 @@ describe('@api RPC Server Acceptance Tests', function () {
14691469
await relay.call('eth_call', [callData, '123'], requestId);
14701470
Assertions.expectedError();
14711471
} catch (error) {
1472-
Assertions.jsonRpcError(error,predefined.INVALID_PARAMETER(1, 'Expected 0x prefixed string representing the hash (32 bytes) in object, 0x prefixed hexadecimal block number, or the string "latest", "earliest" or "pending'));
1472+
Assertions.jsonRpcError(error,predefined.INVALID_PARAMETER(1, 'Expected 0x prefixed string representing the hash (32 bytes) in object, 0x prefixed hexadecimal block number, or the string "latest", "earliest" or "pending, value: 123'));
14731473
}
14741474
});
14751475

@@ -1485,7 +1485,7 @@ describe('@api RPC Server Acceptance Tests', function () {
14851485
Assertions.expectedError();
14861486
} catch (error) {
14871487

1488-
Assertions.jsonRpcError(error,predefined.INVALID_PARAMETER(`'blockHash' for BlockHashObject`, 'Expected 0x prefixed string representing the hash (32 bytes) of a block'));
1488+
Assertions.jsonRpcError(error,predefined.INVALID_PARAMETER(`'blockHash' for BlockHashObject`, 'Expected 0x prefixed string representing the hash (32 bytes) of a block, value: 0x123'));
14891489
}
14901490
});
14911491

@@ -1500,7 +1500,7 @@ describe('@api RPC Server Acceptance Tests', function () {
15001500
await relay.call('eth_call', [callData, {'blockNumber' : '123'}], requestId);
15011501
Assertions.expectedError();
15021502
} catch (error) {
1503-
Assertions.jsonRpcError(error,predefined.INVALID_PARAMETER(`'blockNumber' for BlockNumberObject`, 'Expected 0x prefixed hexadecimal block number, or the string "latest", "earliest" or "pending"'));
1503+
Assertions.jsonRpcError(error,predefined.INVALID_PARAMETER(`'blockNumber' for BlockNumberObject`, 'Expected 0x prefixed hexadecimal block number, or the string "latest", "earliest" or "pending", value: 123'));
15041504
}
15051505
});
15061506
});

0 commit comments

Comments
 (0)