Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 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 @@ -4,7 +4,10 @@ import { useUpdateTransactionPayAmount } from './useUpdateTransactionPayAmount';
import { simpleSendTransactionControllerMock } from '../../__mocks__/controllers/transaction-controller-mock';
import { transactionApprovalControllerMock } from '../../__mocks__/controllers/approval-controller-mock';
import { otherControllersMock } from '../../__mocks__/controllers/other-controllers-mock';
import { updateAtomicBatchData } from '../../../../../util/transaction-controller';
import {
updateAtomicBatchData,
updateTransaction,
} from '../../../../../util/transaction-controller';
import {
updateMoneyAccountDepositTokenAmount,
updateMoneyAccountWithdrawTokenAmount,
Expand All @@ -15,11 +18,15 @@ import {
} from '@metamask/transaction-controller';
import { useUpdateTokenAmount } from '../transactions/useUpdateTokenAmount';
import Logger from '../../../../../util/Logger';
import { useTransactionPayRequiredTokens } from './useTransactionPayData';
import { TransactionPayRequiredToken } from '@metamask/transaction-pay-controller';
import { Hex } from '@metamask/utils';

jest.mock('../../../../../util/transaction-controller');
jest.mock('../../../../UI/Money/utils/moneyAccountTransactions');
jest.mock('../transactions/useUpdateTokenAmount');
jest.mock('../../../../../util/Logger');
jest.mock('./useTransactionPayData');

const moneyAccountDepositMeta: Partial<TransactionMeta> = {
type: TransactionType.moneyAccountDeposit,
Expand Down Expand Up @@ -57,13 +64,17 @@ function runHook({

describe('useUpdateTransactionPayAmount', () => {
const updateAtomicBatchDataMock = jest.mocked(updateAtomicBatchData);
const updateTransactionMock = jest.mocked(updateTransaction);
const updateMoneyAccountDepositTokenAmountMock = jest.mocked(
updateMoneyAccountDepositTokenAmount,
);
const updateMoneyAccountWithdrawTokenAmountMock = jest.mocked(
updateMoneyAccountWithdrawTokenAmount,
);
const useUpdateTokenAmountMock = jest.mocked(useUpdateTokenAmount);
const useTransactionPayRequiredTokensMock = jest.mocked(
useTransactionPayRequiredTokens,
);
const updateTokenAmountMock = jest.fn();
const loggerErrorMock = jest.mocked(Logger.error);

Expand All @@ -73,6 +84,7 @@ describe('useUpdateTransactionPayAmount', () => {
useUpdateTokenAmountMock.mockReturnValue({
updateTokenAmount: updateTokenAmountMock,
});
useTransactionPayRequiredTokensMock.mockReturnValue([]);
});

async function flushPromises() {
Expand Down Expand Up @@ -259,4 +271,152 @@ describe('useUpdateTransactionPayAmount', () => {
expect.stringContaining('Failed to prepare Money Account withdraw'),
);
});

describe('syncMoneyAccountDepositRequiredAssets', () => {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Minor, this isn't an exported function, so should be something like updates requires assets amount?

const TOKEN_ADDRESS_MOCK = '0xToken' as Hex;
const existingRequiredAsset = {
address: TOKEN_ADDRESS_MOCK,
amount: '0x0' as Hex,
standard: 'erc20',
};
const moneyAccountDepositMetaWithRequiredAssets = {
...moneyAccountDepositMeta,
requiredAssets: [existingRequiredAsset],
};

beforeEach(() => {
updateMoneyAccountDepositTokenAmountMock.mockResolvedValue([]);
useTransactionPayRequiredTokensMock.mockReturnValue([
{ decimals: 6 } as TransactionPayRequiredToken,
]);
});

it('calls updateTransaction with hex-encoded amount when requiredAssets exist', async () => {
const { result } = runHook({
transactionMeta: moneyAccountDepositMetaWithRequiredAssets,
});

result.current.updateTransactionPayAmount('1');

await flushPromises();

expect(updateTransactionMock).toHaveBeenCalledTimes(1);
expect(updateTransactionMock).toHaveBeenCalledWith(
expect.objectContaining({
requiredAssets: [{ ...existingRequiredAsset, amount: '0xf4240' }],
}),
'Money Account deposit: sync requiredAssets amount',
);
});

it('rounds fractional atomic amounts up before encoding', async () => {
const { result } = runHook({
transactionMeta: moneyAccountDepositMetaWithRequiredAssets,
});

result.current.updateTransactionPayAmount('1.0000005');

await flushPromises();

expect(updateTransactionMock).toHaveBeenCalledWith(
expect.objectContaining({
requiredAssets: [{ ...existingRequiredAsset, amount: '0xf4241' }],
}),
expect.any(String),
);
});

it('does not call updateTransaction when transactionMeta has no requiredAssets', async () => {
const { result } = runHook({ transactionMeta: moneyAccountDepositMeta });

result.current.updateTransactionPayAmount('1');

await flushPromises();

expect(updateTransactionMock).not.toHaveBeenCalled();
});

it('does not call updateTransaction when no required tokens are available', async () => {
useTransactionPayRequiredTokensMock.mockReturnValue([]);

const { result } = runHook({
transactionMeta: moneyAccountDepositMetaWithRequiredAssets,
});

result.current.updateTransactionPayAmount('1');

await flushPromises();

expect(updateTransactionMock).not.toHaveBeenCalled();
});

it('does not call updateTransaction when computed amount matches existing amount', async () => {
const { result } = runHook({
transactionMeta: {
...moneyAccountDepositMeta,
requiredAssets: [{ ...existingRequiredAsset, amount: '0xf4240' }],
},
});

result.current.updateTransactionPayAmount('1');

await flushPromises();

expect(updateTransactionMock).not.toHaveBeenCalled();
});

it('does not run sync logic for non-deposit transaction types', async () => {
updateMoneyAccountWithdrawTokenAmountMock.mockResolvedValue([]);

const { result } = runHook({
transactionMeta: {
...moneyAccountWithdrawMeta,
requiredAssets: [existingRequiredAsset],
},
});

result.current.updateTransactionPayAmount('1');

await flushPromises();

expect(updateTransactionMock).not.toHaveBeenCalled();
});

it('logs an error when updateTransaction throws', async () => {
const error = new Error('updateTransaction failed');
updateTransactionMock.mockImplementation(() => {
throw error;
});

const { result } = runHook({
transactionMeta: moneyAccountDepositMetaWithRequiredAssets,
});

result.current.updateTransactionPayAmount('1');

await flushPromises();

expect(loggerErrorMock).toHaveBeenCalledWith(
error,
'Failed to sync Money Account deposit requiredAssets amount',
);
});

it('still applies money account deposit updates after syncing requiredAssets', async () => {
updateMoneyAccountDepositTokenAmountMock.mockResolvedValue([
{ nestedTransactionIndex: 0, transactionData: '0xaaaa' },
]);

const { result } = runHook({
transactionMeta: moneyAccountDepositMetaWithRequiredAssets,
});

result.current.updateTransactionPayAmount('1');

await flushPromises();

expect(updateTransactionMock).toHaveBeenCalledTimes(1);
expect(updateAtomicBatchDataMock).toHaveBeenCalledTimes(1);
});
});
});
Original file line number Diff line number Diff line change
@@ -1,18 +1,25 @@
import { useCallback } from 'react';
import { BigNumber } from 'bignumber.js';
import { toHex } from '@metamask/controller-utils';
import {
TransactionMeta,
TransactionType,
} from '@metamask/transaction-controller';
import { Hex } from '@metamask/utils';
import { useTransactionMetadataRequest } from '../transactions/useTransactionMetadataRequest';
import { useUpdateTokenAmount } from '../transactions/useUpdateTokenAmount';
import { updateAtomicBatchData } from '../../../../../util/transaction-controller';
import {
updateAtomicBatchData,
updateTransaction,
} from '../../../../../util/transaction-controller';
import {
updateMoneyAccountDepositTokenAmount,
updateMoneyAccountWithdrawTokenAmount,
} from '../../../../UI/Money/utils/moneyAccountTransactions';
import { UpdateTransactionPayAmountCall } from '../../types/transactions';
import { hasTransactionType } from '../../utils/transaction';
import Logger from '../../../../../util/Logger';
import { useTransactionPayRequiredTokens } from './useTransactionPayData';

type MoneyAccountAmountUpdater = (
transactionMeta: TransactionMeta,
Expand All @@ -22,6 +29,7 @@ type MoneyAccountAmountUpdater = (
export function useUpdateTransactionPayAmount() {
const transactionMeta = useTransactionMetadataRequest();
const { updateTokenAmount } = useUpdateTokenAmount();
const requiredTokens = useTransactionPayRequiredTokens();

const applyMoneyAccountAmountUpdates = useCallback(
async (
Expand Down Expand Up @@ -72,6 +80,11 @@ export function useUpdateTransactionPayAmount() {
TransactionType.moneyAccountDeposit,
])
) {
syncMoneyAccountDepositRequiredAssets(
transactionMeta,
amountHuman,
requiredTokens?.[0]?.decimals,
);
await applyMoneyAccountAmountUpdates(
amountHuman,
updateMoneyAccountDepositTokenAmount,
Expand All @@ -95,8 +108,45 @@ export function useUpdateTransactionPayAmount() {

updateTokenAmount(amountHuman);
},
[transactionMeta, applyMoneyAccountAmountUpdates, updateTokenAmount],
[
transactionMeta,
applyMoneyAccountAmountUpdates,
updateTokenAmount,
requiredTokens,
],
);

return { updateTransactionPayAmount };
}

function syncMoneyAccountDepositRequiredAssets(
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Minor, this is a more general updateRequiredAssetsAmount for any type to use if needed?

transactionMeta: TransactionMeta,
amountHuman: string,
decimals: number | undefined,
): void {
const existing = transactionMeta.requiredAssets;
if (!existing?.length || decimals === undefined) return;

try {
const amount = toHex(
new BigNumber(amountHuman)
.shiftedBy(decimals)
.decimalPlaces(0, BigNumber.ROUND_UP)
.toFixed(0),
) as Hex;
if (existing[0].amount === amount) return;

updateTransaction(
{
...transactionMeta,
requiredAssets: [{ ...existing[0], amount }, ...existing.slice(1)],
},
'Money Account deposit: sync requiredAssets amount',
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Minor, Update requiredAssets amount as can be generic?

);
} catch (error) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Minor, not sure how this would throw as it's a synchronous state update.

Logger.error(
error as Error,
'Failed to sync Money Account deposit requiredAssets amount',
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Minor, also can be generic.

);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,38 @@ describe('Transaction Pay Utils', () => {

expect(getTokenAddress(transactionMeta)).toBe(TO_MOCK);
});

it('returns first requiredAsset address if no nested transfer and requiredAssets present', () => {
const requiredAssetAddress =
'0xrequiredAssetAddress00000000000000000000' as Hex;
const transactionMeta = {
txParams: {
data: '0x1234',
to: TO_MOCK,
},
requiredAssets: [
{
address: requiredAssetAddress,
amount: '0x1' as Hex,
standard: 'erc20',
},
],
} as TransactionMeta;

expect(getTokenAddress(transactionMeta)).toBe(requiredAssetAddress);
});

it('falls back to txParams.to when requiredAssets is empty', () => {
const transactionMeta = {
txParams: {
data: '0x1234',
to: TO_MOCK,
},
requiredAssets: [],
} as unknown as TransactionMeta;

expect(getTokenAddress(transactionMeta)).toBe(TO_MOCK);
});
});

describe('getAvailableTokens', () => {
Expand Down
5 changes: 5 additions & 0 deletions app/components/Views/confirmations/utils/transaction-pay.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,11 @@ export function getTokenAddress(
return nestedCall.to;
}

const requiredAssetAddress = transactionMeta?.requiredAssets?.[0]?.address;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Minor, if we have requiredAssets set, that should probably take priority over a found token transfer call above?

if (requiredAssetAddress) {
return requiredAssetAddress;
}
Comment thread
jpuri marked this conversation as resolved.

return transactionMeta?.txParams?.to as Hex;
}

Expand Down
Loading