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.

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

feat: transaction with CONTRACT_NEGATIVE_VALUE breaks some routes #3387

Merged
Merged
7 changes: 5 additions & 2 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion packages/relay/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,8 @@
},
"dependencies": {
"@ethersproject/asm": "^5.7.0",
"@hashgraph/sdk": "^2.54.0-beta.1",
"@hashgraph/json-rpc-config-service": "file:../config-service",
"@hashgraph/sdk": "^2.54.0-beta.1",
"@keyvhq/core": "^1.6.9",
"axios": "^1.4.0",
"axios-retry": "^3.5.1",
Expand All @@ -60,6 +60,7 @@
"dotenv": "^16.0.0",
"ethers": "^6.7.0",
"find-config": "^1.0.0",
"json-bigint": "^1.0.0",
"keccak": "^3.0.2",
"keyv": "^4.2.2",
"keyv-file": "^0.3.0",
Expand Down
58 changes: 52 additions & 6 deletions packages/relay/src/formatters.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,12 @@
*
*/

import { ConfigService } from '@hashgraph/json-rpc-config-service/dist/services';
import { BigNumber } from '@hashgraph/sdk/lib/Transfer';
import { BigNumber as BN } from 'bignumber.js';
import crypto from 'crypto';

import constants from './lib/constants';
import { BigNumber as BN } from 'bignumber.js';
import { BigNumber } from '@hashgraph/sdk/lib/Transfer';
import { ConfigService } from '@hashgraph/json-rpc-config-service/dist/services';
import { Transaction, Transaction1559, Transaction2930 } from './lib/model';

const EMPTY_HEX = '0x';
Expand Down Expand Up @@ -178,7 +179,7 @@
transactionIndex: nullableNumberTo0x(cr.transaction_index),
type: cr.type === null ? '0x0' : nanOrNumberTo0x(cr.type),
v: cr.v === null ? '0x0' : nanOrNumberTo0x(cr.v),
value: nanOrNumberTo0x(tinybarsToWeibars(cr.amount)),
value: nanOrNumberInt64To0x(tinybarsToWeibars(cr.amount, true)),
// for legacy EIP155 with tx.chainId=0x0, mirror-node will return a '0x' (EMPTY_HEX) value for contract result's chain_id
// which is incompatibile with certain tools (i.e. foundry). By setting this field, chainId, to undefined, the end jsonrpc
// object will leave out this field, which is the proper behavior for other tools to be compatible with.
Expand Down Expand Up @@ -265,6 +266,40 @@
return input == null || Number.isNaN(input) ? numberTo0x(0) : numberTo0x(input);
};

const nanOrNumberInt64To0x = (input: number | BigNumber | bigint | null): string => {
// converting to string and then back to int is fixing a typescript warning
if (input && Number(input) < 0) {
// the hex of a negative number can be obtained from the binary value of that number positive value
// the binary value needs to be negated and then to be incremented by 1

// how the transformation works (using 16 bits)
// a 16 bits integer variables have values from -32768 to +32767, so:
// 0 - 0x0000 - 0000 0000 0000 0000
// 32767 - 0x7fff - 0111 1111 1111 1111
// -32768 - 0x8000 - 1000 0000 0000 0000
// -1 - 0xffff - 1111 1111 1111 1111

// converting int16 -10 will be done as following:
// - make it positive = 10
// - 16 bits binary value of 10 = 0000 0000 0000 1010
// - inverse the bits = 1111 1111 1111 0101
// - adding +1 = 1111 1111 1111 0110
// - 1111 1111 1111 0110 bits = 0xfff6

// we're using 64 bits integer because that's the type returned by the mirror node - int64
const bits = 64;
// this mathematical expression serves as a shortcut for performing the two’s complement conversion
// e.g. input = -10
// we have: (BigInt(1) << BigInt(bits)) = 1 << 64 = 2^64 = 18446744073709551616
// then: (BigInt(input.toString()) + (BigInt(1) << BigInt(bits))) = -10 + 2^64 = 18446744073709551606
// this effectively represents -10 in an unsigned 64-bit representation:18446744073709551606 = 0xFFFFFFFFFFFFFFF6
// finally, the modulo operation: % (1 << 64)
natanasow marked this conversation as resolved.
Show resolved Hide resolved
return numberTo0x((BigInt(input.toString()) + (BigInt(1) << BigInt(bits))) % (BigInt(1) << BigInt(bits)));
natanasow marked this conversation as resolved.
Show resolved Hide resolved
natanasow marked this conversation as resolved.
Show resolved Hide resolved
}

return nanOrNumberTo0x(input);
};

const toHash32 = (value: string): string => {
return value.substring(0, 66);
};
Expand Down Expand Up @@ -303,8 +338,18 @@
return data.replace(/^0x/, '').substring(0, 8);
};

const tinybarsToWeibars = (value: number | null) => {
if (value && value < 0) throw new Error('Invalid value - cannot pass negative number');
const tinybarsToWeibars = (value: number | null, allowNegativeValues: boolean = false) => {
if (value && value < 0) {
// negative amount can be received only by CONTRACT_NEGATIVE_VALUE revert
// e.g. tx https://hashscan.io/mainnet/transaction/1735241436.856862230
// that's not a valid revert in the Ethereum world so we must NOT multiply
// the amount sent via CONTRACT_CALL SDK call by TINYBAR_TO_WEIBAR_COEF
// also, keep in mind that the mirror node returned amount is typed with int64
if (allowNegativeValues) return value;

throw new Error('Invalid value - cannot pass negative number');

Check warning on line 350 in packages/relay/src/formatters.ts

View check run for this annotation

Codecov / codecov/patch

packages/relay/src/formatters.ts#L350

Added line #L350 was not covered by tests
}

if (value && value > constants.TOTAL_SUPPLY_TINYBARS)
throw new Error('Value cannot be more than the total supply of tinybars in the blockchain');

Expand All @@ -324,6 +369,7 @@
numberTo0x,
nullableNumberTo0x,
nanOrNumberTo0x,
nanOrNumberInt64To0x,
toHash32,
toNullableBigNumber,
toNullIfEmptyHex,
Expand Down
25 changes: 25 additions & 0 deletions packages/relay/src/lib/clients/mirrorNodeClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import { install as betterLookupInstall } from 'better-lookup';
import { ethers } from 'ethers';
import http from 'http';
import https from 'https';
import JSONBigInt from 'json-bigint';
import { Logger } from 'pino';
import { Histogram, Registry } from 'prom-client';

Expand Down Expand Up @@ -361,6 +362,30 @@ export class MirrorNodeClient {
if (pathLabel == MirrorNodeClient.GET_CONTRACTS_RESULTS_OPCODES) {
response = await this.web3Client.get<T>(path, axiosRequestConfig);
} else {
// JavaScript supports integers only up to 53 bits. When a number exceeding this limit
// is converted to a JS Number type, precision is lost due to rounding.
// To prevent this, `transformResponse` is used to intercept
// and process the response before Axios’s default JSON.parse conversion.
// JSONBigInt reads the string representation from the received JSON
// and converts large numbers into BigNumber objects to maintain accuracy.
axiosRequestConfig['transformResponse'] = [
(data) => {
// if the data is not valid, just return it to stick to the current behaviour
if (data) {
try {
// try to parse it, if the json is valid, numbers within it will be converted
// this case will happen on almost every GET mirror node call
return JSONBigInt.parse(data);
} catch (e) {
// in some unit tests, the mocked returned json is not property formatted
// so we have to preprocess it here with JSON.stringify()
return JSONBigInt.parse(JSON.stringify(data));
}
quiet-node marked this conversation as resolved.
Show resolved Hide resolved
}

return data;
},
];
response = await this.restClient.get<T>(path, axiosRequestConfig);
}
} else {
Expand Down
86 changes: 68 additions & 18 deletions packages/relay/tests/lib/formatters.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,10 @@
*
*/

import { BigNumber as BN } from 'bignumber.js';
import { expect } from 'chai';
import { AbiCoder, keccak256 } from 'ethers';

import {
ASCIIToHex,
decodeErrorMessage,
Expand All @@ -31,23 +34,22 @@ import {
isHex,
isValidEthereumAddress,
mapKeysAndValues,
nanOrNumberInt64To0x,
nanOrNumberTo0x,
nullableNumberTo0x,
numberTo0x,
parseNumericEnvVar,
prepend0x,
strip0x,
tinybarsToWeibars,
toHash32,
toHexString,
toNullableBigNumber,
toNullIfEmptyHex,
trimPrecedingZeros,
weibarHexToTinyBarInt,
tinybarsToWeibars,
} from '../../src/formatters';
import constants from '../../src/lib/constants';
import { BigNumber as BN } from 'bignumber.js';
import { AbiCoder, keccak256 } from 'ethers';
import { overrideEnvsInMochaDescribe } from '../helpers';

describe('Formatters', () => {
Expand Down Expand Up @@ -399,6 +401,48 @@ describe('Formatters', () => {
});
});

describe('nanOrNumberInt64To0x', () => {
it('should return 0x0 for nullable input', () => {
expect(nanOrNumberInt64To0x(null)).to.equal('0x0');
});
it('should return 0x0 for NaN input', () => {
expect(nanOrNumberInt64To0x(NaN)).to.equal('0x0');
});

for (const [testName, testValues] of Object.entries({
'2 digits': ['-10', '0xfffffffffffffff6'],
'6 digits': ['-851969', '0xfffffffffff2ffff'],
'19 digits -6917529027641081857': ['-6917529027641081857', '0x9fffffffffffffff'],
'19 digits -9223372036586340353': ['-9223372036586340353', '0x800000000fffffff'],
})) {
it(`should convert negative int64 number (${testName})`, () => {
expect(nanOrNumberInt64To0x(BigInt(testValues[0]))).to.equal(testValues[1]);
});
}

for (const [bits, testValues] of Object.entries({
10: ['593', '0x251'],
50: ['844424930131967', '0x2ffffffffffff'],
51: ['1970324836974591', '0x6ffffffffffff'],
52: ['3096224743817215', '0xaffffffffffff'],
53: ['9007199254740991', '0x1fffffffffffff'],
54: ['13510798882111487', '0x2fffffffffffff'],
55: ['31525197391593471', '0x6fffffffffffff'],
56: ['49539595901075455', '0xafffffffffffff'],
57: ['144115188075855871', '0x1ffffffffffffff'],
58: ['216172782113783807', '0x2ffffffffffffff'],
59: ['504403158265495551', '0x6ffffffffffffff'],
60: ['792633534417207295', '0xaffffffffffffff'],
61: ['2305843009213693951', '0x1fffffffffffffff'],
62: ['3458764513820540927', '0x2fffffffffffffff'],
63: ['8070450532247928831', '0x6fffffffffffffff'],
})) {
it(`should convert positive ${bits} bits number`, () => {
expect(nanOrNumberInt64To0x(BigInt(testValues[0]))).to.equal(testValues[1]);
});
}
});

describe('toHash32', () => {
it('should format more than 32 bytes hash to 32 bytes', () => {
expect(
Expand Down Expand Up @@ -735,27 +779,33 @@ describe('Formatters', () => {
});

describe('tinybarsToWeibars', () => {
it('should convert tinybars to weibars', () => {
expect(tinybarsToWeibars(10)).to.eql(100000000000);
});
for (const allowNegativeValues of [true, false]) {
it(`should convert tinybars to weibars allowNegativeValues = ${allowNegativeValues}`, () => {
expect(tinybarsToWeibars(10, allowNegativeValues)).to.eql(100000000000);
});

it('should return null if null is passed', () => {
expect(tinybarsToWeibars(null)).to.eql(null);
});
it(`should return null if null is passed allowNegativeValues = ${allowNegativeValues}`, () => {
expect(tinybarsToWeibars(null, allowNegativeValues)).to.eql(null);
});

it('should return 0 for 0 input', () => {
expect(tinybarsToWeibars(0)).to.eql(0);
});
it(`should return 0 for 0 input allowNegativeValues = ${allowNegativeValues}`, () => {
expect(tinybarsToWeibars(0, allowNegativeValues)).to.eql(0);
});

it(`should throw an error when value is larger than the total supply of tinybars allowNegativeValues = ${allowNegativeValues}`, () => {
expect(() => tinybarsToWeibars(constants.TOTAL_SUPPLY_TINYBARS * 10, allowNegativeValues)).to.throw(
Error,
'Value cannot be more than the total supply of tinybars in the blockchain',
);
});
}

it('should throw an error when value is smaller than 0', () => {
expect(() => tinybarsToWeibars(-10)).to.throw(Error, 'Invalid value - cannot pass negative number');
expect(() => tinybarsToWeibars(-10, false)).to.throw(Error, 'Invalid value - cannot pass negative number');
});

it('should throw an error when value is larger than the total supply of tinybars', () => {
expect(() => tinybarsToWeibars(constants.TOTAL_SUPPLY_TINYBARS * 10)).to.throw(
Error,
'Value cannot be more than the total supply of tinybars in the blockchain',
);
it('should return the negative number if allowNegativeValues flag is set to true', () => {
expect(tinybarsToWeibars(-10, true)).to.eql(-10);
});
});
});
41 changes: 40 additions & 1 deletion packages/server/tests/acceptance/rpc_batch1.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,14 @@
// External resources
import { ConfigService } from '@hashgraph/json-rpc-config-service/dist/services';
// Other imports
import { numberTo0x, prepend0x } from '@hashgraph/json-rpc-relay/dist/formatters';
import { formatTransactionId, numberTo0x, prepend0x } from '@hashgraph/json-rpc-relay/dist/formatters';
import Constants from '@hashgraph/json-rpc-relay/dist/lib/constants';
// Errors and constants from local resources
import { predefined } from '@hashgraph/json-rpc-relay/dist/lib/errors/JsonRpcError';
import { RequestDetails } from '@hashgraph/json-rpc-relay/dist/lib/types';
import {
AccountCreateTransaction,
ContractFunctionParameters,
FileInfo,
FileInfoQuery,
Hbar,
Expand Down Expand Up @@ -726,6 +727,44 @@ describe('@api-batch-1 RPC Server Acceptance Tests', function () {
expect(blockNumber).to.be.oneOf([mirrorBlockNumber, mirrorBlockNumber + 1]);
});
});

it('should execute "eth_getBlockByNumber", hydrated transactions = true for a block that contains a call with CONTRACT_NEGATIVE_VALUE status', async function () {
let transactionId;
let hasContractNegativeValueError = false;
try {
await servicesNode.executeContractCallWithAmount(
mirrorContractDetails.contract_id,
'',
new ContractFunctionParameters(),
500_000,
-100,
requestId,
);
} catch (e: any) {
// regarding the docs and HederaResponseCodes.sol the CONTRACT_NEGATIVE_VALUE code equals 96;
expect(e.status._code).to.equal(96);
hasContractNegativeValueError = true;
transactionId = e.transactionId;
}
expect(hasContractNegativeValueError).to.be.true;

// waiting for at least one block time for data to be populated in the mirror node
// because on the step above we sent a sdk call
await new Promise((r) => setTimeout(r, 2100));
const mirrorResult = await mirrorNode.get(
`/contracts/results/${formatTransactionId(transactionId.toString())}`,
requestId,
);
const txHash = mirrorResult.hash;
const blockResult = await relay.call(
RelayCalls.ETH_ENDPOINTS.ETH_GET_BLOCK_BY_NUMBER,
[numberTo0x(mirrorResult.block_number), true],
requestIdPrefix,
);
expect(blockResult.transactions).to.not.be.empty;
expect(blockResult.transactions.map((tx) => tx.hash)).to.contain(txHash);
natanasow marked this conversation as resolved.
Show resolved Hide resolved
expect(blockResult.transactions.filter((tx) => tx.hash == txHash)[0].value).to.equal('0xffffffffffffff9c');
});
});

describe('Transaction related RPC Calls', () => {
Expand Down
Loading
Loading