chore: refactor SwapsController so it extends from BaseControllerV2

[nikoferro]

This PR updates the controller so it extends from BaseController V2. It also updates tests to adjust to the new controller interface.

These changes will also involve updates on the mobile app to adjust to the new version

[martahj]

Looks good! I think at least one other person should review this because I don't yet feel confident enough in my knowledge of what to look out for, but I didn't see any issues. I did have one small question on one of the helpers

[martahj]

Is it ok for any objects included in the old chainCache to maintain the same reference rather than being deep-cloned?

[nikoferro]

In this case, for the state to be properly updated, we need to avoid shallow copies. Ideally we would keep each part of the chainCache as a top level key on the state, but since the potential chain id(s) used are unknown, we are keeping this inside this object.

[nikoferro]

Moving this back to draft as i noticed some bugs when testing its integration with the mobile client

[nikoferro]

This is ready for review again 🚀

[MajorLift]

nit: The concept of a config object and the configure method is deprecated in BaseControllerV2. We've generally been moving config object properties to private class fields, and also to constructor options to provide initial values.

I've added suggestions below which should get us most of the way to applying these changes.

If any of these properties do need to be included in state, they should probably be added to the top level so that the state stays normalized and the StateMetadata type is automatically updated.

Some criteria for determining whether a config property may belong in state:

• The property needs to be accessible by other controllers (not just e.g. internal test files).
• The property is owned by this controller, and does not belong in the state of other controllers.
• Changes to the property need to be announced with a stateChange event. Other controllers contain logic that rely on this broadcast.
• The property needs to be persisted, or contains PII and needs to be anonymized.
• The property should never be mutated individually, and should always be updated along with the rest of state through an immutable data management scheme i.e. it requires guarantees of strong consistency with the rest of state.

[MajorLift]

Removing as TxParams assertions

[MajorLift]

nit: Class properties are public by default and the public keyword doesn't add any functionality at runtime (unlike e.g. the #-prefix which actually keeps private properties hidden). All public keywords can be safely removed.

[nikoferro]

Went ahead and did this change, removed every public keyword, and also added the # prefix to the methods we know for sure we can keep private.

There are a few that we are spying on, so i kept those with the private keyword. Still public at a language level, but at least we keep TS warnings over those.

Eventually we would want to refactor a bit the controller so we need to test only public methods

[nikoferro]

@MajorLift i think pretty much all your suggestions were included. A lot of it was related to the exclusion of a config object. I ended up keeping those values out of the state so i had to change some things a bit.

[MajorLift]

Catch block fix - same as above: #280 (comment).

diff --git a/src/SwapsController.ts b/src/SwapsController.ts
index 7dc2675..3d9b011 100644
--- a/src/SwapsController.ts
+++ b/src/SwapsController.ts
@@ -14,7 +14,11 @@ import type {
 } from '@metamask/gas-fee-controller';
 import { GAS_ESTIMATE_TYPES } from '@metamask/gas-fee-controller';
 import type { Provider } from '@metamask/network-controller';
-import { getKnownPropertyNames, type Hex } from '@metamask/utils';
+import {
+  getKnownPropertyNames,
+  isErrorWithMessage,
+  type Hex,
+} from '@metamask/utils';
 import { Mutex } from 'async-mutex';
 import { BigNumber } from 'bignumber.js';
 import abiERC20 from 'human-standard-token-abi';
@@ -477,11 +481,19 @@ export default class SwapsController extends BaseController<
         threshold: quotesLastFetched - timeStarted,
         usedGasEstimate: gasFeeEstimates,
       };
-    } catch (error: any) {
-      const errorKey = Object.values(SwapsError).includes(error.message)
-        ? error.message
-        : SwapsError.ERROR_FETCHING_QUOTES;
-      this.stopPollingAndResetState({ key: errorKey, description: error });
+    } catch (error: unknown) {
+      if (isErrorWithMessage(error)) {
+        const errorKey = ((message): message is SwapsError =>
+          Object.values(SwapsError).find(
+            (swapsError) => swapsError === message,
+          ) !== undefined)(error.message)
+          ? error.message
+          : SwapsError.ERROR_FETCHING_QUOTES;
+        this.stopPollingAndResetState({
+          key: errorKey,
+          description: JSON.stringify(error),
+        });
+      }
       return {
         nextQuotesState: null,
         threshold: null,

[MajorLift]

any fixes and type improvements for swapsUtil.ts.

diff --git a/src/swapsUtil.ts b/src/swapsUtil.ts
index 66e671f..2bc2774 100644
--- a/src/swapsUtil.ts
+++ b/src/swapsUtil.ts
@@ -5,6 +5,7 @@ import {
   query,
   timeoutFetch,
 } from '@metamask/controller-utils';
+import type EthQuery from '@metamask/eth-query';
 import type {
   EthGasPriceEstimate,
   GasFeeState,
@@ -15,7 +16,7 @@ import type {
 import { GAS_ESTIMATE_TYPES } from '@metamask/gas-fee-controller';
 import type { TransactionParams } from '@metamask/transaction-controller';
 import type { Hex } from '@metamask/utils';
-import { add0x } from '@metamask/utils';
+import { add0x, getKnownPropertyNames } from '@metamask/utils';
 import { BigNumber } from 'bignumber.js';
 import { BN } from 'bn.js';
@@ -57,19 +58,21 @@ import type {
 // /
 // / BEGIN: Lifted from now unexported normalizeTransaction in @metamask/transaction-controller@3.0.0
 // /
-export const TX_NORMALIZERS: { [param in keyof TransactionParams]: any } = {
+export const TX_NORMALIZERS = {
   data: (data: string) => add0x(data),
-  from: (from: string) => add0x(from).toLowerCase(),
+  from: (from: string) => add0x(from).toLowerCase() as Hex,
   gas: (gas: string) => add0x(gas),
   gasPrice: (gasPrice: string) => add0x(gasPrice),
   nonce: (nonce: string) => add0x(nonce),
-  to: (to: string) => add0x(to).toLowerCase(),
+  to: (to: string) => add0x(to).toLowerCase() as Hex,
   value: (value: string) => add0x(value),
   maxFeePerGas: (maxFeePerGas: string) => add0x(maxFeePerGas),
   maxPriorityFeePerGas: (maxPriorityFeePerGas: string) =>
     add0x(maxPriorityFeePerGas),
   estimatedBaseFee: (maxPriorityFeePerGas: string) =>
     add0x(maxPriorityFeePerGas),
+} satisfies {
+  [param in keyof TransactionParams]: (arg: string) => Hex;
 };
 
 /**
@@ -77,16 +80,17 @@ export const TX_NORMALIZERS: { [param in keyof TransactionParams]: any } = {
  * @param transaction - Transaction object to normalize.
  * @returns Normalized Transaction object.
  */
-export function normalizeTransaction(transaction: TransactionParams) {
+export function normalizeTransaction(
+  transaction: TransactionParams,
+): Pick<TransactionParams, keyof typeof TX_NORMALIZERS> {
   const normalizedTransaction: TransactionParams = { from: '' };
-  let key: keyof TransactionParams;
-  for (key in TX_NORMALIZERS) {
-    if (transaction[key]) {
+  getKnownPropertyNames(TX_NORMALIZERS).forEach((key) => {
+    if (key in transaction && transaction[key]) {
       normalizedTransaction[key] = TX_NORMALIZERS[key](
-        transaction[key],
-      ) as never;
+        transaction[key] as NonNullable<TransactionParams[typeof key]>,
+      );
     }
-  }
+  });
   return normalizedTransaction;
 }
@@ -287,7 +291,11 @@ export async function fetchTradesInfo(
   const tradeURL = `${getBaseApiURL(
     APIType.TRADES,
     chainId,
-  )}?${new URLSearchParams(urlParams as Record<any, any>).toString()}`;
+  )}?${new URLSearchParams(
+    Object.fromEntries(
+      Object.entries(urlParams).map(([key, value]) => [key, String(value)]),
+    ),
+  ).toString()}`;
 
   const tradesResponse = await timeoutFetch(
     tradeURL,
@@ -316,14 +324,13 @@ export async function fetchTradesInfo(
             : BNToHex(new BN(MAX_GAS_LIMIT)),
         });
 
-        return {
-          ...aggIdTradeMap,
+        return Object.assign(aggIdTradeMap, {
           [quote.aggregator]: {
             ...quote,
             slippage,
             trade: constructedTrade,
           },
-        };
+        });
       }
 
       return aggIdTradeMap;
@@ -737,7 +744,7 @@ export function calcTokenAmount(value: number | BigNumber, decimals: number) {
  */
 export async function estimateGas(
   transaction: Omit<TxParams, 'gas'> & Partial<Pick<TxParams, 'gas'>>,
-  ethQuery: any,
+  ethQuery: EthQuery,
 ) {
   const estimatedTransaction = { ...transaction };
   const { value, data } = estimatedTransaction;
@@ -785,7 +792,7 @@ export function constructTxParams({
   gas?: string;
   gasPrice?: string;
   amount?: string;
-}): any {
+}): Pick<TransactionParams, keyof typeof TX_NORMALIZERS> {
   const txParams: TransactionParams = {
     data,
     from,
@@ -842,7 +849,7 @@ export function isGasFeeStateLegacy(
  * @returns Whether the object is of type EthGasPriceEstimate.
  */
 export function isEthGasPriceEstimate(
-  object: any,
+  object: Record<string, unknown> | undefined,
 ): object is EthGasPriceEstimate {
   return Boolean(object) && object?.gasPrice !== undefined;
 }
@@ -853,7 +860,7 @@ export function isEthGasPriceEstimate(
  * @returns Whether the object is of type CustomEthGasPriceEstimate.
  */
 export function isCustomEthGasPriceEstimate(
-  object: any,
+  object: Record<string, unknown> | undefined,
 ): object is CustomEthGasPriceEstimate {
   return Boolean(object) && object?.gasPrice !== undefined;
 }
@@ -863,8 +870,11 @@ export function isCustomEthGasPriceEstimate(
  * @param object - The object to be evaluated.
  * @returns Whether the object is of type CustomGasFee.
  */
-export function isCustomGasFee(object: any): object is CustomGasFee {
+export function isCustomGasFee(
+  object: Record<string, unknown> | undefined,
+): object is CustomGasFee {
   return (
+    object !== undefined &&
     Boolean(object) &&
     'maxFeePerGas' in object &&
     'maxPriorityFeePerGas' in object

[martahj]

@MajorLift Updated the PR with the type changes you suggested :)