[Low priority] Fix a bunch of nits in sdk (#2418)

* Fix a bunch of nits

chain id enum nit

more utils nits

fix the rest of the nits

fix comment being moved

remove comment that is no longer needed

* add comment to isL2Provider so jsdocs works and change comment

* yarn changeset
Co-authored-by: Will Cory <willcory@Wills-MacBook-Pro.local>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

.changeset/mean-spies-trade.md

+---
+'@eth-optimism/sdk': minor
+---
+
+New isL2Provider helper function. Internal cleanups.

packages/sdk/src/adapters/dai-bridge.ts

@@ -20,29 +20,29 @@ export class DAIBridgeAdapter extends StandardBridgeAdapter {
       [
         {
           inputs: [],
-          name: 'l1Token',
+          name: 'l1Token' as const,
           outputs: [
             {
-              internalType: 'address',
-              name: '',
-              type: 'address',
+              internalType: 'address' as const,
+              name: '' as const,
+              type: 'address' as const,
             },
           ],
-          stateMutability: 'view',
-          type: 'function',
+          stateMutability: 'view' as const,
+          type: 'function' as const,
         },
         {
           inputs: [],
-          name: 'l2Token',
+          name: 'l2Token' as const,
           outputs: [
             {
-              internalType: 'address',
-              name: '',
-              type: 'address',
+              internalType: 'address' as const,
+              name: '' as const,
+              type: 'address' as const,
             },
           ],
-          stateMutability: 'view',
-          type: 'function',
+          stateMutability: 'view' as const,
+          type: 'function' as const,
         },
       ],
       this.messenger.l1Provider

packages/sdk/src/adapters/eth-bridge.ts

@@ -119,7 +119,7 @@ export class ETHBridgeAdapter extends StandardBridgeAdapter {
       opts?: {
         overrides?: Overrides
       }
-    ): Promise<TransactionRequest> => {
+    ): Promise<never> => {
       throw new Error(`approvals not necessary for ETH bridge`)
     },
 

packages/sdk/src/cross-chain-messenger.ts

@@ -109,7 +109,7 @@ export class CrossChainMessenger implements ICrossChainMessenger {
     if (Provider.isProvider(this.l1SignerOrProvider)) {
       return this.l1SignerOrProvider
     } else {
-      return this.l1SignerOrProvider.provider as any
+      return this.l1SignerOrProvider.provider
     }
   }
 
@@ -117,7 +117,7 @@ export class CrossChainMessenger implements ICrossChainMessenger {
     if (Provider.isProvider(this.l2SignerOrProvider)) {
       return this.l2SignerOrProvider
     } else {
-      return this.l2SignerOrProvider.provider as any
+      return this.l2SignerOrProvider.provider
     }
   }
 
@@ -144,10 +144,7 @@ export class CrossChainMessenger implements ICrossChainMessenger {
     } = {}
   ): Promise<CrossChainMessage[]> {
     // Wait for the transaction receipt if the input is waitable.
-    // TODO: Maybe worth doing this with more explicit typing but whatever for now.
-    if (typeof (transaction as any).wait === 'function') {
-      await (transaction as any).wait()
-    }
+    await (transaction as TransactionResponse).wait?.()
 
     // Convert the input to a transaction hash.
     const txHash = toTransactionHash(transaction)
@@ -821,7 +818,7 @@ export class CrossChainMessenger implements ICrossChainMessenger {
     )
 
     const stateTrieProof = await makeStateTrieProof(
-      this.l2Provider as any,
+      this.l2Provider as ethers.providers.JsonRpcProvider,
       resolved.blockNumber,
       this.contracts.l2.OVM_L2ToL1MessagePasser.address,
       messageSlot

packages/sdk/src/interfaces/l2-provider.ts

@@ -83,4 +83,10 @@ export type L2Provider<TProvider extends Provider> = TProvider & {
    * @returns Estimated total gas cost.
    */
   estimateTotalGasCost(tx: TransactionRequest): Promise<BigNumber>
+
+  /**
+   * Internal property to determine if a provider is a L2Provider
+   * You are likely looking for the isL2Provider function
+   */
+  _isL2Provider: true
 }

packages/sdk/src/interfaces/types.ts

@@ -9,6 +9,16 @@ import { Contract, BigNumber } from 'ethers'
 import { ICrossChainMessenger } from './cross-chain-messenger'
 import { IBridgeAdapter } from './bridge-adapter'
 
+/**
+ * Commonly used Chain IDs
+ */
+export enum Chain {
+  MAINNET = 1,
+  GOERLI = 5,
+  KOVAN = 42,
+  HARDHAT_LOCAL = 31337,
+}
+
 /**
  * L1 contract references.
  */

packages/sdk/src/l2-provider.ts

+import assert from 'assert'
+
 import { Provider, TransactionRequest } from '@ethersproject/abstract-provider'
 import { serialize } from '@ethersproject/transactions'
 import { Contract, BigNumber } from 'ethers'
@@ -7,6 +9,8 @@ import cloneDeep from 'lodash/cloneDeep'
 import { L2Provider, ProviderLike, NumberLike } from './interfaces'
 import { toProvider, toNumber, toBigNumber } from './utils'
 
+type ProviderTypeIsWrong = any
+
 /**
  * Returns a Contract object for the GasPriceOracle.
  *
@@ -115,6 +119,24 @@ export const estimateTotalGasCost = async (
   return l1GasCost.add(l2GasCost)
 }
 
+/**
+ * Determines if a given Provider is an L2Provider.  Will coerce type
+ * if true
+ *
+ * @param provider The provider to check
+ * @returns Boolean
+ * @example
+ * if (isL2Provider(provider)) {
+ *   // typescript now knows it is of type L2Provider
+ *   const gasPrice = await provider.estimateL2GasPrice(tx)
+ * }
+ */
+export const isL2Provider = <TProvider extends Provider>(
+  provider: TProvider
+): provider is L2Provider<TProvider> => {
+  return Boolean((provider as L2Provider<TProvider>)._isL2Provider)
+}
+
 /**
  * Returns an provider wrapped as an Optimism L2 provider. Adds a few extra helper functions to
  * simplify the process of estimating the gas usage for a transaction on Optimism. Returns a COPY
@@ -126,22 +148,21 @@ export const estimateTotalGasCost = async (
 export const asL2Provider = <TProvider extends Provider>(
   provider: TProvider
 ): L2Provider<TProvider> => {
-  // Make a copy of the provider since we'll be modifying some internals and don't want to mess
-  // with the original object.
-  const l2Provider = cloneDeep(provider) as any
-
   // Skip if we've already wrapped this provider.
-  if (l2Provider._isL2Provider) {
-    return l2Provider
+  if (isL2Provider(provider)) {
+    return provider
   }
 
+  // Make a copy of the provider since we'll be modifying some internals and don't want to mess
+  // with the original object.
+  const l2Provider = cloneDeep(provider) as L2Provider<TProvider>
+
   // Not exactly sure when the provider wouldn't have a formatter function, but throw an error if
   // it doesn't have one. The Provider type doesn't define it but every provider I've dealt with
   // seems to have it.
-  const formatter = l2Provider.formatter
-  if (formatter === undefined) {
-    throw new Error(`provider.formatter must be defined`)
-  }
+  // TODO this may be fixed if library has gotten updated since
+  const formatter = (l2Provider as ProviderTypeIsWrong).formatter
+  assert(formatter, `provider.formatter must be defined`)
 
   // Modify the block formatter to return the state root. Not strictly related to Optimism, just a
   // generally useful thing that really should've been on the Ethers block object to begin with.

packages/sdk/src/utils/chain-constants.ts

+import { Chain } from '../interfaces'
+
 export const DEPOSIT_CONFIRMATION_BLOCKS = {
-  // Mainnet
-  1: 50,
-  // Goerli
-  5: 12,
-  // Kovan
-  42: 12,
-  // Hardhat Local
+  [Chain.MAINNET]: 50 as const,
+  [Chain.GOERLI]: 12 as const,
+  [Chain.KOVAN]: 12 as const,
   // 2 just for testing purposes
-  31337: 2,
+  [Chain.HARDHAT_LOCAL]: 2 as const,
 }
 
 export const CHAIN_BLOCK_TIMES = {
-  // Mainnet
-  1: 13,
-  // Goerli
-  5: 15,
-  // Kovan
-  42: 4,
-  // Hardhat Local
-  31337: 1,
+  [Chain.MAINNET]: 13 as const,
+  [Chain.GOERLI]: 15 as const,
+  [Chain.KOVAN]: 4 as const,
+  [Chain.HARDHAT_LOCAL]: 1 as const,
 }

packages/sdk/src/utils/coercion.ts

@@ -29,9 +29,9 @@ export const toSignerOrProvider = (
   if (typeof signerOrProvider === 'string') {
     return new ethers.providers.JsonRpcProvider(signerOrProvider)
   } else if (Provider.isProvider(signerOrProvider)) {
-    return signerOrProvider as Provider
+    return signerOrProvider
   } else if (Signer.isSigner(signerOrProvider)) {
-    return signerOrProvider as Signer
+    return signerOrProvider
   } else {
     throw new Error('Invalid provider')
   }
@@ -48,7 +48,7 @@ export const toProvider = (provider: ProviderLike): Provider => {
   if (typeof provider === 'string') {
     return new ethers.providers.JsonRpcProvider(provider)
   } else if (Provider.isProvider(provider)) {
-    return provider as Provider
+    return provider
   } else {
     throw new Error('Invalid provider')
   }
@@ -66,7 +66,6 @@ export const toTransactionHash = (transaction: TransactionLike): string => {
       ethers.utils.isHexString(transaction, 32),
       'Invalid transaction hash'
     )
-
     return transaction
   } else if ((transaction as TransactionReceipt).transactionHash) {
     return (transaction as TransactionReceipt).transactionHash

packages/sdk/src/utils/contracts.ts

@@ -13,6 +13,7 @@ import {
   BridgeAdapters,
   BridgeAdapterData,
   ICrossChainMessenger,
+  Chain,
 } from '../interfaces'
 import {
   StandardBridgeAdapter,
@@ -40,9 +41,9 @@ export const DEFAULT_L2_CONTRACT_ADDRESSES: OEL2ContractsLike = {
  * back to the original contract names so we can look them up.
  */
 const NAME_REMAPPING = {
-  AddressManager: 'Lib_AddressManager',
-  OVM_L1BlockNumber: 'iOVM_L1BlockNumber',
-  WETH: 'WETH9',
+  AddressManager: 'Lib_AddressManager' as const,
+  OVM_L1BlockNumber: 'iOVM_L1BlockNumber' as const,
+  WETH: 'WETH9' as const,
 }
 
 /**
@@ -53,51 +54,59 @@ const NAME_REMAPPING = {
 export const CONTRACT_ADDRESSES: {
   [l1ChainId: number]: OEContractsLike
 } = {
-  // Mainnet
-  1: {
+  [Chain.MAINNET]: {
     l1: {
-      AddressManager: '0xdE1FCfB0851916CA5101820A69b13a4E276bd81F',
-      L1CrossDomainMessenger: '0x25ace71c97B33Cc4729CF772ae268934F7ab5fA1',
-      L1StandardBridge: '0x99C9fc46f92E8a1c0deC1b1747d010903E884bE1',
-      StateCommitmentChain: '0xBe5dAb4A2e9cd0F27300dB4aB94BeE3A233AEB19',
-      CanonicalTransactionChain: '0x5E4e65926BA27467555EB562121fac00D24E9dD2',
-      BondManager: '0xcd626E1328b41fCF24737F137BcD4CE0c32bc8d1',
+      AddressManager: '0xdE1FCfB0851916CA5101820A69b13a4E276bd81F' as const,
+      L1CrossDomainMessenger:
+        '0x25ace71c97B33Cc4729CF772ae268934F7ab5fA1' as const,
+      L1StandardBridge: '0x99C9fc46f92E8a1c0deC1b1747d010903E884bE1' as const,
+      StateCommitmentChain:
+        '0xBe5dAb4A2e9cd0F27300dB4aB94BeE3A233AEB19' as const,
+      CanonicalTransactionChain:
+        '0x5E4e65926BA27467555EB562121fac00D24E9dD2' as const,
+      BondManager: '0xcd626E1328b41fCF24737F137BcD4CE0c32bc8d1' as const,
     },
     l2: DEFAULT_L2_CONTRACT_ADDRESSES,
   },
-  // Kovan
-  42: {
+  [Chain.KOVAN]: {
     l1: {
-      AddressManager: '0x100Dd3b414Df5BbA2B542864fF94aF8024aFdf3a',
-      L1CrossDomainMessenger: '0x4361d0F75A0186C05f971c566dC6bEa5957483fD',
-      L1StandardBridge: '0x22F24361D548e5FaAfb36d1437839f080363982B',
-      StateCommitmentChain: '0xD7754711773489F31A0602635f3F167826ce53C5',
-      CanonicalTransactionChain: '0xf7B88A133202d41Fe5E2Ab22e6309a1A4D50AF74',
-      BondManager: '0xc5a603d273E28185c18Ba4d26A0024B2d2F42740',
+      AddressManager: '0x100Dd3b414Df5BbA2B542864fF94aF8024aFdf3a' as const,
+      L1CrossDomainMessenger:
+        '0x4361d0F75A0186C05f971c566dC6bEa5957483fD' as const,
+      L1StandardBridge: '0x22F24361D548e5FaAfb36d1437839f080363982B' as const,
+      StateCommitmentChain:
+        '0xD7754711773489F31A0602635f3F167826ce53C5' as const,
+      CanonicalTransactionChain:
+        '0xf7B88A133202d41Fe5E2Ab22e6309a1A4D50AF74' as const,
+      BondManager: '0xc5a603d273E28185c18Ba4d26A0024B2d2F42740' as const,
     },
     l2: DEFAULT_L2_CONTRACT_ADDRESSES,
   },
-  // Goerli
-  5: {
+  [Chain.GOERLI]: {
     l1: {
-      AddressManager: '0x2F7E3cAC91b5148d336BbffB224B4dC79F09f01D',
-      L1CrossDomainMessenger: '0xEcC89b9EDD804850C4F343A278Be902be11AaF42',
-      L1StandardBridge: '0x73298186A143a54c20ae98EEE5a025bD5979De02',
-      StateCommitmentChain: '0x1afcA918eff169eE20fF8AB6Be75f3E872eE1C1A',
-      CanonicalTransactionChain: '0x2ebA8c4EfDB39A8Cd8f9eD65c50ec079f7CEBD81',
-      BondManager: '0xE5AE60bD6F8DEe4D0c2BC9268e23B92F1cacC58F',
+      AddressManager: '0x2F7E3cAC91b5148d336BbffB224B4dC79F09f01D' as const,
+      L1CrossDomainMessenger:
+        '0xEcC89b9EDD804850C4F343A278Be902be11AaF42' as const,
+      L1StandardBridge: '0x73298186A143a54c20ae98EEE5a025bD5979De02' as const,
+      StateCommitmentChain:
+        '0x1afcA918eff169eE20fF8AB6Be75f3E872eE1C1A' as const,
+      CanonicalTransactionChain:
+        '0x2ebA8c4EfDB39A8Cd8f9eD65c50ec079f7CEBD81' as const,
+      BondManager: '0xE5AE60bD6F8DEe4D0c2BC9268e23B92F1cacC58F' as const,
     },
     l2: DEFAULT_L2_CONTRACT_ADDRESSES,
   },
-  // Hardhat local
-  31337: {
+  [Chain.HARDHAT_LOCAL]: {
     l1: {
-      AddressManager: '0x5FbDB2315678afecb367f032d93F642f64180aa3',
-      L1CrossDomainMessenger: '0x8A791620dd6260079BF849Dc5567aDC3F2FdC318',
-      L1StandardBridge: '0x610178dA211FEF7D417bC0e6FeD39F05609AD788',
-      StateCommitmentChain: '0xDc64a140Aa3E981100a9becA4E685f962f0cF6C9',
-      CanonicalTransactionChain: '0xCf7Ed3AccA5a467e9e704C703E8D87F634fB0Fc9',
-      BondManager: '0x5FC8d32690cc91D4c39d9d3abcBD16989F875707',
+      AddressManager: '0x5FbDB2315678afecb367f032d93F642f64180aa3' as const,
+      L1CrossDomainMessenger:
+        '0x8A791620dd6260079BF849Dc5567aDC3F2FdC318' as const,
+      L1StandardBridge: '0x610178dA211FEF7D417bC0e6FeD39F05609AD788' as const,
+      StateCommitmentChain:
+        '0xDc64a140Aa3E981100a9becA4E685f962f0cF6C9' as const,
+      CanonicalTransactionChain:
+        '0xCf7Ed3AccA5a467e9e704C703E8D87F634fB0Fc9' as const,
+      BondManager: '0x5FC8d32690cc91D4c39d9d3abcBD16989F875707' as const,
     },
     l2: DEFAULT_L2_CONTRACT_ADDRESSES,
   },
@@ -111,7 +120,7 @@ export const BRIDGE_ADAPTER_DATA: {
 } = {
   // TODO: Maybe we can pull these automatically from the token list?
   // Alternatively, check against the token list in CI.
-  1: {
+  [Chain.MAINNET]: {
     Standard: {
       Adapter: StandardBridgeAdapter,
       l1Bridge: CONTRACT_ADDRESSES[1].l1.L1StandardBridge,
@@ -124,16 +133,16 @@ export const BRIDGE_ADAPTER_DATA: {
     },
     BitBTC: {
       Adapter: StandardBridgeAdapter,
-      l1Bridge: '0xaBA2c5F108F7E820C049D5Af70B16ac266c8f128',
-      l2Bridge: '0x158F513096923fF2d3aab2BcF4478536de6725e2',
+      l1Bridge: '0xaBA2c5F108F7E820C049D5Af70B16ac266c8f128' as const,
+      l2Bridge: '0x158F513096923fF2d3aab2BcF4478536de6725e2' as const,
     },
     DAI: {
       Adapter: DAIBridgeAdapter,
-      l1Bridge: '0x10E6593CDda8c58a1d0f14C5164B376352a55f2F',
-      l2Bridge: '0x467194771dAe2967Aef3ECbEDD3Bf9a310C76C65',
+      l1Bridge: '0x10E6593CDda8c58a1d0f14C5164B376352a55f2F' as const,
+      l2Bridge: '0x467194771dAe2967Aef3ECbEDD3Bf9a310C76C65' as const,
     },
   },
-  42: {
+  [Chain.KOVAN]: {
     Standard: {
       Adapter: StandardBridgeAdapter,
       l1Bridge: CONTRACT_ADDRESSES[42].l1.L1StandardBridge,
@@ -146,21 +155,21 @@ export const BRIDGE_ADAPTER_DATA: {
     },
     BitBTC: {
       Adapter: StandardBridgeAdapter,
-      l1Bridge: '0x0b651A42F32069d62d5ECf4f2a7e5Bd3E9438746',
-      l2Bridge: '0x0CFb46528a7002a7D8877a5F7a69b9AaF1A9058e',
+      l1Bridge: '0x0b651A42F32069d62d5ECf4f2a7e5Bd3E9438746' as const,
+      l2Bridge: '0x0CFb46528a7002a7D8877a5F7a69b9AaF1A9058e' as const,
     },
     USX: {
       Adapter: StandardBridgeAdapter,
-      l1Bridge: '0x40E862341b2416345F02c41Ac70df08525150dC7',
-      l2Bridge: '0xB4d37826b14Cd3CB7257A2A5094507d701fe715f',
+      l1Bridge: '0x40E862341b2416345F02c41Ac70df08525150dC7' as const,
+      l2Bridge: '0xB4d37826b14Cd3CB7257A2A5094507d701fe715f' as const,
     },
     DAI: {
       Adapter: DAIBridgeAdapter,
-      l1Bridge: '0xb415e822C4983ecD6B1c1596e8a5f976cf6CD9e3',
-      l2Bridge: '0x467194771dAe2967Aef3ECbEDD3Bf9a310C76C65',
+      l1Bridge: '0xb415e822C4983ecD6B1c1596e8a5f976cf6CD9e3' as const,
+      l2Bridge: '0x467194771dAe2967Aef3ECbEDD3Bf9a310C76C65' as const,
     },
   },
-  5: {
+  [Chain.GOERLI]: {
     Standard: {
       Adapter: StandardBridgeAdapter,
       l1Bridge: CONTRACT_ADDRESSES[5].l1.L1StandardBridge,
@@ -172,7 +181,7 @@ export const BRIDGE_ADAPTER_DATA: {
       l2Bridge: predeploys.L2StandardBridge,
     },
   },
-  31337: {
+  [Chain.HARDHAT_LOCAL]: {
     Standard: {
       Adapter: StandardBridgeAdapter,
       l1Bridge: CONTRACT_ADDRESSES[31337].l1.L1StandardBridge,
@@ -274,21 +283,29 @@ export const getAllOEContracts = (
   }
 
   // Attach all L1 contracts.
-  const l1Contracts: OEL1Contracts = {} as any
+  const l1Contracts = {} as OEL1Contracts
   for (const [contractName, contractAddress] of Object.entries(addresses.l1)) {
-    l1Contracts[contractName] = getOEContract(contractName as any, l1ChainId, {
-      address: opts.overrides?.l1?.[contractName] || contractAddress,
-      signerOrProvider: opts.l1SignerOrProvider,
-    })
+    l1Contracts[contractName] = getOEContract(
+      contractName as keyof OEL1Contracts,
+      l1ChainId,
+      {
+        address: opts.overrides?.l1?.[contractName] || contractAddress,
+        signerOrProvider: opts.l1SignerOrProvider,
+      }
+    )
   }
 
   // Attach all L2 contracts.
-  const l2Contracts: OEL2Contracts = {} as any
+  const l2Contracts = {} as OEL2Contracts
   for (const [contractName, contractAddress] of Object.entries(addresses.l2)) {
-    l2Contracts[contractName] = getOEContract(contractName as any, l1ChainId, {
-      address: opts.overrides?.l2?.[contractName] || contractAddress,
-      signerOrProvider: opts.l2SignerOrProvider,
-    })
+    l2Contracts[contractName] = getOEContract(
+      contractName as keyof OEL2Contracts,
+      l1ChainId,
+      {
+        address: opts.overrides?.l2?.[contractName] || contractAddress,
+        signerOrProvider: opts.l2SignerOrProvider,
+      }
+    )
   }
 
   return {

packages/sdk/src/utils/misc-utils.ts

@@ -8,10 +8,13 @@
  * @param keys Keys to omit from the returned object.
  * @returns A copy of the given object with the given keys omitted.
  */
-export const omit = (obj: any, ...keys: string[]) => {
+export const omit = <T extends object, K extends string | number | symbol>(
+  obj: T,
+  ...keys: K[]
+): Omit<T, K> => {
   const copy = { ...obj }
   for (const key of keys) {
-    delete copy[key]
+    delete copy[key as string]
   }
   return copy
 }