From 32b0f4d41674eca675bdc3ded6168257dfb9ac1f Mon Sep 17 00:00:00 2001 From: Dmitry Date: Mon, 4 Dec 2023 15:59:27 +0100 Subject: [PATCH 01/25] Handling ERC4626 vaults in AcreRouter AcreRouter contract should manage ERC4626 vaults within the Acre system. Owner of the contract should be able to add or remove vaults. --- core/contracts/AcreRouter.sol | 55 +++++++++++++++++++++++++++++++++++ core/package.json | 3 ++ 2 files changed, 58 insertions(+) create mode 100644 core/contracts/AcreRouter.sol diff --git a/core/contracts/AcreRouter.sol b/core/contracts/AcreRouter.sol new file mode 100644 index 000000000..9063ed59f --- /dev/null +++ b/core/contracts/AcreRouter.sol @@ -0,0 +1,55 @@ +// SPDX-License-Identifier: GPL-3.0-only +pragma solidity ^0.8.21; + +import "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol"; + +/// @title AcreRouter +/// @notice AcreRouter is a contract that routes TBTC from stBTC (Acre) to +/// a given vault and back. Vaults supply yield strategies with TBTC that +/// generate yield for Bitcoin holders. +contract AcreRouter is OwnableUpgradeable { + struct Vault { + bool approved; + } + + /// @notice Approved vaults within the Yiern Modules that implement ERC4626 + /// standard. These vaults deposit assets to yield strategies, e.g. + /// Uniswap V3 WBTC/TBTC pool. Vault can be a part of Acre ecosystem + /// or can be implemented externally. As long as it complies with + /// ERC4626 standard and is approved by the owner it can be + /// plugged into Acre. + address[] public vaults; + mapping(address => Vault) public vaultsInfo; + + event VaultAdded(address indexed vault); + event VaultRemoved(address indexed vault); + + /// @notice Adds a vault to the list of approved vaults. + /// @param vault Address of the vault to add. + function addVault(address vault) external onlyOwner { + require(!vaultsInfo[vault].approved, "Vault already approved"); + + vaults.push(vault); + vaultsInfo[vault].approved = true; + + emit VaultAdded(vault); + } + + /// @notice Removes a vault from the list of approved vaults. + /// @param vault Address of the vault to remove. + function removeVault(address vault) external onlyOwner { + require(vaultsInfo[vault].approved, "Not a vault"); + + delete vaultsInfo[vault]; + + for (uint256 i = 0; i < vaults.length; i++) { + if (vaults[i] == vault) { + vaults[i] = vaults[vaults.length - 1]; + vaults.pop(); + break; + } + } + + emit VaultRemoved(vault); + } +} diff --git a/core/package.json b/core/package.json index 49f094744..5e0d6a7dc 100644 --- a/core/package.json +++ b/core/package.json @@ -55,5 +55,8 @@ "ts-node": "^10.9.1", "typechain": "^8.3.2", "typescript": "^5.3.2" + }, + "dependencies": { + "@openzeppelin/contracts-upgradeable": "^5.0.0" } } From 3246552ee84fea7360e641a9a7f4bb1316b2fee5 Mon Sep 17 00:00:00 2001 From: Dmitry Date: Mon, 4 Dec 2023 16:23:23 +0100 Subject: [PATCH 02/25] Adding pnpm-lock.yaml --- pnpm-lock.yaml | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 371dfb09f..b41a02bbc 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -19,6 +19,10 @@ importers: version: 11.2.1 core: + dependencies: + '@openzeppelin/contracts-upgradeable': + specifier: ^5.0.0 + version: 5.0.0(@openzeppelin/contracts@5.0.0) devDependencies: '@nomicfoundation/hardhat-chai-matchers': specifier: ^2.0.2 @@ -4421,6 +4425,18 @@ packages: - supports-color dev: true + /@openzeppelin/contracts-upgradeable@5.0.0(@openzeppelin/contracts@5.0.0): + resolution: {integrity: sha512-D54RHzkOKHQ8xUssPgQe2d/U92mwaiBDY7qCCVGq6VqwQjsT3KekEQ3bonev+BLP30oZ0R1U6YC8/oLpizgC5Q==} + peerDependencies: + '@openzeppelin/contracts': 5.0.0 + dependencies: + '@openzeppelin/contracts': 5.0.0 + dev: false + + /@openzeppelin/contracts@5.0.0: + resolution: {integrity: sha512-bv2sdS6LKqVVMLI5+zqnNrNU/CA+6z6CmwFXm/MzmOPBRSO5reEJN7z0Gbzvs0/bv/MZZXNklubpwy3v2+azsw==} + dev: false + /@openzeppelin/defender-admin-client@1.52.0(debug@4.3.4): resolution: {integrity: sha512-CKs5mMLL7+nXyugsHaAw0aPfLwFNA+vq7ftuJ3sWUKdbQRZsJ+/189HAwp2/BJC64yUbarEeWqOh3jNpaKRJLw==} dependencies: From a3ba20f8ac2a744f8fae34c1b7f95dad463f3e7e Mon Sep 17 00:00:00 2001 From: Dmitry Date: Wed, 6 Dec 2023 14:23:46 +0100 Subject: [PATCH 03/25] Replacing OZ upgradable lib to a non-upgradable one Upgradability will be handled down the road in a separate PR(s) --- core/contracts/AcreRouter.sol | 13 ++++++++++--- core/package.json | 2 +- pnpm-lock.yaml | 12 ++---------- 3 files changed, 13 insertions(+), 14 deletions(-) diff --git a/core/contracts/AcreRouter.sol b/core/contracts/AcreRouter.sol index 9063ed59f..c7224e324 100644 --- a/core/contracts/AcreRouter.sol +++ b/core/contracts/AcreRouter.sol @@ -1,13 +1,13 @@ // SPDX-License-Identifier: GPL-3.0-only pragma solidity ^0.8.21; -import "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol"; +import "@openzeppelin/contracts/access/Ownable.sol"; /// @title AcreRouter /// @notice AcreRouter is a contract that routes TBTC from stBTC (Acre) to /// a given vault and back. Vaults supply yield strategies with TBTC that /// generate yield for Bitcoin holders. -contract AcreRouter is OwnableUpgradeable { +contract AcreRouter is Ownable { struct Vault { bool approved; } @@ -24,6 +24,8 @@ contract AcreRouter is OwnableUpgradeable { event VaultAdded(address indexed vault); event VaultRemoved(address indexed vault); + constructor() Ownable(msg.sender) {} + /// @notice Adds a vault to the list of approved vaults. /// @param vault Address of the vault to add. function addVault(address vault) external onlyOwner { @@ -40,11 +42,12 @@ contract AcreRouter is OwnableUpgradeable { function removeVault(address vault) external onlyOwner { require(vaultsInfo[vault].approved, "Not a vault"); - delete vaultsInfo[vault]; + vaultsInfo[vault].approved = false; for (uint256 i = 0; i < vaults.length; i++) { if (vaults[i] == vault) { vaults[i] = vaults[vaults.length - 1]; + // slither-disable-next-line costly-loop vaults.pop(); break; } @@ -52,4 +55,8 @@ contract AcreRouter is OwnableUpgradeable { emit VaultRemoved(vault); } + + function vaultsLength() external view returns (uint256) { + return vaults.length; + } } diff --git a/core/package.json b/core/package.json index 5e0d6a7dc..603cb8643 100644 --- a/core/package.json +++ b/core/package.json @@ -57,6 +57,6 @@ "typescript": "^5.3.2" }, "dependencies": { - "@openzeppelin/contracts-upgradeable": "^5.0.0" + "@openzeppelin/contracts": "^5.0.0" } } diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index b41a02bbc..362ab37be 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -20,9 +20,9 @@ importers: core: dependencies: - '@openzeppelin/contracts-upgradeable': + '@openzeppelin/contracts': specifier: ^5.0.0 - version: 5.0.0(@openzeppelin/contracts@5.0.0) + version: 5.0.0 devDependencies: '@nomicfoundation/hardhat-chai-matchers': specifier: ^2.0.2 @@ -4425,14 +4425,6 @@ packages: - supports-color dev: true - /@openzeppelin/contracts-upgradeable@5.0.0(@openzeppelin/contracts@5.0.0): - resolution: {integrity: sha512-D54RHzkOKHQ8xUssPgQe2d/U92mwaiBDY7qCCVGq6VqwQjsT3KekEQ3bonev+BLP30oZ0R1U6YC8/oLpizgC5Q==} - peerDependencies: - '@openzeppelin/contracts': 5.0.0 - dependencies: - '@openzeppelin/contracts': 5.0.0 - dev: false - /@openzeppelin/contracts@5.0.0: resolution: {integrity: sha512-bv2sdS6LKqVVMLI5+zqnNrNU/CA+6z6CmwFXm/MzmOPBRSO5reEJN7z0Gbzvs0/bv/MZZXNklubpwy3v2+azsw==} dev: false From 072f854937630ec09e8658b117b7bb6bfd3093f9 Mon Sep 17 00:00:00 2001 From: Dmitry Date: Wed, 6 Dec 2023 14:24:55 +0100 Subject: [PATCH 04/25] Drafting tests for AcreRouter --- core/test/AcreRouter.test.ts | 148 +++++++++++++++++++++++++++++++++++ 1 file changed, 148 insertions(+) create mode 100644 core/test/AcreRouter.test.ts diff --git a/core/test/AcreRouter.test.ts b/core/test/AcreRouter.test.ts new file mode 100644 index 000000000..af3e358b9 --- /dev/null +++ b/core/test/AcreRouter.test.ts @@ -0,0 +1,148 @@ +import { ethers, getUnnamedAccounts, getNamedAccounts } from "hardhat" +import { HardhatEthersSigner } from "@nomicfoundation/hardhat-ethers/signers" +import { Address } from "hardhat-deploy/types" +import { expect } from "chai" +import { + SnapshotRestorer, + takeSnapshot, +} from "@nomicfoundation/hardhat-toolbox/network-helpers" +import type { AcreRouter } from "../typechain" + +describe("AcreRouter", () => { + let snapshot: SnapshotRestorer + + let acreRouter: AcreRouter + let deployerSigner: HardhatEthersSigner + let governanceSigner: HardhatEthersSigner + let thirdPartySigner: HardhatEthersSigner + let vault1: Address + let vault2: Address + let vault3: Address + let vault4: Address + + // TODO: + // Put some of this setup to deployment scripts and/or helpers e.g. ownership + // transfer, acreRouter deployment etc. See: https://github.com/thesis/acre/pull/58 + // and https://github.com/thesis/acre/pull/64 + before(async () => { + const accounts = await getUnnamedAccounts() + ;[vault1, vault2, vault3, vault4] = accounts + const { deployer, governance } = await getNamedAccounts() + deployerSigner = await ethers.getSigner(deployer) + governanceSigner = await ethers.getSigner(governance) + thirdPartySigner = await ethers.getSigner(accounts[0]) + const AcreRouter = await ethers.getContractFactory("AcreRouter") + acreRouter = await AcreRouter.connect(deployerSigner).deploy() + acreRouter.transferOwnership(governance) + }) + + beforeEach(async () => { + snapshot = await takeSnapshot() + }) + + afterEach(async () => { + await snapshot.restore() + }) + + describe("addVault", () => { + context("when caller is not a governance account", () => { + it("should revert when adding a vault", async () => { + await expect( + acreRouter.connect(thirdPartySigner).addVault(vault1), + ).to.be.revertedWithCustomError( + acreRouter, + "OwnableUnauthorizedAccount", + ) + }) + }) + + context("when caller is a governance account", () => { + it("should be able to add vaults", async () => { + await acreRouter.connect(governanceSigner).addVault(vault1) + await acreRouter.connect(governanceSigner).addVault(vault2) + await acreRouter.connect(governanceSigner).addVault(vault3) + + expect(await acreRouter.vaults(0)).to.equal(vault1) + const isVault1Approved = await acreRouter.vaultsInfo(vault1) + expect(isVault1Approved).to.equal(true) + + expect(await acreRouter.vaults(1)).to.equal(vault2) + const isVault2Approved = await acreRouter.vaultsInfo(vault1) + expect(isVault2Approved).to.equal(true) + + expect(await acreRouter.vaults(2)).to.equal(vault3) + const isVault3Approved = await acreRouter.vaultsInfo(vault1) + expect(isVault3Approved).to.equal(true) + }) + + it("should not be able to add the same vault twice", async () => { + await acreRouter.connect(governanceSigner).addVault(vault1) + await expect( + acreRouter.connect(governanceSigner).addVault(vault1), + ).to.be.revertedWith("Vault already approved") + }) + + it("should emit an event when adding a vault", async () => { + await expect(acreRouter.connect(governanceSigner).addVault(vault1)) + .to.emit(acreRouter, "VaultAdded") + .withArgs(vault1) + }) + }) + }) + + describe("removeVault", () => { + beforeEach(async () => { + await acreRouter.connect(governanceSigner).addVault(vault1) + await acreRouter.connect(governanceSigner).addVault(vault2) + await acreRouter.connect(governanceSigner).addVault(vault3) + }) + + context("when caller is not a governance account", () => { + it("should revert when adding a vault", async () => { + await expect( + acreRouter.connect(thirdPartySigner).removeVault(vault1), + ).to.be.revertedWithCustomError( + acreRouter, + "OwnableUnauthorizedAccount", + ) + }) + }) + + context("when caller is a governance account", () => { + it("should be able to remove vaults", async () => { + await acreRouter.connect(governanceSigner).removeVault(vault1) + + // Last vault replaced the first vault in the 'vaults' array + expect(await acreRouter.vaults(0)).to.equal(vault3) + const isVault1Approved = await acreRouter.vaultsInfo(vault1) + expect(isVault1Approved).to.equal(false) + expect(await acreRouter.vaultsLength()).to.equal(2) + + await acreRouter.connect(governanceSigner).removeVault(vault2) + + // Last vault (vault2) was removed from the 'vaults' array + expect(await acreRouter.vaults(0)).to.equal(vault3) + expect(await acreRouter.vaultsLength()).to.equal(1) + const isVault2Approved = await acreRouter.vaultsInfo(vault2) + expect(isVault2Approved).to.equal(false) + + await acreRouter.connect(governanceSigner).removeVault(vault3) + expect(await acreRouter.vaultsLength()).to.equal(0) + const isVault3Approved = await acreRouter.vaultsInfo(vault3) + expect(isVault3Approved).to.equal(false) + }) + + it("should not be able to remove a vault that is not approved", async () => { + await expect( + acreRouter.connect(governanceSigner).removeVault(vault4), + ).to.be.revertedWith("Not a vault") + }) + + it("should emit an event when removing a vault", async () => { + await expect(acreRouter.connect(governanceSigner).removeVault(vault1)) + .to.emit(acreRouter, "VaultRemoved") + .withArgs(vault1) + }) + }) + }) +}) From cd91d0719ed5f987a5c24f1996dd28a4b9e81d1d Mon Sep 17 00:00:00 2001 From: Jakub Nowakowski Date: Tue, 5 Dec 2023 17:21:06 +0100 Subject: [PATCH 05/25] Add helpers to get deployed contract Get contract deployed with hardhat deployment scripts to use them in tests. --- core/test/helpers/context.ts | 15 +++++++++++++++ core/test/helpers/contract.ts | 22 ++++++++++++++++++++++ 2 files changed, 37 insertions(+) create mode 100644 core/test/helpers/context.ts create mode 100644 core/test/helpers/contract.ts diff --git a/core/test/helpers/context.ts b/core/test/helpers/context.ts new file mode 100644 index 000000000..7131b8264 --- /dev/null +++ b/core/test/helpers/context.ts @@ -0,0 +1,15 @@ +import { deployments } from "hardhat" + +import { getDeployedContract } from "./contract" + +import type { Acre, TestERC20 } from "../../typechain" + +// eslint-disable-next-line import/prefer-default-export +export async function deployment() { + await deployments.fixture() + + const tbtc: TestERC20 = await getDeployedContract("TBTC") + const acre: Acre = await getDeployedContract("Acre") + + return { tbtc, acre } +} diff --git a/core/test/helpers/contract.ts b/core/test/helpers/contract.ts new file mode 100644 index 000000000..88a83812a --- /dev/null +++ b/core/test/helpers/contract.ts @@ -0,0 +1,22 @@ +import { ethers } from "ethers" +import { deployments } from "hardhat" + +import type { BaseContract } from "ethers" +import { getUnnamedSigner } from "./signer" + +/** + * Get instance of a contract from Hardhat Deployments. + * @param deploymentName Name of the contract deployment. + * @returns Deployed Ethers contract instance. + */ +// eslint-disable-next-line import/prefer-default-export +export async function getDeployedContract( + deploymentName: string, +): Promise { + const { address, abi } = await deployments.get(deploymentName) + + // Use default unnamed signer from index 0 to initialize the contract runner. + const [defaultSigner] = await getUnnamedSigner() + + return new ethers.BaseContract(address, abi, defaultSigner) as T +} From 4bf5442fa800927e8b0140b9a418d906230fe649 Mon Sep 17 00:00:00 2001 From: Jakub Nowakowski Date: Tue, 5 Dec 2023 17:21:47 +0100 Subject: [PATCH 06/25] Add helpers for hardhat signers Hardhat have named signers that can be defined in the hardhat.config.ts file and unnamed signers. We need to use them separately in tests. --- core/test/helpers/index.ts | 3 +++ core/test/helpers/signer.ts | 37 +++++++++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+) create mode 100644 core/test/helpers/index.ts create mode 100644 core/test/helpers/signer.ts diff --git a/core/test/helpers/index.ts b/core/test/helpers/index.ts new file mode 100644 index 000000000..27ddcb0b9 --- /dev/null +++ b/core/test/helpers/index.ts @@ -0,0 +1,3 @@ +export * from "./context" +export * from "./contract" +export * from "./signer" diff --git a/core/test/helpers/signer.ts b/core/test/helpers/signer.ts new file mode 100644 index 000000000..c6fea1c0e --- /dev/null +++ b/core/test/helpers/signer.ts @@ -0,0 +1,37 @@ +import { ethers, getNamedAccounts, getUnnamedAccounts } from "hardhat" + +import type { HardhatEthersSigner } from "@nomicfoundation/hardhat-ethers/signers" + +/** + * Get named Hardhat Ethers Signers. + * @returns Map of named Hardhat Ethers Signers. + */ +export async function getNamedSigner(): Promise<{ + [name: string]: HardhatEthersSigner +}> { + const namedSigners: { [name: string]: HardhatEthersSigner } = {} + + await Promise.all( + Object.entries(await getNamedAccounts()).map(async ([name, address]) => { + namedSigners[name] = await ethers.getSigner(address) + }), + ) + + return namedSigners +} + +/** + * Get unnamed Hardhat Ethers Signers. + * @returns Array of unnamed Hardhat Ethers Signers. + */ +export async function getUnnamedSigner(): Promise { + const unnamedSigners: HardhatEthersSigner[] = [] + + await Promise.all( + (await getUnnamedAccounts()).map(async (address) => { + unnamedSigners.push(await ethers.getSigner(address)) + }), + ) + + return unnamedSigners +} From 16676e9620c86e1c03a4d8157a733fd3743f0115 Mon Sep 17 00:00:00 2001 From: Jakub Nowakowski Date: Tue, 5 Dec 2023 17:24:27 +0100 Subject: [PATCH 07/25] Improve test fixture used in Acre test - use Acre and TBTC contracts deployed with hardhat dpeloyment scripts - use unnamed signers as stakers, to not overlap with named signers (deployer and governance) --- core/test/Acre.test.ts | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/core/test/Acre.test.ts b/core/test/Acre.test.ts index 74e764ca0..4a5960e3e 100644 --- a/core/test/Acre.test.ts +++ b/core/test/Acre.test.ts @@ -1,23 +1,23 @@ import { - SnapshotRestorer, - loadFixture, takeSnapshot, + loadFixture, } from "@nomicfoundation/hardhat-toolbox/network-helpers" -import { HardhatEthersSigner } from "@nomicfoundation/hardhat-ethers/signers" -import { ethers } from "hardhat" import { expect } from "chai" import { ContractTransactionResponse, ZeroAddress } from "ethers" -import type { TestERC20, Acre } from "../typechain" + +import type { HardhatEthersSigner } from "@nomicfoundation/hardhat-ethers/signers" +import type { SnapshotRestorer } from "@nomicfoundation/hardhat-toolbox/network-helpers" +import { deployment } from "./helpers/context" +import { getUnnamedSigner } from "./helpers/signer" + import { to1e18 } from "./utils" -async function acreFixture() { - const [staker1, staker2] = await ethers.getSigners() +import type { Acre, TestERC20 } from "../typechain" - const TestERC20 = await ethers.getContractFactory("TestERC20") - const tbtc = await TestERC20.deploy() +async function fixture() { + const { tbtc, acre } = await deployment() - const Acre = await ethers.getContractFactory("Acre") - const acre = await Acre.deploy(await tbtc.getAddress()) + const [staker1, staker2] = await getUnnamedSigner() const amountToMint = to1e18(100000) tbtc.mint(staker1, amountToMint) @@ -33,7 +33,7 @@ describe("Acre", () => { let staker2: HardhatEthersSigner before(async () => { - ;({ acre, tbtc, staker1, staker2 } = await loadFixture(acreFixture)) + ;({ acre, tbtc, staker1, staker2 } = await loadFixture(fixture)) }) describe("stake", () => { From 9a9c5137224e4450f1854c83e787db63314778b3 Mon Sep 17 00:00:00 2001 From: Dmitry Date: Thu, 7 Dec 2023 15:34:39 +0100 Subject: [PATCH 08/25] Adding AcreRouter to deployed context --- core/test/helpers/context.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/core/test/helpers/context.ts b/core/test/helpers/context.ts index 7131b8264..ec4c56cbd 100644 --- a/core/test/helpers/context.ts +++ b/core/test/helpers/context.ts @@ -2,7 +2,7 @@ import { deployments } from "hardhat" import { getDeployedContract } from "./contract" -import type { Acre, TestERC20 } from "../../typechain" +import type { Acre, AcreRouter, TestERC20 } from "../../typechain" // eslint-disable-next-line import/prefer-default-export export async function deployment() { @@ -10,6 +10,7 @@ export async function deployment() { const tbtc: TestERC20 = await getDeployedContract("TBTC") const acre: Acre = await getDeployedContract("Acre") + const acreRouter: AcreRouter = await getDeployedContract("AcreRouter") - return { tbtc, acre } + return { tbtc, acre, acreRouter } } From 2ae92ecbc447c6a6fafd6b1edd8ac83f3e15dbfa Mon Sep 17 00:00:00 2001 From: Dmitry Date: Thu, 7 Dec 2023 15:35:22 +0100 Subject: [PATCH 09/25] Adding AcreRouter deployment scripts --- core/deploy/02_deploy_acre_router.ts | 22 +++++++++++++++++++ .../22_transfer_ownership_acre_router.ts | 21 ++++++++++++++++++ 2 files changed, 43 insertions(+) create mode 100644 core/deploy/02_deploy_acre_router.ts create mode 100644 core/deploy/22_transfer_ownership_acre_router.ts diff --git a/core/deploy/02_deploy_acre_router.ts b/core/deploy/02_deploy_acre_router.ts new file mode 100644 index 000000000..be2e45b11 --- /dev/null +++ b/core/deploy/02_deploy_acre_router.ts @@ -0,0 +1,22 @@ +import type { HardhatRuntimeEnvironment } from "hardhat/types" +import type { DeployFunction } from "hardhat-deploy/types" + +const func: DeployFunction = async (hre: HardhatRuntimeEnvironment) => { + const { getNamedAccounts, deployments } = hre + const { deployer } = await getNamedAccounts() + + await deployments.deploy("AcreRouter", { + from: deployer, + args: [], + log: true, + waitConfirmations: 1, + }) + + // TODO: Add Etherscan verification + // TODO: Add Tenderly verification +} + +export default func + +func.tags = ["AcreRouter"] +func.dependencies = ["Acre"] diff --git a/core/deploy/22_transfer_ownership_acre_router.ts b/core/deploy/22_transfer_ownership_acre_router.ts new file mode 100644 index 000000000..7652e04d2 --- /dev/null +++ b/core/deploy/22_transfer_ownership_acre_router.ts @@ -0,0 +1,21 @@ +import type { HardhatRuntimeEnvironment } from "hardhat/types" +import type { DeployFunction } from "hardhat-deploy/types" + +const func: DeployFunction = async (hre: HardhatRuntimeEnvironment) => { + const { getNamedAccounts, deployments } = hre + const { deployer, governance } = await getNamedAccounts() + const { log } = deployments + + log(`transferring ownership of AcreRouter contract to ${governance}`) + + await deployments.execute( + "AcreRouter", + { from: deployer, log: true, waitConfirmations: 1 }, + "transferOwnership", + governance, + ) +} + +export default func + +func.tags = ["TransferOwnershipAcreRouter"] From fd4a2879f59ef8541e995dea24631460a2df5fef Mon Sep 17 00:00:00 2001 From: Dmitry Date: Thu, 7 Dec 2023 15:36:13 +0100 Subject: [PATCH 10/25] Refactoring tests adjusting for helper methods --- core/test/AcreRouter.test.ts | 57 ++++++++++++++++-------------------- 1 file changed, 26 insertions(+), 31 deletions(-) diff --git a/core/test/AcreRouter.test.ts b/core/test/AcreRouter.test.ts index af3e358b9..a457b9b16 100644 --- a/core/test/AcreRouter.test.ts +++ b/core/test/AcreRouter.test.ts @@ -1,4 +1,4 @@ -import { ethers, getUnnamedAccounts, getNamedAccounts } from "hardhat" +import { getUnnamedAccounts } from "hardhat" import { HardhatEthersSigner } from "@nomicfoundation/hardhat-ethers/signers" import { Address } from "hardhat-deploy/types" import { expect } from "chai" @@ -7,33 +7,28 @@ import { takeSnapshot, } from "@nomicfoundation/hardhat-toolbox/network-helpers" import type { AcreRouter } from "../typechain" +import { deployment } from "./helpers/context" +import { getNamedSigner, getUnnamedSigner } from "./helpers/signer" describe("AcreRouter", () => { let snapshot: SnapshotRestorer let acreRouter: AcreRouter - let deployerSigner: HardhatEthersSigner - let governanceSigner: HardhatEthersSigner - let thirdPartySigner: HardhatEthersSigner + let governance: HardhatEthersSigner + let thirdParty: HardhatEthersSigner let vault1: Address let vault2: Address let vault3: Address let vault4: Address - // TODO: - // Put some of this setup to deployment scripts and/or helpers e.g. ownership - // transfer, acreRouter deployment etc. See: https://github.com/thesis/acre/pull/58 - // and https://github.com/thesis/acre/pull/64 before(async () => { const accounts = await getUnnamedAccounts() ;[vault1, vault2, vault3, vault4] = accounts - const { deployer, governance } = await getNamedAccounts() - deployerSigner = await ethers.getSigner(deployer) - governanceSigner = await ethers.getSigner(governance) - thirdPartySigner = await ethers.getSigner(accounts[0]) - const AcreRouter = await ethers.getContractFactory("AcreRouter") - acreRouter = await AcreRouter.connect(deployerSigner).deploy() - acreRouter.transferOwnership(governance) + + governance = (await getNamedSigner()).governance + ;[thirdParty] = await getUnnamedSigner() + + acreRouter = (await deployment()).acreRouter }) beforeEach(async () => { @@ -48,7 +43,7 @@ describe("AcreRouter", () => { context("when caller is not a governance account", () => { it("should revert when adding a vault", async () => { await expect( - acreRouter.connect(thirdPartySigner).addVault(vault1), + acreRouter.connect(thirdParty).addVault(vault1), ).to.be.revertedWithCustomError( acreRouter, "OwnableUnauthorizedAccount", @@ -58,9 +53,9 @@ describe("AcreRouter", () => { context("when caller is a governance account", () => { it("should be able to add vaults", async () => { - await acreRouter.connect(governanceSigner).addVault(vault1) - await acreRouter.connect(governanceSigner).addVault(vault2) - await acreRouter.connect(governanceSigner).addVault(vault3) + await acreRouter.connect(governance).addVault(vault1) + await acreRouter.connect(governance).addVault(vault2) + await acreRouter.connect(governance).addVault(vault3) expect(await acreRouter.vaults(0)).to.equal(vault1) const isVault1Approved = await acreRouter.vaultsInfo(vault1) @@ -76,14 +71,14 @@ describe("AcreRouter", () => { }) it("should not be able to add the same vault twice", async () => { - await acreRouter.connect(governanceSigner).addVault(vault1) + await acreRouter.connect(governance).addVault(vault1) await expect( - acreRouter.connect(governanceSigner).addVault(vault1), + acreRouter.connect(governance).addVault(vault1), ).to.be.revertedWith("Vault already approved") }) it("should emit an event when adding a vault", async () => { - await expect(acreRouter.connect(governanceSigner).addVault(vault1)) + await expect(acreRouter.connect(governance).addVault(vault1)) .to.emit(acreRouter, "VaultAdded") .withArgs(vault1) }) @@ -92,15 +87,15 @@ describe("AcreRouter", () => { describe("removeVault", () => { beforeEach(async () => { - await acreRouter.connect(governanceSigner).addVault(vault1) - await acreRouter.connect(governanceSigner).addVault(vault2) - await acreRouter.connect(governanceSigner).addVault(vault3) + await acreRouter.connect(governance).addVault(vault1) + await acreRouter.connect(governance).addVault(vault2) + await acreRouter.connect(governance).addVault(vault3) }) context("when caller is not a governance account", () => { it("should revert when adding a vault", async () => { await expect( - acreRouter.connect(thirdPartySigner).removeVault(vault1), + acreRouter.connect(thirdParty).removeVault(vault1), ).to.be.revertedWithCustomError( acreRouter, "OwnableUnauthorizedAccount", @@ -110,7 +105,7 @@ describe("AcreRouter", () => { context("when caller is a governance account", () => { it("should be able to remove vaults", async () => { - await acreRouter.connect(governanceSigner).removeVault(vault1) + await acreRouter.connect(governance).removeVault(vault1) // Last vault replaced the first vault in the 'vaults' array expect(await acreRouter.vaults(0)).to.equal(vault3) @@ -118,7 +113,7 @@ describe("AcreRouter", () => { expect(isVault1Approved).to.equal(false) expect(await acreRouter.vaultsLength()).to.equal(2) - await acreRouter.connect(governanceSigner).removeVault(vault2) + await acreRouter.connect(governance).removeVault(vault2) // Last vault (vault2) was removed from the 'vaults' array expect(await acreRouter.vaults(0)).to.equal(vault3) @@ -126,7 +121,7 @@ describe("AcreRouter", () => { const isVault2Approved = await acreRouter.vaultsInfo(vault2) expect(isVault2Approved).to.equal(false) - await acreRouter.connect(governanceSigner).removeVault(vault3) + await acreRouter.connect(governance).removeVault(vault3) expect(await acreRouter.vaultsLength()).to.equal(0) const isVault3Approved = await acreRouter.vaultsInfo(vault3) expect(isVault3Approved).to.equal(false) @@ -134,12 +129,12 @@ describe("AcreRouter", () => { it("should not be able to remove a vault that is not approved", async () => { await expect( - acreRouter.connect(governanceSigner).removeVault(vault4), + acreRouter.connect(governance).removeVault(vault4), ).to.be.revertedWith("Not a vault") }) it("should emit an event when removing a vault", async () => { - await expect(acreRouter.connect(governanceSigner).removeVault(vault1)) + await expect(acreRouter.connect(governance).removeVault(vault1)) .to.emit(acreRouter, "VaultRemoved") .withArgs(vault1) }) From 1c855459606963149363d1c81bbafc940ad9192b Mon Sep 17 00:00:00 2001 From: Dmitry Date: Thu, 7 Dec 2023 16:29:00 +0100 Subject: [PATCH 11/25] Adding func-visibility rule --- core/.solhint.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/core/.solhint.json b/core/.solhint.json index f54af0b9a..435aa7886 100644 --- a/core/.solhint.json +++ b/core/.solhint.json @@ -1,4 +1,5 @@ { "extends": "thesis", - "plugins": [] + "plugins": [], + "rules": { "func-visibility": ["error", { "ignoreConstructors": true }] } } From 2bbf761c41e06f66c75b0ced597724e939377f04 Mon Sep 17 00:00:00 2001 From: Dmitry Date: Thu, 7 Dec 2023 16:29:34 +0100 Subject: [PATCH 12/25] Referencing correct vault names in tests --- core/test/AcreRouter.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/test/AcreRouter.test.ts b/core/test/AcreRouter.test.ts index a457b9b16..e19b68f07 100644 --- a/core/test/AcreRouter.test.ts +++ b/core/test/AcreRouter.test.ts @@ -62,11 +62,11 @@ describe("AcreRouter", () => { expect(isVault1Approved).to.equal(true) expect(await acreRouter.vaults(1)).to.equal(vault2) - const isVault2Approved = await acreRouter.vaultsInfo(vault1) + const isVault2Approved = await acreRouter.vaultsInfo(vault2) expect(isVault2Approved).to.equal(true) expect(await acreRouter.vaults(2)).to.equal(vault3) - const isVault3Approved = await acreRouter.vaultsInfo(vault1) + const isVault3Approved = await acreRouter.vaultsInfo(vault3) expect(isVault3Approved).to.equal(true) }) From 3a05a45e3ab62f29ad113330c3ee8b36bf827dd8 Mon Sep 17 00:00:00 2001 From: Jakub Nowakowski Date: Fri, 8 Dec 2023 13:32:35 +0100 Subject: [PATCH 13/25] Replace realpath with another resolution As per https://github.com/thesis/acre/pull/58#discussion_r1418754955 realpath is not available out of the box on OSX. Instead of installing additional package, we changed the way the path is resolved. --- core/scripts/fetch_external_artifacts.sh | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/core/scripts/fetch_external_artifacts.sh b/core/scripts/fetch_external_artifacts.sh index 5c043568e..c75c98122 100755 --- a/core/scripts/fetch_external_artifacts.sh +++ b/core/scripts/fetch_external_artifacts.sh @@ -1,7 +1,10 @@ #! /bin/bash set -eou pipefail -ROOT_DIR="$(realpath "$(dirname $0)/../")" +ROOT_DIR=$( + cd "$(dirname $0)/../" + pwd -P +) TMP_DIR=${ROOT_DIR}/tmp/external-artifacts EXTERNAL_ARTIFACTS_DIR=${ROOT_DIR}/external From aa133deee055d45fee2659f66052c50a19b83e5a Mon Sep 17 00:00:00 2001 From: Dmitry Date: Mon, 11 Dec 2023 13:31:09 +0100 Subject: [PATCH 14/25] Removing a rule that already exist in parent config --- core/.solhint.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/.solhint.json b/core/.solhint.json index 435aa7886..7afe6405c 100644 --- a/core/.solhint.json +++ b/core/.solhint.json @@ -1,5 +1,5 @@ { "extends": "thesis", "plugins": [], - "rules": { "func-visibility": ["error", { "ignoreConstructors": true }] } + "rules": {} } From 9e0ffd9959bca489a23f44f710f0fda236d7bf4b Mon Sep 17 00:00:00 2001 From: Dmitry Date: Mon, 11 Dec 2023 13:36:23 +0100 Subject: [PATCH 15/25] Renaming AcreRouter -> Dispatcher --- .../{AcreRouter.sol => Dispatcher.sol} | 6 +- core/deploy/02_deploy_acre_router.ts | 4 +- .../22_transfer_ownership_acre_router.ts | 4 +- ...{AcreRouter.test.ts => Dispatcher.test.ts} | 76 +++++++++---------- core/test/helpers/context.ts | 6 +- 5 files changed, 48 insertions(+), 48 deletions(-) rename core/contracts/{AcreRouter.sol => Dispatcher.sol} (94%) rename core/test/{AcreRouter.test.ts => Dispatcher.test.ts} (57%) diff --git a/core/contracts/AcreRouter.sol b/core/contracts/Dispatcher.sol similarity index 94% rename from core/contracts/AcreRouter.sol rename to core/contracts/Dispatcher.sol index c7224e324..02801b743 100644 --- a/core/contracts/AcreRouter.sol +++ b/core/contracts/Dispatcher.sol @@ -3,11 +3,11 @@ pragma solidity ^0.8.21; import "@openzeppelin/contracts/access/Ownable.sol"; -/// @title AcreRouter -/// @notice AcreRouter is a contract that routes TBTC from stBTC (Acre) to +/// @title Dispatcher +/// @notice Dispatcher is a contract that routes TBTC from stBTC (Acre) to /// a given vault and back. Vaults supply yield strategies with TBTC that /// generate yield for Bitcoin holders. -contract AcreRouter is Ownable { +contract Dispatcher is Ownable { struct Vault { bool approved; } diff --git a/core/deploy/02_deploy_acre_router.ts b/core/deploy/02_deploy_acre_router.ts index be2e45b11..bf99d4d73 100644 --- a/core/deploy/02_deploy_acre_router.ts +++ b/core/deploy/02_deploy_acre_router.ts @@ -5,7 +5,7 @@ const func: DeployFunction = async (hre: HardhatRuntimeEnvironment) => { const { getNamedAccounts, deployments } = hre const { deployer } = await getNamedAccounts() - await deployments.deploy("AcreRouter", { + await deployments.deploy("Dispatcher", { from: deployer, args: [], log: true, @@ -18,5 +18,5 @@ const func: DeployFunction = async (hre: HardhatRuntimeEnvironment) => { export default func -func.tags = ["AcreRouter"] +func.tags = ["Dispatcher"] func.dependencies = ["Acre"] diff --git a/core/deploy/22_transfer_ownership_acre_router.ts b/core/deploy/22_transfer_ownership_acre_router.ts index 7652e04d2..2d7748580 100644 --- a/core/deploy/22_transfer_ownership_acre_router.ts +++ b/core/deploy/22_transfer_ownership_acre_router.ts @@ -6,10 +6,10 @@ const func: DeployFunction = async (hre: HardhatRuntimeEnvironment) => { const { deployer, governance } = await getNamedAccounts() const { log } = deployments - log(`transferring ownership of AcreRouter contract to ${governance}`) + log(`transferring ownership of Dispatcher contract to ${governance}`) await deployments.execute( - "AcreRouter", + "Dispatcher", { from: deployer, log: true, waitConfirmations: 1 }, "transferOwnership", governance, diff --git a/core/test/AcreRouter.test.ts b/core/test/Dispatcher.test.ts similarity index 57% rename from core/test/AcreRouter.test.ts rename to core/test/Dispatcher.test.ts index e19b68f07..c9037b850 100644 --- a/core/test/AcreRouter.test.ts +++ b/core/test/Dispatcher.test.ts @@ -6,14 +6,14 @@ import { SnapshotRestorer, takeSnapshot, } from "@nomicfoundation/hardhat-toolbox/network-helpers" -import type { AcreRouter } from "../typechain" +import type { Dispatcher } from "../typechain" import { deployment } from "./helpers/context" import { getNamedSigner, getUnnamedSigner } from "./helpers/signer" -describe("AcreRouter", () => { +describe("Dispatcher", () => { let snapshot: SnapshotRestorer - let acreRouter: AcreRouter + let dispatcher: Dispatcher let governance: HardhatEthersSigner let thirdParty: HardhatEthersSigner let vault1: Address @@ -28,7 +28,7 @@ describe("AcreRouter", () => { governance = (await getNamedSigner()).governance ;[thirdParty] = await getUnnamedSigner() - acreRouter = (await deployment()).acreRouter + dispatcher = (await deployment()).dispatcher }) beforeEach(async () => { @@ -43,9 +43,9 @@ describe("AcreRouter", () => { context("when caller is not a governance account", () => { it("should revert when adding a vault", async () => { await expect( - acreRouter.connect(thirdParty).addVault(vault1), + dispatcher.connect(thirdParty).addVault(vault1), ).to.be.revertedWithCustomError( - acreRouter, + dispatcher, "OwnableUnauthorizedAccount", ) }) @@ -53,33 +53,33 @@ describe("AcreRouter", () => { context("when caller is a governance account", () => { it("should be able to add vaults", async () => { - await acreRouter.connect(governance).addVault(vault1) - await acreRouter.connect(governance).addVault(vault2) - await acreRouter.connect(governance).addVault(vault3) + await dispatcher.connect(governance).addVault(vault1) + await dispatcher.connect(governance).addVault(vault2) + await dispatcher.connect(governance).addVault(vault3) - expect(await acreRouter.vaults(0)).to.equal(vault1) - const isVault1Approved = await acreRouter.vaultsInfo(vault1) + expect(await dispatcher.vaults(0)).to.equal(vault1) + const isVault1Approved = await dispatcher.vaultsInfo(vault1) expect(isVault1Approved).to.equal(true) - expect(await acreRouter.vaults(1)).to.equal(vault2) - const isVault2Approved = await acreRouter.vaultsInfo(vault2) + expect(await dispatcher.vaults(1)).to.equal(vault2) + const isVault2Approved = await dispatcher.vaultsInfo(vault2) expect(isVault2Approved).to.equal(true) - expect(await acreRouter.vaults(2)).to.equal(vault3) - const isVault3Approved = await acreRouter.vaultsInfo(vault3) + expect(await dispatcher.vaults(2)).to.equal(vault3) + const isVault3Approved = await dispatcher.vaultsInfo(vault3) expect(isVault3Approved).to.equal(true) }) it("should not be able to add the same vault twice", async () => { - await acreRouter.connect(governance).addVault(vault1) + await dispatcher.connect(governance).addVault(vault1) await expect( - acreRouter.connect(governance).addVault(vault1), + dispatcher.connect(governance).addVault(vault1), ).to.be.revertedWith("Vault already approved") }) it("should emit an event when adding a vault", async () => { - await expect(acreRouter.connect(governance).addVault(vault1)) - .to.emit(acreRouter, "VaultAdded") + await expect(dispatcher.connect(governance).addVault(vault1)) + .to.emit(dispatcher, "VaultAdded") .withArgs(vault1) }) }) @@ -87,17 +87,17 @@ describe("AcreRouter", () => { describe("removeVault", () => { beforeEach(async () => { - await acreRouter.connect(governance).addVault(vault1) - await acreRouter.connect(governance).addVault(vault2) - await acreRouter.connect(governance).addVault(vault3) + await dispatcher.connect(governance).addVault(vault1) + await dispatcher.connect(governance).addVault(vault2) + await dispatcher.connect(governance).addVault(vault3) }) context("when caller is not a governance account", () => { it("should revert when adding a vault", async () => { await expect( - acreRouter.connect(thirdParty).removeVault(vault1), + dispatcher.connect(thirdParty).removeVault(vault1), ).to.be.revertedWithCustomError( - acreRouter, + dispatcher, "OwnableUnauthorizedAccount", ) }) @@ -105,37 +105,37 @@ describe("AcreRouter", () => { context("when caller is a governance account", () => { it("should be able to remove vaults", async () => { - await acreRouter.connect(governance).removeVault(vault1) + await dispatcher.connect(governance).removeVault(vault1) // Last vault replaced the first vault in the 'vaults' array - expect(await acreRouter.vaults(0)).to.equal(vault3) - const isVault1Approved = await acreRouter.vaultsInfo(vault1) + expect(await dispatcher.vaults(0)).to.equal(vault3) + const isVault1Approved = await dispatcher.vaultsInfo(vault1) expect(isVault1Approved).to.equal(false) - expect(await acreRouter.vaultsLength()).to.equal(2) + expect(await dispatcher.vaultsLength()).to.equal(2) - await acreRouter.connect(governance).removeVault(vault2) + await dispatcher.connect(governance).removeVault(vault2) // Last vault (vault2) was removed from the 'vaults' array - expect(await acreRouter.vaults(0)).to.equal(vault3) - expect(await acreRouter.vaultsLength()).to.equal(1) - const isVault2Approved = await acreRouter.vaultsInfo(vault2) + expect(await dispatcher.vaults(0)).to.equal(vault3) + expect(await dispatcher.vaultsLength()).to.equal(1) + const isVault2Approved = await dispatcher.vaultsInfo(vault2) expect(isVault2Approved).to.equal(false) - await acreRouter.connect(governance).removeVault(vault3) - expect(await acreRouter.vaultsLength()).to.equal(0) - const isVault3Approved = await acreRouter.vaultsInfo(vault3) + await dispatcher.connect(governance).removeVault(vault3) + expect(await dispatcher.vaultsLength()).to.equal(0) + const isVault3Approved = await dispatcher.vaultsInfo(vault3) expect(isVault3Approved).to.equal(false) }) it("should not be able to remove a vault that is not approved", async () => { await expect( - acreRouter.connect(governance).removeVault(vault4), + dispatcher.connect(governance).removeVault(vault4), ).to.be.revertedWith("Not a vault") }) it("should emit an event when removing a vault", async () => { - await expect(acreRouter.connect(governance).removeVault(vault1)) - .to.emit(acreRouter, "VaultRemoved") + await expect(dispatcher.connect(governance).removeVault(vault1)) + .to.emit(dispatcher, "VaultRemoved") .withArgs(vault1) }) }) diff --git a/core/test/helpers/context.ts b/core/test/helpers/context.ts index ec4c56cbd..34875e43d 100644 --- a/core/test/helpers/context.ts +++ b/core/test/helpers/context.ts @@ -2,7 +2,7 @@ import { deployments } from "hardhat" import { getDeployedContract } from "./contract" -import type { Acre, AcreRouter, TestERC20 } from "../../typechain" +import type { Acre, Dispatcher, TestERC20 } from "../../typechain" // eslint-disable-next-line import/prefer-default-export export async function deployment() { @@ -10,7 +10,7 @@ export async function deployment() { const tbtc: TestERC20 = await getDeployedContract("TBTC") const acre: Acre = await getDeployedContract("Acre") - const acreRouter: AcreRouter = await getDeployedContract("AcreRouter") + const dispatcher: Dispatcher = await getDeployedContract("Dispatcher") - return { tbtc, acre, acreRouter } + return { tbtc, acre, dispatcher } } From b56c0ec251325348d9be63e746480e3ca4d0df4b Mon Sep 17 00:00:00 2001 From: Dmitry Date: Mon, 11 Dec 2023 13:50:27 +0100 Subject: [PATCH 16/25] Changing a comment for vaults array --- core/contracts/Dispatcher.sol | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/core/contracts/Dispatcher.sol b/core/contracts/Dispatcher.sol index 02801b743..ff2bddd18 100644 --- a/core/contracts/Dispatcher.sol +++ b/core/contracts/Dispatcher.sol @@ -12,12 +12,12 @@ contract Dispatcher is Ownable { bool approved; } - /// @notice Approved vaults within the Yiern Modules that implement ERC4626 - /// standard. These vaults deposit assets to yield strategies, e.g. - /// Uniswap V3 WBTC/TBTC pool. Vault can be a part of Acre ecosystem - /// or can be implemented externally. As long as it complies with - /// ERC4626 standard and is approved by the owner it can be - /// plugged into Acre. + /// @notice Approved Yield Vaults that implement ERC4626 standard. These + /// vaults deposit assets to yield strategies, e.g. Uniswap V3 + /// WBTC/TBTC pool. Vault can be a part of Acre ecosystem or can be + /// implemented externally. As long as it complies with ERC4626 + /// standard and is approved by the owner it can be plugged into + /// Acre. address[] public vaults; mapping(address => Vault) public vaultsInfo; From d7e8ac65a3a2bb5e9d42062ff5001e0690dfb75b Mon Sep 17 00:00:00 2001 From: Dmitry Date: Mon, 11 Dec 2023 13:53:09 +0100 Subject: [PATCH 17/25] Renaming Vault -> VaultInfo struct --- core/contracts/Dispatcher.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/contracts/Dispatcher.sol b/core/contracts/Dispatcher.sol index ff2bddd18..fb9dfdca0 100644 --- a/core/contracts/Dispatcher.sol +++ b/core/contracts/Dispatcher.sol @@ -8,7 +8,7 @@ import "@openzeppelin/contracts/access/Ownable.sol"; /// a given vault and back. Vaults supply yield strategies with TBTC that /// generate yield for Bitcoin holders. contract Dispatcher is Ownable { - struct Vault { + struct VaultInfo { bool approved; } @@ -19,7 +19,7 @@ contract Dispatcher is Ownable { /// standard and is approved by the owner it can be plugged into /// Acre. address[] public vaults; - mapping(address => Vault) public vaultsInfo; + mapping(address => VaultInfo) public vaultsInfo; event VaultAdded(address indexed vault); event VaultRemoved(address indexed vault); From 01086fce2f4abda3c67f5a1d01502a5fc9ea7711 Mon Sep 17 00:00:00 2001 From: Dmitry Date: Mon, 11 Dec 2023 14:09:02 +0100 Subject: [PATCH 18/25] Approve->authorize for vaults and functions around it --- core/contracts/Dispatcher.sol | 22 ++++++------ core/test/Dispatcher.test.ts | 64 +++++++++++++++++------------------ 2 files changed, 43 insertions(+), 43 deletions(-) diff --git a/core/contracts/Dispatcher.sol b/core/contracts/Dispatcher.sol index fb9dfdca0..ed5a0b4d6 100644 --- a/core/contracts/Dispatcher.sol +++ b/core/contracts/Dispatcher.sol @@ -9,14 +9,14 @@ import "@openzeppelin/contracts/access/Ownable.sol"; /// generate yield for Bitcoin holders. contract Dispatcher is Ownable { struct VaultInfo { - bool approved; + bool authorized; } - /// @notice Approved Yield Vaults that implement ERC4626 standard. These + /// @notice Authorized Yield Vaults that implement ERC4626 standard. These /// vaults deposit assets to yield strategies, e.g. Uniswap V3 /// WBTC/TBTC pool. Vault can be a part of Acre ecosystem or can be /// implemented externally. As long as it complies with ERC4626 - /// standard and is approved by the owner it can be plugged into + /// standard and is authorized by the owner it can be plugged into /// Acre. address[] public vaults; mapping(address => VaultInfo) public vaultsInfo; @@ -26,23 +26,23 @@ contract Dispatcher is Ownable { constructor() Ownable(msg.sender) {} - /// @notice Adds a vault to the list of approved vaults. + /// @notice Adds a vault to the list of authorized vaults. /// @param vault Address of the vault to add. - function addVault(address vault) external onlyOwner { - require(!vaultsInfo[vault].approved, "Vault already approved"); + function authorizeVault(address vault) external onlyOwner { + require(!vaultsInfo[vault].authorized, "Vault already authorized"); vaults.push(vault); - vaultsInfo[vault].approved = true; + vaultsInfo[vault].authorized = true; emit VaultAdded(vault); } - /// @notice Removes a vault from the list of approved vaults. + /// @notice Removes a vault from the list of authorized vaults. /// @param vault Address of the vault to remove. - function removeVault(address vault) external onlyOwner { - require(vaultsInfo[vault].approved, "Not a vault"); + function deauthorizeVault(address vault) external onlyOwner { + require(vaultsInfo[vault].authorized, "Not a vault"); - vaultsInfo[vault].approved = false; + vaultsInfo[vault].authorized = false; for (uint256 i = 0; i < vaults.length; i++) { if (vaults[i] == vault) { diff --git a/core/test/Dispatcher.test.ts b/core/test/Dispatcher.test.ts index c9037b850..5c29d54e8 100644 --- a/core/test/Dispatcher.test.ts +++ b/core/test/Dispatcher.test.ts @@ -39,11 +39,11 @@ describe("Dispatcher", () => { await snapshot.restore() }) - describe("addVault", () => { + describe("authorizeVault", () => { context("when caller is not a governance account", () => { it("should revert when adding a vault", async () => { await expect( - dispatcher.connect(thirdParty).addVault(vault1), + dispatcher.connect(thirdParty).authorizeVault(vault1), ).to.be.revertedWithCustomError( dispatcher, "OwnableUnauthorizedAccount", @@ -53,49 +53,49 @@ describe("Dispatcher", () => { context("when caller is a governance account", () => { it("should be able to add vaults", async () => { - await dispatcher.connect(governance).addVault(vault1) - await dispatcher.connect(governance).addVault(vault2) - await dispatcher.connect(governance).addVault(vault3) + await dispatcher.connect(governance).authorizeVault(vault1) + await dispatcher.connect(governance).authorizeVault(vault2) + await dispatcher.connect(governance).authorizeVault(vault3) expect(await dispatcher.vaults(0)).to.equal(vault1) - const isVault1Approved = await dispatcher.vaultsInfo(vault1) - expect(isVault1Approved).to.equal(true) + const isVault1Authorized = await dispatcher.vaultsInfo(vault1) + expect(isVault1Authorized).to.equal(true) expect(await dispatcher.vaults(1)).to.equal(vault2) - const isVault2Approved = await dispatcher.vaultsInfo(vault2) - expect(isVault2Approved).to.equal(true) + const isVault2Authorized = await dispatcher.vaultsInfo(vault2) + expect(isVault2Authorized).to.equal(true) expect(await dispatcher.vaults(2)).to.equal(vault3) - const isVault3Approved = await dispatcher.vaultsInfo(vault3) - expect(isVault3Approved).to.equal(true) + const isVault3Authorized = await dispatcher.vaultsInfo(vault3) + expect(isVault3Authorized).to.equal(true) }) it("should not be able to add the same vault twice", async () => { - await dispatcher.connect(governance).addVault(vault1) + await dispatcher.connect(governance).authorizeVault(vault1) await expect( - dispatcher.connect(governance).addVault(vault1), - ).to.be.revertedWith("Vault already approved") + dispatcher.connect(governance).authorizeVault(vault1), + ).to.be.revertedWith("Vault already authorized") }) it("should emit an event when adding a vault", async () => { - await expect(dispatcher.connect(governance).addVault(vault1)) + await expect(dispatcher.connect(governance).authorizeVault(vault1)) .to.emit(dispatcher, "VaultAdded") .withArgs(vault1) }) }) }) - describe("removeVault", () => { + describe("deauthorizeVault", () => { beforeEach(async () => { - await dispatcher.connect(governance).addVault(vault1) - await dispatcher.connect(governance).addVault(vault2) - await dispatcher.connect(governance).addVault(vault3) + await dispatcher.connect(governance).authorizeVault(vault1) + await dispatcher.connect(governance).authorizeVault(vault2) + await dispatcher.connect(governance).authorizeVault(vault3) }) context("when caller is not a governance account", () => { it("should revert when adding a vault", async () => { await expect( - dispatcher.connect(thirdParty).removeVault(vault1), + dispatcher.connect(thirdParty).deauthorizeVault(vault1), ).to.be.revertedWithCustomError( dispatcher, "OwnableUnauthorizedAccount", @@ -105,36 +105,36 @@ describe("Dispatcher", () => { context("when caller is a governance account", () => { it("should be able to remove vaults", async () => { - await dispatcher.connect(governance).removeVault(vault1) + await dispatcher.connect(governance).deauthorizeVault(vault1) // Last vault replaced the first vault in the 'vaults' array expect(await dispatcher.vaults(0)).to.equal(vault3) - const isVault1Approved = await dispatcher.vaultsInfo(vault1) - expect(isVault1Approved).to.equal(false) + const isVault1Authorized = await dispatcher.vaultsInfo(vault1) + expect(isVault1Authorized).to.equal(false) expect(await dispatcher.vaultsLength()).to.equal(2) - await dispatcher.connect(governance).removeVault(vault2) + await dispatcher.connect(governance).deauthorizeVault(vault2) // Last vault (vault2) was removed from the 'vaults' array expect(await dispatcher.vaults(0)).to.equal(vault3) expect(await dispatcher.vaultsLength()).to.equal(1) - const isVault2Approved = await dispatcher.vaultsInfo(vault2) - expect(isVault2Approved).to.equal(false) + const isVault2Authorized = await dispatcher.vaultsInfo(vault2) + expect(isVault2Authorized).to.equal(false) - await dispatcher.connect(governance).removeVault(vault3) + await dispatcher.connect(governance).deauthorizeVault(vault3) expect(await dispatcher.vaultsLength()).to.equal(0) - const isVault3Approved = await dispatcher.vaultsInfo(vault3) - expect(isVault3Approved).to.equal(false) + const isVault3Authorized = await dispatcher.vaultsInfo(vault3) + expect(isVault3Authorized).to.equal(false) }) - it("should not be able to remove a vault that is not approved", async () => { + it("should not be able to remove a vault that is not authorized", async () => { await expect( - dispatcher.connect(governance).removeVault(vault4), + dispatcher.connect(governance).deauthorizeVault(vault4), ).to.be.revertedWith("Not a vault") }) it("should emit an event when removing a vault", async () => { - await expect(dispatcher.connect(governance).removeVault(vault1)) + await expect(dispatcher.connect(governance).deauthorizeVault(vault1)) .to.emit(dispatcher, "VaultRemoved") .withArgs(vault1) }) From f477e4500a7fffdb1082f666b711be4c92de70c4 Mon Sep 17 00:00:00 2001 From: Dmitry Date: Mon, 11 Dec 2023 15:17:27 +0100 Subject: [PATCH 19/25] Replacing require err messages with custom errors --- core/contracts/Dispatcher.sol | 11 +++++++++-- core/test/Dispatcher.test.ts | 4 ++-- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/core/contracts/Dispatcher.sol b/core/contracts/Dispatcher.sol index ed5a0b4d6..88e14f103 100644 --- a/core/contracts/Dispatcher.sol +++ b/core/contracts/Dispatcher.sol @@ -8,6 +8,9 @@ import "@openzeppelin/contracts/access/Ownable.sol"; /// a given vault and back. Vaults supply yield strategies with TBTC that /// generate yield for Bitcoin holders. contract Dispatcher is Ownable { + error VaultAlreadyAuthorized(); + error VaultUnauthorized(); + struct VaultInfo { bool authorized; } @@ -29,7 +32,9 @@ contract Dispatcher is Ownable { /// @notice Adds a vault to the list of authorized vaults. /// @param vault Address of the vault to add. function authorizeVault(address vault) external onlyOwner { - require(!vaultsInfo[vault].authorized, "Vault already authorized"); + if (vaultsInfo[vault].authorized) { + revert VaultAlreadyAuthorized(); + } vaults.push(vault); vaultsInfo[vault].authorized = true; @@ -40,7 +45,9 @@ contract Dispatcher is Ownable { /// @notice Removes a vault from the list of authorized vaults. /// @param vault Address of the vault to remove. function deauthorizeVault(address vault) external onlyOwner { - require(vaultsInfo[vault].authorized, "Not a vault"); + if (!vaultsInfo[vault].authorized) { + revert VaultUnauthorized(); + } vaultsInfo[vault].authorized = false; diff --git a/core/test/Dispatcher.test.ts b/core/test/Dispatcher.test.ts index 5c29d54e8..2305d34e3 100644 --- a/core/test/Dispatcher.test.ts +++ b/core/test/Dispatcher.test.ts @@ -74,7 +74,7 @@ describe("Dispatcher", () => { await dispatcher.connect(governance).authorizeVault(vault1) await expect( dispatcher.connect(governance).authorizeVault(vault1), - ).to.be.revertedWith("Vault already authorized") + ).to.be.revertedWithCustomError(dispatcher, "VaultAlreadyAuthorized") }) it("should emit an event when adding a vault", async () => { @@ -130,7 +130,7 @@ describe("Dispatcher", () => { it("should not be able to remove a vault that is not authorized", async () => { await expect( dispatcher.connect(governance).deauthorizeVault(vault4), - ).to.be.revertedWith("Not a vault") + ).to.be.revertedWithCustomError(dispatcher, "VaultUnauthorized") }) it("should emit an event when removing a vault", async () => { From d0bd5dd79dc4c8d7fcb1aad6121d0657fcf7ae13 Mon Sep 17 00:00:00 2001 From: Dmitry Date: Mon, 11 Dec 2023 15:28:49 +0100 Subject: [PATCH 20/25] Renaming events and returning the entire vaults array - Renamed VauldAdded->VaultAuthorized and VaultRemoved->VaultDeauthorized events - Returning vaults array instead of vaults array length --- core/contracts/Dispatcher.sol | 12 ++++++------ core/test/Dispatcher.test.ts | 10 +++++----- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/core/contracts/Dispatcher.sol b/core/contracts/Dispatcher.sol index 88e14f103..309da2ae8 100644 --- a/core/contracts/Dispatcher.sol +++ b/core/contracts/Dispatcher.sol @@ -24,8 +24,8 @@ contract Dispatcher is Ownable { address[] public vaults; mapping(address => VaultInfo) public vaultsInfo; - event VaultAdded(address indexed vault); - event VaultRemoved(address indexed vault); + event VaultAuthorized(address indexed vault); + event VaultDeauthorized(address indexed vault); constructor() Ownable(msg.sender) {} @@ -39,7 +39,7 @@ contract Dispatcher is Ownable { vaults.push(vault); vaultsInfo[vault].authorized = true; - emit VaultAdded(vault); + emit VaultAuthorized(vault); } /// @notice Removes a vault from the list of authorized vaults. @@ -60,10 +60,10 @@ contract Dispatcher is Ownable { } } - emit VaultRemoved(vault); + emit VaultDeauthorized(vault); } - function vaultsLength() external view returns (uint256) { - return vaults.length; + function getVaults() external view returns (address[] memory) { + return vaults; } } diff --git a/core/test/Dispatcher.test.ts b/core/test/Dispatcher.test.ts index 2305d34e3..6aeea01fc 100644 --- a/core/test/Dispatcher.test.ts +++ b/core/test/Dispatcher.test.ts @@ -79,7 +79,7 @@ describe("Dispatcher", () => { it("should emit an event when adding a vault", async () => { await expect(dispatcher.connect(governance).authorizeVault(vault1)) - .to.emit(dispatcher, "VaultAdded") + .to.emit(dispatcher, "VaultAuthorized") .withArgs(vault1) }) }) @@ -111,18 +111,18 @@ describe("Dispatcher", () => { expect(await dispatcher.vaults(0)).to.equal(vault3) const isVault1Authorized = await dispatcher.vaultsInfo(vault1) expect(isVault1Authorized).to.equal(false) - expect(await dispatcher.vaultsLength()).to.equal(2) + expect((await dispatcher.getVaults()).length).to.equal(2) await dispatcher.connect(governance).deauthorizeVault(vault2) // Last vault (vault2) was removed from the 'vaults' array expect(await dispatcher.vaults(0)).to.equal(vault3) - expect(await dispatcher.vaultsLength()).to.equal(1) + expect((await dispatcher.getVaults()).length).to.equal(1) const isVault2Authorized = await dispatcher.vaultsInfo(vault2) expect(isVault2Authorized).to.equal(false) await dispatcher.connect(governance).deauthorizeVault(vault3) - expect(await dispatcher.vaultsLength()).to.equal(0) + expect((await dispatcher.getVaults()).length).to.equal(0) const isVault3Authorized = await dispatcher.vaultsInfo(vault3) expect(isVault3Authorized).to.equal(false) }) @@ -135,7 +135,7 @@ describe("Dispatcher", () => { it("should emit an event when removing a vault", async () => { await expect(dispatcher.connect(governance).deauthorizeVault(vault1)) - .to.emit(dispatcher, "VaultRemoved") + .to.emit(dispatcher, "VaultDeauthorized") .withArgs(vault1) }) }) From cf0d47d893f5795308b3f88337274fc35fead4e8 Mon Sep 17 00:00:00 2001 From: Dmitry Date: Mon, 11 Dec 2023 15:31:57 +0100 Subject: [PATCH 21/25] Adding func.dependencies for Dispatcher ownership transfer --- core/deploy/22_transfer_ownership_acre_router.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/core/deploy/22_transfer_ownership_acre_router.ts b/core/deploy/22_transfer_ownership_acre_router.ts index 2d7748580..686d378c4 100644 --- a/core/deploy/22_transfer_ownership_acre_router.ts +++ b/core/deploy/22_transfer_ownership_acre_router.ts @@ -19,3 +19,4 @@ const func: DeployFunction = async (hre: HardhatRuntimeEnvironment) => { export default func func.tags = ["TransferOwnershipAcreRouter"] +func.dependencies = ["Dispatcher"] From b131f8206a9a79535a10f423f6612fd41f3ce90a Mon Sep 17 00:00:00 2001 From: Dmitry Date: Mon, 11 Dec 2023 16:06:18 +0100 Subject: [PATCH 22/25] Refactoring Dispatcher tests: - Replaced Address type with a String type - Added test fixture just like we have in Acre.test.ts - Various renames --- core/test/Dispatcher.test.ts | 119 ++++++++++++++++++++--------------- 1 file changed, 68 insertions(+), 51 deletions(-) diff --git a/core/test/Dispatcher.test.ts b/core/test/Dispatcher.test.ts index 6aeea01fc..5307dcc27 100644 --- a/core/test/Dispatcher.test.ts +++ b/core/test/Dispatcher.test.ts @@ -1,34 +1,41 @@ -import { getUnnamedAccounts } from "hardhat" +import { ethers } from "hardhat" import { HardhatEthersSigner } from "@nomicfoundation/hardhat-ethers/signers" -import { Address } from "hardhat-deploy/types" import { expect } from "chai" import { SnapshotRestorer, takeSnapshot, + loadFixture, } from "@nomicfoundation/hardhat-toolbox/network-helpers" import type { Dispatcher } from "../typechain" import { deployment } from "./helpers/context" import { getNamedSigner, getUnnamedSigner } from "./helpers/signer" +async function fixture() { + const { dispatcher } = await deployment() + const { governance } = await getNamedSigner() + const [thirdParty] = await getUnnamedSigner() + + return { dispatcher, governance, thirdParty } +} + describe("Dispatcher", () => { let snapshot: SnapshotRestorer let dispatcher: Dispatcher let governance: HardhatEthersSigner let thirdParty: HardhatEthersSigner - let vault1: Address - let vault2: Address - let vault3: Address - let vault4: Address + let vaultAddress1: string + let vaultAddress2: string + let vaultAddress3: string + let vaultAddress4: string before(async () => { - const accounts = await getUnnamedAccounts() - ;[vault1, vault2, vault3, vault4] = accounts - - governance = (await getNamedSigner()).governance - ;[thirdParty] = await getUnnamedSigner() + ;({ dispatcher, governance, thirdParty } = await loadFixture(fixture)) - dispatcher = (await deployment()).dispatcher + vaultAddress1 = await ethers.Wallet.createRandom().getAddress() + vaultAddress2 = await ethers.Wallet.createRandom().getAddress() + vaultAddress3 = await ethers.Wallet.createRandom().getAddress() + vaultAddress4 = await ethers.Wallet.createRandom().getAddress() }) beforeEach(async () => { @@ -43,7 +50,7 @@ describe("Dispatcher", () => { context("when caller is not a governance account", () => { it("should revert when adding a vault", async () => { await expect( - dispatcher.connect(thirdParty).authorizeVault(vault1), + dispatcher.connect(thirdParty).authorizeVault(vaultAddress1), ).to.be.revertedWithCustomError( dispatcher, "OwnableUnauthorizedAccount", @@ -53,49 +60,54 @@ describe("Dispatcher", () => { context("when caller is a governance account", () => { it("should be able to add vaults", async () => { - await dispatcher.connect(governance).authorizeVault(vault1) - await dispatcher.connect(governance).authorizeVault(vault2) - await dispatcher.connect(governance).authorizeVault(vault3) - - expect(await dispatcher.vaults(0)).to.equal(vault1) - const isVault1Authorized = await dispatcher.vaultsInfo(vault1) - expect(isVault1Authorized).to.equal(true) - - expect(await dispatcher.vaults(1)).to.equal(vault2) - const isVault2Authorized = await dispatcher.vaultsInfo(vault2) - expect(isVault2Authorized).to.equal(true) - - expect(await dispatcher.vaults(2)).to.equal(vault3) - const isVault3Authorized = await dispatcher.vaultsInfo(vault3) - expect(isVault3Authorized).to.equal(true) + await dispatcher.connect(governance).authorizeVault(vaultAddress1) + await dispatcher.connect(governance).authorizeVault(vaultAddress2) + await dispatcher.connect(governance).authorizeVault(vaultAddress3) + + expect(await dispatcher.vaults(0)).to.equal(vaultAddress1) + const isVaultAddress1Authorized = + await dispatcher.vaultsInfo(vaultAddress1) + expect(isVaultAddress1Authorized).to.equal(true) + + expect(await dispatcher.vaults(1)).to.equal(vaultAddress2) + const isVaultAddress2Authorized = + await dispatcher.vaultsInfo(vaultAddress2) + expect(isVaultAddress2Authorized).to.equal(true) + + expect(await dispatcher.vaults(2)).to.equal(vaultAddress3) + const isVaultAddress3Authorized = + await dispatcher.vaultsInfo(vaultAddress3) + expect(isVaultAddress3Authorized).to.equal(true) }) it("should not be able to add the same vault twice", async () => { - await dispatcher.connect(governance).authorizeVault(vault1) + await dispatcher.connect(governance).authorizeVault(vaultAddress1) await expect( - dispatcher.connect(governance).authorizeVault(vault1), + dispatcher.connect(governance).authorizeVault(vaultAddress1), ).to.be.revertedWithCustomError(dispatcher, "VaultAlreadyAuthorized") }) it("should emit an event when adding a vault", async () => { - await expect(dispatcher.connect(governance).authorizeVault(vault1)) + await expect( + dispatcher.connect(governance).authorizeVault(vaultAddress1), + ) .to.emit(dispatcher, "VaultAuthorized") - .withArgs(vault1) + .withArgs(vaultAddress1) }) }) }) describe("deauthorizeVault", () => { beforeEach(async () => { - await dispatcher.connect(governance).authorizeVault(vault1) - await dispatcher.connect(governance).authorizeVault(vault2) - await dispatcher.connect(governance).authorizeVault(vault3) + await dispatcher.connect(governance).authorizeVault(vaultAddress1) + await dispatcher.connect(governance).authorizeVault(vaultAddress2) + await dispatcher.connect(governance).authorizeVault(vaultAddress3) }) context("when caller is not a governance account", () => { it("should revert when adding a vault", async () => { await expect( - dispatcher.connect(thirdParty).deauthorizeVault(vault1), + dispatcher.connect(thirdParty).deauthorizeVault(vaultAddress1), ).to.be.revertedWithCustomError( dispatcher, "OwnableUnauthorizedAccount", @@ -105,38 +117,43 @@ describe("Dispatcher", () => { context("when caller is a governance account", () => { it("should be able to remove vaults", async () => { - await dispatcher.connect(governance).deauthorizeVault(vault1) + await dispatcher.connect(governance).deauthorizeVault(vaultAddress1) // Last vault replaced the first vault in the 'vaults' array - expect(await dispatcher.vaults(0)).to.equal(vault3) - const isVault1Authorized = await dispatcher.vaultsInfo(vault1) - expect(isVault1Authorized).to.equal(false) + expect(await dispatcher.vaults(0)).to.equal(vaultAddress3) + const isVaultAddress1Authorized = + await dispatcher.vaultsInfo(vaultAddress1) + expect(isVaultAddress1Authorized).to.equal(false) expect((await dispatcher.getVaults()).length).to.equal(2) - await dispatcher.connect(governance).deauthorizeVault(vault2) + await dispatcher.connect(governance).deauthorizeVault(vaultAddress2) - // Last vault (vault2) was removed from the 'vaults' array - expect(await dispatcher.vaults(0)).to.equal(vault3) + // Last vault (vaultAddress2) was removed from the 'vaults' array + expect(await dispatcher.vaults(0)).to.equal(vaultAddress3) expect((await dispatcher.getVaults()).length).to.equal(1) - const isVault2Authorized = await dispatcher.vaultsInfo(vault2) - expect(isVault2Authorized).to.equal(false) + const isVaultAddress2Authorized = + await dispatcher.vaultsInfo(vaultAddress2) + expect(isVaultAddress2Authorized).to.equal(false) - await dispatcher.connect(governance).deauthorizeVault(vault3) + await dispatcher.connect(governance).deauthorizeVault(vaultAddress3) expect((await dispatcher.getVaults()).length).to.equal(0) - const isVault3Authorized = await dispatcher.vaultsInfo(vault3) - expect(isVault3Authorized).to.equal(false) + const isVaultAddress3Authorized = + await dispatcher.vaultsInfo(vaultAddress3) + expect(isVaultAddress3Authorized).to.equal(false) }) it("should not be able to remove a vault that is not authorized", async () => { await expect( - dispatcher.connect(governance).deauthorizeVault(vault4), + dispatcher.connect(governance).deauthorizeVault(vaultAddress4), ).to.be.revertedWithCustomError(dispatcher, "VaultUnauthorized") }) it("should emit an event when removing a vault", async () => { - await expect(dispatcher.connect(governance).deauthorizeVault(vault1)) + await expect( + dispatcher.connect(governance).deauthorizeVault(vaultAddress1), + ) .to.emit(dispatcher, "VaultDeauthorized") - .withArgs(vault1) + .withArgs(vaultAddress1) }) }) }) From e94c5fc0f4dd7f232b5c67261ebf84991ba9e0ee Mon Sep 17 00:00:00 2001 From: Dmitry Date: Mon, 11 Dec 2023 16:32:21 +0100 Subject: [PATCH 23/25] Simplifying tests --- core/test/Dispatcher.test.ts | 40 ++++++++++++++++-------------------- 1 file changed, 18 insertions(+), 22 deletions(-) diff --git a/core/test/Dispatcher.test.ts b/core/test/Dispatcher.test.ts index 5307dcc27..1fdecc5d9 100644 --- a/core/test/Dispatcher.test.ts +++ b/core/test/Dispatcher.test.ts @@ -59,28 +59,22 @@ describe("Dispatcher", () => { }) context("when caller is a governance account", () => { - it("should be able to add vaults", async () => { + it("should be able to authorize vaults", async () => { await dispatcher.connect(governance).authorizeVault(vaultAddress1) await dispatcher.connect(governance).authorizeVault(vaultAddress2) await dispatcher.connect(governance).authorizeVault(vaultAddress3) expect(await dispatcher.vaults(0)).to.equal(vaultAddress1) - const isVaultAddress1Authorized = - await dispatcher.vaultsInfo(vaultAddress1) - expect(isVaultAddress1Authorized).to.equal(true) + expect(await dispatcher.vaultsInfo(vaultAddress1)).to.be.equal(true) expect(await dispatcher.vaults(1)).to.equal(vaultAddress2) - const isVaultAddress2Authorized = - await dispatcher.vaultsInfo(vaultAddress2) - expect(isVaultAddress2Authorized).to.equal(true) + expect(await dispatcher.vaultsInfo(vaultAddress2)).to.be.equal(true) expect(await dispatcher.vaults(2)).to.equal(vaultAddress3) - const isVaultAddress3Authorized = - await dispatcher.vaultsInfo(vaultAddress3) - expect(isVaultAddress3Authorized).to.equal(true) + expect(await dispatcher.vaultsInfo(vaultAddress3)).to.be.equal(true) }) - it("should not be able to add the same vault twice", async () => { + it("should not be able to authorize the same vault twice", async () => { await dispatcher.connect(governance).authorizeVault(vaultAddress1) await expect( dispatcher.connect(governance).authorizeVault(vaultAddress1), @@ -116,14 +110,12 @@ describe("Dispatcher", () => { }) context("when caller is a governance account", () => { - it("should be able to remove vaults", async () => { + it("should be able to authorize vaults", async () => { await dispatcher.connect(governance).deauthorizeVault(vaultAddress1) // Last vault replaced the first vault in the 'vaults' array expect(await dispatcher.vaults(0)).to.equal(vaultAddress3) - const isVaultAddress1Authorized = - await dispatcher.vaultsInfo(vaultAddress1) - expect(isVaultAddress1Authorized).to.equal(false) + expect(await dispatcher.vaultsInfo(vaultAddress1)).to.be.equal(false) expect((await dispatcher.getVaults()).length).to.equal(2) await dispatcher.connect(governance).deauthorizeVault(vaultAddress2) @@ -131,18 +123,22 @@ describe("Dispatcher", () => { // Last vault (vaultAddress2) was removed from the 'vaults' array expect(await dispatcher.vaults(0)).to.equal(vaultAddress3) expect((await dispatcher.getVaults()).length).to.equal(1) - const isVaultAddress2Authorized = - await dispatcher.vaultsInfo(vaultAddress2) - expect(isVaultAddress2Authorized).to.equal(false) + expect(await dispatcher.vaultsInfo(vaultAddress2)).to.be.equal(false) await dispatcher.connect(governance).deauthorizeVault(vaultAddress3) expect((await dispatcher.getVaults()).length).to.equal(0) - const isVaultAddress3Authorized = - await dispatcher.vaultsInfo(vaultAddress3) - expect(isVaultAddress3Authorized).to.equal(false) + expect(await dispatcher.vaultsInfo(vaultAddress3)).to.be.equal(false) }) - it("should not be able to remove a vault that is not authorized", async () => { + it("should be able to deauthorize a vault and authorize it again", async () => { + await dispatcher.connect(governance).deauthorizeVault(vaultAddress1) + expect(await dispatcher.vaultsInfo(vaultAddress1)).to.be.equal(false) + + await dispatcher.connect(governance).authorizeVault(vaultAddress1) + expect(await dispatcher.vaultsInfo(vaultAddress1)).to.be.equal(true) + }) + + it("should not be able to deauthorize a vault that is not authorized", async () => { await expect( dispatcher.connect(governance).deauthorizeVault(vaultAddress4), ).to.be.revertedWithCustomError(dispatcher, "VaultUnauthorized") From e21a50b8fe3268e34c5e5b9da22daecf930a048c Mon Sep 17 00:00:00 2001 From: Jakub Nowakowski Date: Wed, 13 Dec 2023 08:45:28 +0100 Subject: [PATCH 24/25] Improve signers mapping MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Rafał Czajkowski <57687279+r-czajkowski@users.noreply.github.com> --- core/test/helpers/signer.ts | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/core/test/helpers/signer.ts b/core/test/helpers/signer.ts index c6fea1c0e..22dcda331 100644 --- a/core/test/helpers/signer.ts +++ b/core/test/helpers/signer.ts @@ -25,13 +25,7 @@ export async function getNamedSigner(): Promise<{ * @returns Array of unnamed Hardhat Ethers Signers. */ export async function getUnnamedSigner(): Promise { - const unnamedSigners: HardhatEthersSigner[] = [] + const accounts = await getUnnamedAccounts() - await Promise.all( - (await getUnnamedAccounts()).map(async (address) => { - unnamedSigners.push(await ethers.getSigner(address)) - }), - ) - - return unnamedSigners + return await Promise.all(accounts.map(ethers.getSigner)) } From 2831ef6791b974690550a78e386e184dcd9f3190 Mon Sep 17 00:00:00 2001 From: Jakub Nowakowski Date: Wed, 13 Dec 2023 09:22:12 +0100 Subject: [PATCH 25/25] Don't return awaited promise in getUnnamedSigner Fix `@typescript-eslint/return-await` error returned by eslint. --- core/test/helpers/signer.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/test/helpers/signer.ts b/core/test/helpers/signer.ts index 22dcda331..0ae57f35e 100644 --- a/core/test/helpers/signer.ts +++ b/core/test/helpers/signer.ts @@ -27,5 +27,5 @@ export async function getNamedSigner(): Promise<{ export async function getUnnamedSigner(): Promise { const accounts = await getUnnamedAccounts() - return await Promise.all(accounts.map(ethers.getSigner)) + return Promise.all(accounts.map(ethers.getSigner)) }