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

chore: cherry pick feat: transaction with CONTRACT_NEGATIVE_VALUE breaks some routes (#3387) to release/0.64 #3436

Merged
merged 3 commits into from
Jan 31, 2025
Merged
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
7 changes: 3 additions & 4 deletions .github/workflows/acceptance-workflow.yml
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ on:
description: 'Codecov upload token'
required: true


env:
OPERATOR_ID_MAIN: ${{ inputs.operator_id }}

Expand Down Expand Up @@ -108,15 +107,15 @@ jobs:

- name: Upload Heap Snapshots
if: ${{ !cancelled() }}
uses: actions/upload-artifact@a8a3f3ad30e3422c9c7b888a15615d19a852ae32 # v3.1.3
uses: actions/upload-artifact@65c4c4a1ddee5b72f698fdd19549f0f0fb45cf08 # v4.6.0
with:
name: Heap Snapshots
path: "**/*.heapsnapshot"
path: '**/*.heapsnapshot'
if-no-files-found: ignore

- name: Upload Test Results
if: always()
uses: actions/upload-artifact@65462800fd760344b1a7b4382951275a0abb4808 # v4.3.3
uses: actions/upload-artifact@65c4c4a1ddee5b72f698fdd19549f0f0fb45cf08 # v4.6.0
with:
name: Test Results (${{ inputs.testfilter }})
path: test-*.xml
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,10 @@ jobs:

- name: Upload Heap Snapshots
if: ${{ !cancelled() }}
uses: actions/upload-artifact@a8a3f3ad30e3422c9c7b888a15615d19a852ae32 # v3.1.3
uses: actions/upload-artifact@65c4c4a1ddee5b72f698fdd19549f0f0fb45cf08 # v4.6.0
with:
name: Heap Snapshots
path: "**/*.heapsnapshot"
path: '**/*.heapsnapshot'
if-no-files-found: ignore

- name: Upload coverage report
Expand Down
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)
return numberTo0x((BigInt(input.toString()) + (BigInt(1) << BigInt(bits))) % (BigInt(1) << BigInt(bits)));
}

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));
}
}

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);
});
});
});
Loading
Loading