From 0e622f051dc058edb3f57cf4178af6eff5f0bc5b Mon Sep 17 00:00:00 2001 From: Peter Sanderson Date: Mon, 16 Dec 2024 14:39:03 +0100 Subject: [PATCH] fix: useRbfForm test, also try to tame the fixure beast --- .../ChangeFee/ReplaceTxButton.tsx | 3 +- .../TxDetailModal/TxDetailModal.tsx | 21 ++-- .../hooks/wallet/__fixtures__/useRbfForm.ts | 95 +++++++++++++++---- .../wallet/__tests__/useRbfForm.test.tsx | 52 ++++++---- suite-common/wallet-types/src/transaction.ts | 6 ++ 5 files changed, 128 insertions(+), 49 deletions(-) diff --git a/packages/suite/src/components/suite/modals/ReduxModal/UserContextModal/TxDetailModal/ChangeFee/ReplaceTxButton.tsx b/packages/suite/src/components/suite/modals/ReduxModal/UserContextModal/TxDetailModal/ChangeFee/ReplaceTxButton.tsx index 4e8017c376d..5a3d67b7062 100644 --- a/packages/suite/src/components/suite/modals/ReduxModal/UserContextModal/TxDetailModal/ChangeFee/ReplaceTxButton.tsx +++ b/packages/suite/src/components/suite/modals/ReduxModal/UserContextModal/TxDetailModal/ChangeFee/ReplaceTxButton.tsx @@ -2,8 +2,7 @@ import { NewModal } from '@trezor/components'; import { Translation } from 'src/components/suite'; import { useDevice } from 'src/hooks/suite'; - -import { useRbfContext } from '../../../../../../../hooks/wallet/useRbfForm'; +import { useRbfContext } from 'src/hooks/wallet/useRbfForm'; export const ReplaceTxButton = () => { const { device, isLocked } = useDevice(); diff --git a/packages/suite/src/components/suite/modals/ReduxModal/UserContextModal/TxDetailModal/TxDetailModal.tsx b/packages/suite/src/components/suite/modals/ReduxModal/UserContextModal/TxDetailModal/TxDetailModal.tsx index 4b9a0903008..f22ca36df8d 100644 --- a/packages/suite/src/components/suite/modals/ReduxModal/UserContextModal/TxDetailModal/TxDetailModal.tsx +++ b/packages/suite/src/components/suite/modals/ReduxModal/UserContextModal/TxDetailModal/TxDetailModal.tsx @@ -8,18 +8,25 @@ import { selectAllPendingTransactions, selectIsPhishingTransaction, } from '@suite-common/wallet-core'; -import { ChainedTransactions, SelectedAccountLoaded } from '@suite-common/wallet-types'; -import { RequiredKey } from '@trezor/type-utils'; +import { + ChainedTransactions, + SelectedAccountLoaded, + WalletAccountTransactionWithRequiredRbfParams, +} from '@suite-common/wallet-types'; import { useSelector } from 'src/hooks/suite'; import { Translation } from 'src/components/suite'; import { Account, WalletAccountTransaction } from 'src/types/wallet'; +import { RbfContext, useRbf } from 'src/hooks/wallet/useRbfForm'; import { AdvancedTxDetails, TabID } from './AdvancedTxDetails/AdvancedTxDetails'; import { ChangeFee } from './ChangeFee/ChangeFee'; import { ReplaceTxButton } from './ChangeFee/ReplaceTxButton'; import { TxDetailModalBase } from './TxDetailModalBase'; -import { RbfContext, useRbf } from '../../../../../../hooks/wallet/useRbfForm'; + +const hasRbfParams = ( + tx: WalletAccountTransaction, +): tx is WalletAccountTransactionWithRequiredRbfParams => tx.rbfParams !== undefined; type DetailModalProps = { tx: WalletAccountTransaction; @@ -64,7 +71,7 @@ const DetailModal = ({ tx, onCancel, tab, onChangeFeeClick, chainedTxs }: Detail }; type BumpFeeModalProps = { - tx: RequiredKey; + tx: WalletAccountTransactionWithRequiredRbfParams; onCancel: () => void; onBackClick: () => void; onShowChained: () => void; @@ -140,18 +147,16 @@ export const TxDetailModal = ({ tx, rbfForm, onCancel }: TxDetailModalProps) => setTab(undefined); }; - const { rbfParams } = tx; // This is just for trick to enforce type-narrowing - if ( + hasRbfParams(tx) && section === 'CHANGE_FEE' && networkFeatures?.includes('rbf') && - rbfParams !== undefined && !tx.deadline && selectedAccount.status === 'loaded' ) { return ( ({ +const txDummyData = { + deviceState: 'A@B:1', + descriptor: '', + type: 'sent', + txid: '', + amount: '', + fee: '', + targets: [], + tokens: [], + internalTransfers: [], + details: { + vin: [], + vout: [], + size: 0, + totalInput: '', + totalOutput: '', + }, +} satisfies Partial; + +// This type-magic here is for 2 reasons: +// +// 1. WalletAccountTransaction has rbfParams as optional, but we want to +// enforce it in this fixture as we test only this case here +// +// 2. We need to add `required` into AccountUtxo because this is then passed +// down into utxo-lib where it is present on ComposeInput. This is not +// ideal and pretty magic, maybe subject of future refactor. +// +type HackedTxType = WalletAccountTransactionWithRequiredRbfParams & { + rbfParams: WalletAccountTransactionWithRequiredRbfParams['rbfParams'] & { + utxo: Array; + }; +}; + +const PREPARE_TX = (params: Partial = {}): HackedTxType => ({ symbol: 'btc', rbfParams: { txid: 'ABCD', @@ -110,9 +151,21 @@ const PREPARE_TX = (params = {}) => ({ baseFee: 175, ...params, }, + ...txDummyData, }); -export const composeAndSign = [ +type ComposeAndSignFixture = { + description: string; + store: any; + tx: WalletAccountTransactionWithRequiredRbfParams; + composedLevels: any; + composeTransactionCalls: number; + chainedTxs?: ChainedTransactions; + signedTx?: any; + decreasedOutputs?: boolean | string; +}; + +export const composeAndSign: ComposeAndSignFixture[] = [ { description: 'change-output reduced by fee. outputs order not affected. change was at the end of original tx.', @@ -152,7 +205,7 @@ export const composeAndSign = [ ], }, }, - composeTransactionCalls: 2, + composeTransactionCalls: 1, signedTx: { outputs: [ { @@ -178,10 +231,10 @@ export const composeAndSign = [ }, }, chainedTxs: { - own: [{ txid: 'aaaa', fee: '500' }], + own: [{ symbol: 'btc', ...txDummyData, txid: 'aaaa', fee: '500' }], others: [ - { txid: 'bbbb', fee: '500' }, - { txid: 'cccc', fee: '5000' }, + { symbol: 'btc', ...txDummyData, txid: 'bbbb', fee: '500' }, + { symbol: 'btc', ...txDummyData, txid: 'cccc', fee: '5000' }, ], }, tx: PREPARE_TX({ @@ -207,7 +260,7 @@ export const composeAndSign = [ feePerByte: '34.34', // 3.79 (old) + 4 (new) + 26.55 for chainedTxs }, }, - composeTransactionCalls: 2, + composeTransactionCalls: 1, signedTx: { outputs: [ { @@ -218,7 +271,7 @@ export const composeAndSign = [ }, { address_n: [2147483692, 2147483648, 2147483648, 1, 0], - amount: '9239', + amount: '3239', orig_index: 1, orig_hash: 'ABCD', }, @@ -249,7 +302,7 @@ export const composeAndSign = [ ], }, }, - composeTransactionCalls: 2, + composeTransactionCalls: 1, signedTx: { outputs: [ { @@ -304,7 +357,7 @@ export const composeAndSign = [ ], }, }, - composeTransactionCalls: 2, + composeTransactionCalls: 1, signedTx: { outputs: [ // change-output is gone @@ -358,7 +411,7 @@ export const composeAndSign = [ ], }, }, - composeTransactionCalls: 4, // 1. normal fee, 2. custom fee + composeTransactionCalls: 2, // 1. normal fee, 2. custom fee signedTx: { outputs: [ // change-output is gone @@ -432,7 +485,7 @@ export const composeAndSign = [ ], }, }, - composeTransactionCalls: 2, + composeTransactionCalls: 1, signedTx: { outputs: [ { @@ -503,7 +556,7 @@ export const composeAndSign = [ ], }, }, - composeTransactionCalls: 4, // 1. normal fee, 2. custom fee + composeTransactionCalls: 2, // 1. normal fee, 2. custom fee signedTx: { inputs: [{ prev_hash: DCBA }, { prev_hash: ABCD }], outputs: [ @@ -578,7 +631,7 @@ export const composeAndSign = [ ], }, }, - composeTransactionCalls: 4, // 1. normal fee, 2. custom fee + composeTransactionCalls: 2, // 1. normal fee, 2. custom fee signedTx: { inputs: [{ prev_hash: DCBA }, { prev_hash: ABCD }], outputs: [ @@ -648,7 +701,7 @@ export const composeAndSign = [ ], }, }, - composeTransactionCalls: 6, // 1. normal fee, 2. custom fee, 3. send-max + composeTransactionCalls: 3, // 1. normal fee, 2. custom fee, 3. send-max decreasedOutputs: true, signedTx: { inputs: [{ prev_hash: DCBA }], @@ -695,7 +748,7 @@ export const composeAndSign = [ feeRate: '1.37', changeAddress: undefined, }), - composeTransactionCalls: 2, // 1. immediate send-max + composeTransactionCalls: 1, // 1. immediate send-max decreasedOutputs: true, composedLevels: { normal: { @@ -754,7 +807,7 @@ export const composeAndSign = [ ], }, }, - composeTransactionCalls: 6, // 1. normal fee, 2. custom fee, 3 send-max + composeTransactionCalls: 3, // 1. normal fee, 2. custom fee, 3 send-max decreasedOutputs: true, signedTx: { inputs: [{ prev_hash: DCBA }], @@ -816,7 +869,7 @@ export const composeAndSign = [ ], }, }, - composeTransactionCalls: 2, + composeTransactionCalls: 1, signedTx: { // outputs are restored outputs: [ @@ -878,7 +931,7 @@ export const composeAndSign = [ error: 'NOT-ENOUGH-FUNDS', }, }, - composeTransactionCalls: 8, // 1. normal fee, 2. custom fee, 3. send-max normal fee, 4. send-max custom fee + composeTransactionCalls: 4, // 1. normal fee, 2. custom fee, 3. send-max normal fee, 4. send-max custom fee // tx is not signed }, { @@ -943,7 +996,7 @@ export const composeAndSign = [ feePerByte: '15.33', // 11.33 (old) + 4 (new) }, }, - composeTransactionCalls: 2, // 1. immediate send-max + composeTransactionCalls: 1, // 1. immediate send-max decreasedOutputs: 'TR_NOT_ENOUGH_ANONYMIZED_FUNDS_RBF_WARNING', signedTx: { inputs: [{ prev_hash: DCBA }], @@ -1029,7 +1082,7 @@ export const composeAndSign = [ feePerByte: '15.33', // 11.33 (old) + 4 (new) }, }, - composeTransactionCalls: 2, // 1. immediate send-max + composeTransactionCalls: 1, // 1. immediate send-max decreasedOutputs: 'TR_UTXO_REGISTERED_IN_COINJOIN_RBF_WARNING', signedTx: { inputs: [{ prev_hash: DCBA }], diff --git a/packages/suite/src/hooks/wallet/__tests__/useRbfForm.test.tsx b/packages/suite/src/hooks/wallet/__tests__/useRbfForm.test.tsx index 657595f8bb9..a09c036d578 100644 --- a/packages/suite/src/hooks/wallet/__tests__/useRbfForm.test.tsx +++ b/packages/suite/src/hooks/wallet/__tests__/useRbfForm.test.tsx @@ -2,7 +2,6 @@ import { screen } from '@testing-library/react'; import TrezorConnect from '@trezor/connect'; import { configureMockStore, initPreloadedState } from '@suite-common/test-utils'; -import { SelectedAccountLoaded, RbfTransactionParams } from '@suite-common/wallet-types'; import { renderWithProviders, @@ -14,7 +13,7 @@ import { ChangeFee } from 'src/components/suite/modals/ReduxModal/UserContextMod import { ReplaceTxButton } from 'src/components/suite/modals/ReduxModal/UserContextModal/TxDetailModal/ChangeFee/ReplaceTxButton'; import * as fixtures from '../__fixtures__/useRbfForm'; -import { useRbfContext } from '../useRbfForm'; +import { RbfContext, useRbf, useRbfContext } from '../useRbfForm'; // do not mock jest.unmock('@trezor/connect'); @@ -39,22 +38,30 @@ jest.mock('@trezor/blockchain-link', () => ({ default: class BlockchainLink { name = 'jest-mocked-module'; listeners: Record void> = {}; + constructor(args: any) { this.name = args.name; } + on(...args: any[]) { const [type, fn] = args; this.listeners[type] = fn; } + listenerCount() { return 0; } + connect() { return true; } + disconnect() {} + removeAllListeners() {} + dispose() {} + getInfo() { return { url: this, @@ -66,6 +73,7 @@ jest.mock('@trezor/blockchain-link', () => ({ blockHash: 'abcd', }; } + estimateFee(params: { blocks: number[] }) { return params.blocks.map(() => ({ feePerUnit: '-1' })); } @@ -73,6 +81,7 @@ jest.mock('@trezor/blockchain-link', () => ({ })); type RootReducerState = ReturnType>; + interface Args { send?: Partial; fees?: any; @@ -97,6 +106,7 @@ const initStore = ({ send, fees, selectedAccount, coinjoin }: Args = {}) => { interface TestCallback { getContextValues?: () => any; } + // component rendered inside of SendIndex // callback prop is an object passed from single test case // getContextValues returns actual state of SendFormContext @@ -130,17 +140,25 @@ describe('useRbfForm hook', () => { it(`composeAndSign: ${f.description}`, async () => { const store = initStore(f.store); const callback: TestCallback = {}; - const { unmount } = renderWithProviders( - store, - // @ts-expect-error f.tx is not exact - {}}> - - - , - ); + + const TestComponent = () => { + const contextValues = useRbf({ + rbfParams: f.tx.rbfParams, + chainedTxs: f.chainedTxs, + selectedAccount: f.store.selectedAccount, + }); + + return ( + + {}}> + + + + + ); + }; + + const { unmount } = renderWithProviders(store, ); const composeTransactionSpy = jest.spyOn(TrezorConnect, 'composeTransaction'); @@ -165,13 +183,11 @@ describe('useRbfForm hook', () => { expect(composedLevels).toMatchObject(f.composedLevels); // validate number of calls to '@trezor/connect' - if (typeof f.composeTransactionCalls === 'number') { - expect(composeTransactionSpy).toHaveBeenCalledTimes(f.composeTransactionCalls); - } + expect(composeTransactionSpy).toHaveBeenCalledTimes(f.composeTransactionCalls); - if (f.decreasedOutputs) { + if (f.decreasedOutputs !== undefined) { if (typeof f.decreasedOutputs === 'string') { - expect(() => screen.getByText(f.decreasedOutputs)).not.toThrow(); + expect(() => screen.getByText(f.decreasedOutputs as string)).not.toThrow(); } else { expect(() => findByTestId('@send/decreased-outputs')).not.toThrow(); } diff --git a/suite-common/wallet-types/src/transaction.ts b/suite-common/wallet-types/src/transaction.ts index 84e12810b44..b12313b90d0 100644 --- a/suite-common/wallet-types/src/transaction.ts +++ b/suite-common/wallet-types/src/transaction.ts @@ -15,6 +15,7 @@ import { } from '@trezor/connect'; import { Network, NetworkSymbol } from '@suite-common/wallet-config'; import { TranslationKey } from '@suite-common/intl-types'; +import { RequiredKey } from '@trezor/type-utils'; import { Account } from './account'; @@ -185,6 +186,11 @@ export interface WalletAccountTransaction extends AccountTransaction { deadline?: number; } +export type WalletAccountTransactionWithRequiredRbfParams = RequiredKey< + WalletAccountTransaction, + 'rbfParams' +>; + export interface ChainedTransactions { own: WalletAccountTransaction[]; others: WalletAccountTransaction[];