Skip to content

Commit

Permalink
fix: Change visibility of AmountRow in contract interaction (#29131)
Browse files Browse the repository at this point in the history
<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

## **Description**

<!--
Write a short description of the changes included in this pull request,
also include relevant motivation and context. Have in mind the following
questions:
1. What is the reason for the change?
2. What is the improvement/solution?
-->

This PR makes two visibility change on `AmountRow` in transaction
details.

- Regardless of the amount or simulated value, if transaction details is
toggled, it must show `AmountRow`
- Whenever the Amount being sent doesn't match with a 5% buffer what
we're displaying in the "You send" row of simulations UI for contract
interactions, `AmountRow` must be visible.

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/29131?quickstart=1)

## **Related issues**

Fixes: MetaMask/MetaMask-planning#3783

## **Manual testing steps**

### Scenario I
1. Trigger a contract interaction
2. Enable advanced view
3. With this change, you should now see the Amount row on the advanced
view displaying the native asset value that is being sent along with the
transaction.

### Scenario II
1. Trigger a contract interaction with the below payload (this payload
should make the amount being sent don't match "You send" value within
simulations)
2. With this change, you should now see the Amount row on the default
view displaying the native asset value that is being sent along with the
transaction.
`{
to: "0x4805a248c9611c22a43ce956489c9aadb6108433",
value: "0xDE0B6B3A7640000",
data: "0x3158952e",
}`


## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

<!-- [screenshots/recordings] -->

### **After**

<!-- [screenshots/recordings] -->

## **Pre-merge author checklist**

- [X] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [X] I've completed the PR template to the best of my ability
- [x] I’ve included tests if applicable
- [X] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [X] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

## **Pre-merge reviewer checklist**

- [ ] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [ ] I confirm that this PR addresses all acceptance criteria described
in the ticket it closes and includes the necessary testing evidence such
as recordings and or screenshots.
  • Loading branch information
OGPoyraz authored Dec 12, 2024
1 parent 2e8ef02 commit acdf7c6
Show file tree
Hide file tree
Showing 4 changed files with 300 additions and 21 deletions.
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import React from 'react';
import configureMockStore from 'redux-mock-store';
import thunk from 'redux-thunk';
import { SimulationErrorCode } from '@metamask/transaction-controller';
import { Hex } from '@metamask/utils';
import { toHex } from '@metamask/controller-utils';
import {
getMockConfirmState,
getMockConfirmStateForTransaction,
Expand Down Expand Up @@ -44,21 +45,104 @@ describe('<TransactionDetails />', () => {
expect(container).toMatchSnapshot();
});

it('renders component for transaction details with amount', () => {
const simulationDataMock = {
error: { code: SimulationErrorCode.Disabled },
tokenBalanceChanges: [],
};
const contractInteraction = genUnapprovedContractInteractionConfirmation({
simulationData: simulationDataMock,
chainId: CHAIN_IDS.GOERLI,
describe('AmountRow', () => {
describe('should be in the document', () => {
it('when showAdvancedDetails is true', () => {
const contractInteraction =
genUnapprovedContractInteractionConfirmation({
chainId: CHAIN_IDS.GOERLI,
});
const state = getMockConfirmStateForTransaction(contractInteraction, {
metamask: {
preferences: {
showConfirmationAdvancedDetails: true,
},
},
});
const mockStore = configureMockStore(middleware)(state);
const { getByTestId } = renderWithConfirmContextProvider(
<TransactionDetails />,
mockStore,
);
expect(
getByTestId('transaction-details-amount-row'),
).toBeInTheDocument();
});

it('when value and simulated native balance mismatch', () => {
// Transaction value is set to 0x3782dace9d900000 below mock
const simulationDataMock = {
tokenBalanceChanges: [],
nativeBalanceChange: {
difference: '0x1' as Hex,
isDecrease: false,
previousBalance: '0x2' as Hex,
newBalance: '0x1' as Hex,
},
};
const contractInteraction =
genUnapprovedContractInteractionConfirmation({
simulationData: simulationDataMock,
chainId: CHAIN_IDS.GOERLI,
});
const state = getMockConfirmStateForTransaction(contractInteraction, {
metamask: {
preferences: {
// Intentionally setting to false to test the condition
showConfirmationAdvancedDetails: false,
},
},
});
const mockStore = configureMockStore(middleware)(state);
const { getByTestId } = renderWithConfirmContextProvider(
<TransactionDetails />,
mockStore,
);
expect(
getByTestId('transaction-details-amount-row'),
).toBeInTheDocument();
});
});

it('should not be in the document when value and simulated native balance mismatch is within threshold', () => {
// Transaction value is set to 0x3782dace9d900000 below mock
const transactionValueInDecimal = 4000000000000000000;
const transactionValueInHex = toHex(transactionValueInDecimal);
const newBalanceInDecimal = 1;
const newBalanceInHex = toHex(newBalanceInDecimal);
const previousBalanceInDecimal =
transactionValueInDecimal + newBalanceInDecimal;
const previousBalanceInHex = toHex(previousBalanceInDecimal);

const simulationDataMock = {
tokenBalanceChanges: [],
nativeBalanceChange: {
difference: transactionValueInHex,
isDecrease: true,
previousBalance: previousBalanceInHex,
newBalance: newBalanceInHex,
},
};
const contractInteraction = genUnapprovedContractInteractionConfirmation({
simulationData: simulationDataMock,
chainId: CHAIN_IDS.GOERLI,
});
const state = getMockConfirmStateForTransaction(contractInteraction, {
metamask: {
preferences: {
// Intentionally setting to false to test the condition
showConfirmationAdvancedDetails: false,
},
},
});
const mockStore = configureMockStore(middleware)(state);
const { queryByTestId } = renderWithConfirmContextProvider(
<TransactionDetails />,
mockStore,
);
expect(
queryByTestId('transaction-details-amount-row'),
).not.toBeInTheDocument();
});
const state = getMockConfirmStateForTransaction(contractInteraction);
const mockStore = configureMockStore(middleware)(state);
const { getByTestId } = renderWithConfirmContextProvider(
<TransactionDetails />,
mockStore,
);
expect(getByTestId('transaction-details-amount-row')).toBeInTheDocument();
});
});
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { TransactionMeta } from '@metamask/transaction-controller';
import { isValidAddress } from 'ethereumjs-util';
import React from 'react';
import React, { useMemo } from 'react';
import { useSelector } from 'react-redux';
import {
ConfirmInfoRow,
Expand All @@ -20,6 +20,7 @@ import { ConfirmInfoRowCurrency } from '../../../../../../../components/app/conf
import { PRIMARY } from '../../../../../../../helpers/constants/common';
import { useUserPreferencedCurrency } from '../../../../../../../hooks/useUserPreferencedCurrency';
import { HEX_ZERO } from '../constants';
import { hasValueAndNativeBalanceMismatch as checkValueAndNativeBalanceMismatch } from '../../utils';
import { SigningInWithRow } from '../sign-in-with-row/sign-in-with-row';

export const OriginRow = () => {
Expand Down Expand Up @@ -99,9 +100,8 @@ const AmountRow = () => {
const { currency } = useUserPreferencedCurrency(PRIMARY);

const value = currentConfirmation?.txParams?.value;
const simulationData = currentConfirmation?.simulationData;

if (!value || value === HEX_ZERO || !simulationData?.error) {
if (!value || value === HEX_ZERO) {
return null;
}

Expand Down Expand Up @@ -150,6 +150,11 @@ export const TransactionDetails = () => {
const showAdvancedDetails = useSelector(
selectConfirmationAdvancedDetailsOpen,
);
const { currentConfirmation } = useConfirmContext<TransactionMeta>();
const hasValueAndNativeBalanceMismatch = useMemo(
() => checkValueAndNativeBalanceMismatch(currentConfirmation),
[currentConfirmation],
);

return (
<>
Expand All @@ -159,7 +164,9 @@ export const TransactionDetails = () => {
{showAdvancedDetails && <MethodDataRow />}
<SigningInWithRow />
</ConfirmInfoSection>
<AmountRow />
{(showAdvancedDetails || hasValueAndNativeBalanceMismatch) && (
<AmountRow />
)}
<PaymasterRow />
</>
);
Expand Down
107 changes: 106 additions & 1 deletion ui/pages/confirmations/components/confirm/info/utils.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
import { TransactionMeta } from '@metamask/transaction-controller';
import { toHex } from '@metamask/controller-utils';
import { DecodedTransactionDataSource } from '../../../../../../shared/types/transaction-decode';
import { getIsRevokeSetApprovalForAll } from './utils';
import {
getIsRevokeSetApprovalForAll,
hasValueAndNativeBalanceMismatch,
} from './utils';

describe('getIsRevokeSetApprovalForAll', () => {
it('returns false if no data is passed as an argument', () => {
Expand Down Expand Up @@ -36,3 +41,103 @@ describe('getIsRevokeSetApprovalForAll', () => {
expect(actual).toEqual(true);
});
});

describe('hasValueAndNativeBalanceMismatch', () => {
it('returns false when transaction value matches simulated balance change', () => {
const transactionValueInDecimal = 10000000000000000;
const transactionValueInHex = toHex(transactionValueInDecimal);

const transaction = {
txParams: {
value: transactionValueInHex,
},
simulationData: {
nativeBalanceChange: {
difference: transactionValueInHex,
isDecrease: true,
},
},
} as unknown as TransactionMeta;

expect(hasValueAndNativeBalanceMismatch(transaction)).toBe(false);
});

it('returns false when values differ within threshold', () => {
const transactionValueInDecimal = 10000000000000000;
const transactionValueInHex = toHex(transactionValueInDecimal);

const differenceInDecimal = 10400000000000000;
const differenceInHex = toHex(differenceInDecimal);

const transaction = {
txParams: {
value: transactionValueInHex,
},
simulationData: {
nativeBalanceChange: {
difference: differenceInHex,
isDecrease: true,
},
},
} as unknown as TransactionMeta;

expect(hasValueAndNativeBalanceMismatch(transaction)).toBe(false);
});

it('returns true when values differ beyond threshold', () => {
const transactionValueInDecimal = 10000000000000000;
const transactionValueInHex = toHex(transactionValueInDecimal);

const differenceInDecimal = 1000000000;
const muchSmallerDifferenceInHex = toHex(differenceInDecimal);

const transaction = {
txParams: {
value: transactionValueInHex,
},
simulationData: {
nativeBalanceChange: {
difference: muchSmallerDifferenceInHex,
isDecrease: true,
},
},
} as unknown as TransactionMeta;

expect(hasValueAndNativeBalanceMismatch(transaction)).toBe(true);
});

it('returns true when no simulation data is present', () => {
const transactionValueInDecimal = 10000000000000000;
const transactionValueInHex = toHex(transactionValueInDecimal);

const transaction = {
txParams: {
value: transactionValueInHex,
},
} as unknown as TransactionMeta;

expect(hasValueAndNativeBalanceMismatch(transaction)).toBe(true);
});

it('handles case when value is increased in simulation', () => {
const transactionValueInDecimal = 10000000000000000;
const transactionValueInHex = toHex(transactionValueInDecimal);

const differenceInDecimal = 10000000000000000;
const differenceInHex = toHex(differenceInDecimal);

const transaction = {
txParams: {
value: transactionValueInHex,
},
simulationData: {
nativeBalanceChange: {
difference: differenceInHex,
isDecrease: false,
},
},
} as unknown as TransactionMeta;

expect(hasValueAndNativeBalanceMismatch(transaction)).toBe(true);
});
});
83 changes: 83 additions & 0 deletions ui/pages/confirmations/components/confirm/info/utils.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,15 @@
import { TransactionMeta } from '@metamask/transaction-controller';
import type { Hex } from '@metamask/utils';
import { remove0x } from '@metamask/utils';
import { BN } from 'bn.js';
import { DecodedTransactionDataResponse } from '../../../../../../shared/types/transaction-decode';
import {
BackgroundColor,
TextColor,
} from '../../../../../helpers/constants/design-system';

const VALUE_COMPARISON_PERCENT_THRESHOLD = 5;

export function getIsRevokeSetApprovalForAll(
value: DecodedTransactionDataResponse | undefined,
): boolean {
Expand All @@ -27,3 +33,80 @@ export const getAmountColors = (credit?: boolean, debit?: boolean) => {
}
return { color, backgroundColor };
};

/**
* Calculate the absolute percentage change between two values.
*
* @param originalValue - The first value.
* @param newValue - The second value.
* @returns The percentage change from the first value to the second value.
* If the original value is zero and the new value is not, returns 100.
*/
export function getPercentageChange(
originalValue: InstanceType<typeof BN>,
newValue: InstanceType<typeof BN>,
): number {
const precisionFactor = new BN(10).pow(new BN(18));
const originalValuePrecision = originalValue.mul(precisionFactor);
const newValuePrecision = newValue.mul(precisionFactor);

const difference = newValuePrecision.sub(originalValuePrecision);

if (difference.isZero()) {
return 0;
}

if (originalValuePrecision.isZero() && !newValuePrecision.isZero()) {
return 100;
}

return difference.muln(100).div(originalValuePrecision).abs().toNumber();
}

/**
* Determine if the percentage change between two values is within a threshold.
*
* @param originalValue - The original value.
* @param newValue - The new value.
* @param newNegative - Whether the new value is negative.
* @returns Whether the percentage change between the two values is within a threshold.
*/
function percentageChangeWithinThreshold(
originalValue: Hex,
newValue: Hex,
newNegative?: boolean,
): boolean {
const originalValueBN = new BN(remove0x(originalValue), 'hex');
let newValueBN = new BN(remove0x(newValue), 'hex');

if (newNegative) {
newValueBN = newValueBN.neg();
}

return (
getPercentageChange(originalValueBN, newValueBN) <=
VALUE_COMPARISON_PERCENT_THRESHOLD
);
}

/**
* Determine if a transaction has a value and simulation native balance mismatch.
*
* @param transactionMeta - The transaction metadata.
* @returns Whether the transaction has a value and simulation native balance mismatch.
*/
export function hasValueAndNativeBalanceMismatch(
transactionMeta: TransactionMeta,
): boolean {
const value = transactionMeta?.txParams?.value ?? '0x0';
const nativeBalanceChange =
transactionMeta?.simulationData?.nativeBalanceChange;
const simulatedNativeBalanceDifference =
nativeBalanceChange?.difference ?? '0x0';

return !percentageChangeWithinThreshold(
value as Hex,
simulatedNativeBalanceDifference,
nativeBalanceChange?.isDecrease === false,
);
}

0 comments on commit acdf7c6

Please sign in to comment.