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

fix: unit tests failing in CI #2258

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
93 changes: 68 additions & 25 deletions packages/relay/src/lib/precheck.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/*-
/* -
*
* Hedera JSON RPC Relay
*
Expand Down Expand Up @@ -26,43 +26,69 @@ import constants from './constants';
import { ethers, Transaction } from 'ethers';
import { formatRequestIdMessage, prepend0x } from '../formatters';

/**
* Precheck class for handling various prechecks before sending a raw transaction.
*/
export class Precheck {
private mirrorNodeClient: MirrorNodeClient;
private readonly mirrorNodeClient: MirrorNodeClient;
private readonly chain: string;
private readonly logger: Logger;

/**
* Creates an instance of Precheck.
* @param {MirrorNodeClient} mirrorNodeClient - The MirrorNodeClient instance.
* @param {Logger} logger - The logger instance.
* @param {string} chainId - The chain ID.
*/
constructor(mirrorNodeClient: MirrorNodeClient, logger: Logger, chainId: string) {
this.mirrorNodeClient = mirrorNodeClient;
this.logger = logger;
this.chain = chainId;
}

/**
* Parses the transaction if needed.
* @param {string | Transaction} transaction - The transaction to parse.
* @returns {Transaction} The parsed transaction.
*/
public static parseTxIfNeeded(transaction: string | Transaction): Transaction {
return typeof transaction === 'string' ? Transaction.from(transaction) : transaction;
}

value(tx: Transaction) {
/**
* Checks if the value of the transaction is valid.
* @param {Transaction} tx - The transaction.
*/
value(tx: Transaction): void {
if (tx.data === EthImpl.emptyHex && tx.value < constants.TINYBAR_TO_WEIBAR_COEF) {
throw predefined.VALUE_TOO_LOW;
}
}

/**
* @param transaction
* @param gasPrice
* Sends a raw transaction after performing various prechecks.
* @param {ethers.Transaction} parsedTx - The parsed transaction.
* @param {number} gasPrice - The gas price.
* @param {string} [requestId] - The request ID.
*/
async sendRawTransactionCheck(parsedTx: ethers.Transaction, gasPrice: number, requestId?: string) {
async sendRawTransactionCheck(parsedTx: ethers.Transaction, gasPrice: number, requestId?: string): Promise<void> {
this.transactionType(parsedTx, requestId);
this.gasLimit(parsedTx, requestId);
const mirrorAccountInfo = await this.verifyAccount(parsedTx, requestId);
await this.nonce(parsedTx, mirrorAccountInfo.ethereum_nonce, requestId);
this.nonce(parsedTx, mirrorAccountInfo.ethereum_nonce, requestId);
this.chainId(parsedTx, requestId);
this.value(parsedTx);
this.gasPrice(parsedTx, gasPrice, requestId);
await this.balance(parsedTx, mirrorAccountInfo, requestId);
this.balance(parsedTx, mirrorAccountInfo, requestId);
}

async verifyAccount(tx: Transaction, requestId?: string) {
/**
* Verifies the account.
* @param {Transaction} tx - The transaction.
* @param {string} [requestId] - The request ID.
* @returns {Promise<any>} A Promise.
*/
konstantinabl marked this conversation as resolved.
Show resolved Hide resolved
async verifyAccount(tx: Transaction, requestId?: string): Promise<any> {
const requestIdPrefix = formatRequestIdMessage(requestId);
konstantinabl marked this conversation as resolved.
Show resolved Hide resolved
// verify account
const accountInfo = await this.mirrorNodeClient.getAccount(tx.from!, requestId);
Expand All @@ -81,9 +107,12 @@ export class Precheck {
}

/**
* @param tx
* Checks the nonce of the transaction.
* @param {Transaction} tx - The transaction.
* @param {number} accountInfoNonce - The nonce of the account.
* @param {string} [requestId] - The request ID.
*/
async nonce(tx: Transaction, accountInfoNonce: number, requestId?: string) {
nonce(tx: Transaction, accountInfoNonce: number, requestId?: string): void {
const requestIdPrefix = formatRequestIdMessage(requestId);
this.logger.trace(
`${requestIdPrefix} Nonce precheck for sendRawTransaction(tx.nonce=${tx.nonce}, accountInfoNonce=${accountInfoNonce})`,
Expand All @@ -96,9 +125,11 @@ export class Precheck {
}

/**
* @param tx
* Checks the chain ID of the transaction.
* @param {Transaction} tx - The transaction.
* @param {string} [requestId] - The request ID.
*/
chainId(tx: Transaction, requestId?: string) {
chainId(tx: Transaction, requestId?: string): void {
const requestIdPrefix = formatRequestIdMessage(requestId);
const txChainId = prepend0x(Number(tx.chainId).toString(16));
const passes = this.isLegacyUnprotectedEtx(tx) || txChainId === this.chain;
Expand All @@ -124,10 +155,12 @@ export class Precheck {
}

/**
* @param tx
* @param gasPrice
* Checks the gas price of the transaction.
* @param {Transaction} tx - The transaction.
* @param {number} gasPrice - The gas price.
* @param {string} [requestId] - The request ID.
*/
gasPrice(tx: Transaction, gasPrice: number, requestId?: string) {
gasPrice(tx: Transaction, gasPrice: number, requestId?: string): void {
const requestIdPrefix = formatRequestIdMessage(requestId);
const minGasPrice = BigInt(gasPrice);
const txGasPrice = tx.gasPrice || tx.maxFeePerGas! + tx.maxPriorityFeePerGas!;
Expand Down Expand Up @@ -169,10 +202,12 @@ export class Precheck {
}

/**
* @param tx
* @param callerName
* Checks the balance of the sender account.
* @param {Transaction} tx - The transaction.
* @param {any} account - The account information.
* @param {string} [requestId] - The request ID.
*/
async balance(tx: Transaction, account: any, requestId?: string) {
balance(tx: Transaction, account: any, requestId?: string): void {
const requestIdPrefix = formatRequestIdMessage(requestId);
const result = {
passes: false,
Expand Down Expand Up @@ -221,9 +256,11 @@ export class Precheck {
}

/**
* @param tx
* Checks the gas limit of the transaction.
* @param {Transaction} tx - The transaction.
* @param {string} [requestId] - The request ID.
*/
gasLimit(tx: Transaction, requestId?: string) {
gasLimit(tx: Transaction, requestId?: string): void {
const requestIdPrefix = formatRequestIdMessage(requestId);
const gasLimit = Number(tx.gasLimit);
const failBaseLog = 'Failed gasLimit precheck for sendRawTransaction(transaction=%s).';
Expand Down Expand Up @@ -253,11 +290,12 @@ export class Precheck {
* Calculates the intrinsic gas cost based on the number of bytes in the data field.
* Using a loop that goes through every two characters in the string it counts the zero and non-zero bytes.
* Every two characters that are packed together and are both zero counts towards zero bytes.
* @param data
* @param {string} data - The data with the bytes to be calculated
* @returns {number} The intrinsic gas cost.
* @private
*/
private static transactionIntrinsicGasCost(data: string) {
let trimmedData = data.replace('0x', '');
private static transactionIntrinsicGasCost(data: string): number {
const trimmedData = data.replace('0x', '');

let zeros = 0;
let nonZeros = 0;
Expand All @@ -277,7 +315,8 @@ export class Precheck {

/**
* Converts hex string to bytes array
* @param hex the hex string you want to convert
* @param {string} hex - The hex string you want to convert.
* @returns {Uint8Array} The bytes array.
*/
hexToBytes(hex: string): Uint8Array {
if (hex === '') {
Expand All @@ -293,6 +332,10 @@ export class Precheck {
return Uint8Array.from(Buffer.from(hex, 'hex'));
}

/**
* Checks the size of the transaction.
* @param {string} transaction - The transaction to check.
*/
checkSize(transaction: string): void {
const transactionToBytes: Uint8Array = this.hexToBytes(transaction);
const transactionSize: number = transactionToBytes.length;
Expand Down
20 changes: 20 additions & 0 deletions packages/relay/tests/lib/eth/eth-helpers.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,23 @@
/*-
*
* Hedera JSON RPC Relay
*
* Copyright (C) 2023 Hedera Hashgraph, LLC
konstantinabl marked this conversation as resolved.
Show resolved Hide resolved
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*
*/

import { CacheService } from '../../../src/lib/services/cacheService/cacheService';
import pino from 'pino';
import { Registry } from 'prom-client';
Expand Down
61 changes: 27 additions & 34 deletions packages/relay/tests/lib/precheck.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
*
*/

import { Assertion, expect } from 'chai';
import { expect } from 'chai';
import { Registry } from 'prom-client';
import { Hbar, HbarUnit } from '@hashgraph/sdk';
const registry = new Registry();
Expand All @@ -29,13 +29,14 @@ import { blobVersionedHash, contractAddress1, expectedError, mockData, signTrans
import { MirrorNodeClient } from '../../src/lib/clients';
import axios from 'axios';
import MockAdapter from 'axios-mock-adapter';
import { ethers } from 'ethers';
import { Transaction, ethers } from 'ethers';
import constants from '../../src/lib/constants';
import { JsonRpcError, predefined } from '../../src';
import { CacheService } from '../../src/lib/services/cacheService/cacheService';
const logger = pino();

const limitOrderPostFix = '?order=desc&limit=1';
const transactionsPostFix = '?transactions=false';

describe('Precheck', async function () {
const txWithMatchingChainId =
Expand All @@ -59,10 +60,18 @@ describe('Precheck', async function () {
const parsedTxWithValueLessThanOneTinybarAndNotEmptyData = ethers.Transaction.from(
txWithValueLessThanOneTinybarAndNotEmptyData,
);
const oneTinyBar = ethers.parseUnits('1', 10);

const defaultGasPrice = 720_000_000_000;
const defaultGasLimit = 1_000_000;
const defaultChainId = Number('0x12a');
const defaultTx = {
gasLimit: defaultGasLimit,
gasPrice: defaultGasPrice,
chainId: defaultChainId,
maxFeePerGas: null,
maxPriorityFeePerGas: null,
};

let precheck: Precheck;
let mock: MockAdapter;

Expand Down Expand Up @@ -167,12 +176,6 @@ describe('Precheck', async function () {
});

describe('gasLimit', async function () {
const defaultTx = {
value: oneTinyBar,
gasPrice: defaultGasPrice,
chainId: defaultChainId,
};

function testFailingGasLimitPrecheck(gasLimits, errorCode) {
for (const gasLimit of gasLimits) {
it(`should fail for gasLimit: ${gasLimit}`, async function () {
Expand Down Expand Up @@ -393,13 +396,6 @@ describe('Precheck', async function () {

describe('nonce', async function () {
const defaultNonce = 3;
const defaultTx = {
value: oneTinyBar,
gasPrice: defaultGasPrice,
chainId: defaultChainId,
nonce: defaultNonce,
};

const mirrorAccount = {
ethereum_nonce: defaultNonce,
};
Expand Down Expand Up @@ -450,25 +446,22 @@ describe('Precheck', async function () {
});

describe('account', async function () {
const defaultNonce = 3;
const defaultTx = {
value: oneTinyBar,
gasPrice: defaultGasPrice,
chainId: defaultChainId,
nonce: defaultNonce,
from: mockData.accountEvmAddress,
};

const signed = await signTransaction(defaultTx);
const parsedTx = ethers.Transaction.from(signed);
let parsedTx: Transaction;
let mirrorAccount: any;
const defaultNonce: number = 3;

const mirrorAccount = {
evm_address: mockData.accountEvmAddress,
ethereum_nonce: defaultNonce,
};
before(async () => {
const wallet = ethers.Wallet.createRandom();
const signed = await wallet.signTransaction({ ...defaultTx, from: wallet.address, nonce: defaultNonce });
parsedTx = ethers.Transaction.from(signed);
mirrorAccount = {
evm_address: parsedTx.from,
ethereum_nonce: defaultNonce,
};
});

it(`should fail for missing account`, async function () {
mock.onGet(`accounts/${mockData.accountEvmAddress}${limitOrderPostFix}`).reply(404, mockData.notFound);
mock.onGet(`accounts/${parsedTx.from}${transactionsPostFix}`).reply(404, mockData.notFound);

try {
await precheck.verifyAccount(parsedTx);
Expand All @@ -477,12 +470,12 @@ describe('Precheck', async function () {
expect(e).to.exist;
expect(e.code).to.eq(-32001);
expect(e.name).to.eq('Resource not found');
expect(e.message).to.contain(mockData.accountEvmAddress);
expect(e.message).to.contain(parsedTx.from);
}
});

it(`should not fail for matched account`, async function () {
mock.onGet(`accounts/${mockData.accountEvmAddress}${limitOrderPostFix}`).reply(200, mirrorAccount);
mock.onGet(`accounts/${parsedTx.from}${transactionsPostFix}`).reply(200, mirrorAccount);
const account = await precheck.verifyAccount(parsedTx);

expect(account.ethereum_nonce).to.eq(defaultNonce);
Expand Down