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

fix: circular deps between engine, network utils tx utils #12376

Open
wants to merge 2 commits into
base: fix/circular-deps-handle-network-switch
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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 @@ -13,7 +13,6 @@ import ApproveTransactionReview from '../../components/ApproveTransactionReview'
import AddNickname from '../../components/ApproveTransactionReview/AddNickname';
import Modal from 'react-native-modal';
import { strings } from '../../../../../../locales/i18n';
import { getNetworkNonce } from '../../../../../util/networks';

import {
setTransactionObject,
Expand Down Expand Up @@ -65,7 +64,10 @@ import createStyles from './styles';
import { providerErrors } from '@metamask/rpc-errors';
import { getDeviceId } from '../../../../../core/Ledger/Ledger';
import ExtendedKeyringTypes from '../../../../../constants/keyringTypes';
import { updateTransaction } from '../../../../../util/transaction-controller';
import {
getNetworkNonce,
updateTransaction,
} from '../../../../../util/transaction-controller';
import { withMetricsAwareness } from '../../../../../components/hooks/useMetrics';
import {
selectGasFeeEstimates,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ import { MetaMetricsEvents } from '../../../../../core/Analytics';
import { shallowEqual, renderShortText } from '../../../../../util/general';
import {
isTestNet,
getNetworkNonce,
isMainnetByChainId,
isMultiLayerFeeNetwork,
fetchEstimatedMultiLayerL1Fee,
Expand Down Expand Up @@ -116,7 +115,10 @@ import {
} from '../../../../../selectors/confirmTransaction';
import { selectGasFeeControllerEstimateType } from '../../../../../selectors/gasFeeController';
import { createBuyNavigationDetails } from '../../../../UI/Ramp/routes/utils';
import { updateTransaction } from '../../../../../util/transaction-controller';
import {
getNetworkNonce,
updateTransaction,
} from '../../../../../util/transaction-controller';
import { selectShouldUseSmartTransaction } from '../../../../../selectors/smartTransactionsController';
import { STX_NO_HASH_ERROR } from '../../../../../util/smart-transactions/smart-publish-hook';
import { getSmartTransactionMetricsProperties } from '../../../../../util/smart-transactions';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ import { sumHexWEIs } from '../../../../../../util/conversions';
import { MetaMetricsEvents } from '../../../../../../core/Analytics';
import {
TESTNET_FAUCETS,
getNetworkNonce,
isTestNet,
isTestNetworkWithFaucet,
} from '../../../../../../util/networks';
Expand Down Expand Up @@ -63,6 +62,7 @@ import { getRampNetworks } from '../../../../../../reducers/fiatOrders';
import { createBuyNavigationDetails } from '../../../../../UI/Ramp/routes/utils';
import { withMetricsAwareness } from '../../../../../../components/hooks/useMetrics';
import { selectShouldUseSmartTransaction } from '../../../../../../selectors/smartTransactionsController';
import { getNetworkNonce } from '../../../../../../util/transaction-controller';

const createStyles = (colors) =>
StyleSheet.create({
Expand Down
9 changes: 0 additions & 9 deletions app/util/networks/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ import {
SEPOLIA_BLOCK_EXPLORER,
SEPOLIA_FAUCET,
} from '../../constants/urls';
import { getNonceLock } from '../../util/transaction-controller';

/**
* List of the supported networks
Expand Down Expand Up @@ -349,14 +348,6 @@ export function isPrefixedFormattedHexString(value) {
return regex.prefixedFormattedHexString.test(value);
}

export const getNetworkNonce = async ({ from }) => {
const { nextNonce, releaseLock } = await getNonceLock(from);

releaseLock();

return nextNonce;
};

export function blockTagParamIndex(payload) {
switch (payload.method) {
// blockTag is at index 2
Expand Down
34 changes: 0 additions & 34 deletions app/util/networks/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import {
compareRpcUrls,
getBlockExplorerAddressUrl,
getBlockExplorerTxUrl,
getNetworkNonce,
isPrivateConnection,
} from '.';
import {
Expand All @@ -25,7 +24,6 @@ import {
LINEA_SEPOLIA,
} from '../../../app/constants/network';
import { NetworkSwitchErrorType } from '../../../app/constants/error';
import { getNonceLock } from '../../util/transaction-controller';
import Engine from './../../core/Engine';
import { TransactionMeta } from '@metamask/transaction-controller';

Expand Down Expand Up @@ -78,11 +76,6 @@ jest.mock('./../../core/Engine', () => ({
},
}));

jest.mock('../../util/transaction-controller', () => ({
__esModule: true,
getNonceLock: jest.fn(),
}));

describe('network-utils', () => {
describe('getAllNetworks', () => {
const allNetworks = getAllNetworks();
Expand Down Expand Up @@ -369,33 +362,6 @@ describe('network-utils', () => {
});
});

describe('getNetworkNonce', () => {
const nonceMock = 123;
const fromMock = '0x123';

it('returns value from TransactionController', async () => {
(getNonceLock as jest.Mock).mockReturnValueOnce({
nextNonce: nonceMock,
releaseLock: jest.fn(),
});

expect(await getNetworkNonce({ from: fromMock })).toBe(nonceMock);

expect(getNonceLock).toHaveBeenCalledWith(fromMock);
});

it('releases nonce lock', async () => {
const releaseLockMock = jest.fn();

(getNonceLock as jest.Mock).mockReturnValueOnce({
releaseLock: releaseLockMock,
});

await getNetworkNonce({ from: fromMock });

expect(releaseLockMock).toHaveBeenCalledTimes(1);
});
});
describe('convertNetworkId', () => {
it('converts a number to a string', () => {
expect(convertNetworkId(1)).toEqual('1');
Expand Down
47 changes: 46 additions & 1 deletion app/util/transaction-controller/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { WalletDevice } from '@metamask/transaction-controller';
import * as TransactionControllerUtils from './index';
import Engine from '../../core/Engine';

const { addTransaction, estimateGas, ...proxyMethods } =
const { addTransaction, estimateGas, getNetworkNonce, ...proxyMethods } =
TransactionControllerUtils;

const TRANSACTION_MOCK = { from: '0x0', to: '0x1', value: '0x0' };
Expand Down Expand Up @@ -71,4 +71,49 @@ describe('Transaction Controller Util', () => {
});
});
});

describe('getNetworkNonce', () => {
const nonceMock = 123;
const fromMock = '0x123';

beforeEach(() => {
jest.spyOn(Engine.context.TransactionController, 'getNonceLock');
});

afterEach(() => {
jest.restoreAllMocks();
});

it('returns value from TransactionController', async () => {
(
Engine.context.TransactionController.getNonceLock as jest.Mock
).mockResolvedValueOnce({
nextNonce: nonceMock,
releaseLock: jest.fn(),
});

expect(
await TransactionControllerUtils.getNetworkNonce({ from: fromMock }),
).toBe(nonceMock);

expect(
Engine.context.TransactionController.getNonceLock,
).toHaveBeenCalledWith(fromMock);
});

it('releases nonce lock', async () => {
const releaseLockMock = jest.fn();

(
Engine.context.TransactionController.getNonceLock as jest.Mock
).mockResolvedValueOnce({
nextNonce: nonceMock,
releaseLock: releaseLockMock,
});

await TransactionControllerUtils.getNetworkNonce({ from: fromMock });

expect(releaseLockMock).toHaveBeenCalledTimes(1);
});
});
});
8 changes: 8 additions & 0 deletions app/util/transaction-controller/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,3 +89,11 @@ export function wipeTransactions(
const { TransactionController } = Engine.context;
return TransactionController.wipeTransactions(...args);
}

export const getNetworkNonce = async ({ from }: { from: string }) => {
const { nextNonce, releaseLock } = await getNonceLock(from);

releaseLock();

return nextNonce;
};
Loading