fix: prepareTx choose inputs with fee

Author: raul-oliveira

__tests__/integration/hathorwallet_facade.test.ts

@@ -1603,6 +1603,67 @@ describe('sendTransaction', () => {
     ).toHaveProperty('numTransactions', 1);
   });
 
+  it('should send fee token with manually provided HTR input (no HTR output)', async () => {
+    const hWallet = await generateWalletHelper();
+    await GenesisWalletHelper.injectFunds(hWallet, await hWallet.getAddressAtIndex(0), 10n);
+    const { hash: tokenUid } = await createTokenHelper(
+      hWallet,
+      'FeeTokenManualInput',
+      'FTMI',
+      100n,
+      {
+        tokenVersion: TokenVersion.FEE,
+      }
+    );
+
+    // Get UTXOs for both HTR and the fee token
+    const { utxos: utxosHtr } = await hWallet.getUtxos({ token: NATIVE_TOKEN_UID });
+    const { utxos: utxosToken } = await hWallet.getUtxos({ token: tokenUid });
+
+    // Get the first UTXO of each token
+    const htrUtxo = utxosHtr[0];
+    const tokenUtxo = utxosToken[0];
+
+    // Send transaction with manually provided inputs (HTR + token) and only token output
+    // This tests the scenario where user provides HTR input to pay for fee
+    // but has no HTR output (only token output)
+    const tx = await hWallet.sendManyOutputsTransaction(
+      [
+        {
+          address: await hWallet.getAddressAtIndex(5),
+          value: 50n,
+          token: tokenUid,
+        },
+      ],
+      {
+        inputs: [
+          { txId: htrUtxo.tx_id, index: htrUtxo.index },
+          { txId: tokenUtxo.tx_id, index: tokenUtxo.index },
+        ],
+      }
+    );
+
+    validateFeeAmount(tx.headers, 2n);
+    await waitForTxReceived(hWallet, tx.hash);
+
+    // Validate the transaction was created correctly
+    const decodedTx = await hWallet.getTx(tx.hash);
+
+    // Should have 2 inputs (HTR + token)
+    expect(decodedTx.inputs).toHaveLength(2);
+    expect(decodedTx.inputs).toContainEqual(
+      expect.objectContaining({ tx_id: htrUtxo.tx_id, index: htrUtxo.index })
+    );
+    expect(decodedTx.inputs).toContainEqual(
+      expect.objectContaining({ tx_id: tokenUtxo.tx_id, index: tokenUtxo.index })
+    );
+
+    // Should have outputs: token output (50) + token change (50) + HTR change
+    expect(decodedTx.outputs).toContainEqual(
+      expect.objectContaining({ value: 50n, token: tokenUid })
+    );
+  });
+
   it('should send a multisig transaction', async () => {
     // Initialize 3 wallets from the same multisig and inject funds in them to test
     const mhWallet1 = await generateMultisigWalletHelper({ walletIndex: 0 });

src/errors.ts

@@ -207,7 +207,7 @@ export class UtxoError extends WalletError {}
 export class SendTxError extends WalletError {
   // XXX: There are only two out of dozens of places where this object is used instead of a string.
   //      This should be made consistently for strings
-  errorData: string | { txId: string; index: number } = '';
+  errorData: string | { txId: string; index: number } | { txId: string; index: number }[] = '';
 }
 
 /**

src/new/sendTransaction.ts

@@ -198,6 +198,8 @@ export default class SendTransaction extends EventEmitter {
       }
     }
 
+    const requiresFees: { txId: string; index: number }[] = [];
+
     for (const input of this.inputs) {
       const inputTx = await this.storage.getTx(input.txId);
       if (inputTx === null || !inputTx.outputs[input.index]) {
@@ -207,10 +209,15 @@ export default class SendTransaction extends EventEmitter {
       }
       const spentOut = inputTx.outputs[input.index];
       if (!tokenMap.has(spentOut.token)) {
-        // The input select is from a token that is not in the outputs
-        const err = new SendTxError(ErrorMessages.INVALID_INPUT);
-        err.errorData = { txId: input.txId, index: input.index };
-        throw err;
+        // the inputs should be used to pay fees, otherwise it's an invalid input and it will raise an error after the fee is calculated
+        if (HTR_UID === spentOut.token) {
+          requiresFees.push({ txId: input.txId, index: input.index });
+        } else {
+          // The input select is from a token that is not in the outputs
+          const err = new SendTxError(ErrorMessages.INVALID_INPUT);
+          err.errorData = { txId: input.txId, index: input.index };
+          throw err;
+        }
       }
       tokenMap.set(spentOut.token, false);
       txData.inputs.push({
@@ -223,7 +230,12 @@ export default class SendTransaction extends EventEmitter {
       });
     }
 
-    const shouldChooseHTRInputs = tokenMap.get(HTR_UID) || false;
+    // If the user provided HTR inputs, tokenMap.get(HTR_UID) will be false
+    // In that case, we should NOT choose inputs automatically (accept what user provided)
+    // Otherwise (true or undefined), we should choose HTR inputs if needed for fee
+    const tokenMapHasHTR = tokenMap.has(HTR_UID);
+    let shouldChooseHTRInputs = tokenMap.get(HTR_UID) || false;
+
     // we remove HTR from the tokenMap since we will calculate the fee based on the inputs and outputs
     // and we don't want to select inputs for HTR before that
     tokenMap.delete(HTR_UID);
@@ -245,15 +257,25 @@ export default class SendTransaction extends EventEmitter {
       partialOutputs,
       await tokens.getTokensByManyIds(this.storage, new Set(tokenMap.keys()))
     );
+
+    if (requiresFees.length > 0 && fee === 0n) {
+      const err = new SendTxError(ErrorMessages.INVALID_INPUT);
+      err.errorData = requiresFees;
+      throw err;
+    }
+
     const headers: Header[] = [];
     if (fee > 0) {
       headers.push(new FeeHeader([{ tokenIndex: 0, amount: fee }]));
+      // if any HTR input or output was provided, we need to choose inputs for HTR
+      if (!tokenMapHasHTR) {
+        shouldChooseHTRInputs = true;
+      }
     }
 
-    // We only need to grab HTR inputs if they weren't provided or tha tx has a fee
     const options: IUtxoSelectionOptions = {
       token: HTR_UID,
-      chooseInputs: shouldChooseHTRInputs || fee > 0,
+      chooseInputs: shouldChooseHTRInputs,
     };
 
     if (this.changeAddress) {

src/storage/memory_store.ts

@@ -28,7 +28,6 @@ import {
 } from '../types';
 import { GAP_LIMIT, NATIVE_TOKEN_UID } from '../constants';
 import transactionUtils from '../utils/transaction';
-import tokens from '../utils/tokens';
 
 const DEFAULT_ADDRESSES_WALLET_DATA = {
   lastLoadedAddressIndex: 0,

src/utils/fee.ts

@@ -10,7 +10,7 @@ import { FEE_PER_OUTPUT, NATIVE_TOKEN_UID } from '../constants';
 import { IDataInput, IDataOutputWithToken, ITokenData, IUtxo, TokenVersion } from '../types';
 import Output from '../models/output';
 
-type TokenUtxo = IDataInput | Utxo | IUtxo | IDataInput | IDataOutputWithToken | Output;
+type TokenElement = IDataInput | Utxo | IUtxo | IDataInput | IDataOutputWithToken | Output;
 
 export class Fee {
   /**
@@ -31,8 +31,8 @@ export class Fee {
     outputs: (IDataOutputWithToken | Output)[],
     tokens: Map<string, ITokenData | TokenInfo>
   ): Promise<bigint> {
-    const nonAuthorityInputs = Fee.getNonAuthorityUtxoByTokenUid(inputs);
-    const nonAuthorityOutputs = Fee.getNonAuthorityUtxoByTokenUid(outputs);
+    const nonAuthorityInputs = Fee.groupTokenElementsByTokenUid(inputs);
+    const nonAuthorityOutputs = Fee.groupTokenElementsByTokenUid(outputs);
 
     const tokensSet = new Set([...nonAuthorityInputs.keys(), ...nonAuthorityOutputs.keys()]);
     tokensSet.delete(NATIVE_TOKEN_UID);
@@ -67,8 +67,8 @@ export class Fee {
    * @memberof Fee
    * @static
    */
-  static calculateTokenCreationTxFee(outputs: Omit<TokenUtxo, 'token'>[]): bigint {
-    return BigInt(Fee.getNonAuthorityOutputs(outputs).length) * FEE_PER_OUTPUT;
+  static calculateTokenCreationTxFee(outputs: Omit<TokenElement, 'token'>[]): bigint {
+    return BigInt(Fee.getNonAuthorityTokenElement(outputs).length) * FEE_PER_OUTPUT;
   }
 
   /**
@@ -78,52 +78,52 @@ export class Fee {
    * @memberof Fee
    * @static
    */
-  static getNonAuthorityOutputs(
-    outputs: (TokenUtxo | Omit<TokenUtxo, 'token'>)[]
-  ): (TokenUtxo | Omit<TokenUtxo, 'token'>)[] {
-    return outputs.filter(output => !Fee.isAuthorityUtxo(output as never)); // casting to never since we don't need the token property here.
+  static getNonAuthorityTokenElement(
+    outputs: (TokenElement | Omit<TokenElement, 'token'>)[]
+  ): (TokenElement | Omit<TokenElement, 'token'>)[] {
+    return outputs.filter(output => !Fee.isAuthorityTokenElement(output as never)); // casting to never since we don't need the token property here.
   }
 
   /**
-   * Check if the utxo is an authority utxo by checking the `isAuthority` method or ther `authorities` property.
-   * @param utxo utxo to check
-   * @returns true if the utxo is an authority utxo, false otherwise
+   * Check if the token element is an authority by checking the `isAuthority` method or the `authorities` property.
+   * @param tokenElement token element to check
+   * @returns true if the token element is an authority, false otherwise
    * @memberof Fee
    * @static
    */
-  static isAuthorityUtxo(utxo: TokenUtxo): boolean {
-    if (utxo instanceof Output) {
-      return utxo.isAuthority();
+  static isAuthorityTokenElement(tokenElement: TokenElement): boolean {
+    if (tokenElement instanceof Output) {
+      return tokenElement.isAuthority();
     }
-    return utxo.authorities !== 0n;
+    return tokenElement.authorities !== 0n;
   }
 
   /**
-   * Check if the utxo is a non-authority utxo by checking the isAuthorityUtxo method, then grouping them by token UID.
-   * @param utxos an array of utxos to check
-   * @returns a map where the keys are the token UIDs and the values are arrays of non-authority utxos for that token
+   * Check if the token element is a non-authority token element by checking the isAuthorityTokenElement method, then grouping them by token UID.
+   * @param tokenElements an array of token elements to check
+   * @returns a map where the keys are the token UIDs and the values are arrays of non-authority token elements for that token
    * @memberof Fee
    * @static
    */
-  static getNonAuthorityUtxoByTokenUid(utxos: TokenUtxo[]): Map<string, TokenUtxo[]> {
-    const map = new Map<string, TokenUtxo[]>();
+  static groupTokenElementsByTokenUid(tokenElements: TokenElement[]): Map<string, TokenElement[]> {
+    const map = new Map<string, TokenElement[]>();
 
-    for (const utxo of utxos) {
-      if (!Fee.isAuthorityUtxo(utxo)) {
+    for (const tokenElement of tokenElements) {
+      if (!Fee.isAuthorityTokenElement(tokenElement)) {
         let tokenUid: string = '';
-        if ('token' in utxo) {
-          tokenUid = utxo.token;
+        if ('token' in tokenElement) {
+          tokenUid = tokenElement.token;
         }
-        if ('tokenId' in utxo) {
-          tokenUid = utxo.tokenId;
+        if ('tokenId' in tokenElement) {
+          tokenUid = tokenElement.tokenId;
         }
         if (!tokenUid) {
-          throw new Error('Token UID not found in utxo');
+          throw new Error('Token UID not found in token element');
         }
         if (!map.has(tokenUid)) {
           map.set(tokenUid, []);
         }
-        map.get(tokenUid)?.push(utxo);
+        map.get(tokenUid)?.push(tokenElement);
       }
     }
     return map;

src/utils/tokens.ts

@@ -373,7 +373,7 @@ const tokens = {
    * @param {boolean|null} [options.unshiftData=null] Whether to unshift the data script output.
    * @param {string[]|null} [options.data=null] list of data strings using utf8 encoding to add each as a data script output
    * @param {function} [options.utxoSelection=bestUtxoSelection] Algorithm to select utxos. Use the best method by default
-   * @param {boolean} [options.skipDepositFee=false] if it should skip utxo selection for token deposit fee
+   * @param {boolean} [options.skipDepositFee=false] if it should skip utxo selection for token fees
    * @param {TokenVersion} [options.tokenVersion=TokenVersion.DEPOSIT] Token version to be used for the transaction
    *
    * @returns {Promise<IDataTx>} The transaction data
@@ -468,7 +468,9 @@ const tokens = {
         break;
       case TokenVersion.FEE:
         // is creating a new token
-        if (!isMintingToken) {
+        if (skipDepositFee) {
+          feeAmount = 0n;
+        } else if (!isMintingToken) {
           feeAmount = Fee.calculateTokenCreationTxFee(outputs);
         } else {
           const mappedOutputs = outputs.map(
@@ -486,8 +488,7 @@ const tokens = {
             await tokens.getTokensByManyIds(storage, new Set(tokensArray))
           );
 
-          // TODO-RAUL: check the behaviour of the skipDepositFee and if we should skip the entire fee calculation
-          if (!skipDepositFee && data) {
+          if (data) {
             // The deposit amount will be the quantity of data strings in the array
             // multiplied by the fee (this fee is not related to the trasanction fee that is calculated based in the token version)
             depositAmount += this.getDataFee(data.length);