protocol coverage

.github/workflows/coverage.yml

@@ -1,6 +1,6 @@
 name: coverage 
 
-on: [push, pull_request]
+on: [push]
 
 jobs:
   coverage:

.github/workflows/forge-test.yml

@@ -1,7 +1,7 @@
-on: [push]
-
 name: Build and Test Contracts
 
+on: [push]
+
 jobs:
   check:
     name: Foundry project

integration/integration.test.ts

@@ -1,15 +1,142 @@
 import { ethers } from "hardhat";
 import { expect, use } from "chai";
-import { BigNumber, Contract, Signer } from "ethers";
+import { BigNumber, Contract } from "ethers";
 import { SignerWithAddress } from "@nomiclabs/hardhat-ethers/signers";
 
 import { solidity } from "ethereum-waffle";
 import { signEntitlement } from "./helpers";
 import { getAddress } from "ethers/lib/utils";
-import { writeHeapSnapshot } from "v8";
-import { create } from "domain";
 
 use(solidity);
+
+describe("Protocol", function () {
+  let protocol: Contract;
+  let admin: SignerWithAddress, one: SignerWithAddress, two: SignerWithAddress;
+  beforeEach(async () => {
+    [admin, one, two] = await ethers.getSigners();
+    const protocolFactory = await ethers.getContractFactory("HookProtocol");
+    protocol = await protocolFactory.deploy(admin.address, admin.address);
+  });
+
+  it("can be paused", async () => {
+    await protocol.connect(admin).pause();
+    await expect(protocol.throwWhenPaused()).to.be.reverted;
+  });
+
+  it("can be unpaused", async () => {
+    await protocol.connect(admin).pause();
+    await expect(protocol.throwWhenPaused()).to.be.reverted;
+    await protocol.connect(admin).unpause();
+    await expect(protocol.throwWhenPaused()).not.to.be.reverted;
+  });
+
+  it("vault factory can be set", async () => {
+    await protocol.connect(admin).pause();
+    await expect(protocol.throwWhenPaused()).to.be.reverted;
+    await protocol.connect(admin).unpause();
+    await expect(protocol.throwWhenPaused()).not.to.be.reverted;
+  });
+
+  it("admin role can be revoked", async () => {
+    /// give role to new user
+    await protocol
+      .connect(admin)
+      .grantRole(ethers.utils.id("VAULT_UPGRADER"), one.address);
+    // drop role
+    await protocol
+      .connect(admin)
+      .renounceRole(ethers.utils.id("VAULT_UPGRADER"), admin.address);
+    // new user gives role to a third user
+    await protocol
+      .connect(one)
+      .grantRole(ethers.utils.id("VAULT_UPGRADER"), two.address);
+    expect(
+      await protocol.hasRole(ethers.utils.id("VAULT_UPGRADER"), one.address)
+    ).to.be.true;
+    expect(
+      await protocol.hasRole(ethers.utils.id("VAULT_UPGRADER"), two.address)
+    ).to.be.true;
+    expect(
+      await protocol.hasRole(ethers.utils.id("VAULT_UPGRADER"), admin.address)
+    ).to.be.false;
+  });
+
+  it("collection configs can be set", async () => {
+    await protocol
+      .connect(admin)
+      .setCollectionConfig(two.address, ethers.utils.id("config"), true);
+    expect(
+      await protocol.getCollectionConfig(two.address, ethers.utils.id("config"))
+    ).to.be.true;
+  });
+});
+
+describe("UpgradeableBeacon", function () {
+  let beacon: Contract, impl1: Contract, impl2: Contract, protocol: Contract;
+  let admin: SignerWithAddress;
+  beforeEach(async () => {
+    [admin] = await ethers.getSigners();
+    const protocolFactory = await ethers.getContractFactory("HookProtocol");
+    protocol = await protocolFactory.deploy(admin.address, admin.address);
+
+    const vaultImplFactory = await ethers.getContractFactory(
+      "HookERC721VaultImplV1"
+    );
+
+    const vaultBeaconFactory = await ethers.getContractFactory(
+      "HookUpgradeableBeacon"
+    );
+
+    impl1 = await vaultImplFactory.deploy();
+    impl2 = await vaultImplFactory.deploy();
+
+    beacon = await vaultBeaconFactory.deploy(
+      impl1.address,
+      protocol.address,
+      ethers.utils.id("VAULT_UPGRADER")
+    );
+  });
+
+  it("the beacon should show an implementation", async function () {
+    expect(await beacon.implementation()).to.eq(impl1.address);
+  });
+
+  it("the beacon should be upgradeable by the correct role", async () => {
+    await beacon.connect(admin).upgradeTo(impl2.address);
+    expect(await beacon.implementation()).to.eq(impl2.address);
+  });
+
+  it("the beacon should not be upgradeable by a random address", async () => {
+    const [, actor] = await ethers.getSigners();
+    expect(
+      await protocol.hasRole(ethers.utils.id("VAULT_UPGRADER"), actor.address)
+    ).to.be.false;
+    await expect(
+      beacon.connect(actor).upgradeTo(impl2.address)
+    ).to.be.revertedWith("w");
+    expect(await beacon.implementation()).to.eq(impl1.address);
+  });
+
+  it("the beacon should not be upgradeable by a granted address", async () => {
+    const [, actor] = await ethers.getSigners();
+    expect(
+      await protocol.hasRole(ethers.utils.id("VAULT_UPGRADER"), actor.address)
+    ).to.be.false;
+
+    await protocol
+      .connect(admin)
+      .grantRole(ethers.utils.id("VAULT_UPGRADER"), actor.address);
+    await beacon.connect(actor).upgradeTo(impl2.address);
+    expect(await beacon.implementation()).to.eq(impl2.address);
+  });
+  it("the beacon should not be upgradeable not a non-contract", async () => {
+    const [, actor] = await ethers.getSigners();
+    await expect(
+      beacon.connect(admin).upgradeTo(actor.address)
+    ).to.be.revertedWith("UpgradeableBeacon: implementation is not a contract");
+  });
+});
+
 describe("Vault", function () {
   let vaultFactory: Contract,
     protocol: Contract,
@@ -1949,6 +2076,8 @@ describe("Call Instrument Tests", function () {
       getAddress("0x0000000000000000000000000000000000000000")
     );
 
+    protocol.setCoveredCallFactory(callFactory.address);
+
     // Create another call instrument contract instance
     await callFactory.makeCallInstrument(token.address);
     const callInstrumentAddress = await callFactory.getCallInstrument(

src/HookBeaconProxy.sol

@@ -39,13 +39,6 @@ contract HookBeaconProxy is Proxy, ERC1967Upgrade {
     _upgradeBeaconToAndCall(beacon, data, false);
   }
 
-  /**
-   * @dev Returns the current beacon address.
-   */
-  function _beacon() internal view virtual returns (address) {
-    return _getBeacon();
-  }
-
   /**
    * @dev Returns the current implementation address of the associated beacon.
    */

src/HookProtocol.sol

@@ -8,6 +8,8 @@ import "./interfaces/IHookProtocol.sol";
 
 import "./mixin/PermissionConstants.sol";
 
+import "@openzeppelin/contracts/utils/Address.sol";
+
 /// @dev Other contracts in the protocol refer to this one to get configuration and pausing issues.
 /// to reduce attack surface area, this contract cannot be upgraded; however, additional roles can be
 /// added.
@@ -38,12 +40,12 @@ contract HookProtocol is
     _setupRole(COLLECTION_CONF, admin);
 
     // allow the admin to add and remove other roles
-    _setRoleAdmin(ALLOWLISTER_ROLE, ADMIN_ROLE);
-    _setRoleAdmin(PAUSER_ROLE, ADMIN_ROLE);
-    _setRoleAdmin(VAULT_UPGRADER, ADMIN_ROLE);
-    _setRoleAdmin(CALL_UPGRADER, ADMIN_ROLE);
-    _setRoleAdmin(MARKET_CONF, ADMIN_ROLE);
-    _setRoleAdmin(COLLECTION_CONF, ADMIN_ROLE);
+    _setRoleAdmin(ALLOWLISTER_ROLE, ALLOWLISTER_ROLE);
+    _setRoleAdmin(PAUSER_ROLE, PAUSER_ROLE);
+    _setRoleAdmin(VAULT_UPGRADER, VAULT_UPGRADER);
+    _setRoleAdmin(CALL_UPGRADER, CALL_UPGRADER);
+    _setRoleAdmin(MARKET_CONF, MARKET_CONF);
+    _setRoleAdmin(COLLECTION_CONF, COLLECTION_CONF);
     // set weth
     getWETHAddress = weth;
   }
@@ -96,6 +98,10 @@ contract HookProtocol is
     external
     adminOnly
   {
+    require(
+      Address.isContract(coveredCallFactoryContract),
+      "setCoveredCallFactory: implementation is not a contract"
+    );
     coveredCallContract = coveredCallFactoryContract;
   }
 
@@ -104,6 +110,10 @@ contract HookProtocol is
   /// vault factory.
   /// @param vaultFactoryContract the deployed vault factory
   function setVaultFactory(address vaultFactoryContract) external adminOnly {
+    require(
+      Address.isContract(vaultFactoryContract),
+      "setVaultFactory: implementation is not a contract"
+    );
     vaultContract = vaultFactoryContract;
   }
 }

src/lib/Signatures.sol

@@ -2,10 +2,6 @@ pragma solidity ^0.8.10;
 
 /// @dev A library for validating signatures from ZeroEx
 library Signatures {
-  // '\x19Ethereum Signed Message:\n32\x00\x00\x00\x00'.
-  uint256 private constant ETH_SIGN_HASH_PREFIX =
-    0x19457468657265756d205369676e6564204d6573736167653a0a333200000000;
-
   /// @dev Allowed signature types.
   enum SignatureType {
     EIP712,