Merge branch '108-invalidation-limitation-return-reason-of-the-invali…

…dation' into 'dev' add reason and expectation status to isTxValid result Closes #108 See merge request ergo/rosen-bridge/rosen-chains!125
rosen-bridge · Jul 31, 2024 · 2a0a667 · 2a0a667
2 parents dcc0492 + 1be4396
commit 2a0a667
Show file tree

Hide file tree

Showing 11 changed files with 181 additions and 49 deletions.
diff --git a/.changeset/wet-crabs-tan.md b/.changeset/wet-crabs-tan.md
@@ -0,0 +1,9 @@
+---
+'@rosen-chains/abstract-chain': major
+'@rosen-chains/bitcoin': major
+'@rosen-chains/cardano': major
+'@rosen-chains/ergo': major
+'@rosen-chains/evm': major
+---
+
+add reason and expectation status to isTxValid result
diff --git a/packages/abstract-chain/lib/AbstractChain.ts b/packages/abstract-chain/lib/AbstractChain.ts
@@ -24,6 +24,7 @@ import {
   TokenDetail,
   TransactionAssetBalance,
   TransactionType,
+  ValidityStatus,
 } from './types';
 import PaymentTransaction from './PaymentTransaction';
 
@@ -280,14 +281,16 @@ abstract class AbstractChain<TxType> {
 
   /**
    * checks if a transaction is still valid and can be sent to the network
+   * Note: the `unexpected` field of the details should be true when reason of
+   * being invalid is unexpected and repetition should be prevented
    * @param transaction the transaction
    * @param signingStatus
    * @returns true if the transaction is still valid
    */
   abstract isTxValid: (
     transaction: PaymentTransaction,
     signingStatus: SigningStatus
-  ) => Promise<boolean>;
+  ) => Promise<ValidityStatus>;
 
   /**
    * requests the corresponding signer service to sign the transaction

diff --git a/packages/abstract-chain/lib/types.ts b/packages/abstract-chain/lib/types.ts
@@ -108,6 +108,16 @@ interface TokenDetail {
   decimals: number;
 }
 
+interface ValidityStatus {
+  isValid: boolean;
+  details:
+    | undefined
+    | {
+        reason: string;
+        unexpected: boolean;
+      };
+}
+
 export {
   ConfirmationConfigs,
   ChainConfigs,
@@ -125,4 +135,5 @@ export {
   TransactionType,
   SigningStatus,
   TokenDetail,
+  ValidityStatus,
 };
diff --git a/packages/chains/bitcoin/lib/BitcoinChain.ts b/packages/chains/bitcoin/lib/BitcoinChain.ts
@@ -15,6 +15,7 @@ import {
   SinglePayment,
   TransactionAssetBalance,
   TransactionType,
+  ValidityStatus,
 } from '@rosen-chains/abstract-chain';
 import AbstractBitcoinNetwork from './network/AbstractBitcoinNetwork';
 import BitcoinTransaction from './BitcoinTransaction';
@@ -376,18 +377,27 @@ class BitcoinChain extends AbstractUtxoChain<BitcoinTx, BitcoinUtxo> {
   isTxValid = async (
     transaction: PaymentTransaction,
     signingStatus: SigningStatus = SigningStatus.Signed
-  ): Promise<boolean> => {
+  ): Promise<ValidityStatus> => {
     const tx = Serializer.deserialize(transaction.txBytes);
     for (let i = 0; i < tx.txInputs.length; i++) {
       const boxId = getPsbtTxInputBoxId(tx.txInputs[i]);
       if (!(await this.network.isBoxUnspentAndValid(boxId))) {
         this.logger.debug(
           `Tx [${transaction.txId}] is invalid due to spending invalid input box [${boxId}] at index [${i}]`
         );
-        return false;
+        return {
+          isValid: false,
+          details: {
+            reason: `input [${i}] is spent or invalid`,
+            unexpected: false,
+          },
+        };
       }
     }
-    return true;
+    return {
+      isValid: true,
+      details: undefined,
+    };
   };
 
   /**

diff --git a/packages/chains/bitcoin/tests/BitcoinChain.spec.ts b/packages/chains/bitcoin/tests/BitcoinChain.spec.ts
@@ -9,7 +9,6 @@ import JsonBigInt from '@rosen-bridge/json-bigint';
 import { Psbt } from 'bitcoinjs-lib';
 import {
   BitcoinChain,
-  BitcoinConfigs,
   BitcoinTransaction,
   BitcoinUtxo,
   SEGWIT_INPUT_WEIGHT_UNIT,
@@ -522,7 +521,7 @@ describe('BitcoinChain', () => {
      * - check returned value
      * - check if function got called
      * @expected
-     * - it should return true
+     * - it should return true with no details
      * - `isBoxUnspentAndValidSpy` should have been called with tx input ids
      */
     it('should return true when all tx inputs are valid and ttl is less than current slot', async () => {
@@ -536,7 +535,10 @@ describe('BitcoinChain', () => {
       const bitcoinChain = testUtils.generateChainObject(network);
       const result = await bitcoinChain.isTxValid(payment1);
 
-      expect(result).toEqual(true);
+      expect(result).toEqual({
+        isValid: true,
+        details: undefined,
+      });
       expect(isBoxUnspentAndValidSpy).toHaveBeenCalledWith(
         testData.transaction0Input0BoxId
       );
@@ -554,7 +556,7 @@ describe('BitcoinChain', () => {
      * - check returned value
      * - check if function got called
      * @expected
-     * - it should return false
+     * - it should return false and as expected invalidation
      */
     it('should return false when at least one input is invalid', async () => {
       const payment1 = BitcoinTransaction.fromJson(
@@ -569,7 +571,13 @@ describe('BitcoinChain', () => {
       const bitcoinChain = testUtils.generateChainObject(network);
       const result = await bitcoinChain.isTxValid(payment1);
 
-      expect(result).toEqual(false);
+      expect(result).toEqual({
+        isValid: false,
+        details: {
+          reason: expect.any(String),
+          unexpected: false,
+        },
+      });
       expect(isBoxUnspentAndValidSpy).toHaveBeenCalledWith(
         testData.transaction0Input0BoxId
       );

diff --git a/packages/chains/cardano/lib/CardanoChain.ts b/packages/chains/cardano/lib/CardanoChain.ts
@@ -19,6 +19,7 @@ import {
   SinglePayment,
   TransactionAssetBalance,
   TransactionType,
+  ValidityStatus,
 } from '@rosen-chains/abstract-chain';
 import JSONBigInt from '@rosen-bridge/json-bigint';
 import CardanoTransaction from './CardanoTransaction';
@@ -487,31 +488,54 @@ class CardanoChain extends AbstractUtxoChain<CardanoTx, CardanoUtxo> {
   isTxValid = async (
     transaction: PaymentTransaction,
     _signingStatus: SigningStatus = SigningStatus.Signed
-  ): Promise<boolean> => {
+  ): Promise<ValidityStatus> => {
     const tx = Serializer.deserialize(transaction.txBytes);
     const txBody = tx.body();
 
     // check ttl
     const ttl = txBody.ttl();
     if (ttl && ttl < (await this.network.currentSlot())) {
-      return false;
+      return {
+        isValid: false,
+        details: {
+          reason: `tx ttl, [${ttl}], is past`,
+          unexpected: false,
+        },
+      };
     }
 
     // let valid = true;
     for (let i = 0; i < txBody.inputs().len(); i++) {
-      const box = txBody.inputs().get(i);
-      if (
-        !(await this.network.isBoxUnspentAndValid(CardanoUtils.getBoxId(box)))
-      )
-        return false;
+      const boxId = CardanoUtils.getBoxId(txBody.inputs().get(i));
+      if (!(await this.network.isBoxUnspentAndValid(boxId))) {
+        this.logger.debug(
+          `Tx [${transaction.txId}] is invalid due to spending invalid input box [${boxId}] at index [${i}]`
+        );
+        return {
+          isValid: false,
+          details: {
+            reason: `input [${i}] is spent or invalid`,
+            unexpected: false,
+          },
+        };
+      }
     }
 
     // check if input and output assets match
     const txAssets = await this.getTransactionAssets(transaction);
-    return ChainUtils.isEqualAssetBalance(
+    const isValid = ChainUtils.isEqualAssetBalance(
       txAssets.inputAssets,
       txAssets.outputAssets
     );
+    return {
+      isValid: isValid,
+      details: isValid
+        ? undefined
+        : {
+            reason: `input and output assets are not equal`,
+            unexpected: false,
+          },
+    };
   };
 
   /**

diff --git a/packages/chains/cardano/tests/CardanoChain.spec.ts b/packages/chains/cardano/tests/CardanoChain.spec.ts
@@ -886,7 +886,7 @@ describe('CardanoChain', () => {
      * - call the function
      * - check returned value
      * @expected
-     * - it should return true
+     * - it should return true with no details
      */
     it('should return true when all tx inputs are valid and ttl is less than current slot', async () => {
       // mock PaymentTransaction
@@ -912,7 +912,10 @@ describe('CardanoChain', () => {
       const result = await cardanoChain.isTxValid(payment1);
 
       // check returned value
-      expect(result).toEqual(true);
+      expect(result).toEqual({
+        isValid: true,
+        details: undefined,
+      });
     });
 
     /**
@@ -924,7 +927,7 @@ describe('CardanoChain', () => {
      * - call the function
      * - check returned value
      * @expected
-     * - it should return false
+     * - it should return false and as expected invalidation
      */
     it('should return false when ttl is expired', async () => {
       // mock PaymentTransaction
@@ -941,7 +944,13 @@ describe('CardanoChain', () => {
       const result = await cardanoChain.isTxValid(payment1);
 
       // check returned value
-      expect(result).toEqual(false);
+      expect(result).toEqual({
+        isValid: false,
+        details: {
+          reason: expect.any(String),
+          unexpected: false,
+        },
+      });
     });
 
     /**
@@ -956,7 +965,7 @@ describe('CardanoChain', () => {
      * - call the function
      * - check returned value
      * @expected
-     * - it should return false
+     * - it should return false and as expected invalidation
      */
     it('should return false when at least one input is invalid', async () => {
       // mock PaymentTransaction
@@ -983,7 +992,13 @@ describe('CardanoChain', () => {
       const result = await cardanoChain.isTxValid(payment1);
 
       // check returned value
-      expect(result).toEqual(false);
+      expect(result).toEqual({
+        isValid: false,
+        details: {
+          reason: expect.any(String),
+          unexpected: false,
+        },
+      });
     });
 
     /**
@@ -998,7 +1013,7 @@ describe('CardanoChain', () => {
      * - call the function
      * - check returned value
      * @expected
-     * - it should return false
+     * - it should return false and as expected invalidation
      */
     it('should return false when input and output assets do not match', async () => {
       // mock PaymentTransaction
@@ -1024,7 +1039,13 @@ describe('CardanoChain', () => {
       const result = await cardanoChain.isTxValid(payment1);
 
       // check returned value
-      expect(result).toEqual(false);
+      expect(result).toEqual({
+        isValid: false,
+        details: {
+          reason: expect.any(String),
+          unexpected: false,
+        },
+      });
     });
   });
 

diff --git a/packages/chains/ergo/lib/ErgoChain.ts b/packages/chains/ergo/lib/ErgoChain.ts
@@ -14,6 +14,7 @@ import {
   SinglePayment,
   TransactionAssetBalance,
   TransactionType,
+  ValidityStatus,
 } from '@rosen-chains/abstract-chain';
 import * as wasm from 'ergo-lib-wasm-nodejs';
 import { ERG, ERGO_CHAIN, NUMBER_OF_BLOCKS_PER_YEAR } from './constants';
@@ -601,7 +602,7 @@ class ErgoChain extends AbstractUtxoChain<wasm.Transaction, wasm.ErgoBox> {
   isTxValid = async (
     transaction: PaymentTransaction,
     signingStatus: SigningStatus = SigningStatus.Signed
-  ): Promise<boolean> => {
+  ): Promise<ValidityStatus> => {
     // deserialize transaction
     let tx: wasm.Transaction | wasm.UnsignedTransaction;
     try {
@@ -613,15 +614,26 @@ class ErgoChain extends AbstractUtxoChain<wasm.Transaction, wasm.ErgoBox> {
       tx = Serializer.deserialize(transaction.txBytes).unsigned_tx();
     }
     // check if any input is spent or invalid
-    let valid = true;
     const inputs = tx.inputs();
     for (let i = 0; i < inputs.len(); i++) {
-      const box = inputs.get(i);
-      valid =
-        valid &&
-        (await this.network.isBoxUnspentAndValid(box.box_id().to_str()));
+      const boxId = inputs.get(i).box_id().to_str();
+      if (!(await this.network.isBoxUnspentAndValid(boxId))) {
+        this.logger.debug(
+          `Tx [${transaction.txId}] is invalid due to spending invalid input box [${boxId}] at index [${i}]`
+        );
+        return {
+          isValid: false,
+          details: {
+            reason: `input [${i}] is spent or invalid`,
+            unexpected: false,
+          },
+        };
+      }
     }
-    return valid;
+    return {
+      isValid: true,
+      details: undefined,
+    };
   };
 
   /**