Skip to content

Commit

Permalink
fix: let 0 value transactions pass (#3304)
Browse files Browse the repository at this point in the history 
* fix: let 0 value transactions pass

Signed-off-by: Simeon Nakov <[email protected]>

* fix: brought back precheck for value with additional logic + tests

Signed-off-by: Simeon Nakov <[email protected]>

* fix: changed check to account for transactions with non-zero data

Signed-off-by: Simeon Nakov <[email protected]>

* fix: removed unused import

Signed-off-by: Simeon Nakov <[email protected]>

* More clear error for VALUE_TOO_LOW

Co-authored-by: Nana Essilfie-Conduah <[email protected]>
Signed-off-by: Simeon Nakov <[email protected]>

* fix: changed VALUE_TOO_LOW message in tests as well

Signed-off-by: Simeon Nakov <[email protected]>

---------

Signed-off-by: Simeon Nakov <[email protected]>
Signed-off-by: Simeon Nakov <[email protected]>
Co-authored-by: Nana Essilfie-Conduah <[email protected]>
  • Loading branch information
@simzzz @Nana-EC
simzzz and Nana-EC authored Dec 6, 2024
1 parent cbcc55d commit c006489
Show file tree
Hide file tree
Showing 5 changed files with 89 additions and 40 deletions.
2 changes: 1 addition & 1 deletion packages/relay/src/lib/errors/JsonRpcError.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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}.`;
Expand Down
6 changes: 3 additions & 3 deletions packages/relay/src/lib/eth.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 6 additions & 6 deletions packages/relay/src/lib/precheck.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';

/**
Expand Down Expand Up @@ -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;
}
}
Expand Down
33 changes: 16 additions & 17 deletions packages/relay/tests/lib/eth/eth_estimateGas.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,26 +19,27 @@
*/

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,
NO_TRANSACTIONS,
ONE_TINYBAR_IN_WEI_HEX,
RECEIVER_ADDRESS,
} from './eth-config';
import { overrideEnvsInMochaDescribe, withOverriddenEnvsInMochaTest } from '../../helpers';
import { generateEthTestEnv } from './eth-helpers';

use(chaiAsPromised);

Expand Down Expand Up @@ -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,
Expand All @@ -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 () => {
Expand All @@ -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,
Expand All @@ -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`,
);
});

Expand Down
76 changes: 63 additions & 13 deletions packages/relay/tests/lib/precheck.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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();
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand All @@ -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;
});
});

Expand Down

0 comments on commit c006489

Please sign in to comment.