feat: transaction with CONTRACT_NEGATIVE_VALUE breaks some routes (#…

…3387) * chore: handle contract negative value calls Signed-off-by: nikolay <[email protected]> * chore: remove .only Signed-off-by: nikolay <[email protected]> * chore: fix comments Signed-off-by: nikolay <[email protected]> * chore: edit imports Signed-off-by: nikolay <[email protected]> * chore: resolving comments Signed-off-by: nikolay <[email protected]> * chore: fix json parsing Signed-off-by: nikolay <[email protected]> * chore: add e2e test Signed-off-by: nikolay <[email protected]> * chore: fix test Signed-off-by: nikolay <[email protected]> * chore: fix test Signed-off-by: nikolay <[email protected]> * chore: fix flaky test Signed-off-by: nikolay <[email protected]> * chore: tests Signed-off-by: nikolay <[email protected]> * chore: try to disable the test Signed-off-by: nikolay <[email protected]> * chore: add test Signed-off-by: nikolay <[email protected]> * chore: resolve comments Signed-off-by: nikolay <[email protected]> * chore: simplify the tests Signed-off-by: nikolay <[email protected]> --------- Signed-off-by: nikolay <[email protected]> Revert "feat: transaction with `CONTRACT_NEGATIVE_VALUE` breaks some routes (#3387)" This reverts commit 8bc3991. Reapply "feat: transaction with `CONTRACT_NEGATIVE_VALUE` breaks some routes (#3387)" This reverts commit 8d52b9a07a43cecd6241eaa9a7f2e65c4df8d4e5. Signed-off-by: Logan Nguyen <[email protected]>
hashgraph · Jan 31, 2025 · 1205561 · 1205561
1 parent 5f9c2a4
commit 1205561
Show file tree

Hide file tree

Showing 7 changed files with 193 additions and 31 deletions.
diff --git a/package-lock.json b/package-lock.json
diff --git a/packages/relay/package.json b/packages/relay/package.json
@@ -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",
@@ -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",

diff --git a/packages/relay/src/formatters.ts b/packages/relay/src/formatters.ts
@@ -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';
@@ -178,7 +179,7 @@ const formatContractResult = (cr: any) => {
     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.
@@ -265,6 +266,40 @@ const nanOrNumberTo0x = (input: number | BigNumber | bigint | null): string => {
   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);
 };
@@ -303,8 +338,18 @@ const getFunctionSelector = (data?: string): string => {
   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');
+  }
+
   if (value && value > constants.TOTAL_SUPPLY_TINYBARS)
     throw new Error('Value cannot be more than the total supply of tinybars in the blockchain');
 
@@ -324,6 +369,7 @@ export {
   numberTo0x,
   nullableNumberTo0x,
   nanOrNumberTo0x,
+  nanOrNumberInt64To0x,
   toHash32,
   toNullableBigNumber,
   toNullIfEmptyHex,

diff --git a/packages/relay/src/lib/clients/mirrorNodeClient.ts b/packages/relay/src/lib/clients/mirrorNodeClient.ts
@@ -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';
 
@@ -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 {

diff --git a/packages/relay/tests/lib/formatters.spec.ts b/packages/relay/tests/lib/formatters.spec.ts
@@ -18,7 +18,10 @@
  *
  */
 
+import { BigNumber as BN } from 'bignumber.js';
 import { expect } from 'chai';
+import { AbiCoder, keccak256 } from 'ethers';
+
 import {
   ASCIIToHex,
   decodeErrorMessage,
@@ -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', () => {
@@ -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(
@@ -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);
     });
   });
 });
diff --git a/packages/server/tests/acceptance/rpc_batch1.spec.ts b/packages/server/tests/acceptance/rpc_batch1.spec.ts
@@ -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,
@@ -744,6 +745,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);
+        expect(blockResult.transactions.filter((tx) => tx.hash == txHash)[0].value).to.equal('0xffffffffffffff9c');
+      });
     });
 
     describe('Transaction related RPC Calls', () => {