fix: reverting unneeded hacks to test code

Author: RembrandtK

packages/contracts/.solcover.js

@@ -8,4 +8,16 @@ module.exports = {
   skipFiles,
   istanbulFolder: './reports/coverage',
   configureYulOptimizer: true,
+  mocha: {
+    grep: '@skip-on-coverage',
+    invert: true,
+  },
+  onCompileComplete: async function (/* config */) {
+    // Set environment variable to indicate we're running under coverage
+    process.env.SOLIDITY_COVERAGE = 'true'
+  },
+  onIstanbulComplete: async function (/* config */) {
+    // Clean up environment variable
+    delete process.env.SOLIDITY_COVERAGE
+  },
 }

packages/contracts/addresses.json

@@ -566,22 +566,6 @@
       "txHash": "0x20a0c0f2a26258e1c63f33f3a5651580bb74ea4bf0e66076ebbdda4fa41d5955"
     }
   },
-  "1337": {
-    "Controller": {
-      "address": "0xC0dcc73ab4084494bfFAf8a427Fd3f645217BB1C",
-      "constructorArgs": [],
-      "creationCodeHash": "0x1a9b639b11e33783876f36ee77419a9a2a6a2041e51f78fd993ab012ea151c21",
-      "runtimeCodeHash": "0xec5980ddda2719b394f85b230fb95154a4944ac29d64ca66b9ccb04ee7e175e3",
-      "txHash": "0xd90746512994568e5c431c91ac502d28f17a47d827fb584bb0bf1b85b7e73d33"
-    },
-    "GraphProxyAdmin": {
-      "address": "0x25ec220C451593C9af36131Aa886397a21516533",
-      "constructorArgs": [],
-      "creationCodeHash": "0x4536e01a111e4f1b45c44cb2e38bfac56aca4577accf5bbee5f09c111a099ff4",
-      "runtimeCodeHash": "0x370843a171393282a9b7d05ba36f934831867f136e398505e9c38a9d0c3ecf46",
-      "txHash": "0xcf73cf1c75058d59876cb777959cb71757448265fbe3c415a3f6920fb15cb34e"
-    }
-  },
   "42161": {
     "GraphProxyAdmin": {
       "address": "0x2983936aC20202a6555993448E0d5654AC8Ca5fd",

packages/contracts/test/unit/gns.test.ts

@@ -854,7 +854,7 @@ describe('L1GNS', () => {
       const seqID = toBN('2')
       await legacyGNSMock.connect(me).createLegacySubgraph(seqID, newSubgraph0.subgraphDeploymentID)
       const tx = legacyGNSMock.connect(me).migrateLegacySubgraph(me.address, seqID, newSubgraph0.subgraphMetadata)
-      await expect(tx).emit(legacyGNSMock, 'LegacySubgraphClaimed').withArgs(me.address, seqID)
+      await expect(tx).emit(legacyGNSMock, ' LegacySubgraphClaimed').withArgs(me.address, seqID)
       const expectedSubgraphID = buildLegacySubgraphId(me.address, seqID)
       const migratedSubgraphDeploymentID = await legacyGNSMock.getSubgraphDeploymentID(expectedSubgraphID)
       const migratedNSignal = await legacyGNSMock.getSubgraphNSignal(expectedSubgraphID)
@@ -872,7 +872,7 @@ describe('L1GNS', () => {
       const seqID = toBN('2')
       await legacyGNSMock.connect(me).createLegacySubgraph(seqID, newSubgraph0.subgraphDeploymentID)
       let tx = legacyGNSMock.connect(me).migrateLegacySubgraph(me.address, seqID, newSubgraph0.subgraphMetadata)
-      await expect(tx).emit(legacyGNSMock, 'LegacySubgraphClaimed').withArgs(me.address, seqID)
+      await expect(tx).emit(legacyGNSMock, ' LegacySubgraphClaimed').withArgs(me.address, seqID)
       tx = legacyGNSMock.connect(me).migrateLegacySubgraph(me.address, seqID, newSubgraph0.subgraphMetadata)
       await expect(tx).revertedWith('GNS: Subgraph was already claimed')
     })
@@ -917,12 +917,12 @@ describe('L1GNS', () => {
     }
 
     const publishCurateAndSendSubgraph = async function (
-      beforeTransferCallback?: (_subgraphID: string) => Promise<void>,
+      beforeTransferCallback?: (/* subgraphID: string */) => Promise<void>,
     ): Promise<Subgraph> {
       const subgraph0 = await publishAndCurateOnSubgraph()
 
       if (beforeTransferCallback != null) {
-        await beforeTransferCallback(subgraph0.id)
+        await beforeTransferCallback()
       }
 
       const maxSubmissionCost = toBN('100')
@@ -947,7 +947,7 @@ describe('L1GNS', () => {
       const migrateTx = legacyGNSMock
         .connect(me)
         .migrateLegacySubgraph(me.address, seqID, newSubgraph0.subgraphMetadata)
-      await expect(migrateTx).emit(legacyGNSMock, 'LegacySubgraphClaimed').withArgs(me.address, seqID)
+      await expect(migrateTx).emit(legacyGNSMock, ' LegacySubgraphClaimed').withArgs(me.address, seqID)
       const subgraphID = buildLegacySubgraphId(me.address, seqID)
 
       // Curate on the subgraph

packages/contracts/test/unit/l2/l2GraphTokenGateway.test.ts

@@ -1,41 +1,33 @@
-import { smock } from '@defi-wonderland/smock'
-import { deploy, DeployType, GraphNetworkContracts, helpers, toBN, toGRT } from '@graphprotocol/sdk'
-import { SignerWithAddress } from '@nomiclabs/hardhat-ethers/signers'
+import { FakeContract, smock } from '@defi-wonderland/smock'
 import { expect, use } from 'chai'
 import { constants, ContractTransaction, Signer, utils, Wallet } from 'ethers'
 import hre from 'hardhat'
 
-// Define types for the contracts we'll use in the tests
-type L2GraphToken = any
-type L2GraphTokenGateway = any
-type CallhookReceiverMock = any
-type GraphToken = any
-type L1GraphTokenGateway = any
-type RewardsManager = any
-
+import { CallhookReceiverMock } from '../../../build/types/CallhookReceiverMock'
+import { L2GraphToken } from '../../../build/types/L2GraphToken'
+import { L2GraphTokenGateway } from '../../../build/types/L2GraphTokenGateway'
 import { NetworkFixture } from '../lib/fixtures'
-import { L2ArbitrumMessengerMock } from './l2ArbitrumMessengerMock'
 
-// Initialize smock before using it
-smock.fake // This ensures smock is initialized
 use(smock.matchers)
 
-// Mock the L2ArbitrumMessenger.sendTxToL1 function
-// This is done by monkey-patching the L2ArbitrumMessenger contract
-// We'll use this to verify that the function was called with the correct parameters
-L2ArbitrumMessengerMock.reset()
+import { deploy, DeployType, GraphNetworkContracts, helpers, toBN, toGRT } from '@graphprotocol/sdk'
+import { SignerWithAddress } from '@nomiclabs/hardhat-ethers/signers'
+
+import { GraphToken, L1GraphTokenGateway } from '../../../build/types'
+import { RewardsManager } from '../../../build/types/RewardsManager'
 
 const { AddressZero } = constants
 
 describe('L2GraphTokenGateway', () => {
-  // Remove the graph function call as it's not available in this environment
+  const graph = hre.graph()
   let me: SignerWithAddress
   let governor: SignerWithAddress
   let tokenSender: SignerWithAddress
   let l1Receiver: SignerWithAddress
   let l2Receiver: SignerWithAddress
   let pauseGuardian: SignerWithAddress
   let fixture: NetworkFixture
+  let arbSysMock: FakeContract
 
   let fixtureContracts: GraphNetworkContracts
   let l1MockContracts: GraphNetworkContracts
@@ -56,10 +48,9 @@ describe('L2GraphTokenGateway', () => {
   )
 
   before(async function () {
-    // Get test accounts directly from ethers
-    ;[me, governor, tokenSender, l1Receiver, l2Receiver, pauseGuardian] = await hre.ethers.getSigners()
+    ;[me, governor, tokenSender, l1Receiver, l2Receiver, pauseGuardian] = await graph.getTestAccounts()
 
-    fixture = new NetworkFixture(hre.ethers.provider)
+    fixture = new NetworkFixture(graph.provider)
 
     // Deploy L2
     fixtureContracts = await fixture.load(governor, true)
@@ -84,24 +75,23 @@ describe('L2GraphTokenGateway', () => {
     // Give some funds to the token sender and router mock
     await grt.connect(governor).mint(tokenSender.address, senderTokens)
     await helpers.setBalance(routerMock.address, utils.parseEther('1'))
-
-    // Deploy the ArbSys mock at the correct address
-    const ArbSysMockFactory = await hre.ethers.getContractFactory('ArbSysMock')
-    const arbSysMock = await ArbSysMockFactory.deploy()
-    await arbSysMock.deployed()
-
-    // Get the bytecode of the deployed contract
-    const code = await hre.ethers.provider.getCode(arbSysMock.address)
-
-    // Set the code at the ArbSys precompile address
-    const ARB_SYS_ADDRESS = '0x0000000000000000000000000000000000000064'
-    await hre.ethers.provider.send('hardhat_setCode', [ARB_SYS_ADDRESS, code])
   })
 
   beforeEach(async function () {
     await fixture.setUp()
-    // Reset the L2ArbitrumMessengerMock for each test
-    L2ArbitrumMessengerMock.reset()
+    // Thanks to Livepeer: https://github.com/livepeer/arbitrum-lpt-bridge/blob/main/test/unit/L2/l2LPTGateway.test.ts#L86
+    // Skip smock setup when running under coverage due to provider compatibility issues
+    const isRunningUnderCoverage =
+      hre.network.name === 'coverage' ||
+      process.env.SOLIDITY_COVERAGE === 'true' ||
+      process.env.npm_lifecycle_event === 'test:coverage'
+
+    if (!isRunningUnderCoverage) {
+      arbSysMock = await smock.fake('ArbSys', {
+        address: '0x0000000000000000000000000000000000000064',
+      })
+      arbSysMock.sendTxToL1.returns(1)
+    }
   })
 
   afterEach(async function () {
@@ -242,10 +232,20 @@ describe('L2GraphTokenGateway', () => {
         .emit(l2GraphTokenGateway, 'TxToL1')
         .withArgs(tokenSender.address, l1GRTGatewayMock.address, 1, calldata)
 
-      // We verify the TxToL1 event was emitted with the correct parameters
-      // This is sufficient to verify that sendTxToL1 was called correctly
-      // The event is emitted in the sendTxToL1 function of L2ArbitrumMessenger
+      // For some reason the call count doesn't work properly,
+      // and each function call is counted 12 times.
+      // Possibly related to https://github.com/defi-wonderland/smock/issues/85 ?
+      // expect(arbSysMock.sendTxToL1).to.have.been.calledOnce
+
+      // Only check smock expectations when not running under coverage
+      const isRunningUnderCoverage =
+        hre.network.name === 'coverage' ||
+        process.env.SOLIDITY_COVERAGE === 'true' ||
+        process.env.npm_lifecycle_event === 'test:coverage'
 
+      if (!isRunningUnderCoverage && arbSysMock) {
+        expect(arbSysMock.sendTxToL1).to.have.been.calledWith(l1GRTGatewayMock.address, calldata)
+      }
       const senderBalance = await grt.balanceOf(tokenSender.address)
       expect(senderBalance).eq(toGRT('990'))
     }
@@ -273,26 +273,12 @@ describe('L2GraphTokenGateway', () => {
       })
       it('burns tokens and triggers an L1 call', async function () {
         await grt.connect(tokenSender).approve(l2GraphTokenGateway.address, toGRT('10'))
-
-        // Execute the transfer
         await testValidOutboundTransfer(tokenSender, defaultData)
-
-        // The event checks are already done in testValidOutboundTransfer
-        // We just need to verify the balance change
-        const senderBalance = await grt.balanceOf(tokenSender.address)
-        expect(senderBalance).eq(toGRT('990'))
       })
       it('decodes the sender address from messages sent by the router', async function () {
         await grt.connect(tokenSender).approve(l2GraphTokenGateway.address, toGRT('10'))
         const routerEncodedData = utils.defaultAbiCoder.encode(['address', 'bytes'], [tokenSender.address, defaultData])
-
-        // Execute the transfer
         await testValidOutboundTransfer(routerMock, routerEncodedData)
-
-        // The event checks are already done in testValidOutboundTransfer
-        // We just need to verify the balance change
-        const senderBalance = await grt.balanceOf(tokenSender.address)
-        expect(senderBalance).eq(toGRT('990'))
       })
       it('reverts when called with nonempty calldata', async function () {
         await grt.connect(tokenSender).approve(l2GraphTokenGateway.address, toGRT('10'))

packages/data-edge/test/dataedge.test.ts

@@ -2,20 +2,21 @@ import '@nomiclabs/hardhat-ethers'
 
 import { SignerWithAddress } from '@nomiclabs/hardhat-ethers/signers'
 import { expect } from 'chai'
-import { Contract } from 'ethers'
 import { ethers } from 'hardhat'
 
+import { DataEdge, DataEdge__factory } from '../build/types'
+
 const { getContractFactory, getSigners } = ethers
 const { id, hexConcat, randomBytes, hexlify, defaultAbiCoder } = ethers.utils
 
 describe('DataEdge', () => {
-  let edge: Contract
+  let edge: DataEdge
   let me: SignerWithAddress
 
   beforeEach(async () => {
     ;[me] = await getSigners()
 
-    const factory = await getContractFactory('DataEdge', me)
+    const factory = (await getContractFactory('DataEdge', me)) as DataEdge__factory
     edge = await factory.deploy()
     await edge.deployed()
   })

packages/data-edge/test/eventful-dataedge.test.ts

@@ -2,20 +2,21 @@ import '@nomiclabs/hardhat-ethers'
 
 import { SignerWithAddress } from '@nomiclabs/hardhat-ethers/signers'
 import { expect } from 'chai'
-import { Contract } from 'ethers'
 import { ethers } from 'hardhat'
 
+import { EventfulDataEdge, EventfulDataEdge__factory } from '../build/types'
+
 const { getContractFactory, getSigners } = ethers
 const { id, hexConcat, randomBytes, hexlify, defaultAbiCoder } = ethers.utils
 
 describe('EventfulDataEdge', () => {
-  let edge: Contract
+  let edge: EventfulDataEdge
   let me: SignerWithAddress
 
   beforeEach(async () => {
     ;[me] = await getSigners()
 
-    const factory = await getContractFactory('EventfulDataEdge', me)
+    const factory = (await getContractFactory('EventfulDataEdge', me)) as EventfulDataEdge__factory
     edge = await factory.deploy()
     await edge.deployed()
   })

packages/sdk/src/deployments/network/actions/governed.ts

@@ -27,13 +27,7 @@ export const acceptOwnership: GraphNetworkAction<
     throw new Error(`Contract ${contractName} not found`)
   }
 
-  let pendingGovernor: string
-  try {
-    pendingGovernor = await (contract as GovernedContract).connect(signer).pendingGovernor()
-  } catch (error) {
-    console.log(`Contract ${contract.address} does not have pendingGovernor() method or call failed: ${error}`)
-    return
-  }
+  const pendingGovernor = await (contract as GovernedContract).connect(signer).pendingGovernor()
 
   if (pendingGovernor === ethers.constants.AddressZero) {
     console.log(`No pending governor for ${contract.address}`)
@@ -42,14 +36,9 @@ export const acceptOwnership: GraphNetworkAction<
 
   if (pendingGovernor === signer.address) {
     console.log(`Accepting ownership of ${contract.address}`)
-    try {
-      const tx = await (contract as GovernedContract).connect(signer).acceptOwnership()
-      await tx.wait()
-      return tx
-    } catch (error) {
-      console.log(`Failed to accept ownership of ${contract.address}: ${error}`)
-      return
-    }
+    const tx = await (contract as GovernedContract).connect(signer).acceptOwnership()
+    await tx.wait()
+    return tx
   } else {
     console.log(`Signer ${signer.address} is not the pending governor of ${contract.address}, it is ${pendingGovernor}`)
   }

packages/sdk/src/deployments/network/deployment/contracts/deploy.ts

@@ -194,7 +194,7 @@ export async function deployGraphNetwork(
     for (const contract of GraphNetworkGovernedContractNameList) {
       const tx = await acceptOwnership(loadedContracts, governor, { contractName: contract })
       if (tx) {
-        txs.push(tx)
+        txs.push()
       }
     }
     await Promise.all(txs.map((tx) => tx.wait()))
@@ -226,14 +226,14 @@ export async function deployMockGraphNetwork(l2Deploy: boolean) {
   contractList.push(...(l2Deploy ? GraphNetworkL2ContractNameList : GraphNetworkL1ContractNameList))
   contractList.push('AllocationExchange')
 
-  const contracts: any = {}
+  const contracts: Partial<Record<GraphNetworkContractName, Contract>> = {}
   for (const name of contractList) {
     const fake = Wallet.createRandom()
     await setCode(fake.address, '0x1234')
     contracts[name] = new Contract(fake.address, [], fake)
   }
 
-  return contracts as GraphNetworkContracts // :eyes:
+  return contracts as GraphNetworkContracts
 }
 
 export const deploy = async (

packages/sdk/src/deployments/network/deployment/contracts/load.ts

@@ -92,7 +92,7 @@ export interface GraphNetworkContracts extends ContractList<GraphNetworkContract
 
 // This ensures that local artifacts are preferred over the ones that ship with the sdk in node_modules
 export function getArtifactsPath() {
-  return [path.resolve('artifacts'), path.resolve('node_modules', '@graphprotocol/contracts/artifacts')]
+  return [path.resolve('build/contracts'), path.resolve('node_modules', '@graphprotocol/contracts/build/contracts')]
 }
 export function loadGraphNetworkContracts(
   addressBookPath: string,

packages/token-distribution/test/config.ts

@@ -38,16 +38,13 @@ export interface DateRange {
 
 const dateRange = (months: number): DateRange => {
   const date = new Date(+new Date() - 120) // set start time for a few seconds before
-  const newDate = new Date(date.getTime())
-  newDate.setMonth(date.getMonth() + months)
-  return { startTime: Math.round(+date / 1000), endTime: Math.round(+newDate.getTime() / 1000) }
+  const newDate = new Date().setMonth(date.getMonth() + months)
+  return { startTime: Math.round(+date / 1000), endTime: Math.round(+newDate / 1000) }
 }
 
 const moveTime = (time: number, months: number) => {
   const date = new Date(time * 1000)
-  const newDate = new Date(date.getTime())
-  newDate.setMonth(date.getMonth() + months)
-  return Math.round(newDate.getTime() / 1000)
+  return Math.round(+date.setMonth(date.getMonth() + months) / 1000)
 }
 
 const moveDateRange = (dateRange: DateRange, months: number) => {