Skip to content

Commit 3616571

Browse files
Nana-ECgeorgi-l95
Nana-EC
and
georgi-l95
authored
Add nullable check to estimateGas validation (#849) (#852)
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: Nana Essilfie-Conduah <[email protected]> Signed-off-by: georgi-l95 <[email protected]> Co-authored-by: georgi-l95 <[email protected]>
1 parent 53f8ecc commit 3616571

File tree

6 files changed

+258
-181
lines changed

6 files changed

+258
-181
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -324,7 +324,7 @@ export class EthImpl implements Eth {
324324
// eslint-disable-next-line @typescript-eslint/no-unused-vars
325325
async estimateGas(transaction: any, _blockParam: string | null, requestId?: string) {
326326
const requestIdPrefix = formatRequestIdMessage(requestId);
327-
this.logger.trace(`${requestIdPrefix} estimateGas()`);
327+
this.logger.trace(`${requestIdPrefix} estimateGas(transaction=${JSON.stringify(transaction)}, _blockParam=${_blockParam})`);
328328
if (!transaction || !transaction.data || transaction.data === '0x') {
329329
return EthImpl.gasTxBaseCost;
330330
} 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_batch2.spec.ts‎

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -157,8 +157,8 @@ describe('@api-batch-2 RPC Server Acceptance Tests', function () {
157157
it('should not be able to execute "eth_estimateGas" with wrong from field', async function () {
158158
try {
159159
await relay.call('eth_estimateGas', [{
160-
from: '0x114f60009ee6b84861c0cdae8829751e517bc4d7',
161-
to: '0xae410f34f7487e2cd03396499cebb09b79f45',
160+
from: '0x114f60009ee6b84861c0cdae8829751e517b',
161+
to: '0xae410f34f7487e2cd03396499cebb09b79f45d6e',
162162
value: '0xa688906bd8b00000',
163163
gas: '0xd97010',
164164
accessList: []
@@ -168,8 +168,7 @@ describe('@api-batch-2 RPC Server Acceptance Tests', function () {
168168
const err = JSON.parse(error.body);
169169
expect(error).to.not.be.null;
170170
expect(err.error.name).to.be.equal('Invalid parameter');
171-
expect(err.error.message.endsWith(`Invalid parameter 'to' for TransactionObject: Expected 0x prefixed string representing the address (20 bytes)`)).to.be.true;
172-
}
171+
expect(err.error.message.endsWith(`Invalid parameter 'from' for TransactionObject: Expected 0x prefixed string representing the address (20 bytes), value: 0x114f60009ee6b84861c0cdae8829751e517b`)).to.be.true; }
173172
});
174173

175174
it('should not be able to execute "eth_estimateGas" with wrong to field', async function () {
@@ -186,7 +185,7 @@ describe('@api-batch-2 RPC Server Acceptance Tests', function () {
186185
const err = JSON.parse(error.body);
187186
expect(error).to.not.be.null;
188187
expect(err.error.name).to.be.equal('Invalid parameter');
189-
expect(err.error.message.endsWith(`Invalid parameter 'to' for TransactionObject: Expected 0x prefixed string representing the address (20 bytes)`)).to.be.true;
188+
expect(err.error.message.endsWith(`Invalid parameter 'to' for TransactionObject: Expected 0x prefixed string representing the address (20 bytes), value: 0xae410f34f7487e2cd03396499cebb09b79f45`)).to.be.true;
190189
}
191190
});
192191

@@ -204,7 +203,7 @@ describe('@api-batch-2 RPC Server Acceptance Tests', function () {
204203
const err = JSON.parse(error.body);
205204
expect(error).to.not.be.null;
206205
expect(err.error.name).to.be.equal('Invalid parameter');
207-
expect(err.error.message.endsWith(`Invalid parameter 'value' for TransactionObject: Expected 0x prefixed hexadecimal value`)).to.be.true;
206+
expect(err.error.message.endsWith(`Invalid parameter 'value' for TransactionObject: Expected 0x prefixed hexadecimal value, value: 123`)).to.be.true;
208207
}
209208
});
210209

@@ -222,7 +221,7 @@ describe('@api-batch-2 RPC Server Acceptance Tests', function () {
222221
const err = JSON.parse(error.body);
223222
expect(error).to.not.be.null;
224223
expect(err.error.name).to.be.equal('Invalid parameter');
225-
expect(err.error.message.endsWith(`Invalid parameter 'gas' for TransactionObject: Expected 0x prefixed hexadecimal value`)).to.be.true;
224+
expect(err.error.message.endsWith(`Invalid parameter 'gas' for TransactionObject: Expected 0x prefixed hexadecimal value, value: 123`)).to.be.true;
226225
}
227226
});
228227
});
@@ -647,7 +646,7 @@ describe('@api-batch-2 RPC Server Acceptance Tests', function () {
647646
await relay.call('eth_call', [callData, 'newest'], requestId);
648647
Assertions.expectedError();
649648
} catch (error) {
650-
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'));
649+
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'));
651650
}
652651
});
653652

@@ -662,7 +661,7 @@ describe('@api-batch-2 RPC Server Acceptance Tests', function () {
662661
await relay.call('eth_call', [callData, '123'], requestId);
663662
Assertions.expectedError();
664663
} catch (error) {
665-
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'));
664+
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'));
666665
}
667666
});
668667

@@ -678,7 +677,7 @@ describe('@api-batch-2 RPC Server Acceptance Tests', function () {
678677
Assertions.expectedError();
679678
} catch (error) {
680679

681-
Assertions.jsonRpcError(error,predefined.INVALID_PARAMETER(`'blockHash' for BlockHashObject`, 'Expected 0x prefixed string representing the hash (32 bytes) of a block'));
680+
Assertions.jsonRpcError(error,predefined.INVALID_PARAMETER(`'blockHash' for BlockHashObject`, 'Expected 0x prefixed string representing the hash (32 bytes) of a block, value: 0x123'));
682681
}
683682
});
684683

@@ -693,7 +692,7 @@ describe('@api-batch-2 RPC Server Acceptance Tests', function () {
693692
await relay.call('eth_call', [callData, {'blockNumber' : '123'}], requestId);
694693
Assertions.expectedError();
695694
} catch (error) {
696-
Assertions.jsonRpcError(error,predefined.INVALID_PARAMETER(`'blockNumber' for BlockNumberObject`, 'Expected 0x prefixed hexadecimal block number, or the string "latest", "earliest" or "pending"'));
695+
Assertions.jsonRpcError(error,predefined.INVALID_PARAMETER(`'blockNumber' for BlockNumberObject`, 'Expected 0x prefixed hexadecimal block number, or the string "latest", "earliest" or "pending", value: 123'));
697696
}
698697
});
699698
});

0 commit comments

Comments
 (0)