Merge branch '114-improve-istxvalid' into 'dev'

improve isTxValid in EvmChain

Closes #114

See merge request ergo/rosen-bridge/rosen-chains!134

.changeset/clever-windows-remember.md

@@ -0,0 +1,5 @@
+---
+'@rosen-chains/evm-rpc': minor
+---
+
+add getTransactionByNonce

.changeset/early-drinks-applaud.md

@@ -0,0 +1,5 @@
+---
+'@rosen-chains/evm': major
+---
+
+add getAddressTransactionByNonce to AbstractEvmNetwork

.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

packages/chains/evm/lib/EvmChain.ts

@@ -473,6 +473,13 @@ abstract class EvmChain extends AbstractChain<Transaction> {
       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}]`
       );

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<Transaction> {
   /**
@@ -65,6 +65,14 @@ abstract class AbstractEvmNetwork extends AbstractChainNetwork<Transaction> {
    * @returns the transaction status
    */
   abstract getTransactionStatus: (hash: string) => Promise<EvmTxStatus>;
+
+  /**
+   * 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<TransactionHashes>;
 }
 
 export default AbstractEvmNetwork;

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;
+}

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

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

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<EvmTxStatus> => {
     throw Error('Not mocked');
   };
+
+  getTransactionByNonce = (nonce: number): Promise<TransactionHashes> => {
+    throw Error('Not mocked');
+  };
 }
 
 export default TestEvmNetwork;

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<AddressTxsEntity> => {
+    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;

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<TransactionHashes> => {
+    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

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 hashes 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 hashes 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);
+    });
+  });
 });