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.

Sign up for GitHub

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(mmi): start implementing mmi ofa feature #24749

Merged
merged 17 commits into from
Jun 19, 2024
Merged

feat(mmi): start implementing mmi ofa feature #24749

Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
f69cc6c
Start implementing OFA feature
albertolive May 23, 2024
0dd3ba7
Merge branch 'develop' into MMI-5059-ofa-feature
albertolive May 30, 2024
c2fb078
Merge branch 'develop' into MMI-5059-ofa-feature
albertolive Jun 4, 2024
30c2b8b
Merge branch 'develop' into MMI-5059-ofa-feature
albertolive Jun 6, 2024
916ebb6
Merge branch 'develop' into MMI-5059-ofa-feature
albertolive Jun 7, 2024
aeda3db
Merge branch 'develop' into MMI-5059-ofa-feature
albertolive Jun 7, 2024
f5faf08
Merge branch 'develop' into MMI-5059-ofa-feature
albertolive Jun 10, 2024
b136fc5
Merge branch 'develop' into MMI-5059-ofa-feature
albertolive Jun 12, 2024
854b7f3
add unit tests
albertolive Jun 13, 2024
3182110
Merge branch 'develop' into MMI-5059-ofa-feature
albertolive Jun 13, 2024
5a1ca02
Merge branch 'develop' into MMI-5059-ofa-feature
albertolive Jun 13, 2024
c252f3f
Merge branch 'develop' into MMI-5059-ofa-feature
albertolive Jun 18, 2024
8eda9ba
Merge branch 'develop' into MMI-5059-ofa-feature
albertolive Jun 18, 2024
84521d5
Merge branch 'develop' into MMI-5059-ofa-feature
albertolive Jun 18, 2024
41decc8
Merge branch 'develop' into MMI-5059-ofa-feature
albertolive Jun 19, 2024
7e0604d
changed the way to know when smart transactions are enabled
albertolive Jun 19, 2024
cf59707
fixing eslint issues
albertolive Jun 19, 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
9 changes: 9 additions & 0 deletions ui/pages/confirmations/confirm-transaction-base/confirm-transaction-base.component.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -825,6 +825,8 @@ export default class ConfirmTransactionBase extends Component {
toAddress,
showCustodianDeepLink,
clearConfirmTransaction,
smartTransactionsOptInStatus,
currentChainSupportsSmartTransactions,
} = this.props;
const { noteText } = this.state;

Expand All @@ -836,6 +838,13 @@ export default class ConfirmTransactionBase extends Component {
txData.metadata.note = noteText;
}

if (
smartTransactionsOptInStatus &&
Copy link
Member

Choose a reason for hiding this comment

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

If we're trying to detect if this is a smart transaction, should we be using the getSmartTransactionsEnabled selector as it also checks the feature flag so if the service is live?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the quick response @matthewwalsh0 ! I appreciate it; I didn't know that it existed. I'll do the changes

currentChainSupportsSmartTransactions
) {
txData.origin += '#smartTransaction';
Copy link
Member

Choose a reason for hiding this comment

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

Why are we mutating an existing property in this instance as opposed to using a new property for example?

Is the origin not traditionally a host name so won't this break that assumption and potentially impact other UX?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@matthewwalsh0, we are using the origin because we don’t want to add any new properties for now, as it would impact our Ethereum custodian API.

Copy link
Member

Choose a reason for hiding this comment

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

As long as you're sure it won't break any of the UI, fair enough given this branch is specific to MMI.

}

txData.metadata.custodianPublishesTransaction =
custodianPublishesTransaction;
txData.metadata.rpcUrl = rpcUrl;
Expand Down
124 changes: 113 additions & 11 deletions ui/pages/confirmations/confirm-transaction-base/confirm-transaction-base.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,9 @@ setBackgroundConnection({
}),
),
getGasFeeTimeEstimate: jest.fn(),
promisifiedBackground: jest.fn(),
tryReverseResolveAddress: jest.fn(),
getNextNonce: jest.fn(),
updateTransaction: jest.fn(),
});

const mockTxParamsFromAddress = '0x123456789';
Expand Down Expand Up @@ -470,14 +470,12 @@ describe('Confirm Transaction Base', () => {
const sendTransaction = jest
.fn()
.mockResolvedValue(state.confirmTransaction.txData);
const updateTransaction = jest.fn().mockResolvedValue();
const updateTransactionValue = jest.fn().mockResolvedValue();
const showCustodianDeepLink = jest.fn();
const setWaitForConfirmDeepLinkDialog = jest.fn();

const props = {
sendTransaction,
updateTransaction,
updateTransactionValue,
showCustodianDeepLink,
setWaitForConfirmDeepLinkDialog,
Expand All @@ -497,12 +495,19 @@ describe('Confirm Transaction Base', () => {
expect(sendTransaction).toHaveBeenCalled();
});

it('handleMainSubmit calls sendTransaction correctly', async () => {
it('handleMMISubmit calls sendTransaction correctly and then showCustodianDeepLink', async () => {
const state = {
appState: {
...baseStore.appState,
gasLoadingAnimationIsShowing: false,
},
confirmTransaction: {
...baseStore.confirmTransaction,
txData: {
...baseStore.confirmTransaction.txData,
custodyStatus: true,
},
},
metamask: {
...baseStore.metamask,
accounts: {
Expand All @@ -516,7 +521,9 @@ describe('Confirm Transaction Base', () => {
networksMetadata: {
...baseStore.metamask.networksMetadata,
[NetworkType.mainnet]: {
EIPS: { 1559: true },
EIPS: {
1559: true,
},
status: NetworkStatus.Available,
},
},
Expand All @@ -525,6 +532,47 @@ describe('Confirm Transaction Base', () => {
gasPrice: '0x59682f00',
},
noGasPrice: false,
keyrings: [
{
type: 'Custody',
accounts: [mockTxParamsFromAddress],
},
],
custodyAccountDetails: {
[mockTxParamsFromAddress]: {
address: mockTxParamsFromAddress,
details: 'details',
custodyType: 'testCustody - Saturn',
custodianName: 'saturn-dev',
},
},
mmiConfiguration: {
custodians: [
{
envName: 'saturn-dev',
displayName: 'Saturn Custody',
isNoteToTraderSupported: false,
},
],
},
internalAccounts: {
accounts: {
'cf8dace4-9439-4bd4-b3a8-88c821c8fcb3': {
address: mockTxParamsFromAddress,
id: 'cf8dace4-9439-4bd4-b3a8-88c821c8fcb3',
metadata: {
name: 'Custody Account A',
keyring: {
type: 'Custody',
},
},
options: {},
methods: ETH_EOA_METHODS,
type: EthAccountType.Eoa,
},
},
selectedAccount: 'cf8dace4-9439-4bd4-b3a8-88c821c8fcb3',
},
},
send: {
...baseStore.send,
Expand All @@ -546,12 +594,19 @@ describe('Confirm Transaction Base', () => {
},
};

const sendTransaction = jest.fn().mockResolvedValue();
const sendTransaction = jest
.fn()
.mockResolvedValue(state.confirmTransaction.txData);
const showCustodianDeepLink = jest.fn();
const setWaitForConfirmDeepLinkDialog = jest.fn();

const props = {
sendTransaction,
showCustodianDeepLink,
setWaitForConfirmDeepLinkDialog,
toAddress: mockPropsToAddress,
toAccounts: [{ address: mockPropsToAddress }],
isMainBetaFlask: false,
};

const { getByTestId } = await render({ props, state });
Expand All @@ -560,10 +615,12 @@ describe('Confirm Transaction Base', () => {
await act(async () => {
fireEvent.click(confirmButton);
});
expect(sendTransaction).toHaveBeenCalled();
expect(setWaitForConfirmDeepLinkDialog).toHaveBeenCalled();
await expect(sendTransaction).toHaveBeenCalled();
expect(showCustodianDeepLink).toHaveBeenCalled();
});

it('handleMMISubmit calls sendTransaction correctly and then showCustodianDeepLink', async () => {
it('should append #smartTransaction to txData.origin when smartTransactionsOptInStatus and currentChainSupportsSmartTransactions are true', async () => {
const state = {
appState: {
...baseStore.appState,
Expand All @@ -574,6 +631,7 @@ describe('Confirm Transaction Base', () => {
txData: {
...baseStore.confirmTransaction.txData,
custodyStatus: true,
origin: 'metamask#smartTransaction',
},
},
metamask: {
Expand All @@ -600,6 +658,47 @@ describe('Confirm Transaction Base', () => {
gasPrice: '0x59682f00',
},
noGasPrice: false,
keyrings: [
{
type: 'Custody',
accounts: [mockTxParamsFromAddress],
},
],
custodyAccountDetails: {
[mockTxParamsFromAddress]: {
address: mockTxParamsFromAddress,
details: 'details',
custodyType: 'testCustody - Saturn',
custodianName: 'saturn-dev',
},
},
mmiConfiguration: {
custodians: [
{
envName: 'saturn-dev',
displayName: 'Saturn Custody',
isNoteToTraderSupported: false,
},
],
},
internalAccounts: {
accounts: {
'cf8dace4-9439-4bd4-b3a8-88c821c8fcb3': {
address: mockTxParamsFromAddress,
id: 'cf8dace4-9439-4bd4-b3a8-88c821c8fcb3',
metadata: {
name: 'Custody Account A',
keyring: {
type: 'Custody',
},
},
options: {},
methods: ETH_EOA_METHODS,
type: EthAccountType.Eoa,
},
},
selectedAccount: 'cf8dace4-9439-4bd4-b3a8-88c821c8fcb3',
},
},
send: {
...baseStore.send,
Expand Down Expand Up @@ -642,9 +741,12 @@ describe('Confirm Transaction Base', () => {
await act(async () => {
fireEvent.click(confirmButton);
});
expect(setWaitForConfirmDeepLinkDialog).toHaveBeenCalled();
await expect(sendTransaction).toHaveBeenCalled();
expect(showCustodianDeepLink).toHaveBeenCalled();

expect(sendTransaction).toHaveBeenCalledWith(
expect.objectContaining({
origin: 'metamask#smartTransaction',
}),
);
});

describe('when rendering the recipient value', () => {
Expand Down
Loading