From 2bdc9b5e474f4b6c456e266d0f5449c46f0ce4f5 Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Thu, 7 Dec 2023 10:24:46 +0100 Subject: [PATCH 01/45] Create `StakingParameters` struct Store staking-related parameters in struct such as minimum amount for a single deposit operation and maximum total amount og tBTC token held by Acre. In the future, we should add other staking-related parametrs here. --- core/contracts/Acre.sol | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/core/contracts/Acre.sol b/core/contracts/Acre.sol index 0e1cf640f..abd91c163 100644 --- a/core/contracts/Acre.sol +++ b/core/contracts/Acre.sol @@ -17,6 +17,15 @@ import "@openzeppelin/contracts/token/ERC20/extensions/ERC4626.sol"; contract Acre is ERC4626 { event StakeReferral(bytes32 indexed referral, uint256 assets); + struct StakingParameters { + // Minimum amount for a single deposit operation. + uint256 minimumDepositAmount; + // Maximum total amount of tBTC token held by Acre. + uint256 maximumTotalAssets; + } + + StakingParameters public stakingParameters; + constructor( IERC20 tbtc ) ERC4626(tbtc) ERC20("Acre Staked Bitcoin", "stBTC") {} From 5679d876e54336bc2a00d14d2abfcd8b4bfd6aff Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Thu, 7 Dec 2023 10:28:56 +0100 Subject: [PATCH 02/45] Set default staking parameters in constructor --- core/contracts/Acre.sol | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/core/contracts/Acre.sol b/core/contracts/Acre.sol index abd91c163..6a117568c 100644 --- a/core/contracts/Acre.sol +++ b/core/contracts/Acre.sol @@ -15,8 +15,6 @@ import "@openzeppelin/contracts/token/ERC20/extensions/ERC4626.sol"; /// burning of shares (stBTC), which are represented as standard ERC20 /// tokens, providing a seamless exchange with tBTC tokens. contract Acre is ERC4626 { - event StakeReferral(bytes32 indexed referral, uint256 assets); - struct StakingParameters { // Minimum amount for a single deposit operation. uint256 minimumDepositAmount; @@ -26,9 +24,16 @@ contract Acre is ERC4626 { StakingParameters public stakingParameters; + event StakeReferral(bytes32 indexed referral, uint256 assets); + constructor( IERC20 tbtc - ) ERC4626(tbtc) ERC20("Acre Staked Bitcoin", "stBTC") {} + ) ERC4626(tbtc) ERC20("Acre Staked Bitcoin", "stBTC") { + stakingParameters = StakingParameters({ + minimumDepositAmount: 10000000000000000, // 0.01 tBTC + maximumTotalAssets: 25000000000000000000 // 25 tBTC + }); + } /// @notice Stakes a given amount of tBTC token and mints shares to a /// receiver. From ae7b3fb0b3e6096c9ddeaae8acbdea71540ef418 Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Thu, 7 Dec 2023 15:26:24 +0100 Subject: [PATCH 03/45] Implement min amount limit for a single deposit Only deposits where the amount is greater than or equal to the minimum amount for a single deposit should be possible. --- core/contracts/Acre.sol | 14 +++++++++++ core/test/Acre.test.ts | 51 +++++++++++++++++++++++++++++++++++++++-- 2 files changed, 63 insertions(+), 2 deletions(-) diff --git a/core/contracts/Acre.sol b/core/contracts/Acre.sol index 6a117568c..091c4f578 100644 --- a/core/contracts/Acre.sol +++ b/core/contracts/Acre.sol @@ -35,6 +35,20 @@ contract Acre is ERC4626 { }); } + function deposit( + uint256 assets, + address receiver + ) public override returns (uint256) { + require( + assets >= stakingParameters.minimumDepositAmount, + "Amount is less than minimum" + ); + + uint256 shares = super.deposit(assets, receiver); + + return shares; + } + /// @notice Stakes a given amount of tBTC token and mints shares to a /// receiver. /// @dev This function calls `deposit` function from `ERC4626` contract. The diff --git a/core/test/Acre.test.ts b/core/test/Acre.test.ts index 74e764ca0..91ffdab0b 100644 --- a/core/test/Acre.test.ts +++ b/core/test/Acre.test.ts @@ -161,12 +161,59 @@ describe("Acre", () => { .approve(await acre.getAddress(), amountToStake) }) - it("should not revert", async () => { + it("should revert", async () => { + await expect( + acre + .connect(staker1) + .stake(amountToStake, staker1.address, referral), + ).to.revertedWith("Amount is less than minimum") + }) + }) + + context("when amount to stake is less than minimum", () => { + let amountToStake: bigint + + beforeEach(async () => { + const { minimumDepositAmount } = await acre.stakingParameters() + amountToStake = minimumDepositAmount - 1n + + await tbtc + .connect(staker1) + .approve(await acre.getAddress(), amountToStake) + }) + + it("should revert", async () => { await expect( acre .connect(staker1) .stake(amountToStake, staker1.address, referral), - ).to.not.be.reverted + ).to.revertedWith("Amount is less than minimum") + }) + }) + + context("when amount to stake is equal to the minimum amount", () => { + let amountToStake: bigint + let tx: ContractTransactionResponse + + beforeEach(async () => { + const { minimumDepositAmount } = await acre.stakingParameters() + amountToStake = minimumDepositAmount + + await tbtc + .connect(staker1) + .approve(await acre.getAddress(), amountToStake) + + tx = await acre + .connect(staker1) + .stake(amountToStake, staker1.address, referral) + }) + + it("should receive shares equal to the staked amount", async () => { + await expect(tx).to.changeTokenBalances( + acre, + [staker1.address], + [amountToStake], + ) }) }) From 1765b6d382e7465f9f8ca96f7ffe64bfbfcc84e7 Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Thu, 7 Dec 2023 16:29:49 +0100 Subject: [PATCH 04/45] Fix typo `apporval` -> `approval` --- core/test/Acre.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/test/Acre.test.ts b/core/test/Acre.test.ts index 91ffdab0b..e51dedc44 100644 --- a/core/test/Acre.test.ts +++ b/core/test/Acre.test.ts @@ -234,7 +234,7 @@ describe("Acre", () => { }) context( - "when a staker approved and staked tokens and wants to stake more but w/o another apporval", + "when a staker approved and staked tokens and wants to stake more but w/o another approval", () => { const amountToStake = to1e18(10) From 5494c4461916f1ec10d7e968bfc165f82e1a38ed Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Thu, 7 Dec 2023 16:31:20 +0100 Subject: [PATCH 05/45] Simplify test --- core/test/Acre.test.ts | 38 +++++++++++++++++--------------------- 1 file changed, 17 insertions(+), 21 deletions(-) diff --git a/core/test/Acre.test.ts b/core/test/Acre.test.ts index e51dedc44..5d0952b58 100644 --- a/core/test/Acre.test.ts +++ b/core/test/Acre.test.ts @@ -286,34 +286,30 @@ describe("Acre", () => { }) context("when staker A stakes tokens", () => { - it("should stake tokens correctly", async () => { - await expect( - acre - .connect(staker1) - .stake(staker1AmountToStake, staker1.address, referral), - ).to.be.not.reverted - }) - it("should receive shares equal to a staked amount", async () => { - const shares = await acre.balanceOf(staker1.address) + const tx = await acre + .connect(staker1) + .stake(staker1AmountToStake, staker1.address, referral) - expect(shares).to.eq(staker1AmountToStake) + await expect(tx).to.changeTokenBalances( + acre, + [staker1.address], + [staker1AmountToStake], + ) }) }) context("when staker B stakes tokens", () => { - it("should stake tokens correctly", async () => { - await expect( - acre - .connect(staker2) - .stake(staker2AmountToStake, staker2.address, referral), - ).to.be.not.reverted - }) - it("should receive shares equal to a staked amount", async () => { - const shares = await acre.balanceOf(staker2.address) - - expect(shares).to.eq(staker2AmountToStake) + const tx = await acre + .connect(staker2) + .stake(staker2AmountToStake, staker2.address, referral) + + await expect(tx).to.changeTokenBalances( + acre, + [staker2.address], + [staker2AmountToStake], + ) }) }) }, From 15ff38d1111a21ae17b9c7e6d1a21f14e51ac471 Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Thu, 7 Dec 2023 16:32:26 +0100 Subject: [PATCH 06/45] Override `maxDeposit` fn We want to control how many tBTC tokens the Acre contract can hold. Here we override the `maxDeposit` function that takes into account the staking param, maximum total assets, which determines the total amount of tBTC token held by Acre. --- core/contracts/Acre.sol | 10 +++ core/test/Acre.test.ts | 141 +++++++++++++++++++++++++--------------- 2 files changed, 98 insertions(+), 53 deletions(-) diff --git a/core/contracts/Acre.sol b/core/contracts/Acre.sol index 091c4f578..6b5838b8e 100644 --- a/core/contracts/Acre.sol +++ b/core/contracts/Acre.sol @@ -71,4 +71,14 @@ contract Acre is ERC4626 { return shares; } + + /// @notice Returns the maximum amount of the tBTC token that can be + /// deposited into the vault for the receiver, through a deposit + /// call. It takes into account the staking parameter, maximum total + /// assets, which determines the total amount of tBTC token held by + /// Acre. + /// @return The maximum amount of the tBTC token. + function maxDeposit(address) public view override returns (uint256) { + return stakingParameters.maximumTotalAssets - totalAssets(); + } } diff --git a/core/test/Acre.test.ts b/core/test/Acre.test.ts index 5d0952b58..c7cb3bec0 100644 --- a/core/test/Acre.test.ts +++ b/core/test/Acre.test.ts @@ -50,7 +50,7 @@ describe("Acre", () => { }) context("with a referral", () => { - const amountToStake = to1e18(1000) + const amountToStake = to1e18(1) // In this test case there is only one staker and // the token vault has not earned anythig yet so received shares are @@ -259,9 +259,9 @@ describe("Acre", () => { ) }) - context("when there are two stakers, A and B ", () => { - const staker1AmountToStake = to1e18(75) - const staker2AmountToStake = to1e18(25) + context("when there are two stakers", () => { + const staker1AmountToStake = to1e18(7) + const staker2AmountToStake = to1e18(3) let afterStakesSnapshot: SnapshotRestorer let afterSimulatingYieldSnapshot: SnapshotRestorer @@ -334,14 +334,14 @@ describe("Acre", () => { before(async () => { // Current state: - // Staker A shares = 75 - // Staker B shares = 25 - // Total assets = 75(staker A) + 25(staker B) + 50(yield) + // Staker A shares = 7 + // Staker B shares = 3 + // Total assets = 7(staker A) + 3(staker B) + 5(yield) await afterStakesSnapshot.restore() staker1SharesBefore = await acre.balanceOf(staker1.address) staker2SharesBefore = await acre.balanceOf(staker2.address) - vaultYield = to1e18(50) + vaultYield = to1e18(5) // Simulating yield returned from strategies. The vault now contains // more tokens than deposited which causes the exchange rate to @@ -372,10 +372,10 @@ describe("Acre", () => { const shares = await acre.balanceOf(staker1.address) const availableAssetsToRedeem = await acre.previewRedeem(shares) - // Expected amount w/o rounding: 75 * 150 / 100 = 112.5 - // Expected amount w/ support for rounding: 112499999999999999999 in + // Expected amount w/o rounding: 7 * 15 / 10 = 10.5 + // Expected amount w/ support for rounding: 10499999999999999999 in // tBTC token precision. - const expectedAssetsToRedeem = 112499999999999999999n + const expectedAssetsToRedeem = 10499999999999999999n expect(availableAssetsToRedeem).to.be.eq(expectedAssetsToRedeem) }) @@ -384,65 +384,100 @@ describe("Acre", () => { const shares = await acre.balanceOf(staker2.address) const availableAssetsToRedeem = await acre.previewRedeem(shares) - // Expected amount w/o rounding: 25 * 150 / 100 = 37.5 - // Expected amount w/ support for rounding: 37499999999999999999 in + // Expected amount w/o rounding: 3 * 15 / 10 = 4.5 + // Expected amount w/ support for rounding: 4499999999999999999 in // tBTC token precision. - const expectedAssetsToRedeem = 37499999999999999999n + const expectedAssetsToRedeem = 4499999999999999999n expect(availableAssetsToRedeem).to.be.eq(expectedAssetsToRedeem) }) }) context("when staker A stakes more tokens", () => { - const newAmountToStake = to1e18(20) - // Current state: - // Total assets = 75(staker A) + 25(staker B) + 50(yield) - // Total shares = 75 + 25 = 100 - // 20 * 100 / 150 = 13.(3) -> 13333333333333333333 in stBTC token - /// precision - const expectedSharesToMint = 13333333333333333333n - let sharesBefore: bigint - let availableToRedeemBefore: bigint + context( + "when total tBTC amount after staking would not exceed max amount", + () => { + const newAmountToStake = to1e18(2) + // Current state: + // Total assets = 7(staker A) + 3(staker B) + 5(yield) + // Total shares = 7 + 3 = 10 + // Shares to mint = 2 * 10 / 15 = 1.(3) -> 1333333333333333333 in stBTC + // token precision + const expectedSharesToMint = 1333333333333333333n + let sharesBefore: bigint + let availableToRedeemBefore: bigint + + before(async () => { + await afterSimulatingYieldSnapshot.restore() + + sharesBefore = await acre.balanceOf(staker1.address) + availableToRedeemBefore = await acre.previewRedeem(sharesBefore) + + tbtc.mint(staker1.address, newAmountToStake) + + await tbtc + .connect(staker1) + .approve(await acre.getAddress(), newAmountToStake) - before(async () => { - await afterSimulatingYieldSnapshot.restore() + // State after stake: + // Total assets = 7(staker A) + 3(staker B) + 5(yield) + 2(staker + // A) = 17 + // Total shares = 7 + 3 + 1.(3) = 11.(3) + await acre.stake(newAmountToStake, staker1.address, referral) + }) - sharesBefore = await acre.balanceOf(staker1.address) - availableToRedeemBefore = await acre.previewRedeem(sharesBefore) + it("should receive more shares", async () => { + const shares = await acre.balanceOf(staker1.address) - tbtc.mint(staker1.address, newAmountToStake) + expect(shares).to.be.eq(sharesBefore + expectedSharesToMint) + }) - await tbtc - .connect(staker1) - .approve(await acre.getAddress(), newAmountToStake) + it("should be able to redeem more tokens than before", async () => { + const shares = await acre.balanceOf(staker1.address) + const availableToRedeem = await acre.previewRedeem(shares) - // State after stake: - // Total assets = 75(staker A) + 25(staker B) + 50(yield) + 20(staker - // A) = 170 - // Total shares = 75 + 25 + 13.(3) = 113.(3) - await acre.stake(newAmountToStake, staker1.address, referral) - }) + // Expected amount w/o rounding: 8.(3) * 17 / 11.(3) = 12.5 + // Expected amount w/ support for rounding: 12499999999999999999 in + // tBTC token precision. + const expectedTotalAssetsAvailableToRedeem = 12499999999999999999n - it("should receive more shares", async () => { - const shares = await acre.balanceOf(staker1.address) + expect(availableToRedeem).to.be.greaterThan( + availableToRedeemBefore, + ) + expect(availableToRedeem).to.be.eq( + expectedTotalAssetsAvailableToRedeem, + ) + }) + }, + ) - expect(shares).to.be.eq(sharesBefore + expectedSharesToMint) - }) + context( + "when total tBTC amount after staking would exceed max amount", + () => { + let possibleMaxAmountToStake: bigint + let amountToStake: bigint - it("should be able to redeem more tokens than before", async () => { - const shares = await acre.balanceOf(staker1.address) - const availableToRedeem = await acre.previewRedeem(shares) + before(async () => { + await afterSimulatingYieldSnapshot.restore() - // Expected amount w/o rounding: 88.(3) * 170 / 113.(3) = 132.5 - // Expected amount w/ support for rounding: 132499999999999999999 in - // tBTC token precision. - const expectedTotalAssetsAvailableToRedeem = 132499999999999999999n + // In the current implementation of the `maxDeposit` the + // `address` param is not taken into account - it means it will + // return the same value for any address. + possibleMaxAmountToStake = await acre.maxDeposit(staker1.address) + amountToStake = possibleMaxAmountToStake + 1n - expect(availableToRedeem).to.be.greaterThan(availableToRedeemBefore) - expect(availableToRedeem).to.be.eq( - expectedTotalAssetsAvailableToRedeem, - ) - }) + await tbtc + .connect(staker1) + .approve(await acre.getAddress(), amountToStake) + }) + + it("should revert", async () => { + await expect( + acre.stake(amountToStake, staker1.address, referral), + ).to.be.revertedWithCustomError(acre, "ERC4626ExceededMaxDeposit") + }) + }, + ) }) }) }) From 0efc202a3c3e20cfe087fedc73854e7f703d9df7 Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Thu, 7 Dec 2023 17:57:47 +0100 Subject: [PATCH 07/45] Implement staking limits in `mint` fn In ERC4626 standard users can deposit using 2 functions `mint` and `deposit` so we need to take into account staking limits in `mint` function to not allow deposits that break staking limts rules. --- core/contracts/Acre.sol | 23 +++++++++++++++++++++++ core/test/Acre.test.ts | 38 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+) diff --git a/core/contracts/Acre.sol b/core/contracts/Acre.sol index 6b5838b8e..5bf418dfa 100644 --- a/core/contracts/Acre.sol +++ b/core/contracts/Acre.sol @@ -49,6 +49,20 @@ contract Acre is ERC4626 { return shares; } + function mint( + uint256 shares, + address receiver + ) public override returns (uint256) { + uint256 assets = super.mint(shares, receiver); + + require( + assets >= stakingParameters.minimumDepositAmount, + "Amount is less than minimum" + ); + + return assets; + } + /// @notice Stakes a given amount of tBTC token and mints shares to a /// receiver. /// @dev This function calls `deposit` function from `ERC4626` contract. The @@ -81,4 +95,13 @@ contract Acre is ERC4626 { function maxDeposit(address) public view override returns (uint256) { return stakingParameters.maximumTotalAssets - totalAssets(); } + + /// @notice Returns the maximum amount of the vault shares that can be + /// minted for the receiver, through a mint call. + /// @dev Since the Acre contract limits the maximum total tBTC tokens this + /// function converts the maximum deposit amount to shares. + /// @return The maximum amount of the vault shares. + function maxMint(address receiver) public view override returns (uint256) { + return previewDeposit(maxDeposit(receiver)); + } } diff --git a/core/test/Acre.test.ts b/core/test/Acre.test.ts index c7cb3bec0..7d67e7923 100644 --- a/core/test/Acre.test.ts +++ b/core/test/Acre.test.ts @@ -481,4 +481,42 @@ describe("Acre", () => { }) }) }) + + describe("mint", () => { + context("when staker wants to mint more shares than allowed", () => { + let sharesToMint: bigint + + beforeEach(async () => { + const maxMint = await acre.maxMint(staker1.address) + + sharesToMint = maxMint + 1n + }) + + it("should take into account the max total assets parameter and revert", async () => { + await expect( + acre.mint(sharesToMint, staker1.address), + ).to.be.revertedWithCustomError(acre, "ERC4626ExceededMaxMint") + }) + }) + + context( + "when staker wants to mint less shares than is equal to the min deposit amount", + () => { + let sharesToMint: bigint + + beforeEach(async () => { + const { minimumDepositAmount } = await acre.stakingParameters() + const previewDeposit = await acre.previewDeposit(minimumDepositAmount) + + sharesToMint = previewDeposit - 1n + }) + + it("should take into account the min deposit amount parameter and revert", async () => { + await expect( + acre.mint(sharesToMint, staker1.address), + ).to.be.revertedWith("Amount is less than minimum") + }) + }, + ) + }) }) From 2fda404cf0e6b33872fcaf3b5db1e89cca4dd222 Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Fri, 8 Dec 2023 14:30:38 +0100 Subject: [PATCH 08/45] Add fn that allows owner to update staking params Add function that updates parameters of staking. Only owner can update these parameters so here we also use the `Ownable` contract from `OZ` library to use the `onlyOwner` modifier. Requirements: - Minimum deposit amount must be greater than zero, - Minimum deposit amount must be greater than or equal to the minimum deposit amount in the tBTC system, - Maximum total assets must be greater than zero. --- core/contracts/Acre.sol | 66 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 63 insertions(+), 3 deletions(-) diff --git a/core/contracts/Acre.sol b/core/contracts/Acre.sol index 5bf418dfa..a74151581 100644 --- a/core/contracts/Acre.sol +++ b/core/contracts/Acre.sol @@ -2,6 +2,7 @@ pragma solidity ^0.8.21; import "@openzeppelin/contracts/token/ERC20/extensions/ERC4626.sol"; +import "@openzeppelin/contracts/access/Ownable.sol"; /// @title Acre /// @notice This contract implements the ERC-4626 tokenized vault standard. By @@ -14,7 +15,7 @@ import "@openzeppelin/contracts/token/ERC20/extensions/ERC4626.sol"; /// of yield-bearing vaults. This contract facilitates the minting and /// burning of shares (stBTC), which are represented as standard ERC20 /// tokens, providing a seamless exchange with tBTC tokens. -contract Acre is ERC4626 { +contract Acre is ERC4626, Ownable { struct StakingParameters { // Minimum amount for a single deposit operation. uint256 minimumDepositAmount; @@ -25,16 +26,62 @@ contract Acre is ERC4626 { StakingParameters public stakingParameters; event StakeReferral(bytes32 indexed referral, uint256 assets); + event StakingParametersUpdated( + uint256 minimumDepositAmount, + uint256 maximumTotalAssets + ); constructor( - IERC20 tbtc - ) ERC4626(tbtc) ERC20("Acre Staked Bitcoin", "stBTC") { + IERC20 tbtc, + address initialOwner + ) + ERC4626(tbtc) + ERC20("Acre Staked Bitcoin", "stBTC") + Ownable(initialOwner) + { stakingParameters = StakingParameters({ minimumDepositAmount: 10000000000000000, // 0.01 tBTC maximumTotalAssets: 25000000000000000000 // 25 tBTC }); } + /// @notice Updates parameters of staking. + /// @dev Requirements: + /// - Minimum deposit amount must be greater than zero, + /// - Minimum deposit amount must be greater than or equal to the + /// minimum deposit amount in the tBTC system, + /// - Maximum total assets must be greater than zero. + /// @param minimumDepositAmount New value of the minimum deposit amount. It + /// is the minimum amount for a single deposit operation. + /// @param maximumTotalAssets New value of the maximum total assets amount. + /// It is the maximum amount of the tBTC token that the Acre can + /// hold. + function updateStakingParameters( + uint256 minimumDepositAmount, + uint256 maximumTotalAssets + ) external onlyOwner { + require( + minimumDepositAmount > 0, + "Minimum deposit amount must be greater than zero" + ); + + // TODO: Do we need this requirement? + require( + minimumDepositAmount >= _getTBTCSystemMinDepositAmount(), + "Minimum deposit amount must be greater than or to the equal minimum deposit amount in tBTC system" + ); + + require( + maximumTotalAssets > 0, + "Maximum total assets amount must be greater than zero" + ); + + stakingParameters.minimumDepositAmount = minimumDepositAmount; + stakingParameters.maximumTotalAssets = maximumTotalAssets; + + emit StakingParametersUpdated(minimumDepositAmount, maximumTotalAssets); + } + function deposit( uint256 assets, address receiver @@ -104,4 +151,17 @@ contract Acre is ERC4626 { function maxMint(address receiver) public view override returns (uint256) { return previewDeposit(maxDeposit(receiver)); } + + function _getTBTCSystemMinDepositAmount() private pure returns (uint256) { + // TODO: Implement if we want to make sure the minimum deposit amount is + // greater than or equal the minimum dposit amount in the tBTC system. + + // (uint64 depositDustThreshold, , , ) = tbtcBridge.depositParameters(); + + // // The `depositDustThreshold` is in satoshi so we need to cast to tBTC + // // decimals. + // return 10 ** (ERC20(asset()).decimals() - 8) * depositDustThreshold; + + return 10000000000000000; // 0.01 tBTC + } } From 7ae23cbaab3d638f0e6b801face4ef7c5463812e Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Fri, 8 Dec 2023 15:14:30 +0100 Subject: [PATCH 09/45] Add unit test for `updateStakingParameters` fn --- core/test/Acre.test.ts | 133 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 129 insertions(+), 4 deletions(-) diff --git a/core/test/Acre.test.ts b/core/test/Acre.test.ts index 7d67e7923..ac5d92938 100644 --- a/core/test/Acre.test.ts +++ b/core/test/Acre.test.ts @@ -11,29 +11,30 @@ import type { TestERC20, Acre } from "../typechain" import { to1e18 } from "./utils" async function acreFixture() { - const [staker1, staker2] = await ethers.getSigners() + const [owner, staker1, staker2] = await ethers.getSigners() const TestERC20 = await ethers.getContractFactory("TestERC20") const tbtc = await TestERC20.deploy() const Acre = await ethers.getContractFactory("Acre") - const acre = await Acre.deploy(await tbtc.getAddress()) + const acre = await Acre.deploy(await tbtc.getAddress(), owner) const amountToMint = to1e18(100000) tbtc.mint(staker1, amountToMint) tbtc.mint(staker2, amountToMint) - return { acre, tbtc, staker1, staker2 } + return { acre, tbtc, owner, staker1, staker2 } } describe("Acre", () => { let acre: Acre let tbtc: TestERC20 + let owner: HardhatEthersSigner let staker1: HardhatEthersSigner let staker2: HardhatEthersSigner before(async () => { - ;({ acre, tbtc, staker1, staker2 } = await loadFixture(acreFixture)) + ;({ acre, tbtc, owner, staker1, staker2 } = await loadFixture(acreFixture)) }) describe("stake", () => { @@ -483,6 +484,16 @@ describe("Acre", () => { }) describe("mint", () => { + let snapshot: SnapshotRestorer + + beforeEach(async () => { + snapshot = await takeSnapshot() + }) + + afterEach(async () => { + await snapshot.restore() + }) + context("when staker wants to mint more shares than allowed", () => { let sharesToMint: bigint @@ -519,4 +530,118 @@ describe("Acre", () => { }, ) }) + + describe("updateStakingParameters", () => { + const validMinimumDepositAmount = to1e18(1) + const validMaximumTotalAssetsAmount = to1e18(30) + + context("when is called by owner", () => { + context("when all parameters are valid", () => { + let tx: ContractTransactionResponse + + beforeEach(async () => { + tx = await acre + .connect(owner) + .updateStakingParameters( + validMinimumDepositAmount, + validMaximumTotalAssetsAmount, + ) + }) + + it("should emit StakingParametersUpdated event", async () => { + await expect(tx) + .to.emit(acre, "StakingParametersUpdated") + .withArgs(validMinimumDepositAmount, validMaximumTotalAssetsAmount) + }) + + it("should update parameters correctly", async () => { + const stakingParameters = await acre.stakingParameters() + + expect(stakingParameters.minimumDepositAmount).to.be.eq( + validMinimumDepositAmount, + ) + expect(stakingParameters.maximumTotalAssets).to.be.eq( + validMaximumTotalAssetsAmount, + ) + }) + }) + + context("when minimum deposit amount is invalid", () => { + context("when it is equal to 0", () => { + const minimumDepositAmount = 0 + + it("should revert", async () => { + await expect( + acre + .connect(owner) + .updateStakingParameters( + minimumDepositAmount, + validMaximumTotalAssetsAmount, + ), + ).to.be.revertedWith( + "Minimum deposit amount must be greater than zero", + ) + }) + }) + + context( + "when it is less than the minimum deposit amount in tBTC system", + () => { + // TODO: In the current implementation the minimum deposit amount + // from tBTC system is hardcoded to 0.01 tBTC. We should get this + // value from mocked tBTC Bridge contract. + const minimumDepositAmountInTBTCSystem = 10000000000000000n + + const newMinimumDepositAmount = + minimumDepositAmountInTBTCSystem - 1n + + it("should revert", async () => { + await expect( + acre + .connect(owner) + .updateStakingParameters( + newMinimumDepositAmount, + validMaximumTotalAssetsAmount, + ), + ).to.be.revertedWith( + "Minimum deposit amount must be greater than or to the equal minimum deposit amount in tBTC system", + ) + }) + }, + ) + }) + + context("when the maximum total assets amount is invalid", () => { + const maximumTotalAssets = 0 + + context("when it is equal to 0", () => { + it("should revert", async () => { + await expect( + acre + .connect(owner) + .updateStakingParameters( + validMinimumDepositAmount, + maximumTotalAssets, + ), + ).to.be.revertedWith( + "Maximum total assets amount must be greater than zero", + ) + }) + }) + }) + }) + + context("when it is called by non-owner", () => { + it("should revert", async () => { + await expect( + acre + .connect(staker1) + .updateStakingParameters( + validMinimumDepositAmount, + validMaximumTotalAssetsAmount, + ), + ).to.be.revertedWithCustomError(acre, "OwnableUnauthorizedAccount") + }) + }) + }) }) From a5d022cf6c5fbc1506d2b036dc1cf8b0580bbe8d Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Fri, 8 Dec 2023 15:50:54 +0100 Subject: [PATCH 10/45] Fix unit tests We added a new role `owner` to the test fixture and we need to make sure we call functions with correct staker account. --- core/test/Acre.test.ts | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/core/test/Acre.test.ts b/core/test/Acre.test.ts index ac5d92938..282b045f2 100644 --- a/core/test/Acre.test.ts +++ b/core/test/Acre.test.ts @@ -424,7 +424,9 @@ describe("Acre", () => { // Total assets = 7(staker A) + 3(staker B) + 5(yield) + 2(staker // A) = 17 // Total shares = 7 + 3 + 1.(3) = 11.(3) - await acre.stake(newAmountToStake, staker1.address, referral) + await acre + .connect(staker1) + .stake(newAmountToStake, staker1.address, referral) }) it("should receive more shares", async () => { @@ -505,7 +507,7 @@ describe("Acre", () => { it("should take into account the max total assets parameter and revert", async () => { await expect( - acre.mint(sharesToMint, staker1.address), + acre.connect(staker1).mint(sharesToMint, staker1.address), ).to.be.revertedWithCustomError(acre, "ERC4626ExceededMaxMint") }) }) @@ -520,11 +522,14 @@ describe("Acre", () => { const previewDeposit = await acre.previewDeposit(minimumDepositAmount) sharesToMint = previewDeposit - 1n + await tbtc + .connect(staker1) + .approve(await acre.getAddress(), minimumDepositAmount) }) it("should take into account the min deposit amount parameter and revert", async () => { await expect( - acre.mint(sharesToMint, staker1.address), + acre.connect(staker1).mint(sharesToMint, staker1.address), ).to.be.revertedWith("Amount is less than minimum") }) }, From 4a1de543457244f20dac7f5169f093d5965ef78b Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Fri, 8 Dec 2023 15:58:33 +0100 Subject: [PATCH 11/45] Fix slither errors Fix too many digits error by using `ether` suffix as recomended in slither docs. --- core/contracts/Acre.sol | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/core/contracts/Acre.sol b/core/contracts/Acre.sol index a74151581..c438bcf3c 100644 --- a/core/contracts/Acre.sol +++ b/core/contracts/Acre.sol @@ -39,10 +39,8 @@ contract Acre is ERC4626, Ownable { ERC20("Acre Staked Bitcoin", "stBTC") Ownable(initialOwner) { - stakingParameters = StakingParameters({ - minimumDepositAmount: 10000000000000000, // 0.01 tBTC - maximumTotalAssets: 25000000000000000000 // 25 tBTC - }); + stakingParameters.minimumDepositAmount = 0.01 ether; // 0.01 tBTC + stakingParameters.maximumTotalAssets = 25 ether; // 25 tBTC } /// @notice Updates parameters of staking. @@ -162,6 +160,6 @@ contract Acre is ERC4626, Ownable { // // decimals. // return 10 ** (ERC20(asset()).decimals() - 8) * depositDustThreshold; - return 10000000000000000; // 0.01 tBTC + return 0.01 ether; // 0.01 tBTC } } From afc232335f5a9072f3fdca00ec1efda5fd7700b6 Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Fri, 8 Dec 2023 16:01:07 +0100 Subject: [PATCH 12/45] Fix deployment scripts Since the `Acre` contract inherits from `Ownable` contract from OZ lib we need to pass `initialOwner` to the Acre contract constructor. --- core/deploy/01_deploy_acre.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/deploy/01_deploy_acre.ts b/core/deploy/01_deploy_acre.ts index 531fc6c12..ffcfaf660 100644 --- a/core/deploy/01_deploy_acre.ts +++ b/core/deploy/01_deploy_acre.ts @@ -9,7 +9,7 @@ const func: DeployFunction = async (hre: HardhatRuntimeEnvironment) => { await deployments.deploy("Acre", { from: deployer, - args: [tbtc.address], + args: [tbtc.address, deployer], log: true, waitConfirmations: 1, }) From 7b907fe0e889a8a2440c239f13efaf416ab00eb2 Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Fri, 8 Dec 2023 16:40:37 +0100 Subject: [PATCH 13/45] Simplify overridden `deposit` fn There is no need to assign value from `super.deposit` to the new variable. We can return directly. --- core/contracts/Acre.sol | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/core/contracts/Acre.sol b/core/contracts/Acre.sol index c438bcf3c..3ee03a7aa 100644 --- a/core/contracts/Acre.sol +++ b/core/contracts/Acre.sol @@ -89,9 +89,7 @@ contract Acre is ERC4626, Ownable { "Amount is less than minimum" ); - uint256 shares = super.deposit(assets, receiver); - - return shares; + return super.deposit(assets, receiver); } function mint( From 2ecd47fc4435e05d61bb3f4837334bf6d7111fab Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Tue, 12 Dec 2023 10:43:17 +0100 Subject: [PATCH 14/45] Simplify the Acre constructor We can pass `msg.sender` to the `Ownable` constructor and later in the deployment scripts transfer the ownership at the end of the deployment scenario. --- core/contracts/Acre.sol | 9 ++------- core/deploy/01_deploy_acre.ts | 2 +- core/deploy/21_transfer_ownership_acre.ts | 2 -- core/test/Acre.test.ts | 2 +- 4 files changed, 4 insertions(+), 11 deletions(-) diff --git a/core/contracts/Acre.sol b/core/contracts/Acre.sol index 3ee03a7aa..c77cb4016 100644 --- a/core/contracts/Acre.sol +++ b/core/contracts/Acre.sol @@ -32,13 +32,8 @@ contract Acre is ERC4626, Ownable { ); constructor( - IERC20 tbtc, - address initialOwner - ) - ERC4626(tbtc) - ERC20("Acre Staked Bitcoin", "stBTC") - Ownable(initialOwner) - { + IERC20 tbtc + ) ERC4626(tbtc) ERC20("Acre Staked Bitcoin", "stBTC") Ownable(msg.sender) { stakingParameters.minimumDepositAmount = 0.01 ether; // 0.01 tBTC stakingParameters.maximumTotalAssets = 25 ether; // 25 tBTC } diff --git a/core/deploy/01_deploy_acre.ts b/core/deploy/01_deploy_acre.ts index ffcfaf660..531fc6c12 100644 --- a/core/deploy/01_deploy_acre.ts +++ b/core/deploy/01_deploy_acre.ts @@ -9,7 +9,7 @@ const func: DeployFunction = async (hre: HardhatRuntimeEnvironment) => { await deployments.deploy("Acre", { from: deployer, - args: [tbtc.address, deployer], + args: [tbtc.address], log: true, waitConfirmations: 1, }) diff --git a/core/deploy/21_transfer_ownership_acre.ts b/core/deploy/21_transfer_ownership_acre.ts index c62708641..09a875a1d 100644 --- a/core/deploy/21_transfer_ownership_acre.ts +++ b/core/deploy/21_transfer_ownership_acre.ts @@ -19,5 +19,3 @@ const func: DeployFunction = async (hre: HardhatRuntimeEnvironment) => { export default func func.tags = ["TransferOwnershipAcre"] -// TODO: Enable once Acre extends Ownable -func.skip = async () => true diff --git a/core/test/Acre.test.ts b/core/test/Acre.test.ts index 282b045f2..9d4ae800a 100644 --- a/core/test/Acre.test.ts +++ b/core/test/Acre.test.ts @@ -17,7 +17,7 @@ async function acreFixture() { const tbtc = await TestERC20.deploy() const Acre = await ethers.getContractFactory("Acre") - const acre = await Acre.deploy(await tbtc.getAddress(), owner) + const acre = await Acre.deploy(await tbtc.getAddress()) const amountToMint = to1e18(100000) tbtc.mint(staker1, amountToMint) From 425a06f0c0f650aa70ccf7f68920c4e8389d4e60 Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Tue, 12 Dec 2023 11:31:19 +0100 Subject: [PATCH 15/45] Update `maxDeposit` fn Cover a case where `totalAssets()` value is greater than `stakingParameters.maxiumumTotalAssets`. This can happen when vaults generated yield or `maximumTotalAssets` has been updated to a lower value. --- core/contracts/Acre.sol | 6 +++- core/test/Acre.test.ts | 72 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 1 deletion(-) diff --git a/core/contracts/Acre.sol b/core/contracts/Acre.sol index c77cb4016..0205cffc8 100644 --- a/core/contracts/Acre.sol +++ b/core/contracts/Acre.sol @@ -131,7 +131,11 @@ contract Acre is ERC4626, Ownable { /// Acre. /// @return The maximum amount of the tBTC token. function maxDeposit(address) public view override returns (uint256) { - return stakingParameters.maximumTotalAssets - totalAssets(); + uint256 _totalAssets = totalAssets(); + + if (_totalAssets >= stakingParameters.maximumTotalAssets) return 0; + + return stakingParameters.maximumTotalAssets - _totalAssets; } /// @notice Returns the maximum amount of the vault shares that can be diff --git a/core/test/Acre.test.ts b/core/test/Acre.test.ts index 9d4ae800a..1c2457abe 100644 --- a/core/test/Acre.test.ts +++ b/core/test/Acre.test.ts @@ -279,6 +279,10 @@ describe("Acre", () => { await tbtc.connect(staker2).mint(staker2.address, staker2AmountToStake) }) + after(async () => { + await snapshot.restore() + }) + context( "when the vault is empty and has not yet earned yield from strategies", () => { @@ -539,6 +543,15 @@ describe("Acre", () => { describe("updateStakingParameters", () => { const validMinimumDepositAmount = to1e18(1) const validMaximumTotalAssetsAmount = to1e18(30) + let snapshot: SnapshotRestorer + + beforeEach(async () => { + snapshot = await takeSnapshot() + }) + + afterEach(async () => { + await snapshot.restore() + }) context("when is called by owner", () => { context("when all parameters are valid", () => { @@ -649,4 +662,63 @@ describe("Acre", () => { }) }) }) + + describe("maxDeposit", () => { + let maximumTotalAssets: bigint + let snapshot: SnapshotRestorer + + beforeEach(async () => { + snapshot = await takeSnapshot() + ;({ maximumTotalAssets } = await acre.stakingParameters()) + }) + + afterEach(async () => { + await snapshot.restore() + }) + + context( + "when total assets is greater than maximum total assets amount", + () => { + beforeEach(async () => { + const toMint = maximumTotalAssets + 1n + + await tbtc.mint(await acre.getAddress(), toMint) + }) + + it("should return 0", async () => { + expect(await acre.maxDeposit(staker1.address)).to.be.eq(0) + }) + }, + ) + + context("when the vault is empty", () => { + it("should return maximum total assets amount", async () => { + expect(await acre.maxDeposit(staker1.address)).to.be.eq( + maximumTotalAssets, + ) + }) + }) + + context("when the maximum total amount has not yet been reached", () => { + let expectedValue: bigint + + beforeEach(async () => { + const toMint = to1e18(2) + const newMaximumTotalAssets = to1e18(30) + const minimumDepositAmount = to1e18(1) + + expectedValue = newMaximumTotalAssets - toMint + + await acre.updateStakingParameters( + minimumDepositAmount, + newMaximumTotalAssets, + ) + await tbtc.mint(await acre.getAddress(), toMint) + }) + + it("should return correct value", async () => { + expect(await acre.maxDeposit(staker1.address)).to.be.eq(expectedValue) + }) + }) + }) }) From 8535cfed0e0b29dad9e453917b58a9898711b28b Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Tue, 12 Dec 2023 11:34:56 +0100 Subject: [PATCH 16/45] Move the require check before `super.mint` --- core/contracts/Acre.sol | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/core/contracts/Acre.sol b/core/contracts/Acre.sol index 0205cffc8..c1119abe0 100644 --- a/core/contracts/Acre.sol +++ b/core/contracts/Acre.sol @@ -91,14 +91,12 @@ contract Acre is ERC4626, Ownable { uint256 shares, address receiver ) public override returns (uint256) { - uint256 assets = super.mint(shares, receiver); - require( - assets >= stakingParameters.minimumDepositAmount, + previewMint(shares) >= stakingParameters.minimumDepositAmount, "Amount is less than minimum" ); - return assets; + return super.mint(shares, receiver); } /// @notice Stakes a given amount of tBTC token and mints shares to a From ec6f06e35333032fa337d636ba791ff34826a794 Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Tue, 12 Dec 2023 11:48:12 +0100 Subject: [PATCH 17/45] Update `updateStakingParameters` fn Remove unnecessary requirement. `0.01` BTC is tBTC minting process limitation which is expected to be invoked before `Acre.stake` function. --- core/contracts/Acre.sol | 21 --------------------- core/test/Acre.test.ts | 26 -------------------------- 2 files changed, 47 deletions(-) diff --git a/core/contracts/Acre.sol b/core/contracts/Acre.sol index c1119abe0..b8b2dbc05 100644 --- a/core/contracts/Acre.sol +++ b/core/contracts/Acre.sol @@ -41,8 +41,6 @@ contract Acre is ERC4626, Ownable { /// @notice Updates parameters of staking. /// @dev Requirements: /// - Minimum deposit amount must be greater than zero, - /// - Minimum deposit amount must be greater than or equal to the - /// minimum deposit amount in the tBTC system, /// - Maximum total assets must be greater than zero. /// @param minimumDepositAmount New value of the minimum deposit amount. It /// is the minimum amount for a single deposit operation. @@ -58,12 +56,6 @@ contract Acre is ERC4626, Ownable { "Minimum deposit amount must be greater than zero" ); - // TODO: Do we need this requirement? - require( - minimumDepositAmount >= _getTBTCSystemMinDepositAmount(), - "Minimum deposit amount must be greater than or to the equal minimum deposit amount in tBTC system" - ); - require( maximumTotalAssets > 0, "Maximum total assets amount must be greater than zero" @@ -144,17 +136,4 @@ contract Acre is ERC4626, Ownable { function maxMint(address receiver) public view override returns (uint256) { return previewDeposit(maxDeposit(receiver)); } - - function _getTBTCSystemMinDepositAmount() private pure returns (uint256) { - // TODO: Implement if we want to make sure the minimum deposit amount is - // greater than or equal the minimum dposit amount in the tBTC system. - - // (uint64 depositDustThreshold, , , ) = tbtcBridge.depositParameters(); - - // // The `depositDustThreshold` is in satoshi so we need to cast to tBTC - // // decimals. - // return 10 ** (ERC20(asset()).decimals() - 8) * depositDustThreshold; - - return 0.01 ether; // 0.01 tBTC - } } diff --git a/core/test/Acre.test.ts b/core/test/Acre.test.ts index 1c2457abe..fe2d37110 100644 --- a/core/test/Acre.test.ts +++ b/core/test/Acre.test.ts @@ -601,32 +601,6 @@ describe("Acre", () => { ) }) }) - - context( - "when it is less than the minimum deposit amount in tBTC system", - () => { - // TODO: In the current implementation the minimum deposit amount - // from tBTC system is hardcoded to 0.01 tBTC. We should get this - // value from mocked tBTC Bridge contract. - const minimumDepositAmountInTBTCSystem = 10000000000000000n - - const newMinimumDepositAmount = - minimumDepositAmountInTBTCSystem - 1n - - it("should revert", async () => { - await expect( - acre - .connect(owner) - .updateStakingParameters( - newMinimumDepositAmount, - validMaximumTotalAssetsAmount, - ), - ).to.be.revertedWith( - "Minimum deposit amount must be greater than or to the equal minimum deposit amount in tBTC system", - ) - }) - }, - ) }) context("when the maximum total assets amount is invalid", () => { From 7516de4a7ed10ba3d00e4ba4b695c633947c00ed Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Tue, 12 Dec 2023 11:51:59 +0100 Subject: [PATCH 18/45] Leave TODO in `updateStakingParameters` Leave a TODO to remember about introducing a governable parameters update process. --- core/contracts/Acre.sol | 1 + 1 file changed, 1 insertion(+) diff --git a/core/contracts/Acre.sol b/core/contracts/Acre.sol index b8b2dbc05..63e30e8c8 100644 --- a/core/contracts/Acre.sol +++ b/core/contracts/Acre.sol @@ -51,6 +51,7 @@ contract Acre is ERC4626, Ownable { uint256 minimumDepositAmount, uint256 maximumTotalAssets ) external onlyOwner { + // TODO: Introduce a parameters update process. require( minimumDepositAmount > 0, "Minimum deposit amount must be greater than zero" From 331c1d30f5620e8a5a586d60d16d4516d375cb0a Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Tue, 12 Dec 2023 12:29:16 +0100 Subject: [PATCH 19/45] Define custom errors for the checks The ERC4626 already uses custom errors, so it would be nice to have them here for consistency. --- core/contracts/Acre.sol | 37 ++++++++++++++++++++----------------- core/test/Acre.test.ts | 14 +++++--------- 2 files changed, 25 insertions(+), 26 deletions(-) diff --git a/core/contracts/Acre.sol b/core/contracts/Acre.sol index 63e30e8c8..be4d78a82 100644 --- a/core/contracts/Acre.sol +++ b/core/contracts/Acre.sol @@ -31,6 +31,9 @@ contract Acre is ERC4626, Ownable { uint256 maximumTotalAssets ); + error StakingAmountLessThanMin(uint256 amount, uint256 min); + error InvalidStakingParameter(); + constructor( IERC20 tbtc ) ERC4626(tbtc) ERC20("Acre Staked Bitcoin", "stBTC") Ownable(msg.sender) { @@ -52,15 +55,9 @@ contract Acre is ERC4626, Ownable { uint256 maximumTotalAssets ) external onlyOwner { // TODO: Introduce a parameters update process. - require( - minimumDepositAmount > 0, - "Minimum deposit amount must be greater than zero" - ); - - require( - maximumTotalAssets > 0, - "Maximum total assets amount must be greater than zero" - ); + if (minimumDepositAmount <= 0 || maximumTotalAssets <= 0) { + revert InvalidStakingParameter(); + } stakingParameters.minimumDepositAmount = minimumDepositAmount; stakingParameters.maximumTotalAssets = maximumTotalAssets; @@ -72,10 +69,12 @@ contract Acre is ERC4626, Ownable { uint256 assets, address receiver ) public override returns (uint256) { - require( - assets >= stakingParameters.minimumDepositAmount, - "Amount is less than minimum" - ); + if (assets < stakingParameters.minimumDepositAmount) { + revert StakingAmountLessThanMin( + assets, + stakingParameters.minimumDepositAmount + ); + } return super.deposit(assets, receiver); } @@ -84,10 +83,14 @@ contract Acre is ERC4626, Ownable { uint256 shares, address receiver ) public override returns (uint256) { - require( - previewMint(shares) >= stakingParameters.minimumDepositAmount, - "Amount is less than minimum" - ); + uint256 assets = previewMint(shares); + + if (assets < stakingParameters.minimumDepositAmount) { + revert StakingAmountLessThanMin( + assets, + stakingParameters.minimumDepositAmount + ); + } return super.mint(shares, receiver); } diff --git a/core/test/Acre.test.ts b/core/test/Acre.test.ts index fe2d37110..94a5d451e 100644 --- a/core/test/Acre.test.ts +++ b/core/test/Acre.test.ts @@ -167,7 +167,7 @@ describe("Acre", () => { acre .connect(staker1) .stake(amountToStake, staker1.address, referral), - ).to.revertedWith("Amount is less than minimum") + ).to.revertedWithCustomError(acre, "StakingAmountLessThanMin") }) }) @@ -188,7 +188,7 @@ describe("Acre", () => { acre .connect(staker1) .stake(amountToStake, staker1.address, referral), - ).to.revertedWith("Amount is less than minimum") + ).to.revertedWithCustomError(acre, "StakingAmountLessThanMin") }) }) @@ -534,7 +534,7 @@ describe("Acre", () => { it("should take into account the min deposit amount parameter and revert", async () => { await expect( acre.connect(staker1).mint(sharesToMint, staker1.address), - ).to.be.revertedWith("Amount is less than minimum") + ).to.be.revertedWithCustomError(acre, "StakingAmountLessThanMin") }) }, ) @@ -596,9 +596,7 @@ describe("Acre", () => { minimumDepositAmount, validMaximumTotalAssetsAmount, ), - ).to.be.revertedWith( - "Minimum deposit amount must be greater than zero", - ) + ).to.be.revertedWithCustomError(acre, "InvalidStakingParameter") }) }) }) @@ -615,9 +613,7 @@ describe("Acre", () => { validMinimumDepositAmount, maximumTotalAssets, ), - ).to.be.revertedWith( - "Maximum total assets amount must be greater than zero", - ) + ).to.be.revertedWithCustomError(acre, "InvalidStakingParameter") }) }) }) From bff966536a08c6cddcf466b2026ba33ce49d71f4 Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Tue, 19 Dec 2023 15:04:23 +0100 Subject: [PATCH 20/45] Update `mint` function Check the `super.mint` result inline to optimize the gas usage. In this way we should save ~2k gas if we do not duplicate the `previewMint` call. --- core/contracts/Acre.sol | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/core/contracts/Acre.sol b/core/contracts/Acre.sol index be4d78a82..8fd1ef3d4 100644 --- a/core/contracts/Acre.sol +++ b/core/contracts/Acre.sol @@ -82,17 +82,16 @@ contract Acre is ERC4626, Ownable { function mint( uint256 shares, address receiver - ) public override returns (uint256) { - uint256 assets = previewMint(shares); - - if (assets < stakingParameters.minimumDepositAmount) { + ) public override returns (uint256 assets) { + if ( + (assets = super.mint(shares, receiver)) < + stakingParameters.minimumDepositAmount + ) { revert StakingAmountLessThanMin( assets, stakingParameters.minimumDepositAmount ); } - - return super.mint(shares, receiver); } /// @notice Stakes a given amount of tBTC token and mints shares to a From 9ed875d8bb83ca3a08124e9191a637330280e168 Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Tue, 19 Dec 2023 15:35:37 +0100 Subject: [PATCH 21/45] Update `updateStakingParameters` function We should leave the possibility to set `minimumDepositAmount` to zero. --- core/contracts/Acre.sol | 2 +- core/test/Acre.test.ts | 30 +++++++++++++++++------------- 2 files changed, 18 insertions(+), 14 deletions(-) diff --git a/core/contracts/Acre.sol b/core/contracts/Acre.sol index 8fd1ef3d4..2fab07dd0 100644 --- a/core/contracts/Acre.sol +++ b/core/contracts/Acre.sol @@ -55,7 +55,7 @@ contract Acre is ERC4626, Ownable { uint256 maximumTotalAssets ) external onlyOwner { // TODO: Introduce a parameters update process. - if (minimumDepositAmount <= 0 || maximumTotalAssets <= 0) { + if (minimumDepositAmount < 0 || maximumTotalAssets <= 0) { revert InvalidStakingParameter(); } diff --git a/core/test/Acre.test.ts b/core/test/Acre.test.ts index 8bf29c942..a912d4637 100644 --- a/core/test/Acre.test.ts +++ b/core/test/Acre.test.ts @@ -589,20 +589,24 @@ describe("Acre", () => { }) }) - context("when minimum deposit amount is invalid", () => { - context("when it is equal to 0", () => { - const minimumDepositAmount = 0 + context("when minimum deposit amount is 0", () => { + const newMinimumDepositAmount = 0 - it("should revert", async () => { - await expect( - acre - .connect(owner) - .updateStakingParameters( - minimumDepositAmount, - validMaximumTotalAssetsAmount, - ), - ).to.be.revertedWithCustomError(acre, "InvalidStakingParameter") - }) + beforeEach(async () => { + await acre + .connect(owner) + .updateStakingParameters( + newMinimumDepositAmount, + validMaximumTotalAssetsAmount, + ) + }) + + it("should update the minimum deposit amount correctly", async () => { + const stakingParameters = await acre.stakingParameters() + + expect(stakingParameters.minimumDepositAmount).to.be.eq( + newMinimumDepositAmount, + ) }) }) From f9f230aff688839084791336b0e46281cf3ce999 Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Tue, 19 Dec 2023 15:42:05 +0100 Subject: [PATCH 22/45] Rename error `StakingAmountLessThanMin` -> `DepositAmountLessThanMin` --- core/contracts/Acre.sol | 6 +++--- core/test/Acre.test.ts | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/core/contracts/Acre.sol b/core/contracts/Acre.sol index 2fab07dd0..9aa17b18b 100644 --- a/core/contracts/Acre.sol +++ b/core/contracts/Acre.sol @@ -31,7 +31,7 @@ contract Acre is ERC4626, Ownable { uint256 maximumTotalAssets ); - error StakingAmountLessThanMin(uint256 amount, uint256 min); + error DepositAmountLessThanMin(uint256 amount, uint256 min); error InvalidStakingParameter(); constructor( @@ -70,7 +70,7 @@ contract Acre is ERC4626, Ownable { address receiver ) public override returns (uint256) { if (assets < stakingParameters.minimumDepositAmount) { - revert StakingAmountLessThanMin( + revert DepositAmountLessThanMin( assets, stakingParameters.minimumDepositAmount ); @@ -87,7 +87,7 @@ contract Acre is ERC4626, Ownable { (assets = super.mint(shares, receiver)) < stakingParameters.minimumDepositAmount ) { - revert StakingAmountLessThanMin( + revert DepositAmountLessThanMin( assets, stakingParameters.minimumDepositAmount ); diff --git a/core/test/Acre.test.ts b/core/test/Acre.test.ts index a912d4637..3ea71677d 100644 --- a/core/test/Acre.test.ts +++ b/core/test/Acre.test.ts @@ -172,7 +172,7 @@ describe("Acre", () => { acre .connect(staker1) .stake(amountToStake, staker1.address, referral), - ).to.revertedWithCustomError(acre, "StakingAmountLessThanMin") + ).to.revertedWithCustomError(acre, "DepositAmountLessThanMin") }) }) @@ -193,7 +193,7 @@ describe("Acre", () => { acre .connect(staker1) .stake(amountToStake, staker1.address, referral), - ).to.revertedWithCustomError(acre, "StakingAmountLessThanMin") + ).to.revertedWithCustomError(acre, "DepositAmountLessThanMin") }) }) @@ -539,7 +539,7 @@ describe("Acre", () => { it("should take into account the min deposit amount parameter and revert", async () => { await expect( acre.connect(staker1).mint(sharesToMint, staker1.address), - ).to.be.revertedWithCustomError(acre, "StakingAmountLessThanMin") + ).to.be.revertedWithCustomError(acre, "DepositAmountLessThanMin") }) }, ) From b2c4a800e5d1c92e714480d21e6a671c9e46df6f Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Tue, 19 Dec 2023 17:01:06 +0100 Subject: [PATCH 23/45] Simplify `maxDeposit` function Use ternary operator instead of `if` statement. --- core/contracts/Acre.sol | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/core/contracts/Acre.sol b/core/contracts/Acre.sol index 9aa17b18b..5d9da7418 100644 --- a/core/contracts/Acre.sol +++ b/core/contracts/Acre.sol @@ -126,9 +126,10 @@ contract Acre is ERC4626, Ownable { function maxDeposit(address) public view override returns (uint256) { uint256 _totalAssets = totalAssets(); - if (_totalAssets >= stakingParameters.maximumTotalAssets) return 0; - - return stakingParameters.maximumTotalAssets - _totalAssets; + return + _totalAssets >= stakingParameters.maximumTotalAssets + ? 0 + : stakingParameters.maximumTotalAssets - _totalAssets; } /// @notice Returns the maximum amount of the vault shares that can be From ea67712dc49c33127052f1588fcec0aa32c4d1f2 Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Tue, 19 Dec 2023 17:22:03 +0100 Subject: [PATCH 24/45] Add unit test for `mint` function Add a simple test case to check that `mint` function works if the transaction is within the limits. --- core/test/Acre.test.ts | 45 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/core/test/Acre.test.ts b/core/test/Acre.test.ts index 3ea71677d..24c0514a6 100644 --- a/core/test/Acre.test.ts +++ b/core/test/Acre.test.ts @@ -505,6 +505,51 @@ describe("Acre", () => { await snapshot.restore() }) + context("when minting as first staker", () => { + const amountToStake = to1e18(1) + let tx: ContractTransactionResponse + let sharesToMint: bigint + + beforeEach(async () => { + sharesToMint = await acre.previewDeposit(amountToStake) + + await tbtc + .connect(staker1) + .approve(await acre.getAddress(), amountToStake) + + tx = await acre.connect(staker1).mint(sharesToMint, staker1.address) + }) + + it("should emit Deposit event", () => { + expect(tx).to.emit(acre, "Deposit").withArgs( + // Caller. + staker1.address, + // Receiver. + staker1.address, + // Staked tokens. + amountToStake, + // Received shares. + sharesToMint, + ) + }) + + it("should mint stBTC tokens", async () => { + await expect(tx).to.changeTokenBalances( + acre, + [staker1.address], + [sharesToMint], + ) + }) + + it("should transfer tBTC tokens", async () => { + await expect(tx).to.changeTokenBalances( + tbtc, + [staker1.address, acre], + [-amountToStake, amountToStake], + ) + }) + }) + context("when staker wants to mint more shares than allowed", () => { let sharesToMint: bigint From 43f583db56171b0812b1a27dad802924b8278592 Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Wed, 20 Dec 2023 11:34:40 +0100 Subject: [PATCH 25/45] Fix typo `anythig` -> `anything` and rewrap comment. --- core/test/Acre.test.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/core/test/Acre.test.ts b/core/test/Acre.test.ts index 24c0514a6..159fa46a5 100644 --- a/core/test/Acre.test.ts +++ b/core/test/Acre.test.ts @@ -58,9 +58,9 @@ describe("Acre", () => { context("with a referral", () => { const amountToStake = to1e18(1) - // In this test case there is only one staker and - // the token vault has not earned anythig yet so received shares are - // equal to staked tokens amount. + // In this test case, there is only one staker and the token vault has + // not earned anything yet so received shares are equal to staked tokens + // amount. const expectedReceivedShares = amountToStake let tx: ContractTransactionResponse From becb46e9a116306f3130bfebacf076ef88dff20a Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Wed, 20 Dec 2023 11:36:27 +0100 Subject: [PATCH 26/45] Add unit tests for `maxMint` function --- core/test/Acre.test.ts | 74 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 74 insertions(+) diff --git a/core/test/Acre.test.ts b/core/test/Acre.test.ts index 159fa46a5..aa30d2046 100644 --- a/core/test/Acre.test.ts +++ b/core/test/Acre.test.ts @@ -744,4 +744,78 @@ describe("Acre", () => { }) }) }) + + describe("maxMint", () => { + let maximumTotalAssets: bigint + let snapshot: SnapshotRestorer + + beforeEach(async () => { + snapshot = await takeSnapshot() + ;({ maximumTotalAssets } = await acre.stakingParameters()) + }) + + afterEach(async () => { + await snapshot.restore() + }) + + context( + "when total assets is greater than maximum total assets amount", + () => { + beforeEach(async () => { + const toMint = maximumTotalAssets + 1n + + await tbtc.mint(await acre.getAddress(), toMint) + }) + + it("should return 0", async () => { + expect(await acre.maxMint(staker1.address)).to.be.eq(0) + }) + }, + ) + + context("when the vault is empty", () => { + it("should return maximum total assets amount in shares", async () => { + // When the vault is empty the max shares amount is equal to the maximum + // total assets amount. + expect(await acre.maxMint(staker1.address)).to.be.eq(maximumTotalAssets) + }) + }) + + context("when the maximum total amount has not yet been reached", () => { + let expectedValue: bigint + + beforeEach(async () => { + const toMint = to1e18(2) + const amountToStake = to1e18(3) + const newMaximumTotalAssets = to1e18(30) + const minimumDepositAmount = to1e18(1) + + await acre + .connect(owner) + .updateStakingParameters(minimumDepositAmount, newMaximumTotalAssets) + + // Staker stakes 3 tBTC. + await tbtc + .connect(staker1) + .approve(await acre.getAddress(), amountToStake) + await acre.connect(staker1).deposit(amountToStake, staker1.address) + + // Vault earns 2 tBTC. + await tbtc.mint(await acre.getAddress(), toMint) + + // The current state is: + // Total assets: 5 + // Total supply: 3 + // Maximum total assets: 30 + // Current max deposit: 30 - 2 - 3 = 25 + // Max shares: 25 * 3 / 5 = 15 -> 15000000000000000001 in stBTC + // precision and rounding support. + expectedValue = 15000000000000000001n + }) + + it("should return correct value", async () => { + expect(await acre.maxMint(staker1.address)).to.be.eq(expectedValue) + }) + }) + }) }) From 1c42a377aede695f0ed22c705dd2f067cea4c370 Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Wed, 20 Dec 2023 11:44:30 +0100 Subject: [PATCH 27/45] Update validation of staking parameters The `minimumDepositAmount` is `uint256` type so `minimumDepositAmount < 0` will alwyas be true. We can remove this check and in previous commit we left a possibility to update the `minimumDepositAmount` to `0` so we do not have to validate this value, at least for now. Also `maximumTotalAssets` is `uint256` type so we can't assign negative values - here we only check if `maximumTotalAssets` is not zero. --- core/contracts/Acre.sol | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/core/contracts/Acre.sol b/core/contracts/Acre.sol index 5d9da7418..cfc6246d5 100644 --- a/core/contracts/Acre.sol +++ b/core/contracts/Acre.sol @@ -43,7 +43,6 @@ contract Acre is ERC4626, Ownable { /// @notice Updates parameters of staking. /// @dev Requirements: - /// - Minimum deposit amount must be greater than zero, /// - Maximum total assets must be greater than zero. /// @param minimumDepositAmount New value of the minimum deposit amount. It /// is the minimum amount for a single deposit operation. @@ -55,7 +54,7 @@ contract Acre is ERC4626, Ownable { uint256 maximumTotalAssets ) external onlyOwner { // TODO: Introduce a parameters update process. - if (minimumDepositAmount < 0 || maximumTotalAssets <= 0) { + if (maximumTotalAssets == 0) { revert InvalidStakingParameter(); } From a5903e3b5fccab73ce8e75d47a6e6cd9e066902a Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Wed, 20 Dec 2023 12:01:28 +0100 Subject: [PATCH 28/45] Update `maxMint` function According to the EIP-4626 `previewDeposit` and `convertToShares` slightly differ, especially in terms of taking fees into account. We plan to add fees for deposits and withdrawals that will be included in the previewDeposit function. This will cause `previewDeposit` and `convertToShares` return different values. Here for the `maxMint` calculation, we want to have pure assets to shares conversion, exclusive of the fees. --- core/contracts/Acre.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/contracts/Acre.sol b/core/contracts/Acre.sol index cfc6246d5..4f2c0e604 100644 --- a/core/contracts/Acre.sol +++ b/core/contracts/Acre.sol @@ -137,6 +137,6 @@ contract Acre is ERC4626, Ownable { /// function converts the maximum deposit amount to shares. /// @return The maximum amount of the vault shares. function maxMint(address receiver) public view override returns (uint256) { - return previewDeposit(maxDeposit(receiver)); + return convertToShares(maxDeposit(receiver)); } } From a13b09f8afbc78414dd2040a57239b65a7babd9c Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Wed, 20 Dec 2023 15:52:39 +0100 Subject: [PATCH 29/45] Get rid of `StakingParameter` struct To reduce complexity, especially for contract upgrades and dealing with storage changes in upgradable contracts, which is very tricky. Here we also expose `depositParameters` function that returns all deposit-related parameters in a single call. --- core/contracts/Acre.sol | 69 +++++++++++++++++++---------------------- core/test/Acre.test.ts | 47 +++++++++++++--------------- 2 files changed, 53 insertions(+), 63 deletions(-) diff --git a/core/contracts/Acre.sol b/core/contracts/Acre.sol index 4f2c0e604..38125a417 100644 --- a/core/contracts/Acre.sol +++ b/core/contracts/Acre.sol @@ -16,63 +16,59 @@ import "@openzeppelin/contracts/access/Ownable.sol"; /// burning of shares (stBTC), which are represented as standard ERC20 /// tokens, providing a seamless exchange with tBTC tokens. contract Acre is ERC4626, Ownable { - struct StakingParameters { - // Minimum amount for a single deposit operation. - uint256 minimumDepositAmount; - // Maximum total amount of tBTC token held by Acre. - uint256 maximumTotalAssets; - } - - StakingParameters public stakingParameters; + // Minimum amount for a single deposit operation. + uint256 public minimumDepositAmount; + // Maximum total amount of tBTC token held by Acre. + uint256 public maximumTotalAssets; event StakeReferral(bytes32 indexed referral, uint256 assets); - event StakingParametersUpdated( + event DepositParametersUpdated( uint256 minimumDepositAmount, uint256 maximumTotalAssets ); error DepositAmountLessThanMin(uint256 amount, uint256 min); - error InvalidStakingParameter(); + error InvalidDepositParameter(); constructor( IERC20 tbtc ) ERC4626(tbtc) ERC20("Acre Staked Bitcoin", "stBTC") Ownable(msg.sender) { - stakingParameters.minimumDepositAmount = 0.01 ether; // 0.01 tBTC - stakingParameters.maximumTotalAssets = 25 ether; // 25 tBTC + minimumDepositAmount = 0.01 ether; // 0.01 tBTC + maximumTotalAssets = 25 ether; // 25 tBTC } /// @notice Updates parameters of staking. /// @dev Requirements: /// - Maximum total assets must be greater than zero. - /// @param minimumDepositAmount New value of the minimum deposit amount. It + /// @param _minimumDepositAmount New value of the minimum deposit amount. It /// is the minimum amount for a single deposit operation. - /// @param maximumTotalAssets New value of the maximum total assets amount. + /// @param _maximumTotalAssets New value of the maximum total assets amount. /// It is the maximum amount of the tBTC token that the Acre can /// hold. - function updateStakingParameters( - uint256 minimumDepositAmount, - uint256 maximumTotalAssets + function updateDepositParameters( + uint256 _minimumDepositAmount, + uint256 _maximumTotalAssets ) external onlyOwner { // TODO: Introduce a parameters update process. - if (maximumTotalAssets == 0) { - revert InvalidStakingParameter(); + if (_maximumTotalAssets == 0) { + revert InvalidDepositParameter(); } - stakingParameters.minimumDepositAmount = minimumDepositAmount; - stakingParameters.maximumTotalAssets = maximumTotalAssets; + minimumDepositAmount = _minimumDepositAmount; + maximumTotalAssets = _maximumTotalAssets; - emit StakingParametersUpdated(minimumDepositAmount, maximumTotalAssets); + emit DepositParametersUpdated( + _minimumDepositAmount, + _maximumTotalAssets + ); } function deposit( uint256 assets, address receiver ) public override returns (uint256) { - if (assets < stakingParameters.minimumDepositAmount) { - revert DepositAmountLessThanMin( - assets, - stakingParameters.minimumDepositAmount - ); + if (assets < minimumDepositAmount) { + revert DepositAmountLessThanMin(assets, minimumDepositAmount); } return super.deposit(assets, receiver); @@ -82,14 +78,8 @@ contract Acre is ERC4626, Ownable { uint256 shares, address receiver ) public override returns (uint256 assets) { - if ( - (assets = super.mint(shares, receiver)) < - stakingParameters.minimumDepositAmount - ) { - revert DepositAmountLessThanMin( - assets, - stakingParameters.minimumDepositAmount - ); + if ((assets = super.mint(shares, receiver)) < minimumDepositAmount) { + revert DepositAmountLessThanMin(assets, minimumDepositAmount); } } @@ -126,9 +116,9 @@ contract Acre is ERC4626, Ownable { uint256 _totalAssets = totalAssets(); return - _totalAssets >= stakingParameters.maximumTotalAssets + _totalAssets >= maximumTotalAssets ? 0 - : stakingParameters.maximumTotalAssets - _totalAssets; + : maximumTotalAssets - _totalAssets; } /// @notice Returns the maximum amount of the vault shares that can be @@ -139,4 +129,9 @@ contract Acre is ERC4626, Ownable { function maxMint(address receiver) public view override returns (uint256) { return convertToShares(maxDeposit(receiver)); } + + /// @return Returns deposit parametrs + function depositParameters() public view returns (uint256, uint256) { + return (minimumDepositAmount, maximumTotalAssets); + } } diff --git a/core/test/Acre.test.ts b/core/test/Acre.test.ts index aa30d2046..57f9ce9e9 100644 --- a/core/test/Acre.test.ts +++ b/core/test/Acre.test.ts @@ -180,7 +180,7 @@ describe("Acre", () => { let amountToStake: bigint beforeEach(async () => { - const { minimumDepositAmount } = await acre.stakingParameters() + const [minimumDepositAmount] = await acre.depositParameters() amountToStake = minimumDepositAmount - 1n await tbtc @@ -202,7 +202,7 @@ describe("Acre", () => { let tx: ContractTransactionResponse beforeEach(async () => { - const { minimumDepositAmount } = await acre.stakingParameters() + const [minimumDepositAmount] = await acre.depositParameters() amountToStake = minimumDepositAmount await tbtc @@ -572,7 +572,7 @@ describe("Acre", () => { let sharesToMint: bigint beforeEach(async () => { - const { minimumDepositAmount } = await acre.stakingParameters() + const [minimumDepositAmount] = await acre.depositParameters() const previewDeposit = await acre.previewDeposit(minimumDepositAmount) sharesToMint = previewDeposit - 1n @@ -590,7 +590,7 @@ describe("Acre", () => { ) }) - describe("updateStakingParameters", () => { + describe("updateDepositParameters", () => { const validMinimumDepositAmount = to1e18(1) const validMaximumTotalAssetsAmount = to1e18(30) let snapshot: SnapshotRestorer @@ -610,27 +610,24 @@ describe("Acre", () => { beforeEach(async () => { tx = await acre .connect(owner) - .updateStakingParameters( + .updateDepositParameters( validMinimumDepositAmount, validMaximumTotalAssetsAmount, ) }) - it("should emit StakingParametersUpdated event", async () => { + it("should emit DepositParametersUpdated event", async () => { await expect(tx) - .to.emit(acre, "StakingParametersUpdated") + .to.emit(acre, "DepositParametersUpdated") .withArgs(validMinimumDepositAmount, validMaximumTotalAssetsAmount) }) it("should update parameters correctly", async () => { - const stakingParameters = await acre.stakingParameters() + const [minimumDepositAmount, maximumTotalAssets] = + await acre.depositParameters() - expect(stakingParameters.minimumDepositAmount).to.be.eq( - validMinimumDepositAmount, - ) - expect(stakingParameters.maximumTotalAssets).to.be.eq( - validMaximumTotalAssetsAmount, - ) + expect(minimumDepositAmount).to.be.eq(validMinimumDepositAmount) + expect(maximumTotalAssets).to.be.eq(validMaximumTotalAssetsAmount) }) }) @@ -640,18 +637,16 @@ describe("Acre", () => { beforeEach(async () => { await acre .connect(owner) - .updateStakingParameters( + .updateDepositParameters( newMinimumDepositAmount, validMaximumTotalAssetsAmount, ) }) it("should update the minimum deposit amount correctly", async () => { - const stakingParameters = await acre.stakingParameters() + const [minimumDepositAmount] = await acre.depositParameters() - expect(stakingParameters.minimumDepositAmount).to.be.eq( - newMinimumDepositAmount, - ) + expect(minimumDepositAmount).to.be.eq(newMinimumDepositAmount) }) }) @@ -663,11 +658,11 @@ describe("Acre", () => { await expect( acre .connect(owner) - .updateStakingParameters( + .updateDepositParameters( validMinimumDepositAmount, maximumTotalAssets, ), - ).to.be.revertedWithCustomError(acre, "InvalidStakingParameter") + ).to.be.revertedWithCustomError(acre, "InvalidDepositParameter") }) }) }) @@ -678,7 +673,7 @@ describe("Acre", () => { await expect( acre .connect(staker1) - .updateStakingParameters( + .updateDepositParameters( validMinimumDepositAmount, validMaximumTotalAssetsAmount, ), @@ -693,7 +688,7 @@ describe("Acre", () => { beforeEach(async () => { snapshot = await takeSnapshot() - ;({ maximumTotalAssets } = await acre.stakingParameters()) + maximumTotalAssets = await acre.maximumTotalAssets() }) afterEach(async () => { @@ -735,7 +730,7 @@ describe("Acre", () => { await acre .connect(owner) - .updateStakingParameters(minimumDepositAmount, newMaximumTotalAssets) + .updateDepositParameters(minimumDepositAmount, newMaximumTotalAssets) await tbtc.mint(await acre.getAddress(), toMint) }) @@ -751,7 +746,7 @@ describe("Acre", () => { beforeEach(async () => { snapshot = await takeSnapshot() - ;({ maximumTotalAssets } = await acre.stakingParameters()) + maximumTotalAssets = await acre.maximumTotalAssets() }) afterEach(async () => { @@ -792,7 +787,7 @@ describe("Acre", () => { await acre .connect(owner) - .updateStakingParameters(minimumDepositAmount, newMaximumTotalAssets) + .updateDepositParameters(minimumDepositAmount, newMaximumTotalAssets) // Staker stakes 3 tBTC. await tbtc From 31f74f2c69ed76b787c0c82c5c3409c33163dfdc Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Thu, 21 Dec 2023 07:46:32 +0100 Subject: [PATCH 30/45] Update `Acre` constructor Using `ether` as the unit will be confusing here, let's explicitly use decimals. Here we also leave a TODO - we need to revisit the exact values closer to the launch. --- core/contracts/Acre.sol | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/core/contracts/Acre.sol b/core/contracts/Acre.sol index 38125a417..0612a502d 100644 --- a/core/contracts/Acre.sol +++ b/core/contracts/Acre.sol @@ -33,8 +33,9 @@ contract Acre is ERC4626, Ownable { constructor( IERC20 tbtc ) ERC4626(tbtc) ERC20("Acre Staked Bitcoin", "stBTC") Ownable(msg.sender) { - minimumDepositAmount = 0.01 ether; // 0.01 tBTC - maximumTotalAssets = 25 ether; // 25 tBTC + // TODO: Revisit the exact values closer to the launch. + minimumDepositAmount = 0.01 * 1e18; // 0.01 tBTC + maximumTotalAssets = 25 * 1e18; // 25 tBTC } /// @notice Updates parameters of staking. From 5ec1b805f63fc338de823da96df069b3eb44fbf4 Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Thu, 21 Dec 2023 07:56:14 +0100 Subject: [PATCH 31/45] Rename error `InvalidDepositParameter` -> `InvalidMaximumTotalAssetsParameter`. The previous one was too general - now we exactly know what's wrong with which parameter. --- core/contracts/Acre.sol | 4 ++-- core/test/Acre.test.ts | 5 ++++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/core/contracts/Acre.sol b/core/contracts/Acre.sol index 0612a502d..edf4cebfa 100644 --- a/core/contracts/Acre.sol +++ b/core/contracts/Acre.sol @@ -28,7 +28,7 @@ contract Acre is ERC4626, Ownable { ); error DepositAmountLessThanMin(uint256 amount, uint256 min); - error InvalidDepositParameter(); + error InvalidMaximumTotalAssetsParameter(uint256 maximumTotalAssets); constructor( IERC20 tbtc @@ -52,7 +52,7 @@ contract Acre is ERC4626, Ownable { ) external onlyOwner { // TODO: Introduce a parameters update process. if (_maximumTotalAssets == 0) { - revert InvalidDepositParameter(); + revert InvalidMaximumTotalAssetsParameter(_maximumTotalAssets); } minimumDepositAmount = _minimumDepositAmount; diff --git a/core/test/Acre.test.ts b/core/test/Acre.test.ts index 57f9ce9e9..acc57a02c 100644 --- a/core/test/Acre.test.ts +++ b/core/test/Acre.test.ts @@ -662,7 +662,10 @@ describe("Acre", () => { validMinimumDepositAmount, maximumTotalAssets, ), - ).to.be.revertedWithCustomError(acre, "InvalidDepositParameter") + ).to.be.revertedWithCustomError( + acre, + "InvalidMaximumTotalAssetsParameter", + ) }) }) }) From 1f14cde7e114aff3fb2080f80ec9082f3de97978 Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Thu, 21 Dec 2023 08:16:11 +0100 Subject: [PATCH 32/45] Update Acre docs Add documentation to `deposit` and `mint` functions and fix typos. --- core/contracts/Acre.sol | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/core/contracts/Acre.sol b/core/contracts/Acre.sol index edf4cebfa..aea6a5ff7 100644 --- a/core/contracts/Acre.sol +++ b/core/contracts/Acre.sol @@ -38,7 +38,7 @@ contract Acre is ERC4626, Ownable { maximumTotalAssets = 25 * 1e18; // 25 tBTC } - /// @notice Updates parameters of staking. + /// @notice Updates deposit parameters. /// @dev Requirements: /// - Maximum total assets must be greater than zero. /// @param _minimumDepositAmount New value of the minimum deposit amount. It @@ -64,6 +64,15 @@ contract Acre is ERC4626, Ownable { ); } + /// @notice Mints shares to receiver by depositing exactly amount of + /// tBTC tokens. + /// @dev Takes into account a deposit parameter, minimum deposit amount, + /// which determines the minimum amount for a single deposit operation. + /// The amount of the assets has to be pre-approved in the tBTC + /// contract. + /// @param assets Approved amount of tBTC tokens to deposit. + /// @param receiver The address to which the shares will be minted. + /// @return Minted shares. function deposit( uint256 assets, address receiver @@ -75,6 +84,14 @@ contract Acre is ERC4626, Ownable { return super.deposit(assets, receiver); } + /// @notice Mints shares to receiver by depositing tBTC tokens. + /// @dev Takes into account a deposit parameter, minimum deposit amount, + /// which determines the minimum amount for a single deposit operation. + /// The amount of the assets has to be pre-approved in the tBTC + /// contract. + /// @param shares Amount of shares to mint. To get the amount of share use + /// `previewMint`. + /// @param receiver The address to which the shares will be minted. function mint( uint256 shares, address receiver @@ -109,7 +126,7 @@ contract Acre is ERC4626, Ownable { /// @notice Returns the maximum amount of the tBTC token that can be /// deposited into the vault for the receiver, through a deposit - /// call. It takes into account the staking parameter, maximum total + /// call. It takes into account the deposit parameter, maximum total /// assets, which determines the total amount of tBTC token held by /// Acre. /// @return The maximum amount of the tBTC token. From e8f7aa11c2e5835bac584e48325ab74ecf4c9e84 Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Thu, 21 Dec 2023 08:21:38 +0100 Subject: [PATCH 33/45] Remove unnecessary test We can remove `when amount to stake is 1` test, as we have it covered by `when amount to stake is less than minimum`. --- core/test/Acre.test.ts | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/core/test/Acre.test.ts b/core/test/Acre.test.ts index acc57a02c..ac37db303 100644 --- a/core/test/Acre.test.ts +++ b/core/test/Acre.test.ts @@ -158,24 +158,6 @@ describe("Acre", () => { }, ) - context("when amount to stake is 1", () => { - const amountToStake = 1 - - beforeEach(async () => { - await tbtc - .connect(staker1) - .approve(await acre.getAddress(), amountToStake) - }) - - it("should revert", async () => { - await expect( - acre - .connect(staker1) - .stake(amountToStake, staker1.address, referral), - ).to.revertedWithCustomError(acre, "DepositAmountLessThanMin") - }) - }) - context("when amount to stake is less than minimum", () => { let amountToStake: bigint From ab8d89cb277fe238df54a1cc3e7386afb7442c3c Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Thu, 21 Dec 2023 08:30:41 +0100 Subject: [PATCH 34/45] Add `.withArgs` for all errors that have args --- core/test/Acre.test.ts | 65 ++++++++++++++++++++++++++++++------------ 1 file changed, 47 insertions(+), 18 deletions(-) diff --git a/core/test/Acre.test.ts b/core/test/Acre.test.ts index ac37db303..1e3b85018 100644 --- a/core/test/Acre.test.ts +++ b/core/test/Acre.test.ts @@ -153,16 +153,19 @@ describe("Acre", () => { acre .connect(staker1) .stake(amountToStake, staker1.address, referral), - ).to.be.revertedWithCustomError(tbtc, "ERC20InsufficientAllowance") + ) + .to.be.revertedWithCustomError(tbtc, "ERC20InsufficientAllowance") + .withArgs(await acre.getAddress(), approvedAmount, amountToStake) }) }, ) context("when amount to stake is less than minimum", () => { let amountToStake: bigint + let minimumDepositAmount: bigint beforeEach(async () => { - const [minimumDepositAmount] = await acre.depositParameters() + ;[minimumDepositAmount] = await acre.depositParameters() amountToStake = minimumDepositAmount - 1n await tbtc @@ -175,7 +178,9 @@ describe("Acre", () => { acre .connect(staker1) .stake(amountToStake, staker1.address, referral), - ).to.revertedWithCustomError(acre, "DepositAmountLessThanMin") + ) + .to.revertedWithCustomError(acre, "DepositAmountLessThanMin") + .withArgs(amountToStake, minimumDepositAmount) }) }) @@ -217,7 +222,9 @@ describe("Acre", () => { it("should revert", async () => { await expect( acre.connect(staker1).stake(amountToStake, ZeroAddress, referral), - ).to.be.revertedWithCustomError(acre, "ERC20InvalidReceiver") + ) + .to.be.revertedWithCustomError(acre, "ERC20InvalidReceiver") + .withArgs(ZeroAddress) }) }) @@ -241,7 +248,9 @@ describe("Acre", () => { acre .connect(staker1) .stake(amountToStake, staker1.address, referral), - ).to.be.revertedWithCustomError(acre, "ERC20InsufficientAllowance") + ) + .to.be.revertedWithCustomError(acre, "ERC20InsufficientAllowance") + .withArgs(await acre.getAddress(), 0, amountToStake) }) }, ) @@ -466,9 +475,16 @@ describe("Acre", () => { }) it("should revert", async () => { - await expect( - acre.stake(amountToStake, staker1.address, referral), - ).to.be.revertedWithCustomError(acre, "ERC4626ExceededMaxDeposit") + await expect(acre.stake(amountToStake, staker1.address, referral)) + .to.be.revertedWithCustomError( + acre, + "ERC4626ExceededMaxDeposit", + ) + .withArgs( + staker1.address, + amountToStake, + possibleMaxAmountToStake, + ) }) }, ) @@ -534,17 +550,18 @@ describe("Acre", () => { context("when staker wants to mint more shares than allowed", () => { let sharesToMint: bigint + let maxMint: bigint beforeEach(async () => { - const maxMint = await acre.maxMint(staker1.address) + maxMint = await acre.maxMint(staker1.address) sharesToMint = maxMint + 1n }) it("should take into account the max total assets parameter and revert", async () => { - await expect( - acre.connect(staker1).mint(sharesToMint, staker1.address), - ).to.be.revertedWithCustomError(acre, "ERC4626ExceededMaxMint") + await expect(acre.connect(staker1).mint(sharesToMint, staker1.address)) + .to.be.revertedWithCustomError(acre, "ERC4626ExceededMaxMint") + .withArgs(staker1.address, sharesToMint, maxMint) }) }) @@ -552,9 +569,10 @@ describe("Acre", () => { "when staker wants to mint less shares than is equal to the min deposit amount", () => { let sharesToMint: bigint + let minimumDepositAmount: bigint beforeEach(async () => { - const [minimumDepositAmount] = await acre.depositParameters() + ;[minimumDepositAmount] = await acre.depositParameters() const previewDeposit = await acre.previewDeposit(minimumDepositAmount) sharesToMint = previewDeposit - 1n @@ -564,9 +582,16 @@ describe("Acre", () => { }) it("should take into account the min deposit amount parameter and revert", async () => { + // In this test case, there is only one staker and the token vault has + // not earned anything yet so received shares are equal to staked + // tokens amount. + const depositAmount = sharesToMint + await expect( acre.connect(staker1).mint(sharesToMint, staker1.address), - ).to.be.revertedWithCustomError(acre, "DepositAmountLessThanMin") + ) + .to.be.revertedWithCustomError(acre, "DepositAmountLessThanMin") + .withArgs(depositAmount, minimumDepositAmount) }) }, ) @@ -644,10 +669,12 @@ describe("Acre", () => { validMinimumDepositAmount, maximumTotalAssets, ), - ).to.be.revertedWithCustomError( - acre, - "InvalidMaximumTotalAssetsParameter", ) + .to.be.revertedWithCustomError( + acre, + "InvalidMaximumTotalAssetsParameter", + ) + .withArgs(maximumTotalAssets) }) }) }) @@ -662,7 +689,9 @@ describe("Acre", () => { validMinimumDepositAmount, validMaximumTotalAssetsAmount, ), - ).to.be.revertedWithCustomError(acre, "OwnableUnauthorizedAccount") + ) + .to.be.revertedWithCustomError(acre, "OwnableUnauthorizedAccount") + .withArgs(staker1.address) }) }) }) From 0e33e4bb62d56b4151fc0586e0ffc80aae70aed2 Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Thu, 21 Dec 2023 08:35:20 +0100 Subject: [PATCH 35/45] Celean up Acre unit tests - improve test descriptions, - use `minimumDepositAmount` instead of `depositParameters` in some cases where we only need the minimum deposit amount, - get rid of unnecessary `updateDepositParameters` call in `beforeEach` hook. We can test with default values. --- core/test/Acre.test.ts | 32 ++++++++++---------------------- 1 file changed, 10 insertions(+), 22 deletions(-) diff --git a/core/test/Acre.test.ts b/core/test/Acre.test.ts index 1e3b85018..a97c72d8d 100644 --- a/core/test/Acre.test.ts +++ b/core/test/Acre.test.ts @@ -165,7 +165,7 @@ describe("Acre", () => { let minimumDepositAmount: bigint beforeEach(async () => { - ;[minimumDepositAmount] = await acre.depositParameters() + minimumDepositAmount = await acre.minimumDepositAmount() amountToStake = minimumDepositAmount - 1n await tbtc @@ -189,7 +189,7 @@ describe("Acre", () => { let tx: ContractTransactionResponse beforeEach(async () => { - const [minimumDepositAmount] = await acre.depositParameters() + const minimumDepositAmount = await acre.minimumDepositAmount() amountToStake = minimumDepositAmount await tbtc @@ -548,7 +548,7 @@ describe("Acre", () => { }) }) - context("when staker wants to mint more shares than allowed", () => { + context("when staker wants to mint more shares than max mint limit", () => { let sharesToMint: bigint let maxMint: bigint @@ -566,13 +566,13 @@ describe("Acre", () => { }) context( - "when staker wants to mint less shares than is equal to the min deposit amount", + "when staker wants to mint less shares than the min deposit amount", () => { let sharesToMint: bigint let minimumDepositAmount: bigint beforeEach(async () => { - ;[minimumDepositAmount] = await acre.depositParameters() + minimumDepositAmount = await acre.minimumDepositAmount() const previewDeposit = await acre.previewDeposit(minimumDepositAmount) sharesToMint = previewDeposit - 1n @@ -651,7 +651,7 @@ describe("Acre", () => { }) it("should update the minimum deposit amount correctly", async () => { - const [minimumDepositAmount] = await acre.depositParameters() + const minimumDepositAmount = await acre.minimumDepositAmount() expect(minimumDepositAmount).to.be.eq(newMinimumDepositAmount) }) @@ -737,14 +737,8 @@ describe("Acre", () => { beforeEach(async () => { const toMint = to1e18(2) - const newMaximumTotalAssets = to1e18(30) - const minimumDepositAmount = to1e18(1) + expectedValue = maximumTotalAssets - toMint - expectedValue = newMaximumTotalAssets - toMint - - await acre - .connect(owner) - .updateDepositParameters(minimumDepositAmount, newMaximumTotalAssets) await tbtc.mint(await acre.getAddress(), toMint) }) @@ -796,12 +790,6 @@ describe("Acre", () => { beforeEach(async () => { const toMint = to1e18(2) const amountToStake = to1e18(3) - const newMaximumTotalAssets = to1e18(30) - const minimumDepositAmount = to1e18(1) - - await acre - .connect(owner) - .updateDepositParameters(minimumDepositAmount, newMaximumTotalAssets) // Staker stakes 3 tBTC. await tbtc @@ -816,10 +804,10 @@ describe("Acre", () => { // Total assets: 5 // Total supply: 3 // Maximum total assets: 30 - // Current max deposit: 30 - 2 - 3 = 25 - // Max shares: 25 * 3 / 5 = 15 -> 15000000000000000001 in stBTC + // Current max deposit: 25 - 2 - 3 = 20 + // Max shares: 20 * 3 / 5 = 15 -> 12000000000000000001 in stBTC // precision and rounding support. - expectedValue = 15000000000000000001n + expectedValue = 12000000000000000001n }) it("should return correct value", async () => { From edd69cf04b852200c9e8515a4d64d30716a77e04 Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Wed, 27 Dec 2023 15:34:58 +0100 Subject: [PATCH 36/45] Add missing `await` in `before` test hook --- core/test/Acre.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/test/Acre.test.ts b/core/test/Acre.test.ts index a97c72d8d..0ebf208e9 100644 --- a/core/test/Acre.test.ts +++ b/core/test/Acre.test.ts @@ -414,7 +414,7 @@ describe("Acre", () => { sharesBefore = await acre.balanceOf(staker1.address) availableToRedeemBefore = await acre.previewRedeem(sharesBefore) - tbtc.mint(staker1.address, newAmountToStake) + await tbtc.mint(staker1.address, newAmountToStake) await tbtc .connect(staker1) From a373b59f247d4de9c86743b0287238947329ef2b Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Wed, 27 Dec 2023 15:35:41 +0100 Subject: [PATCH 37/45] Use `convertToShares` instead of `previewDeposit` Use `convertToShares` instead of `previewDeposit` in tests - `convertToShares` does not take fees into account. --- core/test/Acre.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/test/Acre.test.ts b/core/test/Acre.test.ts index 0ebf208e9..d711569d6 100644 --- a/core/test/Acre.test.ts +++ b/core/test/Acre.test.ts @@ -573,9 +573,9 @@ describe("Acre", () => { beforeEach(async () => { minimumDepositAmount = await acre.minimumDepositAmount() - const previewDeposit = await acre.previewDeposit(minimumDepositAmount) + const shares = await acre.convertToShares(minimumDepositAmount) - sharesToMint = previewDeposit - 1n + sharesToMint = shares - 1n await tbtc .connect(staker1) .approve(await acre.getAddress(), minimumDepositAmount) From f5a44292d11d9b29a14e54dddcc207d591eac8a7 Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Wed, 27 Dec 2023 15:38:41 +0100 Subject: [PATCH 38/45] Update `mint` test case For the first staker, shares and stkaed amount will be the same number, we don't need to call `previewDeposit`. --- core/test/Acre.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/test/Acre.test.ts b/core/test/Acre.test.ts index d711569d6..cf98827cf 100644 --- a/core/test/Acre.test.ts +++ b/core/test/Acre.test.ts @@ -509,7 +509,7 @@ describe("Acre", () => { let sharesToMint: bigint beforeEach(async () => { - sharesToMint = await acre.previewDeposit(amountToStake) + sharesToMint = amountToStake await tbtc .connect(staker1) From fc5f5859290dfdc1a03b40378467b46d3b45b1ce Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Wed, 27 Dec 2023 15:45:39 +0100 Subject: [PATCH 39/45] Update `updateDepositParameters` fn We should allow to set the maximum total assets value to `0` so the `maxDeposit` function can return `0` when, for example, deposits are disabled. --- core/contracts/Acre.sol | 7 ------- core/test/Acre.test.ts | 29 ++++++++++++----------------- 2 files changed, 12 insertions(+), 24 deletions(-) diff --git a/core/contracts/Acre.sol b/core/contracts/Acre.sol index aea6a5ff7..3401e83c6 100644 --- a/core/contracts/Acre.sol +++ b/core/contracts/Acre.sol @@ -28,7 +28,6 @@ contract Acre is ERC4626, Ownable { ); error DepositAmountLessThanMin(uint256 amount, uint256 min); - error InvalidMaximumTotalAssetsParameter(uint256 maximumTotalAssets); constructor( IERC20 tbtc @@ -39,8 +38,6 @@ contract Acre is ERC4626, Ownable { } /// @notice Updates deposit parameters. - /// @dev Requirements: - /// - Maximum total assets must be greater than zero. /// @param _minimumDepositAmount New value of the minimum deposit amount. It /// is the minimum amount for a single deposit operation. /// @param _maximumTotalAssets New value of the maximum total assets amount. @@ -51,10 +48,6 @@ contract Acre is ERC4626, Ownable { uint256 _maximumTotalAssets ) external onlyOwner { // TODO: Introduce a parameters update process. - if (_maximumTotalAssets == 0) { - revert InvalidMaximumTotalAssetsParameter(_maximumTotalAssets); - } - minimumDepositAmount = _minimumDepositAmount; maximumTotalAssets = _maximumTotalAssets; diff --git a/core/test/Acre.test.ts b/core/test/Acre.test.ts index cf98827cf..baa124916 100644 --- a/core/test/Acre.test.ts +++ b/core/test/Acre.test.ts @@ -657,25 +657,20 @@ describe("Acre", () => { }) }) - context("when the maximum total assets amount is invalid", () => { - const maximumTotalAssets = 0 + context("when the maximum total assets amount is 0", () => { + const newMaximumTotalAssets = 0 - context("when it is equal to 0", () => { - it("should revert", async () => { - await expect( - acre - .connect(owner) - .updateDepositParameters( - validMinimumDepositAmount, - maximumTotalAssets, - ), + beforeEach(async () => { + await acre + .connect(owner) + .updateDepositParameters( + validMinimumDepositAmount, + newMaximumTotalAssets, ) - .to.be.revertedWithCustomError( - acre, - "InvalidMaximumTotalAssetsParameter", - ) - .withArgs(maximumTotalAssets) - }) + }) + + it("should update parameter correctly", async () => { + expect(await acre.maximumTotalAssets()).to.be.eq(0) }) }) }) From 1356fb07e65ea32fd38a6a4b54eda5cb55168dbf Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Wed, 27 Dec 2023 16:39:17 +0100 Subject: [PATCH 40/45] Allow to remove the limit for deposits We need a way to remove the limit for deposits. In such cases, we will set the value of `maximumTotalAssets` to the maximum (`type(uint256).max`). --- core/contracts/Acre.sol | 6 ++++++ core/test/Acre.test.ts | 40 +++++++++++++++++++++++++++++++++++++++- 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/core/contracts/Acre.sol b/core/contracts/Acre.sol index 3401e83c6..a705b8ba0 100644 --- a/core/contracts/Acre.sol +++ b/core/contracts/Acre.sol @@ -38,6 +38,8 @@ contract Acre is ERC4626, Ownable { } /// @notice Updates deposit parameters. + /// @dev To disable the limit for deposits, set the maximum total assets to + /// maximum (`type(uint256).max`). /// @param _minimumDepositAmount New value of the minimum deposit amount. It /// is the minimum amount for a single deposit operation. /// @param _maximumTotalAssets New value of the maximum total assets amount. @@ -124,6 +126,10 @@ contract Acre is ERC4626, Ownable { /// Acre. /// @return The maximum amount of the tBTC token. function maxDeposit(address) public view override returns (uint256) { + if (maximumTotalAssets == type(uint256).max) { + return type(uint256).max; + } + uint256 _totalAssets = totalAssets(); return diff --git a/core/test/Acre.test.ts b/core/test/Acre.test.ts index baa124916..87fccb743 100644 --- a/core/test/Acre.test.ts +++ b/core/test/Acre.test.ts @@ -5,6 +5,7 @@ import { import { expect } from "chai" import { ContractTransactionResponse, + MaxUint256, ZeroAddress, encodeBytes32String, } from "ethers" @@ -693,11 +694,13 @@ describe("Acre", () => { describe("maxDeposit", () => { let maximumTotalAssets: bigint + let minimumDepositAmount: bigint let snapshot: SnapshotRestorer beforeEach(async () => { snapshot = await takeSnapshot() - maximumTotalAssets = await acre.maximumTotalAssets() + ;[minimumDepositAmount, maximumTotalAssets] = + await acre.depositParameters() }) afterEach(async () => { @@ -741,6 +744,41 @@ describe("Acre", () => { expect(await acre.maxDeposit(staker1.address)).to.be.eq(expectedValue) }) }) + + context("when the deposit limit is disabled", () => { + const maximum = MaxUint256 + + beforeEach(async () => { + await acre + .connect(owner) + .updateDepositParameters(minimumDepositAmount, maximum) + }) + + context("when the vault is empty", () => { + it("should return the maximum value", async () => { + expect(await acre.maxDeposit(staker1.address)).to.be.eq(maximum) + }) + }) + + context("when the vault is not empty", () => { + const amountToStake = to1e18(1) + const referral = encodeBytes32String("referral") + + beforeEach(async () => { + await tbtc + .connect(staker1) + .approve(await acre.getAddress(), amountToStake) + + await acre + .connect(staker1) + .stake(amountToStake, staker1.address, referral) + }) + + it("should return the maximum value", async () => { + expect(await acre.maxDeposit(staker1.address)).to.be.eq(maximum) + }) + }) + }) }) describe("maxMint", () => { From 54bcf1724d0ddf643d42498c1b9898551db3163e Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Thu, 28 Dec 2023 15:05:59 +0100 Subject: [PATCH 41/45] Update `maxMint` function We need to handle a case where there is no maximum limit for deposits. --- core/contracts/Acre.sol | 7 ++++++- core/test/Acre.test.ts | 39 ++++++++++++++++++++++++++++++++++++++- 2 files changed, 44 insertions(+), 2 deletions(-) diff --git a/core/contracts/Acre.sol b/core/contracts/Acre.sol index a705b8ba0..2d968e4a3 100644 --- a/core/contracts/Acre.sol +++ b/core/contracts/Acre.sol @@ -144,7 +144,12 @@ contract Acre is ERC4626, Ownable { /// function converts the maximum deposit amount to shares. /// @return The maximum amount of the vault shares. function maxMint(address receiver) public view override returns (uint256) { - return convertToShares(maxDeposit(receiver)); + uint256 _maxDeposit = maxDeposit(receiver); + + return + _maxDeposit == type(uint256).max + ? type(uint256).max + : convertToShares(_maxDeposit); } /// @return Returns deposit parametrs diff --git a/core/test/Acre.test.ts b/core/test/Acre.test.ts index 87fccb743..d2091098f 100644 --- a/core/test/Acre.test.ts +++ b/core/test/Acre.test.ts @@ -783,11 +783,13 @@ describe("Acre", () => { describe("maxMint", () => { let maximumTotalAssets: bigint + let minimumDepositAmount: bigint let snapshot: SnapshotRestorer beforeEach(async () => { snapshot = await takeSnapshot() - maximumTotalAssets = await acre.maximumTotalAssets() + ;[minimumDepositAmount, maximumTotalAssets] = + await acre.depositParameters() }) afterEach(async () => { @@ -847,5 +849,40 @@ describe("Acre", () => { expect(await acre.maxMint(staker1.address)).to.be.eq(expectedValue) }) }) + + context("when the deposit limit is disabled", () => { + const maximum = MaxUint256 + + beforeEach(async () => { + await acre + .connect(owner) + .updateDepositParameters(minimumDepositAmount, maximum) + }) + + context("when the vault is empty", () => { + it("should return the maximum value", async () => { + expect(await acre.maxMint(staker1.address)).to.be.eq(maximum) + }) + }) + + context("when the vault is not empty", () => { + const amountToStake = to1e18(1) + const referral = encodeBytes32String("referral") + + beforeEach(async () => { + await tbtc + .connect(staker1) + .approve(await acre.getAddress(), amountToStake) + + await acre + .connect(staker1) + .stake(amountToStake, staker1.address, referral) + }) + + it("should return the maximum value", async () => { + expect(await acre.maxMint(staker1.address)).to.be.eq(maximum) + }) + }) + }) }) }) From 0f8b8308d86205044adf3f283486901ab0961cfd Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Thu, 28 Dec 2023 15:27:07 +0100 Subject: [PATCH 42/45] Add unit tests for `deposit` function We overridden `deposit` function and now it takes into account the minimum deposit amount parameter - here we add unit test for this function to cover a cases where the deposit amount is less than minimum and equal to the minimum amount. We indirectly cover this check's test in `stake` tests but we want to add an explicit `deposit` function test that covers it as well. --- core/test/Acre.test.ts | 66 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/core/test/Acre.test.ts b/core/test/Acre.test.ts index d2091098f..6e39f88be 100644 --- a/core/test/Acre.test.ts +++ b/core/test/Acre.test.ts @@ -885,4 +885,70 @@ describe("Acre", () => { }) }) }) + + describe("deposit", () => { + let amountToDeposit: bigint + let minimumDepositAmount: bigint + + beforeEach(async () => { + minimumDepositAmount = await acre.minimumDepositAmount() + }) + + context("when the deposit amount is less than minimum", () => { + beforeEach(() => { + amountToDeposit = minimumDepositAmount - 1n + }) + + it("should revert", async () => { + await expect(acre.deposit(amountToDeposit, staker1.address)) + .to.be.revertedWithCustomError(acre, "DepositAmountLessThanMin") + .withArgs(amountToDeposit, minimumDepositAmount) + }) + }) + + context( + "when the deposit amount is equal to the minimum deposit amount", + () => { + let tx: ContractTransactionResponse + let expectedReceivedShares: bigint + + beforeEach(async () => { + amountToDeposit = minimumDepositAmount + expectedReceivedShares = amountToDeposit + + await tbtc.approve(await acre.getAddress(), amountToDeposit) + tx = await acre.deposit(amountToDeposit, staker1.address) + }) + + it("should emit Deposit event", () => { + expect(tx).to.emit(acre, "Deposit").withArgs( + // Caller. + staker1.address, + // Receiver. + staker1.address, + // Staked tokens. + amountToDeposit, + // Received shares. + expectedReceivedShares, + ) + }) + + it("should mint stBTC tokens", async () => { + await expect(tx).to.changeTokenBalances( + acre, + [staker1.address], + [expectedReceivedShares], + ) + }) + + it("should transfer tBTC tokens", async () => { + await expect(tx).to.changeTokenBalances( + tbtc, + [staker1.address, acre], + [-amountToDeposit, amountToDeposit], + ) + }) + }, + ) + }) }) From 771553b1a7002f44de02c4d1c6a07858a61fc1ed Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Fri, 29 Dec 2023 09:22:49 +0100 Subject: [PATCH 43/45] Fix slither error --- core/contracts/Acre.sol | 1 + 1 file changed, 1 insertion(+) diff --git a/core/contracts/Acre.sol b/core/contracts/Acre.sol index 2d968e4a3..a7109a997 100644 --- a/core/contracts/Acre.sol +++ b/core/contracts/Acre.sol @@ -146,6 +146,7 @@ contract Acre is ERC4626, Ownable { function maxMint(address receiver) public view override returns (uint256) { uint256 _maxDeposit = maxDeposit(receiver); + // slither-disable-next-line incorrect-equality return _maxDeposit == type(uint256).max ? type(uint256).max From cc05de6bca66acc806208fbef04340af606a8241 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Czajkowski?= <57687279+r-czajkowski@users.noreply.github.com> Date: Fri, 29 Dec 2023 12:14:02 +0100 Subject: [PATCH 44/45] Fix typo `parametrs` -> `parameters` Co-authored-by: Jakub Nowakowski --- core/contracts/Acre.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/contracts/Acre.sol b/core/contracts/Acre.sol index a7109a997..83e10b835 100644 --- a/core/contracts/Acre.sol +++ b/core/contracts/Acre.sol @@ -153,7 +153,7 @@ contract Acre is ERC4626, Ownable { : convertToShares(_maxDeposit); } - /// @return Returns deposit parametrs + /// @return Returns deposit parameters. function depositParameters() public view returns (uint256, uint256) { return (minimumDepositAmount, maximumTotalAssets); } From d2fcf4a4ca5220585465e21187f8d3cac10084bb Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Fri, 29 Dec 2023 12:28:50 +0100 Subject: [PATCH 45/45] Add new test case for staking Add a case where total tBTC amount after staking would be equal the max amount, to confirm the transaction will succeed. --- core/test/Acre.test.ts | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/core/test/Acre.test.ts b/core/test/Acre.test.ts index 6e39f88be..776dea47a 100644 --- a/core/test/Acre.test.ts +++ b/core/test/Acre.test.ts @@ -489,6 +489,41 @@ describe("Acre", () => { }) }, ) + + context( + "when total tBTC amount after staking would be equal to the max amount", + () => { + let amountToStake: bigint + let tx: ContractTransactionResponse + + before(async () => { + amountToStake = await acre.maxDeposit(staker1.address) + + await tbtc + .connect(staker1) + .approve(await acre.getAddress(), amountToStake) + + tx = await acre.stake(amountToStake, staker1, referral) + }) + + it("should stake tokens correctly", async () => { + await expect(tx).to.emit(acre, "Deposit") + }) + + it("the max deposit amount should be equal 0", async () => { + expect(await acre.maxDeposit(staker1)).to.eq(0) + }) + + it("should not be able to stake more tokens", async () => { + await expect(acre.stake(amountToStake, staker1, referral)) + .to.be.revertedWithCustomError( + acre, + "ERC4626ExceededMaxDeposit", + ) + .withArgs(staker1.address, amountToStake, 0) + }) + }, + ) }) }) })