Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: permit signature simulation info #24862

Merged
merged 52 commits into from
Jun 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
52 commits
Select commit Hold shift + click to select a range
161814b
Changes in infor section of permit signature types
jpuri May 28, 2024
9ed12f0
update
jpuri May 28, 2024
df04719
update
jpuri May 28, 2024
fa6fc59
update
jpuri May 28, 2024
c3d5774
update
jpuri May 29, 2024
4e5c7fb
Merge branch 'develop' of github.com:MetaMask/metamask-extension into…
jpuri May 29, 2024
d1f85d9
Display simulation info for permit signatures
jpuri May 29, 2024
737d201
update
jpuri May 29, 2024
18fb2c2
merge
jpuri May 29, 2024
0be218d
update
jpuri May 29, 2024
d6164cf
update
jpuri May 29, 2024
c224117
Update
jpuri May 29, 2024
11dc68b
update
jpuri May 30, 2024
a43bcf9
merge
jpuri May 30, 2024
a75ad4a
Update
jpuri May 30, 2024
0d7bae3
update
jpuri May 30, 2024
9d17164
Merge branch 'permit_sign_info_section' into permit_signature_simulat…
legobeat May 31, 2024
e905920
update
jpuri May 31, 2024
22d2cb9
Merge branch 'develop' of github.com:MetaMask/metamask-extension into…
jpuri May 31, 2024
1f6feb1
update
jpuri May 31, 2024
80c2f99
Merge branch 'permit_sign_info_section' into permit_signature_simulat…
jpuri May 31, 2024
da9f628
Changes in message section for siwe signatures
jpuri Jun 1, 2024
5406eb2
Update
jpuri Jun 1, 2024
7a28886
Merge branch 'permit_signature_simulation_info' of github.com:MetaMas…
jpuri Jun 1, 2024
6cfdd13
update
jpuri Jun 1, 2024
84bceab
Merge branch 'develop' into permit_sign_info_section
jpuri Jun 2, 2024
34ab28d
Merge branch 'permit_sign_info_section' into permit_signature_simulat…
jpuri Jun 2, 2024
7007708
Update
jpuri Jun 2, 2024
ec9b3f1
Merge branch 'permit_sign_info_section' of github.com:MetaMask/metama…
jpuri Jun 2, 2024
d712315
Merge branch 'permit_sign_info_section' into permit_signature_simulat…
jpuri Jun 2, 2024
13ad7c0
update
jpuri Jun 2, 2024
9b6923c
Merge branch 'permit_sign_info_section' into permit_signature_simulat…
jpuri Jun 2, 2024
0d23c47
update
jpuri Jun 2, 2024
b7d39d0
Merge branch 'permit_signature_simulation_info' of github.com:MetaMas…
jpuri Jun 2, 2024
48f49b2
merge
jpuri Jun 4, 2024
afc9219
Merge branch 'permit_sign_info_section' into permit_signature_simulat…
jpuri Jun 4, 2024
bfb99bf
update
jpuri Jun 4, 2024
4bd3ff5
Update
jpuri Jun 4, 2024
23f3050
Merge branch 'develop' into permit_signature_simulation_info
jpuri Jun 4, 2024
04dbe96
Update ui/pages/confirmations/components/confirm/info/typed-sign/perm…
jpuri Jun 6, 2024
2768c2a
Update
jpuri Jun 6, 2024
9320061
Update ui/pages/confirmations/components/confirm/info/typed-sign/perm…
jpuri Jun 7, 2024
0ba6528
Merge branch 'develop' into permit_signature_simulation_info
jpuri Jun 7, 2024
873059a
update
jpuri Jun 7, 2024
82538d4
Merge branch 'permit_signature_simulation_info' of github.com:MetaMas…
jpuri Jun 7, 2024
d11f9d5
update
jpuri Jun 7, 2024
2b64f36
update
jpuri Jun 7, 2024
89083dd
Update
jpuri Jun 11, 2024
5207569
Update
jpuri Jun 11, 2024
1adb2ff
Update
jpuri Jun 11, 2024
966267e
Update
jpuri Jun 11, 2024
df189d4
Merge branch 'develop' into permit_signature_simulation_info
jpuri Jun 12, 2024
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
3 changes: 3 additions & 0 deletions app/_locales/en/messages.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`PermitSimulation renders component correctly 1`] = `
<div>
<div
class="mm-box mm-box--margin-bottom-4 mm-box--padding-2 mm-box--background-color-background-default mm-box--rounded-md"
>
<div
class="mm-box confirm-info-row mm-box--margin-top-2 mm-box--margin-bottom-2 mm-box--padding-right-2 mm-box--padding-left-2 mm-box--display-flex mm-box--flex-direction-row mm-box--flex-wrap-wrap mm-box--justify-content-space-between mm-box--color-text-default mm-box--rounded-lg"
style="overflow-wrap: anywhere; min-height: 24px;"
>
<div
class="mm-box mm-box--display-flex mm-box--flex-direction-row mm-box--justify-content-center mm-box--align-items-center"
>
<p
class="mm-box mm-text mm-text--body-md-medium mm-box--color-inherit"
>
Estimated changes
</p>
<div>
<div
aria-describedby="tippy-tooltip-1"
class=""
data-original-title="Estimated changes are what might happen if you go through with this transaction. This is just a prediction, not a guarantee."
data-tooltipped=""
style="display: flex;"
tabindex="0"
>
<span
class="mm-box mm-icon mm-icon--size-sm mm-box--margin-left-1 mm-box--display-inline-block mm-box--color-icon-muted"
style="mask-image: url('./images/icons/question.svg');"
/>
</div>
</div>
</div>
<div
class="mm-box mm-box--display-flex mm-box--gap-2 mm-box--flex-wrap-wrap mm-box--align-items-center"
>
<p
class="mm-box mm-text mm-text--body-md mm-box--color-inherit"
style="white-space: pre-wrap;"
>
This transaction gives permission to withdraw your tokens
</p>
</div>
</div>
<div
class="mm-box confirm-info-row mm-box--margin-top-2 mm-box--margin-bottom-2 mm-box--padding-right-2 mm-box--padding-left-2 mm-box--display-flex mm-box--flex-direction-row mm-box--flex-wrap-wrap mm-box--justify-content-space-between mm-box--color-text-default mm-box--rounded-lg"
style="overflow-wrap: anywhere; min-height: 24px;"
>
<div
class="mm-box mm-box--display-flex mm-box--flex-direction-row mm-box--justify-content-center mm-box--align-items-center"
>
<p
class="mm-box mm-text mm-text--body-md-medium mm-box--color-inherit"
>
Approve spend limit
</p>
</div>
<div
class="mm-box"
>
<div
class="mm-box mm-box--display-flex"
>
<div
class="mm-box mm-box--margin-inline-end-1 mm-box--display-inline"
>
<p
class="mm-box mm-text mm-text--body-md mm-text--text-align-center mm-box--padding-inline-2 mm-box--color-text-default mm-box--background-color-background-alternative mm-box--rounded-xl"
>
3000
</p>
</div>
<div>
<div
class="name name__missing"
>
<span
class="mm-box name__icon mm-icon mm-icon--size-md mm-box--display-inline-block mm-box--color-inherit"
style="mask-image: url('./images/icons/question.svg');"
/>
<p
class="mm-box mm-text name__value mm-text--body-md mm-box--color-text-default"
>
0xCcCCc...ccccC
</p>
</div>
</div>
</div>
<div
class="mm-box"
/>
</div>
</div>
</div>
</div>
`;
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export { default as PermitSimulation } from './permit-simulation';
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import React from 'react';
import configureMockStore from 'redux-mock-store';

import mockState from '../../../../../../../../test/data/mock-state.json';
import { renderWithProvider } from '../../../../../../../../test/lib/render-helpers';
import { permitSignatureMsg } from '../../../../../../../../test/data/confirmations/typed_sign';
import PermitSimulation from './permit-simulation';

describe('PermitSimulation', () => {
it('renders component correctly', () => {
const state = {
...mockState,
confirm: {
currentConfirmation: permitSignatureMsg,
},
};
const mockStore = configureMockStore([])(state);
const { container } = renderWithProvider(<PermitSimulation />, mockStore);
expect(container).toMatchSnapshot();
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
import React, { useMemo } from 'react';
import { useSelector } from 'react-redux';
import { NameType } from '@metamask/name-controller';

import { Numeric } from '../../../../../../../../shared/modules/Numeric';
import Name from '../../../../../../../components/app/name/name';
import {
ConfirmInfoRow,
ConfirmInfoRowText,
} from '../../../../../../../components/app/confirm/info/row';
import { useI18nContext } from '../../../../../../../hooks/useI18nContext';
import { currentConfirmationSelector } from '../../../../../../../selectors';
import { Box, Text } from '../../../../../../../components/component-library';
import {
BackgroundColor,
BorderRadius,
Display,
TextAlign,
} from '../../../../../../../helpers/constants/design-system';
import { parseTypedDataMessage } from '../../../../../utils';
import { SignatureRequestType } from '../../../../../types/confirm';
import useTokenExchangeRate from '../../../../../../../components/app/currency-input/hooks/useTokenExchangeRate';
import { IndividualFiatDisplay } from '../../../../simulation-details/fiat-display';

const PermitSimulation: React.FC = () => {
const t = useI18nContext();
const currentConfirmation = useSelector(
currentConfirmationSelector,
) as SignatureRequestType;

const {
domain: { verifyingContract },
message: { value },
} = parseTypedDataMessage(currentConfirmation.msgParams?.data as string);

const exchangeRate = useTokenExchangeRate(verifyingContract);

const fiatValue = useMemo(() => {
if (exchangeRate && value) {
return exchangeRate.times(new Numeric(value, 10)).toNumber();
}
return undefined;
}, [exchangeRate, value]);

return (
<Box
backgroundColor={BackgroundColor.backgroundDefault}
borderRadius={BorderRadius.MD}
padding={2}
marginBottom={4}
>
<ConfirmInfoRow
label={t('simulationDetailsTitle')}
tooltip={t('simulationDetailsTitleTooltip')}
Copy link
Contributor

Choose a reason for hiding this comment

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

CleanShot 2024-06-07 at 19 52 59@2x

  1. looks like figma users. "i" icon instead of "?" here
  2. I think we could add padding between the label and text below

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Again these are limitations with ConfirmInfoRow. We need to see if its ok to generically change this for all usages.

>
<ConfirmInfoRowText text={t('permitSimulationDetailInfo')} />
</ConfirmInfoRow>
<ConfirmInfoRow label={t('approve')}>
<Box>
<Box display={Display.Flex}>
<Box display={Display.Inline} marginInlineEnd={1}>
<Text
backgroundColor={BackgroundColor.backgroundAlternative}
borderRadius={BorderRadius.XL}
paddingInline={2}
textAlign={TextAlign.Center}
>
{value}
</Text>
</Box>
<Name value={verifyingContract} type={NameType.ETHEREUM_ADDRESS} />
</Box>
<Box>
{fiatValue && <IndividualFiatDisplay fiatAmount={fiatValue} />}
</Box>
</Box>
</ConfirmInfoRow>
</Box>
);
Copy link
Contributor

Choose a reason for hiding this comment

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

The pill component does not appear to be aligned with the label

CleanShot 2024-06-07 at 19 49 29@2x

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. Confirm info row are center aligned and its hard to change for this specifically

};
Copy link
Contributor

Choose a reason for hiding this comment

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

should the values be right aligned?

CleanShot 2024-06-07 at 19 46 54@2x

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey @digiwand : after certain with confirm row wraps, thus this is how it will look for wider values

Copy link

@bschorchit bschorchit Jun 7, 2024

Choose a reason for hiding this comment

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

Derek had made some improvements on how we display long values within simulations here: #24036

Is this something we could re-use? Ideally, we have the same pattern in both places

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep that can be done, but I would prefer to address that in separate PR.


export default PermitSimulation;
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,18 @@ describe('TypedSignInfo', () => {
expect(container).toMatchSnapshot();
});

it('display simulation details for permit signature', () => {
const state = {
...mockState,
confirm: {
currentConfirmation: permitSignatureMsg,
},
};
const mockStore = configureMockStore([])(state);
const { getByText } = renderWithProvider(<TypedSignInfo />, mockStore);
expect(getByText('Estimated changes')).toBeDefined();
});

it('displays "Approving to" for permit signature type', () => {
const state = {
...mockState,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import { EIP712_PRIMARY_TYPE_PERMIT } from '../../../../constants';
import { SignatureRequestType } from '../../../../types/confirm';
import { parseTypedDataMessage } from '../../../../utils';
import { ConfirmInfoRowTypedSignData } from '../../row/typed-sign-data/typedSignData';
import { PermitSimulation } from './permit-simulation';

const TypedSignInfo: React.FC = () => {
const t = useI18nContext();
Expand All @@ -31,13 +32,14 @@ const TypedSignInfo: React.FC = () => {
}

const {
domain,
domain: { verifyingContract },
primaryType,
message: { spender },
} = parseTypedDataMessage(currentConfirmation.msgParams.data as string);

return (
<>
{primaryType === EIP712_PRIMARY_TYPE_PERMIT && <PermitSimulation />}
<Box
backgroundColor={BackgroundColor.backgroundDefault}
borderRadius={BorderRadius.MD}
Expand All @@ -48,7 +50,7 @@ const TypedSignInfo: React.FC = () => {
<>
<Box padding={2}>
<ConfirmInfoRow label={t('approvingTo')}>
<ConfirmInfoRowAddress address={verifyingContract} />
<ConfirmInfoRowAddress address={spender} />
</ConfirmInfoRow>
</Box>
<ConfirmInfoRowDivider />
Expand All @@ -62,10 +64,10 @@ const TypedSignInfo: React.FC = () => {
<ConfirmInfoRowUrl url={currentConfirmation.msgParams.origin} />
</ConfirmInfoRow>
</Box>
{isValidAddress(domain.verifyingContract) && (
{isValidAddress(verifyingContract) && (
<Box padding={2}>
<ConfirmInfoRow label={t('interactingWith')}>
<ConfirmInfoRowAddress address={domain.verifyingContract} />
<ConfirmInfoRowAddress address={verifyingContract} />
</ConfirmInfoRow>
</Box>
)}
Expand Down