From c006489f144fac366da0bf2ea5b76cc59e739165 Mon Sep 17 00:00:00 2001 From: Simeon Nakov Date: Fri, 6 Dec 2024 13:27:06 +0200 Subject: [PATCH] fix: let 0 value transactions pass (#3304) * fix: let 0 value transactions pass Signed-off-by: Simeon Nakov * fix: brought back precheck for value with additional logic + tests Signed-off-by: Simeon Nakov * fix: changed check to account for transactions with non-zero data Signed-off-by: Simeon Nakov * fix: removed unused import Signed-off-by: Simeon Nakov * More clear error for VALUE_TOO_LOW Co-authored-by: Nana Essilfie-Conduah <56320167+Nana-EC@users.noreply.github.com> Signed-off-by: Simeon Nakov * fix: changed VALUE_TOO_LOW message in tests as well Signed-off-by: Simeon Nakov --------- Signed-off-by: Simeon Nakov Signed-off-by: Simeon Nakov Co-authored-by: Nana Essilfie-Conduah <56320167+Nana-EC@users.noreply.github.com> --- packages/relay/src/lib/errors/JsonRpcError.ts | 2 +- packages/relay/src/lib/eth.ts | 6 +- packages/relay/src/lib/precheck.ts | 12 +-- .../tests/lib/eth/eth_estimateGas.spec.ts | 33 ++++---- packages/relay/tests/lib/precheck.spec.ts | 76 +++++++++++++++---- 5 files changed, 89 insertions(+), 40 deletions(-) diff --git a/packages/relay/src/lib/errors/JsonRpcError.ts b/packages/relay/src/lib/errors/JsonRpcError.ts index 24117ab150..bd559a86f5 100644 --- a/packages/relay/src/lib/errors/JsonRpcError.ts +++ b/packages/relay/src/lib/errors/JsonRpcError.ts @@ -162,7 +162,7 @@ export const predefined = { }), VALUE_TOO_LOW: new JsonRpcError({ code: -32602, - message: 'Value below 10_000_000_000 wei which is 1 tinybar', + message: "Value can't be non-zero and less than 10_000_000_000 wei which is 1 tinybar", }), INVALID_CONTRACT_ADDRESS: (address) => { let message = `Invalid Contract Address: ${address}.`; diff --git a/packages/relay/src/lib/eth.ts b/packages/relay/src/lib/eth.ts index e15c7cae1d..337e7bf3a9 100644 --- a/packages/relay/src/lib/eth.ts +++ b/packages/relay/src/lib/eth.ts @@ -656,11 +656,11 @@ export class EthImpl implements Eth { if (isSimpleTransfer) { // Handle Simple Transaction and Hollow Account creation - const isNonZeroValue = Number(transaction.value) > 0; - if (!isNonZeroValue) { + const isZeroOrHigher = Number(transaction.value) >= 0; + if (!isZeroOrHigher) { return predefined.INVALID_PARAMETER( 0, - `Invalid 'value' field in transaction param. Value must be greater than 0`, + `Invalid 'value' field in transaction param. Value must be greater than or equal to 0`, ); } // when account exists return default base gas diff --git a/packages/relay/src/lib/precheck.ts b/packages/relay/src/lib/precheck.ts index f7052aa750..98e247456d 100644 --- a/packages/relay/src/lib/precheck.ts +++ b/packages/relay/src/lib/precheck.ts @@ -18,13 +18,13 @@ * */ -import { JsonRpcError, predefined } from './errors/JsonRpcError'; -import { MirrorNodeClient } from './clients'; -import { EthImpl } from './eth'; -import { Logger } from 'pino'; -import constants from './constants'; import { ethers, Transaction } from 'ethers'; +import { Logger } from 'pino'; + import { prepend0x } from '../formatters'; +import { MirrorNodeClient } from './clients'; +import constants from './constants'; +import { JsonRpcError, predefined } from './errors/JsonRpcError'; import { RequestDetails } from './types'; /** @@ -61,7 +61,7 @@ export class Precheck { * @param {Transaction} tx - The transaction. */ value(tx: Transaction): void { - if (tx.data === EthImpl.emptyHex && tx.value < constants.TINYBAR_TO_WEIBAR_COEF) { + if ((tx.value > 0 && tx.value < constants.TINYBAR_TO_WEIBAR_COEF) || tx.value < 0) { throw predefined.VALUE_TOO_LOW; } } diff --git a/packages/relay/tests/lib/eth/eth_estimateGas.spec.ts b/packages/relay/tests/lib/eth/eth_estimateGas.spec.ts index 6d737045ae..a47b1c26bb 100644 --- a/packages/relay/tests/lib/eth/eth_estimateGas.spec.ts +++ b/packages/relay/tests/lib/eth/eth_estimateGas.spec.ts @@ -19,18 +19,19 @@ */ import { expect, use } from 'chai'; -import { v4 as uuid } from 'uuid'; -import { AbiCoder, keccak256 } from 'ethers'; import chaiAsPromised from 'chai-as-promised'; -import { EthImpl } from '../../../src/lib/eth'; +import { AbiCoder, keccak256 } from 'ethers'; +import { createStubInstance, SinonStub, SinonStubbedInstance, stub } from 'sinon'; +import { v4 as uuid } from 'uuid'; + import { Eth, JsonRpcError } from '../../../src'; -import { generateEthTestEnv } from './eth-helpers'; +import { numberTo0x } from '../../../src/formatters'; +import { SDKClient } from '../../../src/lib/clients'; import constants from '../../../src/lib/constants'; +import { EthImpl } from '../../../src/lib/eth'; import { Precheck } from '../../../src/lib/precheck'; -import { SDKClient } from '../../../src/lib/clients'; -import { numberTo0x } from '../../../src/formatters'; -import { createStubInstance, SinonStub, SinonStubbedInstance, stub } from 'sinon'; import { IContractCallRequest, IContractCallResponse, RequestDetails } from '../../../src/lib/types'; +import { overrideEnvsInMochaDescribe, withOverriddenEnvsInMochaTest } from '../../helpers'; import { ACCOUNT_ADDRESS_1, DEFAULT_NETWORK_FEES, @@ -38,7 +39,7 @@ import { ONE_TINYBAR_IN_WEI_HEX, RECEIVER_ADDRESS, } from './eth-config'; -import { overrideEnvsInMochaDescribe, withOverriddenEnvsInMochaTest } from '../../helpers'; +import { generateEthTestEnv } from './eth-helpers'; use(chaiAsPromised); @@ -270,7 +271,8 @@ describe('@ethEstimateGas Estimate Gas spec', async function () { value: 0, //in tinybars }; await mockContractCall(callData, true, 501, { errorMessage: '', statusCode: 501 }, requestDetails); - const result = await ethImpl.estimateGas( + restMock.onGet(`accounts/${RECEIVER_ADDRESS}${NO_TRANSACTIONS}`).reply(200, { address: RECEIVER_ADDRESS }); + const gas = await ethImpl.estimateGas( { to: RECEIVER_ADDRESS, value: 0, @@ -279,11 +281,7 @@ describe('@ethEstimateGas Estimate Gas spec', async function () { requestDetails, ); - expect(result).to.exist; - expect((result as JsonRpcError).code).to.equal(-32602); - expect((result as JsonRpcError).message).to.equal( - `Invalid parameter 0: Invalid 'value' field in transaction param. Value must be greater than 0`, - ); + expect(gas).to.equal(EthImpl.gasTxBaseCost); }); it('should eth_estimateGas for contract create with input field and absent data field', async () => { @@ -306,13 +304,14 @@ describe('@ethEstimateGas Estimate Gas spec', async function () { it('should eth_estimateGas transfer with invalid value', async function () { const callData: IContractCallRequest = { to: RECEIVER_ADDRESS, - value: null, //in tinybars + value: -100_000_000_000, //in tinybars }; await mockContractCall(callData, true, 501, { errorMessage: '', statusCode: 501 }, requestDetails); + restMock.onGet(`accounts/${RECEIVER_ADDRESS}${NO_TRANSACTIONS}`).reply(200, { address: RECEIVER_ADDRESS }); const result = await ethImpl.estimateGas( { to: RECEIVER_ADDRESS, - value: null, + value: -100_000_000_000, }, null, requestDetails, @@ -321,7 +320,7 @@ describe('@ethEstimateGas Estimate Gas spec', async function () { expect(result).to.exist; expect((result as JsonRpcError).code).to.equal(-32602); expect((result as JsonRpcError).message).to.equal( - `Invalid parameter 0: Invalid 'value' field in transaction param. Value must be greater than 0`, + `Invalid parameter 0: Invalid 'value' field in transaction param. Value must be greater than or equal to 0`, ); }); diff --git a/packages/relay/tests/lib/precheck.spec.ts b/packages/relay/tests/lib/precheck.spec.ts index d53b1b320a..c793c3911a 100644 --- a/packages/relay/tests/lib/precheck.spec.ts +++ b/packages/relay/tests/lib/precheck.spec.ts @@ -19,11 +19,19 @@ */ import { ConfigService } from '@hashgraph/json-rpc-config-service/dist/services'; -import { expect } from 'chai'; -import { Registry } from 'prom-client'; import { Hbar, HbarUnit } from '@hashgraph/sdk'; +import axios from 'axios'; +import MockAdapter from 'axios-mock-adapter'; +import { expect } from 'chai'; +import { ethers, Transaction } from 'ethers'; import pino from 'pino'; +import { Registry } from 'prom-client'; + +import { JsonRpcError, predefined } from '../../src'; +import { MirrorNodeClient } from '../../src/lib/clients'; +import constants from '../../src/lib/constants'; import { Precheck } from '../../src/lib/precheck'; +import { CacheService } from '../../src/lib/services/cacheService/cacheService'; import { blobVersionedHash, contractAddress1, @@ -32,13 +40,6 @@ import { overrideEnvsInMochaDescribe, signTransaction, } from '../helpers'; -import { MirrorNodeClient } from '../../src/lib/clients'; -import axios from 'axios'; -import MockAdapter from 'axios-mock-adapter'; -import { ethers, Transaction } from 'ethers'; -import constants from '../../src/lib/constants'; -import { JsonRpcError, predefined } from '../../src'; -import { CacheService } from '../../src/lib/services/cacheService/cacheService'; import { ONE_TINYBAR_IN_WEI_HEX } from './eth/eth-config'; const registry = new Registry(); @@ -71,6 +72,9 @@ describe('Precheck', async function () { const parsedTxWithValueLessThanOneTinybarAndNotEmptyData = ethers.Transaction.from( txWithValueLessThanOneTinybarAndNotEmptyData, ); + const txWithZeroValue = + '0xf86380843b9aca00825208940000000000000000000000000000000000000000808025a04e557f2008ff383df9a21919860939f60f4c27b9c845b89021ae2a79be4f6790a002f86d6dcefd2ffec72bf4d427091e7375acb6707e49d99893173cbc03515fd6'; + const parsedTxWithZeroValue = ethers.Transaction.from(txWithZeroValue); const defaultGasPrice = 720_000_000_000; const defaultGasLimit = 1_000_000; @@ -117,20 +121,28 @@ describe('Precheck', async function () { }); describe('value', async function () { - it('should throw an exception if value is less than 1 tinybar', async function () { + it('should throw an exception if value is less than 1 tinybar but above 0', async function () { let hasError = false; try { precheck.value(parsedTxWithValueLessThanOneTinybar); } catch (e: any) { expect(e).to.exist; expect(e.code).to.eq(-32602); - expect(e.message).to.eq('Value below 10_000_000_000 wei which is 1 tinybar'); + expect(e.message).to.eq("Value can't be non-zero and less than 10_000_000_000 wei which is 1 tinybar"); hasError = true; } expect(hasError).to.be.true; }); + it('should pass if value is 0', async function () { + try { + precheck.value(parsedTxWithZeroValue); + } catch (e) { + expect(e).to.not.exist; + } + }); + it('should pass if value is more than 1 tinybar', async function () { try { precheck.value(parsedTxWithValueMoreThanOneTinyBar); @@ -139,12 +151,50 @@ describe('Precheck', async function () { } }); - it('should pass if value is less than 1 tinybar and data is not empty', async function () { + it('should throw an exception if value is less than 1 tinybar, above 0, and data is not empty', async function () { + let hasError = false; + try { precheck.value(parsedTxWithValueLessThanOneTinybarAndNotEmptyData); } catch (e: any) { - expect(e).to.not.exist; + expect(e).to.exist; + expect(e.code).to.eq(-32602); + expect(e.message).to.eq("Value can't be non-zero and less than 10_000_000_000 wei which is 1 tinybar"); + hasError = true; + } + expect(hasError).to.be.true; + }); + + it('should throw an exception if value is negative', async function () { + let hasError = false; + const txWithNegativeValue = parsedTxWithValueLessThanOneTinybar.clone(); + txWithNegativeValue.value = -1; + try { + precheck.value(txWithNegativeValue); + } catch (e: any) { + expect(e).to.exist; + expect(e.code).to.eq(-32602); + expect(e.message).to.eq("Value can't be non-zero and less than 10_000_000_000 wei which is 1 tinybar"); + hasError = true; } + + expect(hasError).to.be.true; + }); + + it('should throw an exception if value is negative and more than one tinybar', async function () { + let hasError = false; + const txWithNegativeValue = parsedTxWithValueLessThanOneTinybar.clone(); + txWithNegativeValue.value = -100_000_000; + try { + precheck.value(txWithNegativeValue); + } catch (e: any) { + expect(e).to.exist; + expect(e.code).to.eq(-32602); + expect(e.message).to.eq("Value can't be non-zero and less than 10_000_000_000 wei which is 1 tinybar"); + hasError = true; + } + + expect(hasError).to.be.true; }); });