Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handling ERC4626 vaults in AcreRouter #62

Merged
merged 20 commits into from
Dec 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion core/.solhint.json
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
{
"extends": "thesis",
"plugins": []
"plugins": [],
"rules": {}
}
69 changes: 69 additions & 0 deletions core/contracts/Dispatcher.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
// SPDX-License-Identifier: GPL-3.0-only
pragma solidity ^0.8.21;

import "@openzeppelin/contracts/access/Ownable.sol";

/// @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 Dispatcher is Ownable {
error VaultAlreadyAuthorized();
error VaultUnauthorized();
Comment on lines +11 to +12
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have a warning here. We should define errors below the events declaration.

obraz

Copy link
Member Author

@dimpar dimpar Dec 13, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 I'll change that in the following PR


struct VaultInfo {
bool authorized;
}

/// @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 authorized by the owner it can be plugged into
/// Acre.
address[] public vaults;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just wonder if we need this array. From the dapp/subgprah perspective, we can get the vault address from an event. Not sure about the contracts perspective, maybe other contracts will need this?

Copy link
Member Author

@dimpar dimpar Dec 12, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This array will track all the vaults within Acre ecosystem. vaultsInfo map won't iterate through all the vaults in Acre, you need to know and provide a concrete address. From the dApp perspective maybe it's easy tracking all the authorize/deauthorize events and produce the final result what's in the system, but if a user just want to go to etherscan and see the vaults addresses that won't make it. Besides, it's way easier just to call vaults() and get all the vaults rather than fetching events and then combine them to get the final answer.

mapping(address => VaultInfo) public vaultsInfo;

event VaultAuthorized(address indexed vault);
event VaultDeauthorized(address indexed vault);

constructor() Ownable(msg.sender) {}

/// @notice Adds a vault to the list of authorized vaults.
/// @param vault Address of the vault to add.
function authorizeVault(address vault) external onlyOwner {
if (vaultsInfo[vault].authorized) {
revert VaultAlreadyAuthorized();
}

vaults.push(vault);
vaultsInfo[vault].authorized = true;

emit VaultAuthorized(vault);
}

/// @notice Removes a vault from the list of authorized vaults.
/// @param vault Address of the vault to remove.
function deauthorizeVault(address vault) external onlyOwner {
if (!vaultsInfo[vault].authorized) {
revert VaultUnauthorized();
}

vaultsInfo[vault].authorized = 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;
}
}

emit VaultDeauthorized(vault);
}

function getVaults() external view returns (address[] memory) {
return vaults;
}
}
22 changes: 22 additions & 0 deletions core/deploy/02_deploy_acre_router.ts
Original file line number Diff line number Diff line change
@@ -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("Dispatcher", {
from: deployer,
args: [],
log: true,
waitConfirmations: 1,
})

// TODO: Add Etherscan verification
// TODO: Add Tenderly verification
}

export default func

func.tags = ["Dispatcher"]
func.dependencies = ["Acre"]
22 changes: 22 additions & 0 deletions core/deploy/22_transfer_ownership_acre_router.ts
Original file line number Diff line number Diff line change
@@ -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, governance } = await getNamedAccounts()
const { log } = deployments

log(`transferring ownership of Dispatcher contract to ${governance}`)

await deployments.execute(
"Dispatcher",
{ from: deployer, log: true, waitConfirmations: 1 },
"transferOwnership",
governance,
)
}

export default func

func.tags = ["TransferOwnershipAcreRouter"]
nkuba marked this conversation as resolved.
Show resolved Hide resolved
func.dependencies = ["Dispatcher"]
156 changes: 156 additions & 0 deletions core/test/Dispatcher.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,156 @@
import { ethers } from "hardhat"
import { HardhatEthersSigner } from "@nomicfoundation/hardhat-ethers/signers"
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 vaultAddress1: string
let vaultAddress2: string
let vaultAddress3: string
let vaultAddress4: string

before(async () => {
;({ dispatcher, governance, thirdParty } = await loadFixture(fixture))

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 () => {
snapshot = await takeSnapshot()
})

afterEach(async () => {
await snapshot.restore()
})

describe("authorizeVault", () => {
context("when caller is not a governance account", () => {
it("should revert when adding a vault", async () => {
await expect(
dispatcher.connect(thirdParty).authorizeVault(vaultAddress1),
).to.be.revertedWithCustomError(
dispatcher,
"OwnableUnauthorizedAccount",
)
})
})

context("when caller is a governance account", () => {
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)
expect(await dispatcher.vaultsInfo(vaultAddress1)).to.be.equal(true)

expect(await dispatcher.vaults(1)).to.equal(vaultAddress2)
expect(await dispatcher.vaultsInfo(vaultAddress2)).to.be.equal(true)

expect(await dispatcher.vaults(2)).to.equal(vaultAddress3)
expect(await dispatcher.vaultsInfo(vaultAddress3)).to.be.equal(true)
})

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),
).to.be.revertedWithCustomError(dispatcher, "VaultAlreadyAuthorized")
})

it("should emit an event when adding a vault", async () => {
await expect(
dispatcher.connect(governance).authorizeVault(vaultAddress1),
)
.to.emit(dispatcher, "VaultAuthorized")
.withArgs(vaultAddress1)
})
})
})

describe("deauthorizeVault", () => {
beforeEach(async () => {
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(vaultAddress1),
).to.be.revertedWithCustomError(
dispatcher,
"OwnableUnauthorizedAccount",
)
})
})

context("when caller is a governance account", () => {
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)
expect(await dispatcher.vaultsInfo(vaultAddress1)).to.be.equal(false)
expect((await dispatcher.getVaults()).length).to.equal(2)

await dispatcher.connect(governance).deauthorizeVault(vaultAddress2)

// 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)
expect(await dispatcher.vaultsInfo(vaultAddress2)).to.be.equal(false)

await dispatcher.connect(governance).deauthorizeVault(vaultAddress3)
expect((await dispatcher.getVaults()).length).to.equal(0)
expect(await dispatcher.vaultsInfo(vaultAddress3)).to.be.equal(false)
})

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")
})

it("should emit an event when removing a vault", async () => {
await expect(
dispatcher.connect(governance).deauthorizeVault(vaultAddress1),
)
.to.emit(dispatcher, "VaultDeauthorized")
.withArgs(vaultAddress1)
})
})
})
})
5 changes: 3 additions & 2 deletions core/test/helpers/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,15 @@ import { deployments } from "hardhat"

import { getDeployedContract } from "./contract"

import type { Acre, TestERC20 } from "../../typechain"
import type { Acre, Dispatcher, 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")
const dispatcher: Dispatcher = await getDeployedContract("Dispatcher")

return { tbtc, acre }
return { tbtc, acre, dispatcher }
}
Loading