test: upgrade tests

.github/workflows/ci.yml

@@ -31,11 +31,6 @@ jobs:
           node-version: 22
       - run: npm install
       - run: npm test
-      - uses: actions/cache@v4
-        with:
-          path: fuzzing/corpus
-          key: fuzzing
-      - run: npm run fuzz
 
   verify:
     runs-on: ubuntu-latest

certora/harness/MarketplaceHarness.sol

@@ -10,10 +10,6 @@ import {RequestId, SlotId} from "../../contracts/Requests.sol";
 import {Requests} from "../../contracts/Requests.sol";
 
 contract MarketplaceHarness is Marketplace {
-    constructor(MarketplaceConfig memory config, IERC20 token, IGroth16Verifier verifier)
-        Marketplace(config, token, verifier)
-    {}
-
     function publicPeriodEnd(Period period) public view returns (uint64) {
         return _periodEnd(period);
     }

configuration/configuration.js

@@ -1,4 +1,5 @@
 const fs = require("fs")
+const { loadZkeyHash } = require("../verifier/verifier.js")
 
 const BASE_PATH = __dirname + "/networks"
 
@@ -23,6 +24,17 @@ const DEFAULT_CONFIGURATION = {
   requestDurationLimit: 60*60*24*30 // 30 days
 }
 
+function getDefaultConfig(networkName) {
+  if (networkName === undefined) {
+    throw new TypeError("Network name needs to be specified!")
+  }
+
+  const zkeyHash = loadZkeyHash(networkName)
+  const config = loadConfiguration(networkName)
+  config.proofs.zkeyHash = zkeyHash
+  return config
+}
+
 function loadConfiguration(name) {
   const path = `${BASE_PATH}/${name}/configuration.js`
   if (fs.existsSync(path)) {
@@ -32,4 +44,4 @@ function loadConfiguration(name) {
   }
 }
 
-module.exports = { loadConfiguration }
+module.exports = { loadConfiguration, getDefaultConfig }

contracts/Marketplace.sol

@@ -97,6 +97,13 @@ contract Marketplace is Initializable, SlotReservations, Proofs, StateRetrieval,
     uint64 slotIndex;
   }
 
+  constructor() {
+    // In case that the contract would get deployed without initialization
+    // this prevents attackers to call the initializations themselves with
+    // potentially malicious initialization values.
+    _disableInitializers();
+  }
+
   function initialize (
     MarketplaceConfig memory config,
     IERC20 token_,

contracts/TestMarketplaceUpgrade.sol

@@ -0,0 +1,12 @@
+// SPDX-License-Identifier: MIT
+pragma solidity ^0.8.28;
+
+import "./Marketplace.sol";
+
+// exposes internal functions of Marketplace for testing
+contract TestMarketplaceUpgraded is Marketplace {
+
+  function newShinyMethod() public pure returns (uint256) {
+    return 42;
+  }
+}

ignition/modules/marketplace-test-upgrade.js

@@ -0,0 +1,36 @@
+const { buildModule } = require('@nomicfoundation/hardhat-ignition/modules')
+const MarketplaceModule = require("./marketplace.js")
+
+/**
+ * This module upgrades the Marketplace's Proxy with a new implementation.
+ * It deploys the new Marketplace contract and then calls the Proxy with
+ * the `upgradeAndCall` function, which swaps the implementations.
+ */
+const upgradeModule = buildModule('UpgradeProxyImplementation', (m) => {
+  const config = hre.network.config
+
+  if (!(config && config.tags && config.tags.includes("local"))) {
+    throw new Error("Module is not meant for real deployments!")
+  }
+
+  const proxyAdminOwner = m.getAccount(9)
+  const marketplaceUpgraded = m.contract("TestMarketplaceUpgraded", [])
+  const {proxyAdmin, proxy, token} = m.useModule(MarketplaceModule);
+
+  m.call(proxyAdmin, "upgradeAndCall", [proxy, marketplaceUpgraded, "0x"], {
+    from: proxyAdminOwner,
+  });
+
+  return { proxyAdmin, proxy, token };
+})
+
+/**
+ * The main module that represents the upgraded Marketplace contract.
+ */
+module.exports = buildModule('MarketplaceUpgraded', (m) => {
+  const { proxy, proxyAdmin, token } = m.useModule(upgradeModule)
+
+  const marketplace = m.contractAt('TestMarketplaceUpgraded', proxy)
+
+  return { marketplace, proxy, proxyAdmin, token }
+})

ignition/modules/marketplace.js

@@ -1,15 +1,8 @@
 const { buildModule } = require('@nomicfoundation/hardhat-ignition/modules')
-const { loadZkeyHash } = require("../../verifier/verifier.js")
-const { loadConfiguration } = require("../../configuration/configuration.js")
+const { getDefaultConfig } = require("../../configuration/configuration.js")
 const TokenModule = require("./token.js")
 const VerifierModule = require("./verifier.js")
 
-function getDefaultConfig() {
-  const zkeyHash = loadZkeyHash(hre.network.name)
-  const config = loadConfiguration(hre.network.name)
-  config.proofs.zkeyHash = zkeyHash
-  return config
-}
 
 /**
  * Module that deploy the Marketplace logic
@@ -59,7 +52,7 @@ const proxyModule = buildModule('Proxy', (m) => {
   const { marketplace } = m.useModule(marketplaceLogicModule)
   const { token } = m.useModule(TokenModule)
   const { verifier } = m.useModule(VerifierModule)
-  const configuration = m.getParameter("configuration", getDefaultConfig())
+  const configuration = m.getParameter("configuration", getDefaultConfig(hre.network.name))
   const encodedMarketplaceInitializerCall = m.encodeFunctionCall(
     marketplace,
     "initialize",
@@ -110,7 +103,7 @@ const testProxyModule = buildModule('TestProxy', (m) => {
   const { testMarketplace } = m.useModule(marketplaceLogicModule)
   const { token } = m.useModule(TokenModule)
   const { testVerifier } = m.useModule(VerifierModule)
-  const configuration = m.getParameter("configuration", getDefaultConfig())
+  const configuration = m.getParameter("configuration", getDefaultConfig(hre.network.name))
   const encodedMarketplaceInitializerCall = m.encodeFunctionCall(
     testMarketplace,
     "initialize",

test/Marketplace.test.js

@@ -41,6 +41,7 @@ const {
 } = require("./evm")
 const { getBytes } = require("ethers")
 const MarketplaceModule = require("../ignition/modules/marketplace")
+const MarketplaceUpgradeModule = require("../ignition/modules/marketplace-test-upgrade")
 const { assertDeploymentRejectedWithCustomError } = require("./helpers")
 
 const ACCOUNT_STARTING_BALANCE = 1_000_000_000_000_000n
@@ -86,8 +87,6 @@ describe("Marketplace constructor", function () {
     config.collateral.slashPercentage = 1
     config.collateral.maxNumberOfSlashes = 101
 
-    const expectedError = "Marketplace_MaximumSlashingTooHigh"
-
     const promise = ignition.deploy(MarketplaceModule, {
       parameters: {
         TestProxy: {
@@ -103,6 +102,108 @@ describe("Marketplace constructor", function () {
   })
 })
 
+// Currently, Hardhat Ignition does not track deployments in the Hardhat network,
+// so when Hardhat runs tests on its network, Ignition will deploy new contracts
+// every time it is deploying something. This prevents proper testing of the upgradability
+// because a new Proxy will be deployed on the "upgrade deployment," and it won't reuse the
+// original one.
+// This can be tested manually by running `hardhat node` and then `hardhat test --network localhost`.
+// But even then, the test "should share the same storage" is having issues, which I suspect
+// are related to different network usage.
+// Blocked until this issue is resolved: https://github.com/NomicFoundation/hardhat/issues/6927
+describe.skip("Marketplace upgrades", function () {
+  const config = exampleConfiguration()
+  let originalMarketplace, proxy, token, request, client
+  enableRequestAssertions()
+
+  beforeEach(async function () {
+    await snapshot()
+    await ensureMinimumBlockHeight(256)
+    const {
+      marketplace,
+      proxy: deployedProxy,
+      token: token_,
+    } = await ignition.deploy(MarketplaceModule, {
+      deploymentId: "test",
+      parameters: {
+        TestProxy: {
+          configuration: config,
+        },
+      },
+    })
+
+    proxy = deployedProxy
+    originalMarketplace = marketplace
+    token = token_
+
+    await ensureMinimumBlockHeight(256)
+    ;[client] = await ethers.getSigners()
+
+    request = await exampleRequest()
+    request.client = client.address
+
+    token = token.connect(client)
+    originalMarketplace = marketplace.connect(client)
+  })
+
+  afterEach(async function () {
+    await revert()
+  })
+
+  it("should get upgraded", async () => {
+    expect(() => originalMarketplace.newShinyMethod()).to.throw(TypeError)
+
+    const { marketplace: upgradedMarketplace, proxy: upgradedProxy } =
+      await ignition.deploy(MarketplaceUpgradeModule, {
+        deploymentId: "test",
+        parameters: {
+          TestProxy: {
+            configuration: config,
+          },
+        },
+      })
+    expect(await upgradedMarketplace.newShinyMethod()).to.equal(42)
+    expect(await originalMarketplace.getAddress()).to.equal(
+      await upgradedMarketplace.getAddress(),
+    )
+    expect(await originalMarketplace.configuration()).to.deep.equal(
+      await upgradedMarketplace.configuration(),
+    )
+  })
+
+  it("should share the same storage", async () => {
+    // Old implementation not supporting the shiny method
+    expect(() => originalMarketplace.newShinyMethod()).to.throw(TypeError)
+
+    // We create a Request on the old implementation
+    await token.approve(
+      await originalMarketplace.getAddress(),
+      maxPrice(request),
+    )
+    const now = await currentTime()
+    await setNextBlockTimestamp(now)
+    const expectedExpiry = now + request.expiry
+    await expect(originalMarketplace.requestStorage(request))
+      .to.emit(originalMarketplace, "StorageRequested")
+      .withArgs(requestId(request), askToArray(request.ask), expectedExpiry)
+
+    // Assert that the data is there
+    expect(
+      await originalMarketplace.getRequest(requestId(request)),
+    ).to.be.request(request)
+
+    // Upgrade Marketplace
+    const { marketplace: upgradedMarketplace, token: _token } =
+      await ignition.deploy(MarketplaceUpgradeModule)
+    expect(await upgradedMarketplace.newShinyMethod()).to.equal(42)
+
+    // Assert that the data is there
+    expect(
+      await upgradedMarketplace.getRequest(requestId(request)),
+    ).to.be.request(request)
+  })
+})
+
 describe("Marketplace", function () {
   const proof = exampleProof()
   const config = exampleConfiguration()
@@ -144,6 +245,7 @@ describe("Marketplace", function () {
       {
         parameters: {
           TestProxy: {
+            // TestMarketplace initialization is done in the TestProxy module
             configuration: config,
           },
         },