diff --git a/__tests__/integration/storage/storage.test.ts b/__tests__/integration/storage/storage.test.ts index d42a9d36d..08e8dbc54 100644 --- a/__tests__/integration/storage/storage.test.ts +++ b/__tests__/integration/storage/storage.test.ts @@ -9,7 +9,7 @@ import { waitForWalletReady, waitForTxReceived, } from '../helpers/wallet.helper'; -import { InvalidPasswdError } from '../../../src/errors'; +import { InvalidPasswdError, SendTxError } from '../../../src/errors'; import HathorWallet from '../../../src/new/wallet'; import { loggers } from '../utils/logger.util'; import SendTransaction from '../../../src/new/sendTransaction'; @@ -83,12 +83,15 @@ describe('locked utxos', () => { ], pin: 'xxx', // instance with wrong pin to validate pin as parameter }); - await expect(sendTx.prepareTx()).rejects.toThrow(InvalidPasswdError); + // prepareTx creates an unsigned transaction (pin is not validated here) + await sendTx.prepareTx(); + // signTx with the wrong pin should fail + await expect(sendTx.signTx()).rejects.toThrow(InvalidPasswdError); // Will work with the correct PIN as parameter - await sendTx.prepareTx(DEFAULT_PIN_CODE); + await sendTx.signTx(DEFAULT_PIN_CODE); await sendTx.updateOutputSelected(true); // This shouldn't fail since if we did not have tokens the prepareTx should have failed - const input = sendTx.transaction.inputs[0]; + const input = sendTx.transaction!.inputs[0]; const utxoId = { txId: input.hash, index: input.index }; await expect(hwallet.storage.isUtxoSelectedAsInput(utxoId)).resolves.toBe(true); // Send a transaction spending the only utxo on the wallet. @@ -104,6 +107,134 @@ describe('locked utxos', () => { const storageMem = new Storage(storeMem); await testUnlockWhenSpent(storageMem, walletDataMem); }); + + it('should wrap prepareTx errors into SendTxError', async () => { + const walletData = precalculationHelpers.test.getPrecalculatedWallet(); + const store = new MemoryStore(); + const storage = new Storage(store); + const hwallet = await startWallet(storage, walletData); + const address = await hwallet.getAddressAtIndex(0); + await GenesisWalletHelper.injectFunds(hwallet, address, 1n); + + const sendTx = new SendTransaction({ + storage: hwallet.storage, + outputs: [ + { + type: 'p2pkh', + address: await hwallet.getAddressAtIndex(1), + // Sending more than available should cause prepareTx to fail + value: 9999n, + token: NATIVE_TOKEN_UID, + }, + ], + }); + + await expect(sendTx.prepareTx()).rejects.toThrow(SendTxError); + }); +}); + +describe('run(until) state machine', () => { + afterEach(async () => { + await stopWallets(); + await stopAllWallets(); + await GenesisWalletHelper.clearListeners(); + }); + + it('should stop at prepare-tx and resume to completion', async () => { + const walletData = precalculationHelpers.test.getPrecalculatedWallet(); + const store = new MemoryStore(); + const storage = new Storage(store); + const hwallet = await startWallet(storage, walletData); + const address = await hwallet.getAddressAtIndex(0); + await GenesisWalletHelper.injectFunds(hwallet, address, 2n); + + const sendTx = new SendTransaction({ + storage: hwallet.storage, + pin: DEFAULT_PIN_CODE, + outputs: [ + { + type: 'p2pkh', + address: await hwallet.getAddressAtIndex(1), + value: 1n, + token: NATIVE_TOKEN_UID, + }, + ], + }); + + // Stop after prepare — transaction should exist but be unsigned (no input data) + const preparedTx = await sendTx.run('prepare-tx'); + expect(preparedTx).toBeDefined(); + expect(preparedTx.inputs.length).toBeGreaterThan(0); + for (const input of preparedTx.inputs) { + expect(input.data).toBeNull(); + } + + // Resume from where we left off — should sign, mine and push + const finalTx = await sendTx.run(null); + expect(finalTx.hash).toBeDefined(); + await waitForTxReceived(hwallet, finalTx.hash); + }); + + it('should stop at sign-tx and resume to completion', async () => { + const walletData = precalculationHelpers.test.getPrecalculatedWallet(); + const store = new MemoryStore(); + const storage = new Storage(store); + const hwallet = await startWallet(storage, walletData); + const address = await hwallet.getAddressAtIndex(0); + await GenesisWalletHelper.injectFunds(hwallet, address, 2n); + + const sendTx = new SendTransaction({ + storage: hwallet.storage, + pin: DEFAULT_PIN_CODE, + outputs: [ + { + type: 'p2pkh', + address: await hwallet.getAddressAtIndex(1), + value: 1n, + token: NATIVE_TOKEN_UID, + }, + ], + }); + + // Stop after sign — transaction should be signed (input data populated) + const signedTx = await sendTx.run('sign-tx'); + expect(signedTx).toBeDefined(); + for (const input of signedTx.inputs) { + expect(input.data).not.toBeNull(); + } + + // Resume — should mine and push + const finalTx = await sendTx.run(null); + expect(finalTx.hash).toBeDefined(); + await waitForTxReceived(hwallet, finalTx.hash); + }); + + it('should complete the full flow with run(null)', async () => { + const walletData = precalculationHelpers.test.getPrecalculatedWallet(); + const store = new MemoryStore(); + const storage = new Storage(store); + const hwallet = await startWallet(storage, walletData); + const address = await hwallet.getAddressAtIndex(0); + await GenesisWalletHelper.injectFunds(hwallet, address, 2n); + + const sendTx = new SendTransaction({ + storage: hwallet.storage, + pin: DEFAULT_PIN_CODE, + outputs: [ + { + type: 'p2pkh', + address: await hwallet.getAddressAtIndex(1), + value: 1n, + token: NATIVE_TOKEN_UID, + }, + ], + }); + + // Full flow in one call + const tx = await sendTx.run(null); + expect(tx.hash).toBeDefined(); + await waitForTxReceived(hwallet, tx.hash); + }); }); describe('custom signature method', () => { diff --git a/__tests__/integration/walletservice_facade.test.ts b/__tests__/integration/walletservice_facade.test.ts index 0c554ce58..839abe856 100644 --- a/__tests__/integration/walletservice_facade.test.ts +++ b/__tests__/integration/walletservice_facade.test.ts @@ -2307,7 +2307,7 @@ describe('Fee-based tokens', () => { expect((txData.headers![0] as FeeHeader).entries[0].amount).toBe(2n); // Sign the transaction and run from mining - await sendTx.signTx(sendTx.utxosAddressPath); + await sendTx.signTx(); const tx = await sendTx.runFromMining(); await pollForTx(feeWallet, tx.hash!); diff --git a/__tests__/new/sendTransaction.test.ts b/__tests__/new/sendTransaction.test.ts index a15021bf9..01c4f323f 100644 --- a/__tests__/new/sendTransaction.test.ts +++ b/__tests__/new/sendTransaction.test.ts @@ -169,10 +169,13 @@ test('prepareTxData', async () => { tokens: ['01'], }); - await expect(sendTransaction.prepareTx()).rejects.toThrow('Pin is not set.'); - sendTransaction.pin = '000000'; + // prepareTx does not require a PIN (creates unsigned transaction) await expect(sendTransaction.prepareTx()).resolves.toBe(preparedTx); + // signTx requires a PIN + await expect(sendTransaction.signTx()).rejects.toThrow('Pin is not set.'); + sendTransaction.pin = '000000'; + prepareSpy.mockRestore(); spyGetToken.mockRestore(); }); diff --git a/__tests__/wallet/sendTransactionWalletService.test.ts b/__tests__/wallet/sendTransactionWalletService.test.ts index 264d958eb..5af8b9bd6 100644 --- a/__tests__/wallet/sendTransactionWalletService.test.ts +++ b/__tests__/wallet/sendTransactionWalletService.test.ts @@ -13,7 +13,7 @@ import Network from '../../src/models/network'; import Address from '../../src/models/address'; import { TokenVersion } from '../../src/types'; import FeeHeader from '../../src/headers/fee'; -import { SendTxError } from '../../src/errors'; +import { SendTxError, WalletError } from '../../src/errors'; describe('prepareTxData', () => { let wallet; @@ -2158,7 +2158,7 @@ describe('prepareTx - Fee Tokens', () => { ]; sendTransaction = new SendTransactionWalletService(wallet, { inputs, outputs }); - const { transaction } = await sendTransaction.prepareTx(); + const transaction = await sendTransaction.prepareTx(); // Verify inputs: 1 pre-selected fee token + 1 pre-selected HTR expect(transaction.inputs).toHaveLength(2); @@ -2254,7 +2254,7 @@ describe('prepareTx - Fee Tokens', () => { ]; sendTransaction = new SendTransactionWalletService(wallet, { outputs }); - const { transaction } = await sendTransaction.prepareTx(); + const transaction = await sendTransaction.prepareTx(); // Verify transaction was created expect(transaction).toBeDefined(); @@ -2327,3 +2327,52 @@ describe('prepareTx - Fee Tokens', () => { ); }); }); + +describe('signTx preconditions', () => { + let wallet; + const seed = + 'purse orchard camera cloud piece joke hospital mechanic timber horror shoulder rebuild you decrease garlic derive rebuild random naive elbow depart okay parrot cliff'; + + beforeEach(() => { + wallet = new HathorWalletServiceWallet({ + requestPassword: async () => '123', + seed, + network: new Network('testnet'), + }); + }); + + it('should throw when transaction is null', async () => { + const sendTransaction = new SendTransactionWalletService(wallet); + await expect(sendTransaction.signTx('1234')).rejects.toThrow(WalletError); + await expect(sendTransaction.signTx('1234')).rejects.toThrow( + "Can't sign transaction if it's null." + ); + }); + + it('should throw when pin is not set', async () => { + const sendTransaction = new SendTransactionWalletService(wallet); + // Manually set transaction to bypass the null check + sendTransaction.transaction = { + inputs: [], + getDataToSignHash: jest.fn(), + prepareToSend: jest.fn(), + }; + await expect(sendTransaction.signTx()).rejects.toThrow(SendTxError); + await expect(sendTransaction.signTx()).rejects.toThrow('Pin is not set.'); + }); + + it('should throw when utxosAddressPath does not match inputs', async () => { + const sendTransaction = new SendTransactionWalletService(wallet); + // Set transaction with inputs but leave utxosAddressPath empty + sendTransaction.transaction = { + inputs: [{ hash: 'tx1', index: 0 }], + getDataToSignHash: jest.fn(), + prepareToSend: jest.fn(), + }; + sendTransaction.utxosAddressPath = []; // mismatch: 0 paths vs 1 input + await expect(sendTransaction.signTx('1234')).rejects.toThrow(SendTxError); + await expect(sendTransaction.signTx('1234')).rejects.toThrow( + 'utxosAddressPath length does not match transaction inputs. Call prepareTx() first.' + ); + }); +}); diff --git a/__tests__/wallet/wallet.test.ts b/__tests__/wallet/wallet.test.ts index bce6440e3..9436a79fa 100644 --- a/__tests__/wallet/wallet.test.ts +++ b/__tests__/wallet/wallet.test.ts @@ -56,15 +56,17 @@ jest.mock('../../src/wallet/sendTransactionWalletService', () => { const instance = new ActualSendTransactionWalletService(...args); // Mock all methods except run with appropriate return values - instance.prepareTx = jest.fn().mockResolvedValue({ - utxosAddressPath: [], // Mock utxos address path array - }); - // Set up the transaction mock for signTx to use - instance.transaction = { + const mockTransaction = { getDataToSignHash: jest.fn().mockReturnValue(Buffer.from('mock-hash')), inputs: [], // Mock empty inputs so the for loop doesn't execute prepareToSend: jest.fn(), }; + instance.prepareTx = jest.fn().mockImplementation(() => { + instance.transaction = mockTransaction; + instance.utxosAddressPath = []; + instance._currentStep = 'prepared'; + return Promise.resolve(mockTransaction); + }); instance.runFromMining = jest.fn().mockResolvedValue({}); return instance; diff --git a/src/new/sendTransaction.ts b/src/new/sendTransaction.ts index fd3f004e2..624f2b1f8 100644 --- a/src/new/sendTransaction.ts +++ b/src/new/sendTransaction.ts @@ -32,7 +32,7 @@ import tokens from '../utils/tokens'; import transactionUtils from '../utils/transaction'; import { bestUtxoSelection } from '../utils/utxo'; import MineTransaction from '../wallet/mineTransaction'; -import { OutputType } from '../wallet/types'; +import { ISendTransaction as ISendTransactionInterface, OutputType } from '../wallet/types'; import HathorWallet from './wallet'; import Header from '../headers/base'; import FeeHeader from '../headers/fee'; @@ -79,7 +79,7 @@ export type ISendOutput = ISendDataOutput | ISendTokenOutput; * 'send-error': if an error happens; * 'unexpected-error': if an unexpected error happens; * */ -export default class SendTransaction extends EventEmitter { +export default class SendTransaction extends EventEmitter implements ISendTransactionInterface { wallet: HathorWallet | null; storage: IStorage | null; @@ -98,6 +98,8 @@ export default class SendTransaction extends EventEmitter { mineTransaction: MineTransaction | null = null; + private _currentStep: 'idle' | 'prepared' | 'signed' = 'idle'; + /** * * @param {HathorWallet} wallet Wallet instance @@ -314,8 +316,38 @@ export default class SendTransaction extends EventEmitter { } /** - * Prepare transaction data from inputs and outputs - * Fill the inputs if needed, create output change if needed and sign inputs + * Prepare transaction without signing it. + * Fill the inputs if needed, create output change if needed. + * + * @throws SendTxError + * + * @return {Transaction} Transaction object prepared to be signed + * + * @memberof SendTransaction + * @inner + */ + async prepareTx(): Promise { + if (!this.storage) { + throw new SendTxError('Storage is not set.'); + } + + const txData = this.fullTxData || (await this.prepareTxData()); + try { + this.transaction = await transactionUtils.prepareTransaction(txData, '', this.storage, { + signTx: false, + }); + // This will validate if the transaction has more than the max number of inputs and outputs. + this.transaction.validate(); + this._currentStep = 'prepared'; + return this.transaction; + } catch (e) { + const message = helpers.handlePrepareDataError(e); + throw new SendTxError(message); + } + } + + /** + * Sign the transaction and prepare the tx to be mined * * @param {string | null} pin Pin to use in this method (overwrites this.pin) * @@ -326,20 +358,24 @@ export default class SendTransaction extends EventEmitter { * @memberof SendTransaction * @inner */ - async prepareTx(pin = null): Promise { + async signTx(pin: string | null = null): Promise { if (!this.storage) { throw new SendTxError('Storage is not set.'); } + if (!this.transaction) { + throw new SendTxError('Transaction is not set.'); + } + const pinToUse = pin ?? this.pin ?? ''; - const txData = this.fullTxData || (await this.prepareTxData()); try { if (!pinToUse) { - throw new Error('Pin is not set.'); + throw new SendTxError('Pin is not set.'); } - this.transaction = await transactionUtils.prepareTransaction(txData, pinToUse, this.storage); - // This will validate if the transaction has more than the max number of inputs and outputs. - this.transaction.validate(); + + await transactionUtils.signTransaction(this.transaction, this.storage, pinToUse); + this.transaction.prepareToSend(); + this._currentStep = 'signed'; return this.transaction; } catch (e) { const message = helpers.handlePrepareDataError(e); @@ -536,7 +572,7 @@ export default class SendTransaction extends EventEmitter { * @memberof SendTransaction * @inner */ - async runFromMining(until = null): Promise { + async runFromMining(until: 'mine-tx' | null = null): Promise { try { if (this.transaction === null) { throw new WalletError(ErrorMessages.TRANSACTION_IS_NULL); @@ -581,16 +617,33 @@ export default class SendTransaction extends EventEmitter { * Run sendTransaction from preparing, i.e. prepare, sign, mine and push the tx * * 'until' parameter can be 'prepare-tx' (it will stop before signing the tx), + * 'sign-tx' (it will stop before mining the tx), * or 'mine-tx' (it will stop before send tx proposal, i.e. propagating the tx) * + * Can be called incrementally: run('prepare-tx') then run(null) to continue. + * * @memberof SendTransaction * @inner */ - async run(until = null, pin = null) { + async run( + until: 'prepare-tx' | 'sign-tx' | 'mine-tx' | null = null, + pin: string | null = null + ): Promise { try { - await this.prepareTx(pin); + if (this._currentStep === 'idle') { + await this.prepareTx(); + } + if (until === 'prepare-tx') { - return this.transaction; + return this.transaction!; + } + + if (this._currentStep === 'prepared') { + await this.signTx(pin); + } + + if (until === 'sign-tx') { + return this.transaction!; } const tx = await this.runFromMining(until); diff --git a/src/utils/transaction.ts b/src/utils/transaction.ts index e69ba92bb..043dcc986 100644 --- a/src/utils/transaction.ts +++ b/src/utils/transaction.ts @@ -783,8 +783,8 @@ const transaction = { const tx = this.createTransactionFromData(txData, network); if (newOptions.signTx) { await this.signTransaction(tx, storage, pinCode); + tx.prepareToSend(); } - tx.prepareToSend(); return tx; }, diff --git a/src/wallet/sendTransactionWalletService.ts b/src/wallet/sendTransactionWalletService.ts index 4ff966bb6..5d545bb93 100644 --- a/src/wallet/sendTransactionWalletService.ts +++ b/src/wallet/sendTransactionWalletService.ts @@ -93,6 +93,8 @@ class SendTransactionWalletService extends EventEmitter implements ISendTransact // Fee amount to prepare the transaction private _feeAmount: bigint; + private _currentStep: 'idle' | 'prepared' | 'signed' = 'idle'; + constructor(wallet: HathorWalletServiceWallet, options: optionsType = {}) { super(); @@ -375,7 +377,7 @@ class SendTransactionWalletService extends EventEmitter implements ISendTransact * @memberof SendTransactionWalletService * @inner */ - async prepareTx(): Promise<{ transaction: Transaction; utxosAddressPath: string[] }> { + async prepareTx(): Promise { this.emit('prepare-tx-start'); // We get the full outputs amount for each token // This is useful for (i) getting the utxos for each one @@ -474,8 +476,10 @@ class SendTransactionWalletService extends EventEmitter implements ISendTransact this.transaction.headers.push(new FeeHeader([{ tokenIndex: 0, amount: this._feeAmount }])); } + this.utxosAddressPath = utxosAddressPath; + this._currentStep = 'prepared'; this.emit('prepare-tx-end', this.transaction); - return { transaction: this.transaction, utxosAddressPath }; + return this.transaction; } /** @@ -771,13 +775,21 @@ class SendTransactionWalletService extends EventEmitter implements ISendTransact * @memberof SendTransactionWalletService * @inner */ - async signTx(utxosAddressPath: string[], pin: string | null = null) { + async signTx(pin?: string | null): Promise { if (this.transaction === null) { throw new WalletError("Can't sign transaction if it's null."); } + const pinToUse = pin ?? this.pin ?? ''; + if (!pinToUse) { + throw new SendTxError('Pin is not set.'); + } + if (this.utxosAddressPath.length !== this.transaction.inputs.length) { + throw new SendTxError( + 'utxosAddressPath length does not match transaction inputs. Call prepareTx() first.' + ); + } this.emit('sign-tx-start'); const dataToSignHash = this.transaction.getDataToSignHash(); - const pinToUse = pin ?? this.pin ?? ''; const xprivkey = await this.wallet.storage.getMainXPrivKey(pinToUse); for (const [idx, inputObj] of this.transaction.inputs.entries()) { @@ -785,7 +797,7 @@ class SendTransactionWalletService extends EventEmitter implements ISendTransact xprivkey, dataToSignHash, // the wallet service returns the full BIP44 path, but we only need the address path: - HathorWalletServiceWallet.getAddressIndexFromFullPath(utxosAddressPath[idx]) + HathorWalletServiceWallet.getAddressIndexFromFullPath(this.utxosAddressPath[idx]) ); inputObj.setData(inputData); } @@ -794,7 +806,9 @@ class SendTransactionWalletService extends EventEmitter implements ISendTransact // we can add the timestamp and calculate the weight this.transaction.prepareToSend(); + this._currentStep = 'signed'; this.emit('sign-tx-end', this.transaction); + return this.transaction; } /** @@ -899,7 +913,7 @@ class SendTransactionWalletService extends EventEmitter implements ISendTransact * @memberof SendTransactionWalletService * @inner */ - async runFromMining(until: string | null = null): Promise { + async runFromMining(until: 'mine-tx' | null = null): Promise { try { // This will await until mine tx is fully completed // mineTx method returns a promise that resolves when @@ -933,14 +947,23 @@ class SendTransactionWalletService extends EventEmitter implements ISendTransact * @memberof SendTransactionWalletService * @inner */ - async run(until: string | null = null, pin: string | null = null): Promise { + async run( + until: 'prepare-tx' | 'sign-tx' | 'mine-tx' | null = null, + pin: string | null = null + ): Promise { try { - const preparedData = await this.prepareTx(); + if (this._currentStep === 'idle') { + await this.prepareTx(); + } + if (until === 'prepare-tx') { return this.transaction!; } - await this.signTx(preparedData.utxosAddressPath, pin); + if (this._currentStep === 'prepared') { + await this.signTx(pin); + } + if (until === 'sign-tx') { return this.transaction!; } diff --git a/src/wallet/types.ts b/src/wallet/types.ts index 34e90771f..0ae87b3f6 100644 --- a/src/wallet/types.ts +++ b/src/wallet/types.ts @@ -6,7 +6,7 @@ */ import bitcore from 'bitcore-lib'; -import { TokenVersion, IStorage, OutputValueType, IHistoryTx } from '../types'; +import { TokenVersion, IStorage, OutputValueType, IHistoryTx, IDataTx } from '../types'; import Transaction from '../models/transaction'; import CreateTokenTransaction from '../models/create_token_transaction'; import SendTransactionWalletService from './sendTransactionWalletService'; @@ -487,8 +487,15 @@ export interface IHathorWallet { } export interface ISendTransaction { - run(until: string | null): Promise; - runFromMining(until: string | null): Promise; + run( + until?: 'prepare-tx' | 'sign-tx' | 'mine-tx' | null, + pin?: string | null + ): Promise; + runFromMining(until?: 'mine-tx' | null): Promise; + prepareTx(): Promise; + signTx(pin?: string | null): Promise; + readonly transaction: Transaction | null; + readonly fullTxData: IDataTx | null; } export interface MineTxSuccessData {