Skip to content
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.

Sign up for GitHub

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

Expand eth_call validation to support blockParam object #817

Merged
merged 8 commits into from
Jan 19, 2023
Merged

Expand eth_call validation to support blockParam object #817

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
515b566
add blockParam to eth_call validation
georgi-l95 Jan 17, 2023
fa34e09
bump image versions
georgi-l95 Jan 17, 2023
f3f8c32
revert previous change
georgi-l95 Jan 17, 2023
22bfc3a
fix typo and add more tests
georgi-l95 Jan 18, 2023
834befe
bump images
georgi-l95 Jan 18, 2023
cd2d6fc
revert some changes
georgi-l95 Jan 18, 2023
e74a683
fix some bugs in erc20 test
georgi-l95 Jan 18, 2023
9fdec83
change regex
georgi-l95 Jan 19, 2023
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
1,118 changes: 117 additions & 1,001 deletions package-lock.json
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@
"check:node": "ts-node packages/server/tests/helpers/nodeCheck.ts"
},
"dependencies": {
"@hashgraph/hedera-local": "^2.4.1",
"@hashgraph/hedera-local": "^2.4.3",
"@open-rpc/schema-utils-js": "^1.16.1",
"@types/find-config": "^1.0.1",
"keyv-file": "^0.2.0",
Expand Down
6 changes: 3 additions & 3 deletions packages/relay/src/lib/eth.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1050,7 +1050,7 @@ export class EthImpl implements Eth {

return new Transaction({
accessList: undefined, // we don't support access lists, so punt for now
blockHash: contractResult.block_hash.substring(0, 66),
blockHash: EthImpl.toHash32(contractResult.block_hash),
blockNumber: EthImpl.numberTo0x(contractResult.block_number),
chainId: contractResult.chain_id,
from: fromAddress,
Expand Down Expand Up @@ -1237,7 +1237,7 @@ export class EthImpl implements Eth {
}
}

const blockHash = blockResponse.hash.substring(0, 66);
const blockHash = EthImpl.toHash32(blockResponse.hash);
const transactionArray = showDetails ? transactionObjects : transactionHashes;
return new Block({
baseFeePerGas: await this.gasPrice(requestId),
Expand Down Expand Up @@ -1327,7 +1327,7 @@ export class EthImpl implements Eth {
const sSig = contractResultDetails.s === null ? null : contractResultDetails.s.substring(0, 66);
return new Transaction({
accessList: undefined, // we don't support access lists for now, so punt
blockHash: contractResultDetails.block_hash.substring(0, 66),
blockHash: EthImpl.toHash32(contractResultDetails.block_hash),
blockNumber: EthImpl.numberTo0x(contractResultDetails.block_number),
chainId: contractResultDetails.chain_id,
from: contractResultDetails.from.substring(0, 42),
Expand Down
4 changes: 2 additions & 2 deletions packages/server/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@
"pino-pretty": "^7.6.1"
},
"devDependencies": {
"@hashgraph/hedera-local": "^2.1.3",
"@hashgraph/sdk": "^2.19.0",
"@hashgraph/hedera-local": "^2.4.3",
"@hashgraph/sdk": "^2.19.2",
"@koa/cors": "^3.1.0",
"@types/chai": "^4.3.0",
"@types/cors": "^2.8.12",
Expand Down
1 change: 1 addition & 0 deletions packages/server/src/validator/constants.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ export const DEFAULT_HEX_ERROR = 'Expected 0x prefixed hexadecimal value';
export const HASH_ERROR = '0x prefixed string representing the hash (32 bytes)';
export const ADDRESS_ERROR = 'Expected 0x prefixed string representing the address (20 bytes)';
export const BLOCK_NUMBER_ERROR = 'Expected 0x prefixed hexadecimal block number, or the string "latest", "earliest" or "pending"';
export const BLOCK_PARAMS_ERROR = `Expected ${HASH_ERROR} in object, 0x prefixed hexadecimal block number, or the string "latest", "earliest" or "pending`;
export const BLOCK_HASH_ERROR = `Expected ${HASH_ERROR} of a block`;
export const TRANSACTION_HASH_ERROR = `Expected ${HASH_ERROR} of a transaction`;
export const TOPIC_HASH_ERROR = `Expected ${HASH_ERROR} of a topic`;
2 changes: 1 addition & 1 deletion packages/server/src/validator/methods.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ export const METHODS = {
},
1: {
required: true,
type: 'blockNumber'
type: 'blockParams'
}
},
"eth_sendRawTransaction": {
Expand Down
45 changes: 45 additions & 0 deletions packages/server/src/validator/objectTypes.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,16 @@ import { Validator } from ".";
import { predefined } from '@hashgraph/json-rpc-relay';

export const OBJECTS_VALIDATIONS = {
"blockHashObject": {
"blockHash": {
type: "blockHash"
}
},
"blockNumberObject": {
"blockNumber": {
type: "blockNumber"
}
},
"filter": {
"blockHash": {
type: "blockHash"
Expand Down Expand Up @@ -118,3 +128,38 @@ export class FilterObject {
return this.constructor.name;
}
};

export class BlockHashObject {
blockHash: string;

constructor (param: any) {
Validator.hasUnexpectedParams(param, OBJECTS_VALIDATIONS.blockHashObject, this.name());
this.blockHash = param.blockHash;
}

validate() {
return Validator.validateObject(this, OBJECTS_VALIDATIONS.blockHashObject);
}

name() {
return this.constructor.name;
}
};

export class BlockNumberObject {
blockNumber: string;

constructor (param: any) {
Validator.hasUnexpectedParams(param, OBJECTS_VALIDATIONS.blockNumberObject, this.name());
this.blockNumber = param.blockNumber;
}

validate() {
return Validator.validateObject(this, OBJECTS_VALIDATIONS.blockNumberObject);
}

name() {
return this.constructor.name;
}
};

14 changes: 13 additions & 1 deletion packages/server/src/validator/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,25 @@ export const TYPES = {
error: Constants.BLOCK_HASH_ERROR
},
'blockNumber': {
test: (param: string) => /^0[xX]([1-9A-Fa-f]+[0-9A-Fa-f]{0,13}|0)$/.test(param) && Number.MAX_SAFE_INTEGER >= Number(param) || ["earliest", "latest", "pending"].includes(param),
test: (param: string) => /^0[xX]([1-9A-Fa-f][0-9A-Fa-f]{0,13}|0)$/.test(param) && Number.MAX_SAFE_INTEGER >= Number(param) || ["earliest", "latest", "pending"].includes(param),
error: Constants.BLOCK_NUMBER_ERROR
},
'boolean': {
test: (param: boolean) => param === true || param === false,
error: 'Expected boolean type'
},
'blockParams': {
test: (param: any) => {
if(Object.prototype.toString.call(param) === "[object Object]") {
if (param.hasOwnProperty('blockHash')) {
return new Validator.BlockHashObject(param).validate();
}
return new Validator.BlockNumberObject(param).validate();
}
return /^0[xX]([1-9A-Fa-f]+[0-9A-Fa-f]{0,13}|0)$/.test(param) && Number.MAX_SAFE_INTEGER >= Number(param) || ["earliest", "latest", "pending"].includes(param)
Copy link
Member

@lukelee-sl lukelee-sl Jan 17, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The regex seems odd to me. It allows for 12 bytes and the first nibble cannot be 0? I tested 0x1204567890abcdef123456 for example and the regex does not match for me.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe also add some of the test cases noted in the EIP for additional coverage

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First nibble cannot be 0, because block numbers are not starting with 0, that would mean to be padded, which we have to trim afterwards. For example infura doesn't accepts padded blocknumbers, but alchemy accepts them. We can change the regex to accepts, but we may have to trim it afterwards. I mean it could work both ways.
It allows for this many bytes only, because otherwise it will exceeds javascript max integer size, this is the reason your example is not working. If you cut it down to 0x1204567890abcde, it should work with this regular expression.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess my question is whether the regex should not be /^0[xX]([1-9A-Fa-f][0-9A-Fa-f]{0,13}|0)$/? Unless I am misunderstanding the purpose of what the regex is trying to test. My guess is that it is trying to say the first nibble should not be zero but should be any other hex character and the next up to 13 characters can be any hex character.

Because of the + after [1-9A-Fa-f] I think this allows for any number of hex characters as long as they are not 0 to be part of the first match group. So things like 0x123456789abcedf123456789abcedf123456789abcedf123456789abcedf123456789abcedf123456789abcedf will still match (which I don't think is the intention but correct me if I am wrong). The count of up to 13 characters starts after the first 0is encountered.

Copy link
Collaborator

@dimitrovmaksim dimitrovmaksim Jan 19, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I may have "over-insured" in this case. Generally it should be fine to have a longer than 16 character hex for block number, my reason was that at one point all hex values will always convert to JS's MAX_SAFE_INTEGER, but then again it would take quite the long amout of time until we reach that point, so in the end that size limit may be uneeded.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, you're right, I'll change the regex to /^0[xX]([1-9A-Fa-f][0-9A-Fa-f]{0,13}|0)$/, which will work for our needs.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for double checking.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the feedback, Luke.

},
error: Constants.BLOCK_PARAMS_ERROR
},
"filter": {
test: (param: any) => {
if(Object.prototype.toString.call(param) === "[object Object]") {
Expand Down
24 changes: 13 additions & 11 deletions packages/server/tests/acceptance/erc20.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,9 +171,10 @@ describe('@erc20 Acceptance Tests', async function () {
});

it('emits a transfer event', async function () {
await expect(tx)
.to.emit(contract, 'Transfer')
.withArgs(tokenOwnerWallet.address, toWallet.address, amount);
const transferEvent = (await tx.wait()).events.filter(e => e.event === 'Transfer')[0].args;
expect(transferEvent.from).to.eq(tokenOwnerWallet.address);
expect(transferEvent.to).to.eq(toWallet.address);
expect(transferEvent.value).to.eq(amount);
});

it ('other account transfers tokens back to owner', async function () {
Expand All @@ -194,9 +195,10 @@ describe('@erc20 Acceptance Tests', async function () {

it('emits an approval event', async function () {
const allowance = await contract.allowance(tokenOwner, spender);
await expect(tx)
.to.emit(contract, 'Approval')
.withArgs(tokenOwnerWallet.address, spenderWallet.address, allowance);
const approvalEvent = (await tx.wait()).events.filter(e => e.event === 'Approval')[0].args;
expect(approvalEvent.owner).to.eq(tokenOwnerWallet.address);
expect(approvalEvent.spender).to.eq(spenderWallet.address);
expect(approvalEvent.value).to.eq(allowance);
});

describe('when the token owner has enough balance', function () {
Expand Down Expand Up @@ -224,9 +226,10 @@ describe('@erc20 Acceptance Tests', async function () {
});

it('emits a transfer event', async function () {
await expect(tx)
.to.emit(contract, 'Transfer')
.withArgs(tokenOwnerWallet.address, toWallet.address, amount);
const transferEvent = (await tx.wait()).events.filter(e => e.event === 'Transfer')[0].args;
expect(transferEvent.from).to.eq(tokenOwnerWallet.address);
expect(transferEvent.to).to.eq(toWallet.address);
expect(transferEvent.value).to.eq(amount);
});
});

Expand Down Expand Up @@ -360,9 +363,8 @@ describe('@erc20 Acceptance Tests', async function () {
await servicesNode.associateHTSToken(account.accountId, htsResult.receipt.tokenId, account.privateKey, htsResult.client, requestId);
await servicesNode.approveHTSToken(account.accountId, htsResult.receipt.tokenId, htsResult.client, requestId);
}

// Setup initial balance of token owner account
await servicesNode.transferHTSToken(accounts[0].accountId, htsResult.receipt.tokenId, initialSupply, htsResult.client, requestId);
await servicesNode.transferHTSToken(accounts[0].accountId, htsResult.receipt.tokenId, initialSupply, htsResult.client.operatorAccountId, requestId);
const evmAddress = Utils.idToEvmAddress(htsResult.receipt.tokenId.toString());
return new ethers.Contract(evmAddress, abi, accounts[0].wallet);
};
Expand Down
6 changes: 3 additions & 3 deletions packages/server/tests/acceptance/index.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,9 +125,9 @@ describe('RPC Server Acceptance Tests', function () {

function runLocalHederaNetwork() {
// set env variables for docker images until local-node is updated
process.env['NETWORK_NODE_IMAGE_TAG'] = '0.32.0';
process.env['HAVEGED_IMAGE_TAG'] = '0.32.0';
process.env['MIRROR_IMAGE_TAG'] = '0.70.1';
process.env['NETWORK_NODE_IMAGE_TAG'] = '0.33.2';
process.env['HAVEGED_IMAGE_TAG'] = '0.33.2';
process.env['MIRROR_IMAGE_TAG'] = '0.72.0';

console.log(`Docker container versions, services: ${process.env['NETWORK_NODE_IMAGE_TAG']}, mirror: ${process.env['MIRROR_IMAGE_TAG']}`);

Expand Down
95 changes: 95 additions & 0 deletions packages/server/tests/acceptance/rpc.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1299,6 +1299,101 @@ describe('@api RPC Server Acceptance Tests', function () {
const res = await relay.call('eth_call', [callData, 'latest'], requestId);
expect(res).to.eq(BASIC_CONTRACT_PING_RESULT);
});

it('should execute "eth_call" with correct block number', async function () {
const callData = {
from: '0x' + accounts[2].address,
to: evmAddress,
data: BASIC_CONTRACT_PING_CALL_DATA
};

const res = await relay.call('eth_call', [callData, '0x1'], requestId);
expect(res).to.eq(BASIC_CONTRACT_PING_RESULT);
});

it('should execute "eth_call" with correct block hash object', async function () {
const blockHash = '0xd4e56740f876aef8c010b86a40d5f56745a118d0906a34e69aec8c0db1cb8fa3';
const callData = {
from: '0x' + accounts[2].address,
to: evmAddress,
data: BASIC_CONTRACT_PING_CALL_DATA
};

const res = await relay.call('eth_call', [callData, {'blockHash' : blockHash}], requestId);
expect(res).to.eq(BASIC_CONTRACT_PING_RESULT);
});

it('should execute "eth_call" with correct block number object', async function () {
const callData = {
from: '0x' + accounts[2].address,
to: evmAddress,
data: BASIC_CONTRACT_PING_CALL_DATA
};

const res = await relay.call('eth_call', [callData, {'blockNumber' : '0x1'}], requestId);
expect(res).to.eq(BASIC_CONTRACT_PING_RESULT);
});

it('should fail to execute "eth_call" with wrong block tag', async function () {
const callData = {
from: '0x' + accounts[2].address,
to: evmAddress,
data: BASIC_CONTRACT_PING_CALL_DATA
};

try {
await relay.call('eth_call', [callData, 'newest'], requestId);
Assertions.expectedError();
} catch (error) {
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'));
}
});

it('should fail to execute "eth_call" with wrong block number', async function () {
const callData = {
from: '0x' + accounts[2].address,
to: evmAddress,
data: BASIC_CONTRACT_PING_CALL_DATA
};

try {
await relay.call('eth_call', [callData, '123'], requestId);
Assertions.expectedError();
} catch (error) {
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'));
}
});

it('should fail to execute "eth_call" with wrong block hash object', async function () {
const callData = {
from: '0x' + accounts[2].address,
to: evmAddress,
data: BASIC_CONTRACT_PING_CALL_DATA
};

try {
await relay.call('eth_call', [callData, {'blockHash' : '0x123'}], requestId);
Assertions.expectedError();
} catch (error) {

Assertions.jsonRpcError(error,predefined.INVALID_PARAMETER(`'blockHash' for BlockHashObject`, 'Expected 0x prefixed string representing the hash (32 bytes) of a block'));
}
});

it('should fail to execute "eth_call" with wrong block number object', async function () {
const callData = {
from: '0x' + accounts[2].address,
to: evmAddress,
data: BASIC_CONTRACT_PING_CALL_DATA
};

try {
await relay.call('eth_call', [callData, {'blockNumber' : '123'}], requestId);
Assertions.expectedError();
} catch (error) {
Assertions.jsonRpcError(error,predefined.INVALID_PARAMETER(`'blockNumber' for BlockNumberObject`, 'Expected 0x prefixed hexadecimal block number, or the string "latest", "earliest" or "pending"'));
}
});
});

// Test state changes with getStorageAt
Expand Down
6 changes: 3 additions & 3 deletions packages/server/tests/helpers/prerequisite.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@ const RELAY_URL = process.env.E2E_RELAY_HOST || LOCAL_RELAY_URL;

(function () {
if (USE_LOCAL_NODE) {
process.env['NETWORK_NODE_IMAGE_TAG'] = '0.32.0';
process.env['HAVEGED_IMAGE_TAG'] = '0.32.0';
process.env['MIRROR_IMAGE_TAG'] = '0.70.1';
process.env['NETWORK_NODE_IMAGE_TAG'] = '0.33.2';
process.env['HAVEGED_IMAGE_TAG'] = '0.33.2';
process.env['MIRROR_IMAGE_TAG'] = '0.72.0';

console.log(`Docker container versions, services: ${process.env['NETWORK_NODE_IMAGE_TAG']}, mirror: ${process.env['MIRROR_IMAGE_TAG']}`);

Expand Down
38 changes: 34 additions & 4 deletions packages/server/tests/integration/server.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1085,7 +1085,7 @@ describe('RPC Server', async function() {
}
});

it('validates Block param is valid block hex', async function() {
it('validates Block param is non valid block hex', async function() {
try {
await this.testClient.post('/', {
'id': '2',
Expand All @@ -1096,11 +1096,11 @@ describe('RPC Server', async function() {

Assertions.expectedError();
} catch (error) {
BaseTest.invalidParamError(error.response, Validator.ERROR_CODE, `Invalid parameter 1: ${Validator.BLOCK_NUMBER_ERROR}`);
BaseTest.invalidParamError(error.response, Validator.ERROR_CODE, `Invalid parameter 1: ${Validator.BLOCK_PARAMS_ERROR}`);
}
});

it('validates Block param is valid tag', async function() {
it('validates Block param is non valid tag', async function() {
try {
await this.testClient.post('/', {
'id': '2',
Expand All @@ -1111,7 +1111,37 @@ describe('RPC Server', async function() {

Assertions.expectedError();
} catch (error) {
BaseTest.invalidParamError(error.response, Validator.ERROR_CODE, `Invalid parameter 1: ${Validator.BLOCK_NUMBER_ERROR}`);
BaseTest.invalidParamError(error.response, Validator.ERROR_CODE, `Invalid parameter 1: ${Validator.BLOCK_PARAMS_ERROR}`);
}
});

it('validates Block param is non valid block hash', async function() {
try {
await this.testClient.post('/', {
'id': '2',
'jsonrpc': '2.0',
'method': 'eth_call',
'params': [{"to": "0x0000000000000000000000000000000000000001"}, { "blockHash": "0x123" }]
});

Assertions.expectedError();
} catch (error) {
BaseTest.invalidParamError(error.response, Validator.ERROR_CODE, `Invalid parameter 'blockHash' for BlockHashObject: ${Validator.BLOCK_HASH_ERROR}`);
}
});

it('validates Block param is non valid block number', async function() {
try {
await this.testClient.post('/', {
'id': '2',
'jsonrpc': '2.0',
'method': 'eth_call',
'params': [{"to": "0x0000000000000000000000000000000000000001"}, { "blockNumber": "123" }]
});

Assertions.expectedError();
} catch (error) {
BaseTest.invalidParamError(error.response, Validator.ERROR_CODE, `Invalid parameter 'blockNumber' for BlockNumberObject: ${Validator.BLOCK_NUMBER_ERROR}`);
}
});
});
Expand Down