diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 65250b3..21a78ac 100644 --- a/.github/workflows/ci.yml +++ b/.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 diff --git a/certora/harness/MarketplaceHarness.sol b/certora/harness/MarketplaceHarness.sol index c3119c9..072e43e 100644 --- a/certora/harness/MarketplaceHarness.sol +++ b/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); } diff --git a/configuration/configuration.js b/configuration/configuration.js index 2d0ec3d..bb45d3d 100644 --- a/configuration/configuration.js +++ b/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 } diff --git a/contracts/Marketplace.sol b/contracts/Marketplace.sol index 8a09004..93c0398 100644 --- a/contracts/Marketplace.sol +++ b/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_, diff --git a/contracts/TestMarketplaceUpgrade.sol b/contracts/TestMarketplaceUpgrade.sol new file mode 100644 index 0000000..cc23e03 --- /dev/null +++ b/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; + } +} diff --git a/ignition/modules/marketplace-test-upgrade.js b/ignition/modules/marketplace-test-upgrade.js new file mode 100644 index 0000000..3ff47c9 --- /dev/null +++ b/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 } +}) diff --git a/ignition/modules/marketplace.js b/ignition/modules/marketplace.js index b0fc732..397acad 100644 --- a/ignition/modules/marketplace.js +++ b/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", diff --git a/test/Marketplace.test.js b/test/Marketplace.test.js index 9c472f7..6237f20 100644 --- a/test/Marketplace.test.js +++ b/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, }, },