Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,7 @@ jest.mock('../../../../../selectors/multichain', () => ({
},
],
})),
selectAccountTokensAcrossChainsUnified: jest.fn(() => []),
selectMultichainAssetsRates: jest.fn(() => ({})),
}));

Expand Down Expand Up @@ -1230,6 +1231,7 @@ describe('EarnInputView', () => {

// Should navigate to redesigned confirmations instead of legacy lending deposit confirmation
expect(mockNavigate).toHaveBeenCalledWith('StakeScreens', {
params: { loader: 'default' },
screen: Routes.FULL_SCREEN_CONFIRMATIONS.REDESIGNED_CONFIRMATIONS,
});

Expand Down
14 changes: 10 additions & 4 deletions app/components/UI/Earn/Views/EarnInputView/EarnInputView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,8 @@ import { trace, TraceName } from '../../../../../util/trace';
import { useEndTraceOnMount } from '../../../../hooks/useEndTraceOnMount';
import { EVM_SCOPE } from '../../constants/networks';
import { selectConfirmationRedesignFlags } from '../../../../../selectors/featureFlagController/confirmations';
import { ConfirmationLoader } from '../../../../Views/confirmations/components/confirm/confirm-component';
import { useConfirmNavigation } from '../../../../Views/confirmations/hooks/useConfirmNavigation';
///: BEGIN:ONLY_INCLUDE_IF(tron)
import useTronStake from '../../hooks/useTronStake';
import useTronStakeApy from '../../hooks/useTronStakeApy';
Expand All @@ -86,6 +88,7 @@ const EarnInputView = () => {
// navigation hooks
const navigation = useNavigation();
const route = useRoute<EarnInputViewProps['route']>();
const { navigateToConfirmation } = useConfirmNavigation();
const { token } = route.params;

// We want to keep track of the last quick amount pressed before navigating to review.
Expand Down Expand Up @@ -434,17 +437,19 @@ const EarnInputView = () => {
type: TransactionType.lendingDeposit,
};

// Navigate first so the loader shows while the transaction batch is being added
navigateToConfirmation({
loader: ConfirmationLoader.Default,
stack: 'StakeScreens',
});

addTransactionBatch({
from: (selectedAccount?.address as Hex) || '0x',
networkClientId,
origin: ORIGIN_METAMASK,
transactions: [approveTx, lendingDepositTx],
requireApproval: true,
});

navigation.navigate('StakeScreens', {
screen: Routes.FULL_SCREEN_CONFIRMATIONS.REDESIGNED_CONFIRMATIONS,
});
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unawaited async call after navigation

Medium Severity

The addTransactionBatch function is called without await after navigating to the confirmation screen. Since addTransactionBatch is an async function that returns a Promise, any errors from the transaction batch creation will be unhandled. If the batch fails to be added (validation error, network issue), the user will already be on the confirmation screen with no transaction to confirm. Other uses of addTransactionBatch in the codebase (e.g., PredictController.ts) properly await the result.

Fix in Cursor Fix in Web

};

const createLegacyLendingDepositConfirmation = (
Expand Down Expand Up @@ -491,6 +496,7 @@ const EarnInputView = () => {
amountTokenMinimalUnit,
networkClientId,
navigation,
navigateToConfirmation,
token,
amountFiatNumber,
annualRewardsToken,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -405,7 +405,7 @@ describe('EarnLendingWithdrawalConfirmationView', () => {
);
});

it('handles token import and confirmation via subscription listener when no earnToken is present', async () => {
it('executes withdrawal transaction when no earnToken is present', async () => {
// Update the mock to return no earnToken
(useEarnToken as jest.Mock).mockReturnValueOnce({
earnTokenPair: {
Expand All @@ -426,37 +426,10 @@ describe('EarnLendingWithdrawalConfirmationView', () => {
CONFIRMATION_FOOTER_BUTTON_TEST_IDS.CONFIRM_BUTTON,
);

// Mock the subscription callback
let subscriptionCallback:
| ((event: { transaction: { hash: string; status: string } }) => void)
| undefined;
(
Engine.controllerMessenger.subscribeOnceIf as jest.Mock
).mockImplementation((event, callback) => {
if (event === 'TransactionController:transactionConfirmed') {
subscriptionCallback = callback;
}
return () => {
// Cleanup function
};
});

await act(async () => {
fireEvent.press(footerConfirmationButton);
});

// Simulate transaction submission
await act(async () => {
if (subscriptionCallback) {
subscriptionCallback({
transaction: {
hash: '0x123',
status: 'submitted',
},
});
}
});

// Verify the transaction was executed
expect(
Engine.context.EarnController.executeLendingWithdraw,
Expand All @@ -475,100 +448,9 @@ describe('EarnLendingWithdrawalConfirmationView', () => {
},
underlyingTokenAddress: '0x176211869ca2b568f2a7d4ee941e073a821ee1ff',
});

expect(Engine.context.TokensController.addToken).toHaveBeenCalledTimes(1);
});

it('should handle error adding counter-token on confirmation', async () => {
// Update the mock to return no earnToken
(useEarnToken as jest.Mock).mockReturnValueOnce({
earnTokenPair: {
earnToken: null,
outputToken: mockLineaAUsdc,
},
getTokenSnapshot: jest.fn(),
});

(
Engine.context.NetworkController.findNetworkClientIdByChainId as jest.Mock
).mockImplementationOnce(() => {
throw new Error('Invalid chain ID');
});

const consoleErrorSpy = jest
.spyOn(console, 'error')
.mockImplementation(() => {
// Do nothing
});

const { getByTestId } = renderWithProvider(
<EarnLendingWithdrawalConfirmationView />,
{
state: mockInitialState,
},
);

const footerConfirmationButton = getByTestId(
CONFIRMATION_FOOTER_BUTTON_TEST_IDS.CONFIRM_BUTTON,
);

// Mock the subscription callback
let subscriptionCallback:
| ((event: { transaction: { hash: string; status: string } }) => void)
| undefined;
(
Engine.controllerMessenger.subscribeOnceIf as jest.Mock
).mockImplementation((event, callback) => {
if (event === 'TransactionController:transactionConfirmed') {
subscriptionCallback = callback;
}
return () => {
// Cleanup function
};
});

await act(async () => {
fireEvent.press(footerConfirmationButton);
});

// Simulate transaction submission
await act(async () => {
if (subscriptionCallback) {
subscriptionCallback({
transaction: {
hash: '0x123',
status: 'submitted',
},
});
}
});

// Verify the transaction was executed
expect(
Engine.context.EarnController.executeLendingWithdraw,
).toHaveBeenCalledWith({
amount: '1000000',
chainId: '0xe708',
gasOptions: {
gasLimit: 'none',
},
protocol: LendingProtocol.AAVE,
txOptions: {
deviceConfirmedOn: 'metamask_mobile',
networkClientId: 'linea-mainnet',
origin: 'metamask',
type: 'lendingWithdraw',
},
underlyingTokenAddress: '0x176211869ca2b568f2a7d4ee941e073a821ee1ff',
});

expect(consoleErrorSpy).toHaveBeenCalledTimes(1);

// Clean up the spy
consoleErrorSpy.mockRestore();
});

it('should use MaxUint256 when amountTokenMinimalUnit equals balanceMinimalUnit', async () => {
it('uses MaxUint256 when amountTokenMinimalUnit equals balanceMinimalUnit', async () => {
// Mock the route params to have amountTokenMinimalUnit equal to balanceMinimalUnit
(useRoute as jest.MockedFunction<typeof useRoute>).mockReturnValue({
...defaultRouteParams,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,7 @@ const EarnLendingWithdrawalConfirmationView = () => {

const [isConfirmButtonDisabled, setIsConfirmButtonDisabled] = useState(false);

const { earnTokenPair, getTokenSnapshot, tokenSnapshot } =
useEarnToken(token);
const { earnTokenPair } = useEarnToken(token);

const earnToken = earnTokenPair?.earnToken;
const outputToken = earnTokenPair?.outputToken;
Expand Down Expand Up @@ -181,15 +180,6 @@ const EarnLendingWithdrawalConfirmationView = () => {
// return parseFloat(healthFactor).toFixed(2);
// };

useEffect(() => {
if (!earnToken) {
getTokenSnapshot(
outputToken?.chainId as Hex,
outputToken?.experience.market?.underlying?.address as Hex,
);
}
}, [outputToken, getTokenSnapshot, earnToken]);

// Needed to get token's network name
const networkConfig =
Engine.context.NetworkController.getNetworkConfigurationByChainId(
Expand Down Expand Up @@ -339,36 +329,8 @@ const EarnLendingWithdrawalConfirmationView = () => {
},
(transactionMeta) => transactionMeta.id === transactionId,
);
Engine.controllerMessenger.subscribeOnceIf(
'TransactionController:transactionConfirmed',
() => {
if (!earnToken) {
try {
const tokenNetworkClientId =
Engine.context.NetworkController.findNetworkClientIdByChainId(
tokenSnapshot?.chainId as Hex,
);
Engine.context.TokensController.addToken({
decimals: tokenSnapshot?.token?.decimals || 0,
symbol: tokenSnapshot?.token?.symbol || '',
address: tokenSnapshot?.token?.address || '',
name: tokenSnapshot?.token?.name || '',
networkClientId: tokenNetworkClientId,
}).catch(console.error);
} catch (error) {
console.error(
error,
`error adding counter-token for ${
outputToken?.symbol || outputToken?.ticker || ''
} on confirmation`,
);
}
}
},
(transactionMeta) => transactionMeta.id === transactionId,
);
},
[emitTxMetaMetric, tokenSnapshot, earnToken, outputToken, navigation],
[emitTxMetaMetric, navigation, outputToken?.chainId],
);

// Guards
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,17 @@
import React from 'react';
import { render } from '@testing-library/react-native';
import EarnTransactionMonitor from './EarnTransactionMonitor';
import { useEarnLendingTransactionStatus } from '../hooks/useEarnLendingTransactionStatus';
import { useMusdConversionStatus } from '../hooks/useMusdConversionStatus';

jest.mock('../hooks/useMusdConversionStatus');
jest.mock('../hooks/useEarnLendingTransactionStatus');

describe('EarnTransactionMonitor', () => {
const mockUseMusdConversionStatus = jest.mocked(useMusdConversionStatus);
const mockUseEarnLendingTransactionStatus = jest.mocked(
useEarnLendingTransactionStatus,
);

beforeEach(() => {
jest.clearAllMocks();
Expand All @@ -28,6 +33,12 @@ describe('EarnTransactionMonitor', () => {
expect(mockUseMusdConversionStatus).toHaveBeenCalledTimes(1);
});

it('calls useEarnLendingTransactionStatus hook', () => {
render(<EarnTransactionMonitor />);

expect(mockUseEarnLendingTransactionStatus).toHaveBeenCalledTimes(1);
});

it('returns null', () => {
const { toJSON } = render(<EarnTransactionMonitor />);

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import React from 'react';
import { useEarnLendingTransactionStatus } from '../hooks/useEarnLendingTransactionStatus';
import { useMusdConversionStatus } from '../hooks/useMusdConversionStatus';

/**
Expand All @@ -11,6 +12,9 @@ const EarnTransactionMonitor: React.FC = () => {
// Enable mUSD conversion status monitoring and toasts
useMusdConversionStatus();

// Enable lending transaction token addition (no toasts)
useEarnLendingTransactionStatus();

// This component doesn't render anything
return null;
};
Expand Down
Loading
Loading