From 0a615ac18936137f39c24960efc926d3c77156cf Mon Sep 17 00:00:00 2001 From: HFazelinia Date: Thu, 15 Aug 2024 11:53:48 -0600 Subject: [PATCH 1/3] improve isTxValid in EvmChain --- .changeset/early-drinks-applaud.md | 5 ++ .changeset/wise-trees-search.md | 5 ++ packages/chains/evm/lib/EvmChain.ts | 7 +++ .../evm/lib/network/AbstractEvmNetwork.ts | 10 ++- packages/chains/evm/lib/types.ts | 5 ++ packages/chains/evm/tests/EvmChain.spec.ts | 63 ++++++++++++++++++- packages/chains/evm/tests/TestUtils.ts | 14 ++++- .../evm/tests/network/TestEvmNetwork.ts | 6 +- 8 files changed, 110 insertions(+), 5 deletions(-) create mode 100644 .changeset/early-drinks-applaud.md create mode 100644 .changeset/wise-trees-search.md diff --git a/.changeset/early-drinks-applaud.md b/.changeset/early-drinks-applaud.md new file mode 100644 index 0000000..331c5e0 --- /dev/null +++ b/.changeset/early-drinks-applaud.md @@ -0,0 +1,5 @@ +--- +'@rosen-chains/evm': major +--- + +add getAddressTransactionByNonce to AbstractEvmNetwork diff --git a/.changeset/wise-trees-search.md b/.changeset/wise-trees-search.md new file mode 100644 index 0000000..e8bf6b6 --- /dev/null +++ b/.changeset/wise-trees-search.md @@ -0,0 +1,5 @@ +--- +'@rosen-chains/evm': patch +--- + +improve isTxValid to not invalid the tx when tx is not found diff --git a/packages/chains/evm/lib/EvmChain.ts b/packages/chains/evm/lib/EvmChain.ts index 342dcb6..278244d 100644 --- a/packages/chains/evm/lib/EvmChain.ts +++ b/packages/chains/evm/lib/EvmChain.ts @@ -473,6 +473,13 @@ abstract class EvmChain extends AbstractChain { this.configs.addresses.lock ); if (nextNonce > trx.nonce) { + const hashes = this.network.getTransactionByNonce(trx.nonce); + if ((await hashes).unsignedHash === trx.unsignedHash) { + return { + isValid: true, + details: undefined, + }; + } this.logger.debug( `Tx [${transaction.txId}] invalid: Transaction's nonce [${trx.nonce}] is not available anymore according to address's current nonce [${nextNonce}]` ); diff --git a/packages/chains/evm/lib/network/AbstractEvmNetwork.ts b/packages/chains/evm/lib/network/AbstractEvmNetwork.ts index 6a48016..64139fc 100644 --- a/packages/chains/evm/lib/network/AbstractEvmNetwork.ts +++ b/packages/chains/evm/lib/network/AbstractEvmNetwork.ts @@ -1,6 +1,6 @@ import { AbstractChainNetwork } from '@rosen-chains/abstract-chain'; import { Transaction, TransactionResponse } from 'ethers'; -import { EvmTxStatus } from '../types'; +import { EvmTxStatus, TransactionHashes } from '../types'; abstract class AbstractEvmNetwork extends AbstractChainNetwork { /** @@ -65,6 +65,14 @@ abstract class AbstractEvmNetwork extends AbstractChainNetwork { * @returns the transaction status */ abstract getTransactionStatus: (hash: string) => Promise; + + /** + * gets id and unsigned hash of the transaction of the lock address with specific nonce + * throws error if NO tx is found for that nonce + * @param nonce + * @returns the transaction id and unsigned hash + */ + abstract getTransactionByNonce: (nonce: number) => Promise; } export default AbstractEvmNetwork; diff --git a/packages/chains/evm/lib/types.ts b/packages/chains/evm/lib/types.ts index 5bc634c..ec2f7a2 100644 --- a/packages/chains/evm/lib/types.ts +++ b/packages/chains/evm/lib/types.ts @@ -18,3 +18,8 @@ export enum EvmTxStatus { mempool = 'mempool', notFound = 'not-found', } + +export interface TransactionHashes { + unsignedHash: string; + txId: string; +} diff --git a/packages/chains/evm/tests/EvmChain.spec.ts b/packages/chains/evm/tests/EvmChain.spec.ts index 41f031c..6a4693c 100644 --- a/packages/chains/evm/tests/EvmChain.spec.ts +++ b/packages/chains/evm/tests/EvmChain.spec.ts @@ -1890,18 +1890,20 @@ describe('EvmChain', () => { }); /** - * @target EvmChain.isTxValid should return false when nonce is already used + * @target EvmChain.isTxValid should return false when nonce is + * already used by another transaction * @dependencies * @scenario * - mock PaymentTransaction * - mock getTransactionStatus * - mock getAddressNextAvailableNonce + * - mock getTransactionByNonce * - call the function * - check returned value * @expected * - it should return false and as expected invalidation */ - it('should return false when nonce is already used', async () => { + it('should return false when nonce is already used by another transaction', async () => { // mock PaymentTransaction const eventId = 'bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb'; const txType = TransactionType.payment; @@ -1921,6 +1923,13 @@ describe('EvmChain', () => { // mock getAddressNextAvailableNonce testUtils.mockGetAddressNextAvailableNonce(network, tx.nonce + 1); + // mock getTransactionByNonce + testUtils.mockGetTransactionByNonce(network, { + unsignedHash: + 'b129a7ff43fa447edcd87dc596cd8788742d9f395a84e4b5863e7e9d266acf9c', + txId: '568722d20aabcc3102affe9bcd2637f3b8b46429b93154ec7f42649f9844b85e', + }); + // run test const result = await evmChain.isTxValid(paymentTx, SigningStatus.Signed); @@ -1934,6 +1943,56 @@ describe('EvmChain', () => { }); }); + /** + * @target EvmChain.isTxValid should return true when nonce is + * used by current transaction + * @dependencies + * @scenario + * - mock PaymentTransaction + * - mock getTransactionStatus + * - mock getAddressNextAvailableNonce + * - mock getTransactionByNonce + * - call the function + * - check returned value + * @expected + * - it should return true with no details + */ + it('should return true when nonce is used by current transaction', async () => { + // mock PaymentTransaction + const eventId = 'bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb'; + const txType = TransactionType.payment; + const tx = Transaction.from(TestData.erc20transaction as TransactionLike); + + const paymentTx = new PaymentTransaction( + evmChain.CHAIN, + tx.unsignedHash, + eventId, + Serializer.serialize(tx), + txType + ); + + // mock getTransactionStatus + testUtils.mockGetTransactionStatus(network, EvmTxStatus.succeed); + + // mock getAddressNextAvailableNonce + testUtils.mockGetAddressNextAvailableNonce(network, tx.nonce + 1); + + // mock getTransactionByNonce + testUtils.mockGetTransactionByNonce(network, { + unsignedHash: tx.unsignedHash, + txId: tx.hash!, + }); + + // run test + const result = await evmChain.isTxValid(paymentTx, SigningStatus.Signed); + + // check returned value + expect(result).toEqual({ + isValid: true, + details: undefined, + }); + }); + /** * @target EvmChain.isTxValid should return false when * tx is failed diff --git a/packages/chains/evm/tests/TestUtils.ts b/packages/chains/evm/tests/TestUtils.ts index 594689b..a042df4 100644 --- a/packages/chains/evm/tests/TestUtils.ts +++ b/packages/chains/evm/tests/TestUtils.ts @@ -1,5 +1,10 @@ import * as testData from './testData'; -import { EvmConfigs, EvmTxStatus, TssSignFunction } from '../lib/types'; +import { + EvmConfigs, + EvmTxStatus, + TransactionHashes, + TssSignFunction, +} from '../lib/types'; import EvmChain from '../lib/EvmChain'; import { vi } from 'vitest'; import { AbstractEvmNetwork } from '../lib'; @@ -75,6 +80,13 @@ export const mockGetAddressNextAvailableNonce = ( spyOn(network, 'getAddressNextAvailableNonce').mockResolvedValue(value); }; +export const mockGetTransactionByNonce = ( + network: AbstractEvmNetwork, + value: TransactionHashes +) => { + spyOn(network, 'getTransactionByNonce').mockResolvedValue(value); +}; + export const mockGetMaxPriorityFeePerGas = ( network: AbstractEvmNetwork, value: bigint diff --git a/packages/chains/evm/tests/network/TestEvmNetwork.ts b/packages/chains/evm/tests/network/TestEvmNetwork.ts index f1e586c..763dd3d 100644 --- a/packages/chains/evm/tests/network/TestEvmNetwork.ts +++ b/packages/chains/evm/tests/network/TestEvmNetwork.ts @@ -1,5 +1,5 @@ import { Transaction } from 'ethers'; -import { AbstractEvmNetwork, EvmTxStatus } from '../../lib'; +import { AbstractEvmNetwork, EvmTxStatus, TransactionHashes } from '../../lib'; import { BlockInfo, AssetBalance, @@ -77,6 +77,10 @@ class TestEvmNetwork extends AbstractEvmNetwork { getTransactionStatus = (hash: string): Promise => { throw Error('Not mocked'); }; + + getTransactionByNonce = (nonce: number): Promise => { + throw Error('Not mocked'); + }; } export default TestEvmNetwork; From 2a0948c813917181a0a8cac5bb8bdcbc0a133876 Mon Sep 17 00:00:00 2001 From: HFazelinia Date: Thu, 15 Aug 2024 12:23:25 -0600 Subject: [PATCH 2/3] add getTransactionByNonce to EvmRpcNetwork --- .changeset/clever-windows-remember.md | 5 ++ .../networks/evm-rpc/lib/AddressTxAction.ts | 21 ++++++++ .../networks/evm-rpc/lib/EvmRpcNetwork.ts | 15 ++++++ .../evm-rpc/tests/EvmRpcNetwork.spec.ts | 51 +++++++++++++++++++ 4 files changed, 92 insertions(+) create mode 100644 .changeset/clever-windows-remember.md diff --git a/.changeset/clever-windows-remember.md b/.changeset/clever-windows-remember.md new file mode 100644 index 0000000..a439133 --- /dev/null +++ b/.changeset/clever-windows-remember.md @@ -0,0 +1,5 @@ +--- +'@rosen-chains/evm-rpc': minor +--- + +add getTransactionByNonce diff --git a/packages/networks/evm-rpc/lib/AddressTxAction.ts b/packages/networks/evm-rpc/lib/AddressTxAction.ts index e4b2951..12908a1 100644 --- a/packages/networks/evm-rpc/lib/AddressTxAction.ts +++ b/packages/networks/evm-rpc/lib/AddressTxAction.ts @@ -42,6 +42,27 @@ class AddressTxAction { } return res[0]; }; + + /** + * gets transaction by nonce + * @param nonce + */ + getTxByNonce = async (nonce: number): Promise => { + const res = await this.repository.find({ + where: { + address: this.address, + nonce: nonce, + }, + }); + if (res.length === 0) { + throw new Error(`No transaction is found with nonce [${nonce}]`); + } else if (res.length > 1) { + this.logger.warn( + `Found [${res.length}] transactions with nonce [${nonce}]. returning the first one...` + ); + } + return res[0]; + }; } export default AddressTxAction; diff --git a/packages/networks/evm-rpc/lib/EvmRpcNetwork.ts b/packages/networks/evm-rpc/lib/EvmRpcNetwork.ts index cdaa8a1..2717d5d 100644 --- a/packages/networks/evm-rpc/lib/EvmRpcNetwork.ts +++ b/packages/networks/evm-rpc/lib/EvmRpcNetwork.ts @@ -11,6 +11,7 @@ import { AbstractEvmNetwork, EvmTxStatus, PartialERC20ABI, + TransactionHashes, } from '@rosen-chains/evm'; import { Block, @@ -387,6 +388,20 @@ class EvmRpcNetwork extends AbstractEvmNetwork { } }; + /** + * gets id and unsigned hash of the transaction of the lock address with specific nonce + * throws error if NO tx is found for that nonce + * @param nonce + * @returns the transaction id and unsigned hash + */ + getTransactionByNonce = async (nonce: number): Promise => { + const txRecord = await this.dbAction.getTxByNonce(nonce); + return { + unsignedHash: txRecord.unsignedHash, + txId: txRecord.signedHash, + }; + }; + /** * gets the transaction status (mempool, succeed, failed) from TransactionResponse object * @param TransactionResponse diff --git a/packages/networks/evm-rpc/tests/EvmRpcNetwork.spec.ts b/packages/networks/evm-rpc/tests/EvmRpcNetwork.spec.ts index 33bd246..a393c01 100644 --- a/packages/networks/evm-rpc/tests/EvmRpcNetwork.spec.ts +++ b/packages/networks/evm-rpc/tests/EvmRpcNetwork.spec.ts @@ -619,4 +619,55 @@ describe('EvmRpcNetwork', () => { expect(result).toEqual(EvmTxStatus.failed); }); }); + + describe('getTransactionByNonce', () => { + /** + * @target `EvmRpcNetwork.getTransactionByNonce` should return hashesh when tx is found in database + * @dependencies + * - database + * @scenario + * - insert transaction into database + * - run test + * - check returned value + * @expected + * - it should be expected hashes + */ + it('should return hashesh when tx is found in database', async () => { + const unsignedHash = generateRandomId(); + const signedHash = generateRandomId(); + + const nonce = 10; + await addressTxRepository.insert({ + unsignedHash: unsignedHash, + signedHash: signedHash, + nonce: nonce, + address: testData.lockAddress, + blockId: 'blockId', + extractor: 'custom-extractor', + status: 'succeed', + }); + + const result = await network.getTransactionByNonce(nonce); + expect(result).toEqual({ + unsignedHash: unsignedHash, + txId: signedHash, + }); + }); + + /** + * @target `EvmRpcNetwork.getTransactionByNonce` should throw Error when tx is not found in database + * @dependencies + * - database + * @scenario + * - run test + * - check returned value + * @expected + * - it should throw Error + */ + it('should throw Error when tx is not found in database', async () => { + await expect(async () => { + await network.getTransactionByNonce(10); + }).rejects.toThrow(Error); + }); + }); }); From 6c546135d53d7b9b1af3164b1ef88aab3ed581d1 Mon Sep 17 00:00:00 2001 From: HFazelinia Date: Thu, 15 Aug 2024 15:16:06 -0600 Subject: [PATCH 3/3] fix typos --- packages/networks/evm-rpc/tests/EvmRpcNetwork.spec.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/networks/evm-rpc/tests/EvmRpcNetwork.spec.ts b/packages/networks/evm-rpc/tests/EvmRpcNetwork.spec.ts index a393c01..a3175d7 100644 --- a/packages/networks/evm-rpc/tests/EvmRpcNetwork.spec.ts +++ b/packages/networks/evm-rpc/tests/EvmRpcNetwork.spec.ts @@ -622,7 +622,7 @@ describe('EvmRpcNetwork', () => { describe('getTransactionByNonce', () => { /** - * @target `EvmRpcNetwork.getTransactionByNonce` should return hashesh when tx is found in database + * @target `EvmRpcNetwork.getTransactionByNonce` should return hashes when tx is found in database * @dependencies * - database * @scenario @@ -632,7 +632,7 @@ describe('EvmRpcNetwork', () => { * @expected * - it should be expected hashes */ - it('should return hashesh when tx is found in database', async () => { + it('should return hashes when tx is found in database', async () => { const unsignedHash = generateRandomId(); const signedHash = generateRandomId();