Skip to content

Commit

Permalink
feat: Use a dynamic interval value for smart transaction status polli…
Browse files Browse the repository at this point in the history
…ng (#29703)

## **Description**

We want to use a dynamic interval value for smart transaction status
polling, which is returned by backend. That way we can easily change it
if needed and use the most optimal value.

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

## **Related issues**

Fixes:

## **Manual testing steps**

1. Make sure smart transactions are enabled in Advanced Settings
2. Be on Ethereum Mainnet
3. Submit a smart transaction
4. Check that a network request for checking smart transaction status
happens every second

## **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**

- [ ] 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).
- [ ] I've completed the PR template to the best of my ability
- [ ] I’ve included tests if applicable
- [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [ ] 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
dan437 authored Jan 16, 2025
1 parent 77243cb commit 6b765ba
Show file tree
Hide file tree
Showing 8 changed files with 191 additions and 4 deletions.
5 changes: 3 additions & 2 deletions ui/ducks/swaps/swaps.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ import {
hexWEIToDecGWEI,
} from '../../../shared/modules/conversion.utils';
import { getCurrentChainId } from '../../../shared/modules/selectors/networks';
import { getFeatureFlagsByChainId } from '../../../shared/modules/selectors/feature-flags';
import {
getSelectedAccount,
getTokenExchangeRates,
Expand Down Expand Up @@ -896,12 +897,12 @@ export const signAndSendSwapsSmartTransaction = ({
const { metaData, value: swapTokenValue, slippage } = fetchParams;
const { sourceTokenInfo = {}, destinationTokenInfo = {} } = metaData;
const usedQuote = getUsedQuote(state);
const swapsNetworkConfig = getSwapsNetworkConfig(state);
const selectedNetwork = getSelectedNetwork(state);
const swapsFeatureFlags = getFeatureFlagsByChainId(state);

dispatch(
setSmartTransactionsRefreshInterval(
swapsNetworkConfig?.stxBatchStatusRefreshTime,
swapsFeatureFlags?.smartTransactions?.batchStatusPollingInterval,
),
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,7 @@ export default class ConfirmTransactionBase extends Component {
isSmartTransactionsEnabled: PropTypes.bool,
hasPriorityApprovalRequest: PropTypes.bool,
chainId: PropTypes.string,
setSmartTransactionsRefreshInterval: PropTypes.func,
};

state = {
Expand Down Expand Up @@ -1023,6 +1024,7 @@ export default class ConfirmTransactionBase extends Component {
currentChainSupportsSmartTransactions,
setSwapsFeatureFlags,
fetchSmartTransactionsLiveness,
setSmartTransactionsRefreshInterval,
} = this.props;

const { trackEvent } = this.context;
Expand Down Expand Up @@ -1071,7 +1073,12 @@ export default class ConfirmTransactionBase extends Component {
Promise.all([
fetchSwapsFeatureFlags(),
fetchSmartTransactionsLiveness(),
]).then(([swapsFeatureFlags]) => setSwapsFeatureFlags(swapsFeatureFlags));
]).then(([swapsFeatureFlags]) => {
setSwapsFeatureFlags(swapsFeatureFlags);
setSmartTransactionsRefreshInterval(
swapsFeatureFlags?.smartTransactions?.batchStatusPollingInterval,
);
});
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import {
setSwapsFeatureFlags,
fetchSmartTransactionsLiveness,
setNextNonce,
setSmartTransactionsRefreshInterval,
} from '../../../store/actions';
import { isBalanceSufficient } from '../send/send.utils';
import { shortenAddress, valuesFor } from '../../../helpers/utils/util';
Expand Down Expand Up @@ -486,6 +487,8 @@ export const mapDispatchToProps = (dispatch) => {
setWaitForConfirmDeepLinkDialog: (wait) =>
dispatch(mmiActions.setWaitForConfirmDeepLinkDialog(wait)),
///: END:ONLY_INCLUDE_IF
setSmartTransactionsRefreshInterval: (interval) =>
dispatch(setSmartTransactionsRefreshInterval(interval)),
};
};

Expand Down
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 { fireEvent } from '@testing-library/react';
import { fireEvent, waitFor } from '@testing-library/react';
import nock from 'nock';
import { EthAccountType } from '@metamask/keyring-api';
import {
TransactionStatus,
Expand All @@ -23,6 +24,7 @@ import {
import { defaultBuyableChains } from '../../../ducks/ramps/constants';
import { ETH_EOA_METHODS } from '../../../../shared/constants/eth-methods';
import { mockNetworkState } from '../../../../test/stub/networks';
import { setStorageItem } from '../../../../shared/lib/storage-helpers';
import ConfirmTransactionBase from './confirm-transaction-base.container';

jest.mock('../components/simulation-details/useSimulationMetrics');
Expand All @@ -43,6 +45,7 @@ setBackgroundConnection({
updateTransaction: jest.fn(),
getLastInteractedConfirmationInfo: jest.fn(),
setAlertEnabledness: jest.fn(),
setSwapsFeatureFlags: jest.fn(),
});

const mockTxParamsFromAddress = '0x123456789';
Expand Down Expand Up @@ -1032,4 +1035,76 @@ describe('Confirm Transaction Base', () => {
expect(updateTransactionValue).toHaveBeenCalledTimes(0);
});
});

describe('Smart Transactions Refresh Interval', () => {
const cleanFeatureFlagApiCache = () => {
setStorageItem(
'cachedFetch:https://swap.api.cx.metamask.io/featureFlags',
null,
);
};

beforeEach(() => {
cleanFeatureFlagApiCache();
nock.cleanAll();
});

it('calls setSmartTransactionsRefreshInterval when smart transactions are enabled', async () => {
nock('https://swap.api.cx.metamask.io')
.get('/featureFlags')
.reply(200, {
smartTransactions: {
batchStatusPollingInterval: 1000,
},
});

const state = {
...baseStore,
metamask: {
...baseStore.metamask,
smartTransactionsState: {
fees: {},
liveness: true,
},
},
};

const setSmartTransactionsRefreshInterval = jest.fn();
const props = {
setSmartTransactionsRefreshInterval,
smartTransactionsPreferenceEnabled: true,
currentChainSupportsSmartTransactions: true,
};

await render({ props, state });

await waitFor(() => {
expect(setSmartTransactionsRefreshInterval).toHaveBeenCalledWith(1000);
});
});

it('does not call setSmartTransactionsRefreshInterval when smart transactions are disabled', async () => {
const state = {
...baseStore,
metamask: {
...baseStore.metamask,
smartTransactionsState: {
fees: {},
liveness: false,
},
},
};

const setSmartTransactionsRefreshInterval = jest.fn();
const props = {
setSmartTransactionsRefreshInterval,
smartTransactionsPreferenceEnabled: false,
currentChainSupportsSmartTransactions: false,
};

await render({ props, state });

expect(setSmartTransactionsRefreshInterval).not.toHaveBeenCalled();
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { Hex } from '@metamask/utils';
import {
fetchSmartTransactionsLiveness,
setSwapsFeatureFlags,
setSmartTransactionsRefreshInterval,
} from '../../../store/actions';
import { renderHookWithConfirmContextProvider } from '../../../../test/lib/confirmations/render-helpers';
import { genUnapprovedContractInteractionConfirmation } from '../../../../test/data/confirmations/contract-interaction';
Expand All @@ -22,6 +23,7 @@ jest.mock('../../../store/actions', () => ({
...jest.requireActual('../../../store/actions'),
setSwapsFeatureFlags: jest.fn(),
fetchSmartTransactionsLiveness: jest.fn(),
setSmartTransactionsRefreshInterval: jest.fn(),
}));

jest.mock('../../swaps/swaps.util', () => ({
Expand Down Expand Up @@ -66,6 +68,9 @@ async function runHook({

describe('useSmartTransactionFeatureFlags', () => {
const setSwapsFeatureFlagsMock = jest.mocked(setSwapsFeatureFlags);
const setSmartTransactionsRefreshIntervalMock = jest.mocked(
setSmartTransactionsRefreshInterval,
);
const fetchSwapsFeatureFlagsMock = jest.mocked(fetchSwapsFeatureFlags);
const fetchSmartTransactionsLivenessMock = jest.mocked(
fetchSmartTransactionsLiveness,
Expand Down Expand Up @@ -116,4 +121,36 @@ describe('useSmartTransactionFeatureFlags', () => {

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

it('updates refresh interval when feature flags include interval', async () => {
fetchSwapsFeatureFlagsMock.mockResolvedValue({
smartTransactions: {
batchStatusPollingInterval: 1000,
},
});

await runHook({
smartTransactionsOptInStatus: true,
chainId: CHAIN_IDS.MAINNET,
});

expect(setSmartTransactionsRefreshIntervalMock).toHaveBeenCalledTimes(1);
expect(setSmartTransactionsRefreshIntervalMock).toHaveBeenCalledWith(1000);
});

it('does not update refresh interval when feature flags do not include interval', async () => {
fetchSwapsFeatureFlagsMock.mockResolvedValue({
smartTransactions: {},
});

await runHook({
smartTransactionsOptInStatus: true,
chainId: CHAIN_IDS.MAINNET,
});

expect(setSmartTransactionsRefreshIntervalMock).toHaveBeenCalledTimes(1);
expect(setSmartTransactionsRefreshIntervalMock).toHaveBeenCalledWith(
undefined,
);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { fetchSwapsFeatureFlags } from '../../swaps/swaps.util';
import {
fetchSmartTransactionsLiveness,
setSwapsFeatureFlags,
setSmartTransactionsRefreshInterval,
} from '../../../store/actions';
import { useConfirmContext } from '../context/confirm';

Expand Down Expand Up @@ -40,6 +41,11 @@ export function useSmartTransactionFeatureFlags() {
Promise.all([fetchSwapsFeatureFlags(), fetchSmartTransactionsLiveness()()])
.then(([swapsFeatureFlags]) => {
dispatch(setSwapsFeatureFlags(swapsFeatureFlags));
dispatch(
setSmartTransactionsRefreshInterval(
swapsFeatureFlags.smartTransactions?.batchStatusPollingInterval,
),
);
})
.catch((error) => {
log.debug('Error updating smart transaction feature flags', error);
Expand Down
55 changes: 55 additions & 0 deletions ui/store/actions.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2734,4 +2734,59 @@ describe('Actions', () => {
expect(store.getActions()).toStrictEqual([]);
});
});

describe('setSmartTransactionsRefreshInterval', () => {
afterEach(() => {
sinon.restore();
});

it('calls setStatusRefreshInterval in the background with provided interval', async () => {
const store = mockStore();
const refreshInterval = 1000;

background = {
setStatusRefreshInterval: sinon.stub().callsFake((_, cb) => cb()),
};
setBackgroundConnection(background);

await store.dispatch(
actions.setSmartTransactionsRefreshInterval(refreshInterval),
);

expect(
background.setStatusRefreshInterval.calledWith(
refreshInterval,
sinon.match.func,
),
).toBe(true);
});

it('does not call background if refresh interval is undefined', async () => {
const store = mockStore();

background = {
setStatusRefreshInterval: sinon.stub().callsFake((_, cb) => cb()),
};
setBackgroundConnection(background);

await store.dispatch(
actions.setSmartTransactionsRefreshInterval(undefined),
);

expect(background.setStatusRefreshInterval.called).toBe(false);
});

it('does not call background if refresh interval is null', async () => {
const store = mockStore();

background = {
setStatusRefreshInterval: sinon.stub().callsFake((_, cb) => cb()),
};
setBackgroundConnection(background);

await store.dispatch(actions.setSmartTransactionsRefreshInterval(null));

expect(background.setStatusRefreshInterval.called).toBe(false);
});
});
});
3 changes: 3 additions & 0 deletions ui/store/actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5084,6 +5084,9 @@ export function setSmartTransactionsRefreshInterval(
refreshInterval: number,
): ThunkAction<void, MetaMaskReduxState, unknown, AnyAction> {
return async () => {
if (refreshInterval === undefined || refreshInterval === null) {
return;
}
try {
await submitRequestToBackground('setStatusRefreshInterval', [
refreshInterval,
Expand Down

0 comments on commit 6b765ba

Please sign in to comment.