From 05030cfac884a1ea930971f5d582a5b52d09d26f Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Fri, 24 Nov 2023 15:14:43 +0100 Subject: [PATCH 01/60] Add OpenZeppelin contracts We want to implement the ERC4626 tokenized vault standard based on OpenZeppelin contracts. --- core/package.json | 3 +++ core/yarn.lock | 5 +++++ 2 files changed, 8 insertions(+) diff --git a/core/package.json b/core/package.json index 570eebfbf..a454a7064 100644 --- a/core/package.json +++ b/core/package.json @@ -55,5 +55,8 @@ "ts-node": "^10.9.1", "typechain": "^8.3.2", "typescript": "^5.2.2" + }, + "dependencies": { + "@openzeppelin/contracts": "^5.0.0" } } diff --git a/core/yarn.lock b/core/yarn.lock index cf702692a..43b0c719a 100644 --- a/core/yarn.lock +++ b/core/yarn.lock @@ -874,6 +874,11 @@ table "^6.8.0" undici "^5.14.0" +"@openzeppelin/contracts@^5.0.0": + version "5.0.0" + resolved "https://registry.yarnpkg.com/@openzeppelin/contracts/-/contracts-5.0.0.tgz#ee0e4b4564f101a5c4ee398cd4d73c0bd92b289c" + integrity sha512-bv2sdS6LKqVVMLI5+zqnNrNU/CA+6z6CmwFXm/MzmOPBRSO5reEJN7z0Gbzvs0/bv/MZZXNklubpwy3v2+azsw== + "@openzeppelin/defender-admin-client@^1.48.0": version "1.49.0" resolved "https://registry.yarnpkg.com/@openzeppelin/defender-admin-client/-/defender-admin-client-1.49.0.tgz#ed07318ccba10ac8a8a33cf594fc18b7ab5889f9" From c80b77cc14312e521a8cab56455e09a868dff24c Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Fri, 24 Nov 2023 15:23:02 +0100 Subject: [PATCH 02/60] Update solhint rules Based on [the solhint docs](https://github.com/protofire/solhint/blob/develop/docs/rules/security/func-visibility.md) This rule is required to be true for Solidity `>=0.7.0`. --- core/.solhint.json | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/core/.solhint.json b/core/.solhint.json index 7afe6405c..b53dba35b 100644 --- a/core/.solhint.json +++ b/core/.solhint.json @@ -1,5 +1,7 @@ { "extends": "thesis", "plugins": [], - "rules": {} + "rules": { + "func-visibility": ["off", { "ignoreConstructors": true }] + } } From 3ebb88b5567e8019c0079ca1a1fd7e2e3f7b17da Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Fri, 24 Nov 2023 15:25:51 +0100 Subject: [PATCH 03/60] Initial implementation of the main Acre contract The Acre contract is ERC4626 tokenized vault standard. Here we inherit from abstract contract `ERC4626` from OpenZeppelin lib. We are going to implement some custom features in follow-up work but the core is based on the ERC4626 standard. --- core/contracts/Acre.sol | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/core/contracts/Acre.sol b/core/contracts/Acre.sol index cb6ed5f33..7f82199a0 100644 --- a/core/contracts/Acre.sol +++ b/core/contracts/Acre.sol @@ -1,9 +1,8 @@ // SPDX-License-Identifier: GPL-3.0-only pragma solidity ^0.8.21; -// Uncomment this line to use console.log -// import "hardhat/console.sol"; +import "@openzeppelin/contracts/token/ERC20/extensions/ERC4626.sol"; -contract Acre { - // TODO: add your implementation +contract Acre is ERC4626 { + constructor(IERC20 _token) ERC4626(_token) ERC20("Staking BTC", "stBTC") {} } From c81cfd570008b17a75476b07496b85bd3125b4d7 Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Fri, 24 Nov 2023 16:17:17 +0100 Subject: [PATCH 04/60] Add initial implementation of the stake function. Stakes a given amount of underlying token and mints shares to a receiver. This function calls `deposit` function from `ERC4626` contract. The function should accept `referrer` input parameter that will be later used for accounting (out of scope of this issue). --- core/contracts/Acre.sol | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/core/contracts/Acre.sol b/core/contracts/Acre.sol index 7f82199a0..0d4819ba7 100644 --- a/core/contracts/Acre.sol +++ b/core/contracts/Acre.sol @@ -5,4 +5,21 @@ import "@openzeppelin/contracts/token/ERC20/extensions/ERC4626.sol"; contract Acre is ERC4626 { constructor(IERC20 _token) ERC4626(_token) ERC20("Staking BTC", "stBTC") {} + + /// @notice Stakes a given amount of underlying token and mints shares to a + /// receiver. + /// @dev This function calls `deposit` function from `ERC4626` contract. + /// @param assets Approved amount for the transfer and stake. + /// @param receiver The address to which the shares will be minted. + /// @param referrer Data used for refferal program. + /// @return shares Minted shares. + function stake( + uint256 assets, + address receiver, + bytes32 referrer + ) external returns (uint256 shares) { + require(referrer != bytes32(0), "Referrer can not be empty"); + + return super.deposit(assets, receiver); + } } From e754acab851b72d1b0f91acac208a32697c7221a Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Mon, 27 Nov 2023 11:29:15 +0100 Subject: [PATCH 05/60] Add `@thesis/solidity-contracts` dependency We want to support `receiveApproval`/`approveAndCall` pattern in the Acre contract. This package provides required interfaces to support this pattern. --- core/package.json | 3 ++- core/yarn.lock | 11 +++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/core/package.json b/core/package.json index a454a7064..89acd2dd1 100644 --- a/core/package.json +++ b/core/package.json @@ -57,6 +57,7 @@ "typescript": "^5.2.2" }, "dependencies": { - "@openzeppelin/contracts": "^5.0.0" + "@openzeppelin/contracts": "^5.0.0", + "@thesis/solidity-contracts": "github:thesis/solidity-contracts#c315b9d" } } diff --git a/core/yarn.lock b/core/yarn.lock index 43b0c719a..7359ea7cf 100644 --- a/core/yarn.lock +++ b/core/yarn.lock @@ -874,6 +874,11 @@ table "^6.8.0" undici "^5.14.0" +"@openzeppelin/contracts@^4.1.0": + version "4.9.3" + resolved "https://registry.yarnpkg.com/@openzeppelin/contracts/-/contracts-4.9.3.tgz#00d7a8cf35a475b160b3f0293a6403c511099364" + integrity sha512-He3LieZ1pP2TNt5JbkPA4PNT9WC3gOTOlDcFGJW4Le4QKqwmiNJCRt44APfxMxvq7OugU/cqYuPcSBzOw38DAg== + "@openzeppelin/contracts@^5.0.0": version "5.0.0" resolved "https://registry.yarnpkg.com/@openzeppelin/contracts/-/contracts-5.0.0.tgz#ee0e4b4564f101a5c4ee398cd4d73c0bd92b289c" @@ -1112,6 +1117,12 @@ version "0.0.2" resolved "https://codeload.github.com/thesis/prettier-config/tar.gz/daeaac564056a7885e4366ce12bfde6fd823fc90" +"@thesis/solidity-contracts@github:thesis/solidity-contracts#c315b9d": + version "0.0.1-pre" + resolved "https://codeload.github.com/thesis/solidity-contracts/tar.gz/c315b9d5f0c39253428898474e97552f5e5b9046" + dependencies: + "@openzeppelin/contracts" "^4.1.0" + "@tsconfig/node10@^1.0.7": version "1.0.9" resolved "https://registry.yarnpkg.com/@tsconfig/node10/-/node10-1.0.9.tgz#df4907fc07a886922637b15e02d4cebc4c0021b2" From f36c65cd38ff7ba6a2e04944671f4d4523c294aa Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Mon, 27 Nov 2023 11:35:38 +0100 Subject: [PATCH 06/60] Add support for receive approval pattern to Acre The tBTC token contract that will be a staking token supports this pattern. To be able to stake in one transaction (instead of 2: approve + stake) we must implement the `RecieveApproval` interface. The token staking contract receives approval to spend tokens and create a stake for a given account. --- core/contracts/Acre.sol | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/core/contracts/Acre.sol b/core/contracts/Acre.sol index 0d4819ba7..0c19cac2e 100644 --- a/core/contracts/Acre.sol +++ b/core/contracts/Acre.sol @@ -2,10 +2,34 @@ pragma solidity ^0.8.21; import "@openzeppelin/contracts/token/ERC20/extensions/ERC4626.sol"; +import "@thesis/solidity-contracts/contracts/token/IReceiveApproval.sol"; -contract Acre is ERC4626 { +contract Acre is ERC4626, IReceiveApproval { constructor(IERC20 _token) ERC4626(_token) ERC20("Staking BTC", "stBTC") {} + /// @notice Receives approval of token transfer and stakes the approved + /// amount. + /// @dev Requires that the provided token contract be the same one linked to + /// this contract. + /// @param from The owner of the tokens who approved them to transfer. + /// @param amount Approved amount for the transfer and stake. + /// @param _token Token contract address. + /// @param extraData Extra data for stake. This byte array must have the + /// following values concatenated: + /// - referrer ID (32 bytes) + function receiveApproval( + address from, + uint256 amount, + address _token, + bytes calldata extraData + ) external override { + require(_token == super.asset(), "Unrecognized token"); + // TODO: Decide on the format of the `extradata` variable. + require(extraData.length >= 32, "Corrupted stake data"); + + this.stake(amount, from, bytes32(extraData)); + } + /// @notice Stakes a given amount of underlying token and mints shares to a /// receiver. /// @dev This function calls `deposit` function from `ERC4626` contract. From 14c14e146ab8e882c66036b0a65a20f45ce2e8da Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Mon, 27 Nov 2023 11:57:44 +0100 Subject: [PATCH 07/60] Add test token contract Use this contract in unit tests as tBTC token. --- core/contracts/test/TestToken.sol | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 core/contracts/test/TestToken.sol diff --git a/core/contracts/test/TestToken.sol b/core/contracts/test/TestToken.sol new file mode 100644 index 000000000..5a83eabd8 --- /dev/null +++ b/core/contracts/test/TestToken.sol @@ -0,0 +1,31 @@ +// SPDX-License-Identifier: GPL-3.0-only +pragma solidity ^0.8.21; + +import "@openzeppelin/contracts/token/ERC20/ERC20.sol"; +import "@thesis/solidity-contracts/contracts/token/IApproveAndCall.sol"; +import "@thesis/solidity-contracts/contracts/token/IReceiveApproval.sol"; + +contract TestToken is ERC20, IApproveAndCall { + constructor() ERC20("Test Token", "TEST") {} + + function mint(address account, uint256 value) external { + _mint(account, value); + } + + function approveAndCall( + address spender, + uint256 amount, + bytes memory extraData + ) external returns (bool) { + if (approve(spender, amount)) { + IReceiveApproval(spender).receiveApproval( + msg.sender, + amount, + address(this), + extraData + ); + return true; + } + return false; + } +} From 02c2b3b4495340dffa0867f13538d0dd2bfeb606 Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Mon, 27 Nov 2023 16:54:03 +0100 Subject: [PATCH 08/60] Add basic unit tests for staking --- core/contracts/Acre.sol | 2 +- core/test/Acre.test.ts | 117 +++++++++++++++++++++++++++++++++++++++- 2 files changed, 116 insertions(+), 3 deletions(-) diff --git a/core/contracts/Acre.sol b/core/contracts/Acre.sol index 0c19cac2e..104e1a586 100644 --- a/core/contracts/Acre.sol +++ b/core/contracts/Acre.sol @@ -25,7 +25,7 @@ contract Acre is ERC4626, IReceiveApproval { ) external override { require(_token == super.asset(), "Unrecognized token"); // TODO: Decide on the format of the `extradata` variable. - require(extraData.length >= 32, "Corrupted stake data"); + require(extraData.length == 32, "Corrupted stake data"); this.stake(amount, from, bytes32(extraData)); } diff --git a/core/test/Acre.test.ts b/core/test/Acre.test.ts index 97775ba4b..b4e301df0 100644 --- a/core/test/Acre.test.ts +++ b/core/test/Acre.test.ts @@ -1,5 +1,118 @@ +import { loadFixture } from "@nomicfoundation/hardhat-toolbox/network-helpers" +import { HardhatEthersSigner } from "@nomicfoundation/hardhat-ethers/signers" +import { ethers } from "hardhat" +import { expect } from "chai" +import { WeiPerEther } from "ethers" +import type { TestToken, Acre } from "../typechain" + +async function acreFixture() { + const [_, tokenHolder] = await ethers.getSigners() + const Token = await ethers.getContractFactory("TestToken") + const token = await Token.deploy() + + const amountToMint = WeiPerEther * 10000n + + token.mint(tokenHolder, amountToMint) + + const Acre = await ethers.getContractFactory("Acre") + const acre = await Acre.deploy(await token.getAddress()) + + return { acre, token, tokenHolder } +} + describe("Acre", () => { - describe("Add your tests", () => { - it("Should validate something", async () => {}) + let acre: Acre + let token: TestToken + let tokenHolder: HardhatEthersSigner + + beforeEach(async () => { + ;({ acre, token, tokenHolder } = await loadFixture(acreFixture)) + }) + + describe("Staking", () => { + const referrer = ethers.encodeBytes32String("referrer") + + context("when staking via Acre contract", () => { + beforeEach(async () => { + // Infinite approval for staking contract. + await token + .connect(tokenHolder) + .approve(await acre.getAddress(), ethers.MaxUint256) + }) + + it("should stake tokens and receive shares", async () => { + const tokenHolderAddress = tokenHolder.address + const amountToStake = await token.balanceOf(tokenHolderAddress) + + await expect( + acre + .connect(tokenHolder) + .stake(amountToStake, tokenHolderAddress, referrer), + ) + .to.emit(acre, "Deposit") + .withArgs( + // Caller. + tokenHolderAddress, + // Receiver. + tokenHolderAddress, + // Staked tokens. + amountToStake, + // Received shares. 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. + amountToStake, + ) + expect(await acre.balanceOf(tokenHolderAddress)).to.be.eq(amountToStake) + expect(await token.balanceOf(tokenHolderAddress)).to.be.eq(0) + }) + + it("should revert if the referrer is zero valu", async () => { + const emptyReferrer = ethers.encodeBytes32String("") + + await expect( + acre + .connect(tokenHolder) + .stake(1, tokenHolder.address, emptyReferrer), + ).to.be.revertedWith("Referrer can not be empty") + }) + }) + + context( + "when staking via staking token using approve and call pattern", + () => { + it("should stake tokens", () => { + // TODO: The `ERC4626.deposit` sets the owner as `msg.sender` under + // the hood. Using the approve and call pattern the `msg.sender` is + // token address, so we are actually trying to send tokens from the + // token contract to the receiver, which is impossible. We need to + // decide if we want to override the `deposit` function to allow + // staking via approve and call pattern. + }) + + it("should revert when called not for linked token", async () => { + const FakeToken = await ethers.getContractFactory("TestToken") + const fakeToken = await FakeToken.deploy() + const acreAddress = await acre.getAddress() + + await expect( + fakeToken.connect(tokenHolder).approveAndCall(acreAddress, 1, "0x"), + ).to.be.revertedWith("Unrecognized token") + }) + + it("should revert when extra data is invalid", async () => { + const acreAddress = await acre.getAddress() + const invalidExtraData = ethers.AbiCoder.defaultAbiCoder().encode( + ["bytes32", "address"], + [referrer, tokenHolder.address], + ) + + await expect( + token + .connect(tokenHolder) + .approveAndCall(acreAddress, 1, invalidExtraData), + ).to.be.revertedWith("Corrupted stake data") + }) + }, + ) }) }) From a9b6c0c3ec5107cbbe83ac46f6049eff9dd77977 Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Tue, 28 Nov 2023 13:47:13 +0100 Subject: [PATCH 09/60] Remove unnecessary `super` keyword There is no need to add `super` keyword unless you want to override to function and call the original implementation. --- core/contracts/Acre.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/contracts/Acre.sol b/core/contracts/Acre.sol index 104e1a586..913b10eb1 100644 --- a/core/contracts/Acre.sol +++ b/core/contracts/Acre.sol @@ -23,7 +23,7 @@ contract Acre is ERC4626, IReceiveApproval { address _token, bytes calldata extraData ) external override { - require(_token == super.asset(), "Unrecognized token"); + require(_token == asset(), "Unrecognized token"); // TODO: Decide on the format of the `extradata` variable. require(extraData.length == 32, "Corrupted stake data"); @@ -44,6 +44,6 @@ contract Acre is ERC4626, IReceiveApproval { ) external returns (uint256 shares) { require(referrer != bytes32(0), "Referrer can not be empty"); - return super.deposit(assets, receiver); + return deposit(assets, receiver); } } From a4f9f8725b76f72c5eedaefa9f9ff0cc60ccb7e2 Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Tue, 28 Nov 2023 13:50:48 +0100 Subject: [PATCH 10/60] Update the name of the ERC4626 token We decided to use `Acre Staked Bitcoin` as a full name of the ERC4626 token. --- core/contracts/Acre.sol | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/core/contracts/Acre.sol b/core/contracts/Acre.sol index 913b10eb1..8006b1684 100644 --- a/core/contracts/Acre.sol +++ b/core/contracts/Acre.sol @@ -5,7 +5,9 @@ import "@openzeppelin/contracts/token/ERC20/extensions/ERC4626.sol"; import "@thesis/solidity-contracts/contracts/token/IReceiveApproval.sol"; contract Acre is ERC4626, IReceiveApproval { - constructor(IERC20 _token) ERC4626(_token) ERC20("Staking BTC", "stBTC") {} + constructor( + IERC20 _token + ) ERC4626(_token) ERC20("Acre Staked Bitcoin", "stBTC") {} /// @notice Receives approval of token transfer and stakes the approved /// amount. From 028319fce92aca1023ec770de54cde2e4fc8d8e3 Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Tue, 28 Nov 2023 14:02:11 +0100 Subject: [PATCH 11/60] Remove unnecessary undersocre for `_token` Use `token` as it's how it is defined in the `IReceiveApproval` interface. It also doesn't collide with another property so the underscore is unnecessary. --- core/contracts/Acre.sol | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/core/contracts/Acre.sol b/core/contracts/Acre.sol index 8006b1684..cf890c22e 100644 --- a/core/contracts/Acre.sol +++ b/core/contracts/Acre.sol @@ -15,17 +15,17 @@ contract Acre is ERC4626, IReceiveApproval { /// this contract. /// @param from The owner of the tokens who approved them to transfer. /// @param amount Approved amount for the transfer and stake. - /// @param _token Token contract address. + /// @param token Token contract address. /// @param extraData Extra data for stake. This byte array must have the /// following values concatenated: /// - referrer ID (32 bytes) function receiveApproval( address from, uint256 amount, - address _token, + address token, bytes calldata extraData ) external override { - require(_token == asset(), "Unrecognized token"); + require(token == asset(), "Unrecognized token"); // TODO: Decide on the format of the `extradata` variable. require(extraData.length == 32, "Corrupted stake data"); From f1a126b0d97eab4b3af115bef81ff16ad4004355 Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Tue, 28 Nov 2023 14:11:35 +0100 Subject: [PATCH 12/60] Update the `stake` function This function is called by `receiveApproval` internally so we should use `public` instead of `external`. --- core/contracts/Acre.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/contracts/Acre.sol b/core/contracts/Acre.sol index cf890c22e..91c2fc714 100644 --- a/core/contracts/Acre.sol +++ b/core/contracts/Acre.sol @@ -29,7 +29,7 @@ contract Acre is ERC4626, IReceiveApproval { // TODO: Decide on the format of the `extradata` variable. require(extraData.length == 32, "Corrupted stake data"); - this.stake(amount, from, bytes32(extraData)); + stake(amount, from, bytes32(extraData)); } /// @notice Stakes a given amount of underlying token and mints shares to a @@ -43,7 +43,7 @@ contract Acre is ERC4626, IReceiveApproval { uint256 assets, address receiver, bytes32 referrer - ) external returns (uint256 shares) { + ) public returns (uint256 shares) { require(referrer != bytes32(0), "Referrer can not be empty"); return deposit(assets, receiver); From 684d667cc517a9b50d4942d0d36c3a6ed03f4dca Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Tue, 28 Nov 2023 14:26:08 +0100 Subject: [PATCH 13/60] Update the Acre unit tests Rename `token` -> `tbtc`. --- core/test/Acre.test.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/core/test/Acre.test.ts b/core/test/Acre.test.ts index b4e301df0..c82ef81eb 100644 --- a/core/test/Acre.test.ts +++ b/core/test/Acre.test.ts @@ -22,11 +22,11 @@ async function acreFixture() { describe("Acre", () => { let acre: Acre - let token: TestToken + let tbtc: TestToken let tokenHolder: HardhatEthersSigner beforeEach(async () => { - ;({ acre, token, tokenHolder } = await loadFixture(acreFixture)) + ;({ acre, token: tbtc, tokenHolder } = await loadFixture(acreFixture)) }) describe("Staking", () => { @@ -35,14 +35,14 @@ describe("Acre", () => { context("when staking via Acre contract", () => { beforeEach(async () => { // Infinite approval for staking contract. - await token + await tbtc .connect(tokenHolder) .approve(await acre.getAddress(), ethers.MaxUint256) }) it("should stake tokens and receive shares", async () => { const tokenHolderAddress = tokenHolder.address - const amountToStake = await token.balanceOf(tokenHolderAddress) + const amountToStake = await tbtc.balanceOf(tokenHolderAddress) await expect( acre @@ -63,7 +63,7 @@ describe("Acre", () => { amountToStake, ) expect(await acre.balanceOf(tokenHolderAddress)).to.be.eq(amountToStake) - expect(await token.balanceOf(tokenHolderAddress)).to.be.eq(0) + expect(await tbtc.balanceOf(tokenHolderAddress)).to.be.eq(0) }) it("should revert if the referrer is zero valu", async () => { @@ -107,7 +107,7 @@ describe("Acre", () => { ) await expect( - token + tbtc .connect(tokenHolder) .approveAndCall(acreAddress, 1, invalidExtraData), ).to.be.revertedWith("Corrupted stake data") From 829c3ed5d76ba032699497ddf8348eb7598fe058 Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Tue, 28 Nov 2023 14:34:55 +0100 Subject: [PATCH 14/60] Rename param in `stake` function `referrer` -> `referral` --- core/contracts/Acre.sol | 8 ++++---- core/test/Acre.test.ts | 6 +++--- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/core/contracts/Acre.sol b/core/contracts/Acre.sol index 91c2fc714..a25e2252a 100644 --- a/core/contracts/Acre.sol +++ b/core/contracts/Acre.sol @@ -18,7 +18,7 @@ contract Acre is ERC4626, IReceiveApproval { /// @param token Token contract address. /// @param extraData Extra data for stake. This byte array must have the /// following values concatenated: - /// - referrer ID (32 bytes) + /// - referral (32 bytes) function receiveApproval( address from, uint256 amount, @@ -37,14 +37,14 @@ contract Acre is ERC4626, IReceiveApproval { /// @dev This function calls `deposit` function from `ERC4626` contract. /// @param assets Approved amount for the transfer and stake. /// @param receiver The address to which the shares will be minted. - /// @param referrer Data used for refferal program. + /// @param referral Data used for referral program. /// @return shares Minted shares. function stake( uint256 assets, address receiver, - bytes32 referrer + bytes32 referral ) public returns (uint256 shares) { - require(referrer != bytes32(0), "Referrer can not be empty"); + require(referral != bytes32(0), "Referral can not be empty"); return deposit(assets, receiver); } diff --git a/core/test/Acre.test.ts b/core/test/Acre.test.ts index c82ef81eb..03f40393f 100644 --- a/core/test/Acre.test.ts +++ b/core/test/Acre.test.ts @@ -30,7 +30,7 @@ describe("Acre", () => { }) describe("Staking", () => { - const referrer = ethers.encodeBytes32String("referrer") + const referral = ethers.encodeBytes32String("referral") context("when staking via Acre contract", () => { beforeEach(async () => { @@ -47,7 +47,7 @@ describe("Acre", () => { await expect( acre .connect(tokenHolder) - .stake(amountToStake, tokenHolderAddress, referrer), + .stake(amountToStake, tokenHolderAddress, referral), ) .to.emit(acre, "Deposit") .withArgs( @@ -103,7 +103,7 @@ describe("Acre", () => { const acreAddress = await acre.getAddress() const invalidExtraData = ethers.AbiCoder.defaultAbiCoder().encode( ["bytes32", "address"], - [referrer, tokenHolder.address], + [referral, tokenHolder.address], ) await expect( From ffaaba693b2a902112625230ff6373254d1e8b7b Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Tue, 28 Nov 2023 14:43:34 +0100 Subject: [PATCH 15/60] Update `Acre` unit tests We've updated the revert message in `stake` function when validating the `referral` param. Here we adjust the revert message in unit tests to the current version of contract. --- 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 03f40393f..0f0d228dc 100644 --- a/core/test/Acre.test.ts +++ b/core/test/Acre.test.ts @@ -73,7 +73,7 @@ describe("Acre", () => { acre .connect(tokenHolder) .stake(1, tokenHolder.address, emptyReferrer), - ).to.be.revertedWith("Referrer can not be empty") + ).to.be.revertedWith("Referral can not be empty") }) }) From b8a5b963602d8bb34bf9bcb3c43dc78f9fdeac2a Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Thu, 30 Nov 2023 09:10:36 +0100 Subject: [PATCH 16/60] Fix typo `valu` -> `value` --- 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 0f0d228dc..2425fc671 100644 --- a/core/test/Acre.test.ts +++ b/core/test/Acre.test.ts @@ -66,7 +66,7 @@ describe("Acre", () => { expect(await tbtc.balanceOf(tokenHolderAddress)).to.be.eq(0) }) - it("should revert if the referrer is zero valu", async () => { + it("should revert if the referrer is zero value", async () => { const emptyReferrer = ethers.encodeBytes32String("") await expect( From 6132a7bb36405402273683f2f9cb6a91ed707a43 Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Thu, 30 Nov 2023 10:34:20 +0100 Subject: [PATCH 17/60] Add more unit tests for staking flow Unit test stakig flow where there are more than one staker and the vault earns yield from strategies. --- core/test/Acre.test.ts | 189 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 187 insertions(+), 2 deletions(-) diff --git a/core/test/Acre.test.ts b/core/test/Acre.test.ts index 2425fc671..893a7b71a 100644 --- a/core/test/Acre.test.ts +++ b/core/test/Acre.test.ts @@ -1,4 +1,8 @@ -import { loadFixture } from "@nomicfoundation/hardhat-toolbox/network-helpers" +import { + SnapshotRestorer, + loadFixture, + takeSnapshot, +} from "@nomicfoundation/hardhat-toolbox/network-helpers" import { HardhatEthersSigner } from "@nomicfoundation/hardhat-ethers/signers" import { ethers } from "hardhat" import { expect } from "chai" @@ -25,21 +29,27 @@ describe("Acre", () => { let tbtc: TestToken let tokenHolder: HardhatEthersSigner - beforeEach(async () => { + before(async () => { ;({ acre, token: tbtc, tokenHolder } = await loadFixture(acreFixture)) }) describe("Staking", () => { const referral = ethers.encodeBytes32String("referral") + let snapshot: SnapshotRestorer context("when staking via Acre contract", () => { beforeEach(async () => { + snapshot = await takeSnapshot() // Infinite approval for staking contract. await tbtc .connect(tokenHolder) .approve(await acre.getAddress(), ethers.MaxUint256) }) + afterEach(async () => { + await snapshot.restore() + }) + it("should stake tokens and receive shares", async () => { const tokenHolderAddress = tokenHolder.address const amountToStake = await tbtc.balanceOf(tokenHolderAddress) @@ -114,5 +124,180 @@ describe("Acre", () => { }) }, ) + + context("when there are two stakers, A and B ", () => { + type Staker = { + signer: HardhatEthersSigner + address: string + amountToStake: bigint + } + let stakerA: Staker + let stakerB: Staker + let afterStakesSnapshot: SnapshotRestorer + let afterSimulatingYieldSnapshot: SnapshotRestorer + + before(async () => { + const [staker1, staker2] = await ethers.getSigners() + const staker1AmountToStake = WeiPerEther * 10000n + const staker2AmountToStake = WeiPerEther * 5000n + // Infinite approval for staking contract. + await tbtc + .connect(staker1) + .approve(await acre.getAddress(), ethers.MaxUint256) + await tbtc + .connect(staker2) + .approve(await acre.getAddress(), ethers.MaxUint256) + + // Mint tokens. + await tbtc.connect(staker1).mint(staker1.address, staker1AmountToStake) + await tbtc.connect(staker2).mint(staker2.address, staker2AmountToStake) + + stakerA = { + signer: staker1, + address: staker1.address, + amountToStake: staker1AmountToStake, + } + stakerB = { + signer: staker2, + address: staker2.address, + amountToStake: staker2AmountToStake, + } + }) + + context( + "when the vault is empty and has not yet earned yield from strategies", + () => { + after(async () => { + afterStakesSnapshot = await takeSnapshot() + }) + + context("when staker A stakes tokens", () => { + it("should stake tokens", async () => { + await expect( + acre + .connect(stakerA.signer) + .stake(stakerA.amountToStake, stakerA.address, referral), + ).to.be.not.reverted + }) + + it("should receive shares equal to a staked amount", async () => { + const shares = await acre.balanceOf(stakerA.address) + + expect(shares).to.eq(stakerA.amountToStake) + }) + }) + + context("when staker B stakes tokens", () => { + it("should stake tokens correctly", async () => { + await expect( + acre + .connect(stakerB.signer) + .stake(stakerB.amountToStake, stakerB.address, referral), + ).to.be.not.reverted + }) + + it("should receive shares equal to a staked amount", async () => { + const shares = await acre.balanceOf(stakerB.address) + + expect(shares).to.eq(stakerB.amountToStake) + }) + }) + }, + ) + + context("when the vault has stakes", () => { + before(async () => { + await afterStakesSnapshot.restore() + }) + + it("the total assets amount should be equal to all staked tokens", async () => { + const totalAssets = await acre.totalAssets() + + expect(totalAssets).to.eq( + stakerA.amountToStake + stakerB.amountToStake, + ) + }) + }) + + context("when vault earns yield", () => { + let stakerASharesBefore: bigint + let stakerBSharesBefore: bigint + let vaultYield: bigint + + before(async () => { + await afterStakesSnapshot.restore() + + stakerASharesBefore = await acre.balanceOf(stakerA.address) + stakerBSharesBefore = await acre.balanceOf(stakerB.address) + vaultYield = WeiPerEther * 10000n + + // Simulating yield returned from strategies. The vault now contains + // more tokens than deposited which causes the exchange rate to + // change. + await tbtc.mint(await acre.getAddress(), vaultYield) + }) + + after(async () => { + afterSimulatingYieldSnapshot = await takeSnapshot() + }) + + it("the vault should hold more asstes", async () => { + expect(await acre.totalAssets()).to.be.eq( + stakerA.amountToStake + stakerB.amountToStake + vaultYield, + ) + }) + + it("the staker's shares should be the same", async () => { + expect(await acre.balanceOf(stakerA.address)).to.be.eq( + stakerASharesBefore, + ) + expect(await acre.balanceOf(stakerB.address)).to.be.eq( + stakerBSharesBefore, + ) + }) + + it("the staker A should be able to redeem more tokens than before", async () => { + const shares = await acre.balanceOf(stakerA.address) + const availableAssetsToRedeem = await acre.previewRedeem(shares) + + expect(availableAssetsToRedeem).to.be.greaterThan( + stakerA.amountToStake, + ) + }) + + it("the staker B should be able to redeem more tokens than before", async () => { + const shares = await acre.balanceOf(stakerB.address) + const availableAssetsToRedeem = await acre.previewRedeem(shares) + + expect(availableAssetsToRedeem).to.be.greaterThan( + stakerB.amountToStake, + ) + }) + }) + + context("when staker A stakes more tokens", () => { + let sharesBefore: bigint + let availableToRedeemBefore: bigint + + before(async () => { + await afterSimulatingYieldSnapshot.restore() + + sharesBefore = await acre.balanceOf(stakerA.address) + availableToRedeemBefore = await acre.previewRedeem(sharesBefore) + + tbtc.mint(stakerA.address, stakerA.amountToStake) + + await acre.stake(stakerA.amountToStake, stakerA.address, referral) + }) + + it("should receive more shares", async () => { + const shares = await acre.balanceOf(stakerA.address) + const availableToRedeem = await acre.previewRedeem(shares) + + expect(shares).to.be.greaterThan(sharesBefore) + expect(availableToRedeem).to.be.greaterThan(availableToRedeemBefore) + }) + }) + }) }) }) From 3a3cb35ab5c7fe8ff0bedec3d16a78bdfb480b95 Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Thu, 30 Nov 2023 10:44:12 +0100 Subject: [PATCH 18/60] Add `Staked` event Instead of this require we want to emit an event that will include the referral. The off-chain accounting will need it to calculate the fee shares. To not duplicate the `Deposit` event from `ERC4626` the `Staked` event has only context for referral program. We can get other values from the `Deposit` event since it will be emitted in the same transaction. --- core/contracts/Acre.sol | 8 +++++-- core/test/Acre.test.ts | 46 ++++++++++++++++++++++++----------------- 2 files changed, 33 insertions(+), 21 deletions(-) diff --git a/core/contracts/Acre.sol b/core/contracts/Acre.sol index a25e2252a..a7eadb38f 100644 --- a/core/contracts/Acre.sol +++ b/core/contracts/Acre.sol @@ -5,6 +5,8 @@ import "@openzeppelin/contracts/token/ERC20/extensions/ERC4626.sol"; import "@thesis/solidity-contracts/contracts/token/IReceiveApproval.sol"; contract Acre is ERC4626, IReceiveApproval { + event Staked(bytes32 indexed referral, uint256 assets, uint256 shares); + constructor( IERC20 _token ) ERC4626(_token) ERC20("Acre Staked Bitcoin", "stBTC") {} @@ -44,8 +46,10 @@ contract Acre is ERC4626, IReceiveApproval { address receiver, bytes32 referral ) public returns (uint256 shares) { - require(referral != bytes32(0), "Referral can not be empty"); + shares = deposit(assets, receiver); + + emit Staked(referral, assets, shares); - return deposit(assets, receiver); + return shares; } } diff --git a/core/test/Acre.test.ts b/core/test/Acre.test.ts index 893a7b71a..c85cb713e 100644 --- a/core/test/Acre.test.ts +++ b/core/test/Acre.test.ts @@ -54,36 +54,44 @@ describe("Acre", () => { const tokenHolderAddress = tokenHolder.address const amountToStake = await tbtc.balanceOf(tokenHolderAddress) - await expect( - acre - .connect(tokenHolder) - .stake(amountToStake, tokenHolderAddress, referral), + const tx = await acre + .connect(tokenHolder) + .stake(amountToStake, tokenHolderAddress, referral) + + const stakedTokens = amountToStake + + // 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. + const receivedShares = amountToStake + + expect(tx).to.emit(acre, "Deposit").withArgs( + // Caller. + tokenHolderAddress, + // Receiver. + tokenHolderAddress, + // Staked tokens. + stakedTokens, + // Received shares. + receivedShares, ) - .to.emit(acre, "Deposit") - .withArgs( - // Caller. - tokenHolderAddress, - // Receiver. - tokenHolderAddress, - // Staked tokens. - amountToStake, - // Received shares. 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. - amountToStake, - ) + + expect(tx) + .to.emit(acre, "Staked") + .withArgs(referral, stakedTokens, receivedShares) + expect(await acre.balanceOf(tokenHolderAddress)).to.be.eq(amountToStake) expect(await tbtc.balanceOf(tokenHolderAddress)).to.be.eq(0) }) - it("should revert if the referrer is zero value", async () => { + it("should not revert if the referral is zero value", async () => { const emptyReferrer = ethers.encodeBytes32String("") await expect( acre .connect(tokenHolder) .stake(1, tokenHolder.address, emptyReferrer), - ).to.be.revertedWith("Referral can not be empty") + ).to.be.not.reverted }) }) From 6837e74915b3647bbe37758a1159fcd4b1a5e07e Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Thu, 30 Nov 2023 12:23:17 +0100 Subject: [PATCH 19/60] Revert "Add support for receive approval pattern to Acre" This reverts commit f36c65cd38ff7ba6a2e04944671f4d4523c294aa. To support receive approval pattern we need to override the `ERC4626.deposit` function to allow pass an `owner`. The OZ implementation sets the owner as `msg.sender` by default under the hood see https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/token/ERC20/extensions/ERC4626.sol#L178 and we can't change the owner. W/o this update using approve and call pattern to stake tokens, the `msg.sender` is tBTC token address, so we are actually trying to send tokens from the token contract to the receiver, which is impossible. We are going to address it in a separate PR. --- core/contracts/Acre.sol | 26 +------------------------- 1 file changed, 1 insertion(+), 25 deletions(-) diff --git a/core/contracts/Acre.sol b/core/contracts/Acre.sol index a7eadb38f..fda7ab237 100644 --- a/core/contracts/Acre.sol +++ b/core/contracts/Acre.sol @@ -2,38 +2,14 @@ pragma solidity ^0.8.21; import "@openzeppelin/contracts/token/ERC20/extensions/ERC4626.sol"; -import "@thesis/solidity-contracts/contracts/token/IReceiveApproval.sol"; -contract Acre is ERC4626, IReceiveApproval { +contract Acre is ERC4626 { event Staked(bytes32 indexed referral, uint256 assets, uint256 shares); constructor( IERC20 _token ) ERC4626(_token) ERC20("Acre Staked Bitcoin", "stBTC") {} - /// @notice Receives approval of token transfer and stakes the approved - /// amount. - /// @dev Requires that the provided token contract be the same one linked to - /// this contract. - /// @param from The owner of the tokens who approved them to transfer. - /// @param amount Approved amount for the transfer and stake. - /// @param token Token contract address. - /// @param extraData Extra data for stake. This byte array must have the - /// following values concatenated: - /// - referral (32 bytes) - function receiveApproval( - address from, - uint256 amount, - address token, - bytes calldata extraData - ) external override { - require(token == asset(), "Unrecognized token"); - // TODO: Decide on the format of the `extradata` variable. - require(extraData.length == 32, "Corrupted stake data"); - - stake(amount, from, bytes32(extraData)); - } - /// @notice Stakes a given amount of underlying token and mints shares to a /// receiver. /// @dev This function calls `deposit` function from `ERC4626` contract. From c960102f5650f9d3165297ac8c18814d7f1bc759 Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Thu, 30 Nov 2023 12:29:15 +0100 Subject: [PATCH 20/60] Remove unit tests for receive approval pattern --- core/test/Acre.test.ts | 38 -------------------------------------- 1 file changed, 38 deletions(-) diff --git a/core/test/Acre.test.ts b/core/test/Acre.test.ts index c85cb713e..3ba383b1a 100644 --- a/core/test/Acre.test.ts +++ b/core/test/Acre.test.ts @@ -95,44 +95,6 @@ describe("Acre", () => { }) }) - context( - "when staking via staking token using approve and call pattern", - () => { - it("should stake tokens", () => { - // TODO: The `ERC4626.deposit` sets the owner as `msg.sender` under - // the hood. Using the approve and call pattern the `msg.sender` is - // token address, so we are actually trying to send tokens from the - // token contract to the receiver, which is impossible. We need to - // decide if we want to override the `deposit` function to allow - // staking via approve and call pattern. - }) - - it("should revert when called not for linked token", async () => { - const FakeToken = await ethers.getContractFactory("TestToken") - const fakeToken = await FakeToken.deploy() - const acreAddress = await acre.getAddress() - - await expect( - fakeToken.connect(tokenHolder).approveAndCall(acreAddress, 1, "0x"), - ).to.be.revertedWith("Unrecognized token") - }) - - it("should revert when extra data is invalid", async () => { - const acreAddress = await acre.getAddress() - const invalidExtraData = ethers.AbiCoder.defaultAbiCoder().encode( - ["bytes32", "address"], - [referral, tokenHolder.address], - ) - - await expect( - tbtc - .connect(tokenHolder) - .approveAndCall(acreAddress, 1, invalidExtraData), - ).to.be.revertedWith("Corrupted stake data") - }) - }, - ) - context("when there are two stakers, A and B ", () => { type Staker = { signer: HardhatEthersSigner From 58d2ddd55c7098626e7f9e8205a392f4394a37a5 Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Thu, 30 Nov 2023 14:41:44 +0100 Subject: [PATCH 21/60] Rename variable in fixture Rename `token` -> `tbtc`. Acre works only with tBTC token - to simplify let's use `tbtc`. --- core/test/Acre.test.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/core/test/Acre.test.ts b/core/test/Acre.test.ts index 3ba383b1a..c7fdf4a02 100644 --- a/core/test/Acre.test.ts +++ b/core/test/Acre.test.ts @@ -12,16 +12,16 @@ import type { TestToken, Acre } from "../typechain" async function acreFixture() { const [_, tokenHolder] = await ethers.getSigners() const Token = await ethers.getContractFactory("TestToken") - const token = await Token.deploy() + const tbtc = await Token.deploy() const amountToMint = WeiPerEther * 10000n - token.mint(tokenHolder, amountToMint) + tbtc.mint(tokenHolder, amountToMint) const Acre = await ethers.getContractFactory("Acre") - const acre = await Acre.deploy(await token.getAddress()) + const acre = await Acre.deploy(await tbtc.getAddress()) - return { acre, token, tokenHolder } + return { acre, tbtc, tokenHolder } } describe("Acre", () => { @@ -30,7 +30,7 @@ describe("Acre", () => { let tokenHolder: HardhatEthersSigner before(async () => { - ;({ acre, token: tbtc, tokenHolder } = await loadFixture(acreFixture)) + ;({ acre, tbtc, tokenHolder } = await loadFixture(acreFixture)) }) describe("Staking", () => { From 05b20912ef0e9a2cb69e48395c5833aacf61db8b Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Thu, 30 Nov 2023 15:05:41 +0100 Subject: [PATCH 22/60] Update unit test for staking To be closer to a real world scenarios we get rid of infinite approval dor staking contract and approve an amount that will be staked - just to make sure Acre can't spend more than it was approved for. Here we also add additional test case where staker tries to stake more than approved. --- core/test/Acre.test.ts | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/core/test/Acre.test.ts b/core/test/Acre.test.ts index c7fdf4a02..79f4b5ede 100644 --- a/core/test/Acre.test.ts +++ b/core/test/Acre.test.ts @@ -14,7 +14,7 @@ async function acreFixture() { const Token = await ethers.getContractFactory("TestToken") const tbtc = await Token.deploy() - const amountToMint = WeiPerEther * 10000n + const amountToMint = WeiPerEther * 100000n tbtc.mint(tokenHolder, amountToMint) @@ -38,12 +38,14 @@ describe("Acre", () => { let snapshot: SnapshotRestorer context("when staking via Acre contract", () => { + const amountToStake = 1000n * WeiPerEther + beforeEach(async () => { snapshot = await takeSnapshot() - // Infinite approval for staking contract. + await tbtc .connect(tokenHolder) - .approve(await acre.getAddress(), ethers.MaxUint256) + .approve(await acre.getAddress(), amountToStake) }) afterEach(async () => { @@ -52,7 +54,7 @@ describe("Acre", () => { it("should stake tokens and receive shares", async () => { const tokenHolderAddress = tokenHolder.address - const amountToStake = await tbtc.balanceOf(tokenHolderAddress) + const balanceOfBeforeStake = await tbtc.balanceOf(tokenHolderAddress) const tx = await acre .connect(tokenHolder) @@ -81,7 +83,9 @@ describe("Acre", () => { .withArgs(referral, stakedTokens, receivedShares) expect(await acre.balanceOf(tokenHolderAddress)).to.be.eq(amountToStake) - expect(await tbtc.balanceOf(tokenHolderAddress)).to.be.eq(0) + expect(await tbtc.balanceOf(tokenHolderAddress)).to.be.eq( + balanceOfBeforeStake - amountToStake, + ) }) it("should not revert if the referral is zero value", async () => { @@ -90,9 +94,17 @@ describe("Acre", () => { await expect( acre .connect(tokenHolder) - .stake(1, tokenHolder.address, emptyReferrer), + .stake(amountToStake, tokenHolder.address, emptyReferrer), ).to.be.not.reverted }) + + it("should revert if a staker wants to stake more tokens than approved", async () => { + await expect( + acre + .connect(tokenHolder) + .stake(amountToStake + 1n, tokenHolder.address, referral), + ).to.be.reverted + }) }) context("when there are two stakers, A and B ", () => { From a77b1a460be3e1a8e319282ac34306dc0a15ee6a Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Thu, 30 Nov 2023 15:15:07 +0100 Subject: [PATCH 23/60] Rename variable `emptyReferrer` -> `emptyReferral` --- 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 79f4b5ede..6648c81bf 100644 --- a/core/test/Acre.test.ts +++ b/core/test/Acre.test.ts @@ -89,12 +89,12 @@ describe("Acre", () => { }) it("should not revert if the referral is zero value", async () => { - const emptyReferrer = ethers.encodeBytes32String("") + const emptyReferral = ethers.encodeBytes32String("") await expect( acre .connect(tokenHolder) - .stake(amountToStake, tokenHolder.address, emptyReferrer), + .stake(amountToStake, tokenHolder.address, emptyReferral), ).to.be.not.reverted }) From 4c9eeecd03a46cfef34ea4b0ef7ece6d87dc6222 Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Thu, 30 Nov 2023 15:18:37 +0100 Subject: [PATCH 24/60] Fix typo --- 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 6648c81bf..247942fa1 100644 --- a/core/test/Acre.test.ts +++ b/core/test/Acre.test.ts @@ -223,7 +223,7 @@ describe("Acre", () => { afterSimulatingYieldSnapshot = await takeSnapshot() }) - it("the vault should hold more asstes", async () => { + it("the vault should hold more assets", async () => { expect(await acre.totalAssets()).to.be.eq( stakerA.amountToStake + stakerB.amountToStake + vaultYield, ) From 249fb11d40359f07cc7514acee872f5c603a00dd Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Thu, 30 Nov 2023 15:27:27 +0100 Subject: [PATCH 25/60] Rename variable in tests `tokenHolder` -> `staker` --- core/test/Acre.test.ts | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/core/test/Acre.test.ts b/core/test/Acre.test.ts index 247942fa1..1a74af049 100644 --- a/core/test/Acre.test.ts +++ b/core/test/Acre.test.ts @@ -10,27 +10,27 @@ import { WeiPerEther } from "ethers" import type { TestToken, Acre } from "../typechain" async function acreFixture() { - const [_, tokenHolder] = await ethers.getSigners() + const [_, staker] = await ethers.getSigners() const Token = await ethers.getContractFactory("TestToken") const tbtc = await Token.deploy() const amountToMint = WeiPerEther * 100000n - tbtc.mint(tokenHolder, amountToMint) + tbtc.mint(staker, amountToMint) const Acre = await ethers.getContractFactory("Acre") const acre = await Acre.deploy(await tbtc.getAddress()) - return { acre, tbtc, tokenHolder } + return { acre, tbtc, staker } } describe("Acre", () => { let acre: Acre let tbtc: TestToken - let tokenHolder: HardhatEthersSigner + let staker: HardhatEthersSigner before(async () => { - ;({ acre, tbtc, tokenHolder } = await loadFixture(acreFixture)) + ;({ acre, tbtc, staker } = await loadFixture(acreFixture)) }) describe("Staking", () => { @@ -44,7 +44,7 @@ describe("Acre", () => { snapshot = await takeSnapshot() await tbtc - .connect(tokenHolder) + .connect(staker) .approve(await acre.getAddress(), amountToStake) }) @@ -53,12 +53,12 @@ describe("Acre", () => { }) it("should stake tokens and receive shares", async () => { - const tokenHolderAddress = tokenHolder.address - const balanceOfBeforeStake = await tbtc.balanceOf(tokenHolderAddress) + const stakerAddress = staker.address + const balanceOfBeforeStake = await tbtc.balanceOf(stakerAddress) const tx = await acre - .connect(tokenHolder) - .stake(amountToStake, tokenHolderAddress, referral) + .connect(staker) + .stake(amountToStake, stakerAddress, referral) const stakedTokens = amountToStake @@ -69,9 +69,9 @@ describe("Acre", () => { expect(tx).to.emit(acre, "Deposit").withArgs( // Caller. - tokenHolderAddress, + stakerAddress, // Receiver. - tokenHolderAddress, + stakerAddress, // Staked tokens. stakedTokens, // Received shares. @@ -82,8 +82,8 @@ describe("Acre", () => { .to.emit(acre, "Staked") .withArgs(referral, stakedTokens, receivedShares) - expect(await acre.balanceOf(tokenHolderAddress)).to.be.eq(amountToStake) - expect(await tbtc.balanceOf(tokenHolderAddress)).to.be.eq( + expect(await acre.balanceOf(stakerAddress)).to.be.eq(amountToStake) + expect(await tbtc.balanceOf(stakerAddress)).to.be.eq( balanceOfBeforeStake - amountToStake, ) }) @@ -93,16 +93,16 @@ describe("Acre", () => { await expect( acre - .connect(tokenHolder) - .stake(amountToStake, tokenHolder.address, emptyReferral), + .connect(staker) + .stake(amountToStake, staker.address, emptyReferral), ).to.be.not.reverted }) it("should revert if a staker wants to stake more tokens than approved", async () => { await expect( acre - .connect(tokenHolder) - .stake(amountToStake + 1n, tokenHolder.address, referral), + .connect(staker) + .stake(amountToStake + 1n, staker.address, referral), ).to.be.reverted }) }) From 20086c68caf70b0f3ff161392476d69c0daf4819 Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Thu, 30 Nov 2023 17:07:13 +0100 Subject: [PATCH 26/60] Update staking flow unit tests Be more precise when comparing available amount of assets to redeem. The OZ implementation supports rounding so the available amount to redeem is slightly lower than expected see: 1. Staker A stakes 75 tBTC - receives 75 shares. 2. Staker B stakes 25 tBTC - receives 25 shares. 3. Vault earns 100 tBTC - total assets: 75 + 25 + 100 = 200 tBTC. 4. Available amount to redeem for staker is: - A - 75 * 200 / 100 = 150 -> 149 w/ rounding, - B - 25 * 200 / 100 = 50 -> 49 w/ rounding. --- core/test/Acre.test.ts | 31 +++++++++++++++++++++++++++---- 1 file changed, 27 insertions(+), 4 deletions(-) diff --git a/core/test/Acre.test.ts b/core/test/Acre.test.ts index 1a74af049..a3b714169 100644 --- a/core/test/Acre.test.ts +++ b/core/test/Acre.test.ts @@ -120,8 +120,8 @@ describe("Acre", () => { before(async () => { const [staker1, staker2] = await ethers.getSigners() - const staker1AmountToStake = WeiPerEther * 10000n - const staker2AmountToStake = WeiPerEther * 5000n + const staker1AmountToStake = WeiPerEther * 75n + const staker2AmountToStake = WeiPerEther * 25n // Infinite approval for staking contract. await tbtc .connect(staker1) @@ -154,7 +154,7 @@ describe("Acre", () => { }) context("when staker A stakes tokens", () => { - it("should stake tokens", async () => { + it("should stake tokens correctly", async () => { await expect( acre .connect(stakerA.signer) @@ -205,13 +205,22 @@ describe("Acre", () => { let stakerASharesBefore: bigint let stakerBSharesBefore: bigint let vaultYield: bigint + let expectedTotalAssets: bigint + let expectedTotalSupply: bigint before(async () => { await afterStakesSnapshot.restore() stakerASharesBefore = await acre.balanceOf(stakerA.address) stakerBSharesBefore = await acre.balanceOf(stakerB.address) - vaultYield = WeiPerEther * 10000n + vaultYield = WeiPerEther * 100n + // Staker A shares = 75 + // Staker B shares = 25 + // Total assets = 75(staker A) + 25(staker) + 100(yield) + expectedTotalAssets = + stakerA.amountToStake + stakerB.amountToStake + vaultYield + // Total shares = 75 + 25 = 100 + expectedTotalSupply = stakerA.amountToStake + stakerB.amountToStake // Simulating yield returned from strategies. The vault now contains // more tokens than deposited which causes the exchange rate to @@ -242,18 +251,32 @@ describe("Acre", () => { const shares = await acre.balanceOf(stakerA.address) const availableAssetsToRedeem = await acre.previewRedeem(shares) + // Expected amount w/o rounding: 75 * 200 / 100 = 150 + // Expected amount w/ support for rounding: 149999999999999999999 in + // tBTC token precision. + const expectedAssetsToRedeem = + (shares * (expectedTotalAssets + 1n)) / (expectedTotalSupply + 1n) + expect(availableAssetsToRedeem).to.be.greaterThan( stakerA.amountToStake, ) + expect(availableAssetsToRedeem).to.be.eq(expectedAssetsToRedeem) }) it("the staker B should be able to redeem more tokens than before", async () => { const shares = await acre.balanceOf(stakerB.address) const availableAssetsToRedeem = await acre.previewRedeem(shares) + // Expected amount w/o rounding: 25 * 200 / 100 = 50 + // Expected amount w/ support for rounding: 49999999999999999999 in + // tBTC token precision. + const expectedAssetsToRedeem = + (shares * (expectedTotalAssets + 1n)) / (expectedTotalSupply + 1n) + expect(availableAssetsToRedeem).to.be.greaterThan( stakerB.amountToStake, ) + expect(availableAssetsToRedeem).to.be.eq(expectedAssetsToRedeem) }) }) From 63aac1b03c50b43ea5bd2362345b6bcd4b3ccfe7 Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Mon, 4 Dec 2023 06:18:08 +0100 Subject: [PATCH 27/60] Use unnamed return in the `stake` fn We preffer unnamed return here. --- core/contracts/Acre.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/contracts/Acre.sol b/core/contracts/Acre.sol index fda7ab237..2d67a3151 100644 --- a/core/contracts/Acre.sol +++ b/core/contracts/Acre.sol @@ -21,8 +21,8 @@ contract Acre is ERC4626 { uint256 assets, address receiver, bytes32 referral - ) public returns (uint256 shares) { - shares = deposit(assets, receiver); + ) public returns (uint256) { + uint256 shares = deposit(assets, receiver); emit Staked(referral, assets, shares); From e923f5ee03640f9596374c0a76cc777549f2ab48 Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Mon, 4 Dec 2023 06:21:31 +0100 Subject: [PATCH 28/60] Leave TODO in the `stake` fn We should probably revisit the type of `referral` in the future. --- core/contracts/Acre.sol | 1 + 1 file changed, 1 insertion(+) diff --git a/core/contracts/Acre.sol b/core/contracts/Acre.sol index 2d67a3151..c2fc1631b 100644 --- a/core/contracts/Acre.sol +++ b/core/contracts/Acre.sol @@ -22,6 +22,7 @@ contract Acre is ERC4626 { address receiver, bytes32 referral ) public returns (uint256) { + // TODO: revisit the type of referral. uint256 shares = deposit(assets, receiver); emit Staked(referral, assets, shares); From 5403ac3ccea60e8014109af9cab3d703b1f53434 Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Mon, 4 Dec 2023 06:41:32 +0100 Subject: [PATCH 29/60] Rename constructor parameter in Acre `_token` -> `tbtc`. We know that this will only be the tBTC token, so we want to have a clear parameter name. --- core/contracts/Acre.sol | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/core/contracts/Acre.sol b/core/contracts/Acre.sol index c2fc1631b..2cac7beed 100644 --- a/core/contracts/Acre.sol +++ b/core/contracts/Acre.sol @@ -7,10 +7,10 @@ contract Acre is ERC4626 { event Staked(bytes32 indexed referral, uint256 assets, uint256 shares); constructor( - IERC20 _token - ) ERC4626(_token) ERC20("Acre Staked Bitcoin", "stBTC") {} + IERC20 tbtc + ) ERC4626(tbtc) ERC20("Acre Staked Bitcoin", "stBTC") {} - /// @notice Stakes a given amount of underlying token and mints shares to a + /// @notice Stakes a given amount of tBTC token and mints shares to a /// receiver. /// @dev This function calls `deposit` function from `ERC4626` contract. /// @param assets Approved amount for the transfer and stake. From 6511fefeb016da4a3f75a0b3ee21a1c2b23f0ffc Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Mon, 4 Dec 2023 07:33:18 +0100 Subject: [PATCH 30/60] Remove `@thesis/solidity-contracts` dependency Since we decided to not deal with receive approval pattern during the staking work this dependency is no longer needed at this stage. --- core/contracts/test/TestToken.sol | 21 +-------------------- core/package.json | 3 +-- core/test/Acre.test.ts | 6 +++--- pnpm-lock.yaml | 15 --------------- 4 files changed, 5 insertions(+), 40 deletions(-) diff --git a/core/contracts/test/TestToken.sol b/core/contracts/test/TestToken.sol index 5a83eabd8..44e5e14dc 100644 --- a/core/contracts/test/TestToken.sol +++ b/core/contracts/test/TestToken.sol @@ -2,30 +2,11 @@ pragma solidity ^0.8.21; import "@openzeppelin/contracts/token/ERC20/ERC20.sol"; -import "@thesis/solidity-contracts/contracts/token/IApproveAndCall.sol"; -import "@thesis/solidity-contracts/contracts/token/IReceiveApproval.sol"; -contract TestToken is ERC20, IApproveAndCall { +contract TestERC20 is ERC20 { constructor() ERC20("Test Token", "TEST") {} function mint(address account, uint256 value) external { _mint(account, value); } - - function approveAndCall( - address spender, - uint256 amount, - bytes memory extraData - ) external returns (bool) { - if (approve(spender, amount)) { - IReceiveApproval(spender).receiveApproval( - msg.sender, - amount, - address(this), - extraData - ); - return true; - } - return false; - } } diff --git a/core/package.json b/core/package.json index 783907808..603cb8643 100644 --- a/core/package.json +++ b/core/package.json @@ -57,7 +57,6 @@ "typescript": "^5.3.2" }, "dependencies": { - "@openzeppelin/contracts": "^5.0.0", - "@thesis/solidity-contracts": "github:thesis/solidity-contracts#c315b9d" + "@openzeppelin/contracts": "^5.0.0" } } diff --git a/core/test/Acre.test.ts b/core/test/Acre.test.ts index a3b714169..d8a703fe5 100644 --- a/core/test/Acre.test.ts +++ b/core/test/Acre.test.ts @@ -7,11 +7,11 @@ import { HardhatEthersSigner } from "@nomicfoundation/hardhat-ethers/signers" import { ethers } from "hardhat" import { expect } from "chai" import { WeiPerEther } from "ethers" -import type { TestToken, Acre } from "../typechain" +import type { TestERC20, Acre } from "../typechain" async function acreFixture() { const [_, staker] = await ethers.getSigners() - const Token = await ethers.getContractFactory("TestToken") + const Token = await ethers.getContractFactory("TestERC20") const tbtc = await Token.deploy() const amountToMint = WeiPerEther * 100000n @@ -26,7 +26,7 @@ async function acreFixture() { describe("Acre", () => { let acre: Acre - let tbtc: TestToken + let tbtc: TestERC20 let staker: HardhatEthersSigner before(async () => { diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 2d7a91c33..3e6723390 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -23,9 +23,6 @@ importers: '@openzeppelin/contracts': specifier: ^5.0.0 version: 5.0.0 - '@thesis/solidity-contracts': - specifier: github:thesis/solidity-contracts#c315b9d - version: github.com/thesis/solidity-contracts/c315b9d devDependencies: '@nomicfoundation/hardhat-chai-matchers': specifier: ^2.0.2 @@ -4416,10 +4413,6 @@ packages: - supports-color dev: true - /@openzeppelin/contracts@4.9.3: - resolution: {integrity: sha512-He3LieZ1pP2TNt5JbkPA4PNT9WC3gOTOlDcFGJW4Le4QKqwmiNJCRt44APfxMxvq7OugU/cqYuPcSBzOw38DAg==} - dev: false - /@openzeppelin/contracts@5.0.0: resolution: {integrity: sha512-bv2sdS6LKqVVMLI5+zqnNrNU/CA+6z6CmwFXm/MzmOPBRSO5reEJN7z0Gbzvs0/bv/MZZXNklubpwy3v2+azsw==} dev: false @@ -15362,11 +15355,3 @@ packages: dependencies: solhint: 4.0.0 dev: true - - github.com/thesis/solidity-contracts/c315b9d: - resolution: {tarball: https://codeload.github.com/thesis/solidity-contracts/tar.gz/c315b9d} - name: '@thesis-co/solidity-contracts' - version: 0.0.1-pre - dependencies: - '@openzeppelin/contracts': 4.9.3 - dev: false From 84bdf8e9376aeeb1ffa159f04faa280aae10a761 Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Mon, 4 Dec 2023 07:59:53 +0100 Subject: [PATCH 31/60] Add unit test heleprs Add `to1e18` function to deal with the token amount conversion. --- core/test/Acre.test.ts | 12 ++++++------ core/test/utils/index.ts | 1 + core/test/utils/number.ts | 10 ++++++++++ 3 files changed, 17 insertions(+), 6 deletions(-) create mode 100644 core/test/utils/index.ts create mode 100644 core/test/utils/number.ts diff --git a/core/test/Acre.test.ts b/core/test/Acre.test.ts index d8a703fe5..eee9aff4d 100644 --- a/core/test/Acre.test.ts +++ b/core/test/Acre.test.ts @@ -6,15 +6,15 @@ import { import { HardhatEthersSigner } from "@nomicfoundation/hardhat-ethers/signers" import { ethers } from "hardhat" import { expect } from "chai" -import { WeiPerEther } from "ethers" import type { TestERC20, Acre } from "../typechain" +import { to1e18 } from "./utils" async function acreFixture() { const [_, staker] = await ethers.getSigners() const Token = await ethers.getContractFactory("TestERC20") const tbtc = await Token.deploy() - const amountToMint = WeiPerEther * 100000n + const amountToMint = to1e18(100000) tbtc.mint(staker, amountToMint) @@ -38,7 +38,7 @@ describe("Acre", () => { let snapshot: SnapshotRestorer context("when staking via Acre contract", () => { - const amountToStake = 1000n * WeiPerEther + const amountToStake = to1e18(1000) beforeEach(async () => { snapshot = await takeSnapshot() @@ -120,8 +120,8 @@ describe("Acre", () => { before(async () => { const [staker1, staker2] = await ethers.getSigners() - const staker1AmountToStake = WeiPerEther * 75n - const staker2AmountToStake = WeiPerEther * 25n + const staker1AmountToStake = to1e18(75) + const staker2AmountToStake = to1e18(25) // Infinite approval for staking contract. await tbtc .connect(staker1) @@ -213,7 +213,7 @@ describe("Acre", () => { stakerASharesBefore = await acre.balanceOf(stakerA.address) stakerBSharesBefore = await acre.balanceOf(stakerB.address) - vaultYield = WeiPerEther * 100n + vaultYield = to1e18(100) // Staker A shares = 75 // Staker B shares = 25 // Total assets = 75(staker A) + 25(staker) + 100(yield) diff --git a/core/test/utils/index.ts b/core/test/utils/index.ts new file mode 100644 index 000000000..6654cca0e --- /dev/null +++ b/core/test/utils/index.ts @@ -0,0 +1 @@ +export * from "./number" diff --git a/core/test/utils/number.ts b/core/test/utils/number.ts new file mode 100644 index 000000000..9784b499c --- /dev/null +++ b/core/test/utils/number.ts @@ -0,0 +1,10 @@ +export function to1ePrecision( + n: string | number | bigint, + precision: number, +): bigint { + return BigInt(n) * 10n ** BigInt(precision) +} + +export function to1e18(n: string | number | bigint): bigint { + return to1ePrecision(n, 18) +} From ce16fbade77fd216d5b0e1a90fc632f07d4e909b Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Mon, 4 Dec 2023 09:13:49 +0100 Subject: [PATCH 32/60] Add more unit tests for staking To cover more cases. --- core/test/Acre.test.ts | 185 ++++++++++++++++++++++++++++++----------- 1 file changed, 138 insertions(+), 47 deletions(-) diff --git a/core/test/Acre.test.ts b/core/test/Acre.test.ts index eee9aff4d..9660ced74 100644 --- a/core/test/Acre.test.ts +++ b/core/test/Acre.test.ts @@ -6,6 +6,7 @@ import { import { HardhatEthersSigner } from "@nomicfoundation/hardhat-ethers/signers" import { ethers } from "hardhat" import { expect } from "chai" +import { ContractTransactionResponse, ZeroAddress } from "ethers" import type { TestERC20, Acre } from "../typechain" import { to1e18 } from "./utils" @@ -37,74 +38,164 @@ describe("Acre", () => { const referral = ethers.encodeBytes32String("referral") let snapshot: SnapshotRestorer - context("when staking via Acre contract", () => { - const amountToStake = to1e18(1000) - + context("when staking", () => { beforeEach(async () => { snapshot = await takeSnapshot() - - await tbtc - .connect(staker) - .approve(await acre.getAddress(), amountToStake) }) afterEach(async () => { await snapshot.restore() }) - it("should stake tokens and receive shares", async () => { - const stakerAddress = staker.address - const balanceOfBeforeStake = await tbtc.balanceOf(stakerAddress) - - const tx = await acre - .connect(staker) - .stake(amountToStake, stakerAddress, referral) - - const stakedTokens = amountToStake + context("with a referral", () => { + const amountToStake = to1e18(1000) // 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. - const receivedShares = amountToStake - - expect(tx).to.emit(acre, "Deposit").withArgs( - // Caller. - stakerAddress, - // Receiver. - stakerAddress, - // Staked tokens. - stakedTokens, - // Received shares. - receivedShares, - ) - - expect(tx) - .to.emit(acre, "Staked") - .withArgs(referral, stakedTokens, receivedShares) - - expect(await acre.balanceOf(stakerAddress)).to.be.eq(amountToStake) - expect(await tbtc.balanceOf(stakerAddress)).to.be.eq( - balanceOfBeforeStake - amountToStake, - ) + const expectedReceivedShares = amountToStake + + let tx: ContractTransactionResponse + + beforeEach(async () => { + await tbtc + .connect(staker) + .approve(await acre.getAddress(), amountToStake) + + tx = await acre + .connect(staker) + .stake(amountToStake, staker.address, referral) + }) + + it("should emit Deposit event", () => { + expect(tx).to.emit(acre, "Deposit").withArgs( + // Caller. + staker.address, + // Receiver. + staker.address, + // Staked tokens. + amountToStake, + // Received shares. + expectedReceivedShares, + ) + }) + + it("should emit Staked event", () => { + expect(tx) + .to.emit(acre, "Staked") + .withArgs(referral, amountToStake, expectedReceivedShares) + }) + + it("should mint stBTC tokens", async () => { + await expect(tx).to.changeTokenBalances( + acre, + [staker.address], + [amountToStake], + ) + }) + + it("should transfer tBTC tokens", async () => { + await expect(tx).to.changeTokenBalances( + tbtc, + [staker.address, acre], + [-amountToStake, amountToStake], + ) + }) }) - it("should not revert if the referral is zero value", async () => { + context("without referral", () => { const emptyReferral = ethers.encodeBytes32String("") + let tx: ContractTransactionResponse + + beforeEach(async () => { + await tbtc.connect(staker).approve(await acre.getAddress(), 1) - await expect( - acre + tx = await acre .connect(staker) - .stake(amountToStake, staker.address, emptyReferral), - ).to.be.not.reverted + .stake(1, staker.address, emptyReferral) + }) + + it("should not revert", async () => { + await expect(tx).to.be.not.reverted + }) }) - it("should revert if a staker wants to stake more tokens than approved", async () => { - await expect( - acre + context( + "when amount to stake is greater than the approved amount", + () => { + const amountToStake = to1e18(10) + beforeEach(async () => { + await tbtc + .connect(staker) + .approve(await acre.getAddress(), amountToStake) + }) + + it("should revert", async () => { + await expect( + acre + .connect(staker) + .stake(amountToStake + 1n, staker.address, referral), + ).to.be.reverted + }) + }, + ) + + context("when amount to stake is 1", () => { + const amountToStake = 1 + + beforeEach(async () => { + await tbtc .connect(staker) - .stake(amountToStake + 1n, staker.address, referral), - ).to.be.reverted + .approve(await acre.getAddress(), amountToStake) + }) + + it("should not revert", async () => { + await expect( + acre.connect(staker).stake(amountToStake, staker.address, referral), + ).to.not.be.reverted + }) }) + + context("when the receiver is zero address", () => { + const amountToStake = to1e18(10) + + beforeEach(async () => { + await tbtc + .connect(staker) + .approve(await acre.getAddress(), amountToStake) + }) + + it("should revert", async () => { + await expect( + acre.connect(staker).stake(amountToStake, ZeroAddress, referral), + ).to.be.revertedWithCustomError(acre, "ERC20InvalidReceiver") + }) + }) + + context( + "when a staker approved and staked tokens and wants to stake more but w/o another apporval", + () => { + const amountToStake = to1e18(10) + + beforeEach(async () => { + await tbtc + .connect(staker) + .approve(await acre.getAddress(), amountToStake) + + await acre + .connect(staker) + .stake(amountToStake, staker.address, referral) + }) + + it("should revert", async () => { + await expect( + acre + .connect(staker) + .stake(amountToStake, staker.address, referral), + ).to.be.revertedWithCustomError(acre, "ERC20InsufficientAllowance") + }) + }, + ) }) context("when there are two stakers, A and B ", () => { From c3a2bd89d37d4d3ee9b73fbe3beb3d7a59af248b Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Mon, 4 Dec 2023 09:38:52 +0100 Subject: [PATCH 33/60] Update solhint config We fixed this rule in the `thesis/solhint-config` repo see https://github.com/thesis/solhint-config/pull/3 so we can remove it from this config. --- core/.solhint.json | 5 +---- pnpm-lock.yaml | 8 ++++---- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/core/.solhint.json b/core/.solhint.json index b53dba35b..f54af0b9a 100644 --- a/core/.solhint.json +++ b/core/.solhint.json @@ -1,7 +1,4 @@ { "extends": "thesis", - "plugins": [], - "rules": { - "func-visibility": ["off", { "ignoreConstructors": true }] - } + "plugins": [] } diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 3e6723390..9a82d03ba 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -92,7 +92,7 @@ importers: version: 4.0.0 solhint-config-thesis: specifier: github:thesis/solhint-config - version: github.com/thesis/solhint-config/159587cd6103f21aaa7fbadc17a06717a51b5b42(solhint@4.0.0) + version: github.com/thesis/solhint-config/266de12d96d58f01171e20858b855ec80520de8d(solhint@4.0.0) solidity-coverage: specifier: ^0.8.5 version: 0.8.5(hardhat@2.19.1) @@ -15344,9 +15344,9 @@ packages: prettier: 3.1.0 dev: true - github.com/thesis/solhint-config/159587cd6103f21aaa7fbadc17a06717a51b5b42(solhint@4.0.0): - resolution: {tarball: https://codeload.github.com/thesis/solhint-config/tar.gz/159587cd6103f21aaa7fbadc17a06717a51b5b42} - id: github.com/thesis/solhint-config/159587cd6103f21aaa7fbadc17a06717a51b5b42 + github.com/thesis/solhint-config/266de12d96d58f01171e20858b855ec80520de8d(solhint@4.0.0): + resolution: {tarball: https://codeload.github.com/thesis/solhint-config/tar.gz/266de12d96d58f01171e20858b855ec80520de8d} + id: github.com/thesis/solhint-config/266de12d96d58f01171e20858b855ec80520de8d name: solhint-config-thesis version: 0.1.0 engines: {node: '>=0.10.0'} From b20259eb8cacb331eaecb4117284ec70d03898dc Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Mon, 4 Dec 2023 10:19:17 +0100 Subject: [PATCH 34/60] Add more stakers to the Acre test fixture --- core/test/Acre.test.ts | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/core/test/Acre.test.ts b/core/test/Acre.test.ts index 9660ced74..4a31307fd 100644 --- a/core/test/Acre.test.ts +++ b/core/test/Acre.test.ts @@ -11,27 +11,29 @@ import type { TestERC20, Acre } from "../typechain" import { to1e18 } from "./utils" async function acreFixture() { - const [_, staker] = await ethers.getSigners() + const [staker, staker2] = await ethers.getSigners() const Token = await ethers.getContractFactory("TestERC20") const tbtc = await Token.deploy() const amountToMint = to1e18(100000) tbtc.mint(staker, amountToMint) + tbtc.mint(staker2, amountToMint) const Acre = await ethers.getContractFactory("Acre") const acre = await Acre.deploy(await tbtc.getAddress()) - return { acre, tbtc, staker } + return { acre, tbtc, staker, staker2 } } describe("Acre", () => { let acre: Acre let tbtc: TestERC20 let staker: HardhatEthersSigner + let staker2: HardhatEthersSigner before(async () => { - ;({ acre, tbtc, staker } = await loadFixture(acreFixture)) + ;({ acre, tbtc, staker, staker2 } = await loadFixture(acreFixture)) }) describe("Staking", () => { @@ -210,24 +212,23 @@ describe("Acre", () => { let afterSimulatingYieldSnapshot: SnapshotRestorer before(async () => { - const [staker1, staker2] = await ethers.getSigners() const staker1AmountToStake = to1e18(75) const staker2AmountToStake = to1e18(25) // Infinite approval for staking contract. await tbtc - .connect(staker1) + .connect(staker) .approve(await acre.getAddress(), ethers.MaxUint256) await tbtc .connect(staker2) .approve(await acre.getAddress(), ethers.MaxUint256) // Mint tokens. - await tbtc.connect(staker1).mint(staker1.address, staker1AmountToStake) + await tbtc.connect(staker).mint(staker.address, staker1AmountToStake) await tbtc.connect(staker2).mint(staker2.address, staker2AmountToStake) stakerA = { - signer: staker1, - address: staker1.address, + signer: staker, + address: staker.address, amountToStake: staker1AmountToStake, } stakerB = { From 300b2a916124d52a442f0642bb293b58296f3c9b Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Tue, 5 Dec 2023 09:59:46 +0100 Subject: [PATCH 35/60] Rename variable in Acre unit tests `staker1AmountToStake` -> `stakerAmountToStake`. Just for consistency with the `staker` variable (we do not add `1` suffix). --- 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 4a31307fd..d52c23876 100644 --- a/core/test/Acre.test.ts +++ b/core/test/Acre.test.ts @@ -212,7 +212,7 @@ describe("Acre", () => { let afterSimulatingYieldSnapshot: SnapshotRestorer before(async () => { - const staker1AmountToStake = to1e18(75) + const stakerAmountToStake = to1e18(75) const staker2AmountToStake = to1e18(25) // Infinite approval for staking contract. await tbtc @@ -223,13 +223,13 @@ describe("Acre", () => { .approve(await acre.getAddress(), ethers.MaxUint256) // Mint tokens. - await tbtc.connect(staker).mint(staker.address, staker1AmountToStake) + await tbtc.connect(staker).mint(staker.address, stakerAmountToStake) await tbtc.connect(staker2).mint(staker2.address, staker2AmountToStake) stakerA = { signer: staker, address: staker.address, - amountToStake: staker1AmountToStake, + amountToStake: stakerAmountToStake, } stakerB = { signer: staker2, From 44b3f858d41568f282b9b3d9a9686506fb2bd68f Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Tue, 5 Dec 2023 10:07:42 +0100 Subject: [PATCH 36/60] Update comment in Acre unit tests To clarify that the staker `B` stakes a given amount of tokens. --- 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 d52c23876..496d68e97 100644 --- a/core/test/Acre.test.ts +++ b/core/test/Acre.test.ts @@ -308,7 +308,7 @@ describe("Acre", () => { vaultYield = to1e18(100) // Staker A shares = 75 // Staker B shares = 25 - // Total assets = 75(staker A) + 25(staker) + 100(yield) + // Total assets = 75(staker A) + 25(staker B) + 100(yield) expectedTotalAssets = stakerA.amountToStake + stakerB.amountToStake + vaultYield // Total shares = 75 + 25 = 100 From b84ee1bf108732532f5f04dd9601c1d8589d4514 Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Tue, 5 Dec 2023 11:47:46 +0100 Subject: [PATCH 37/60] Update staking flow unit tests Use more concrete numbers, as we did in previous tests, and put some math in the comments to make the calculations easier to follow. --- core/test/Acre.test.ts | 43 +++++++++++++++++++++++++++++++++++++++--- 1 file changed, 40 insertions(+), 3 deletions(-) diff --git a/core/test/Acre.test.ts b/core/test/Acre.test.ts index 496d68e97..369f80704 100644 --- a/core/test/Acre.test.ts +++ b/core/test/Acre.test.ts @@ -373,26 +373,63 @@ describe("Acre", () => { }) context("when staker A stakes more tokens", () => { + const newAmountToStake = to1e18(20) let sharesBefore: bigint let availableToRedeemBefore: bigint + let totalAssetsBefore: bigint + let totalSupplyBefore: bigint + let expectedSharesToMint: bigint + let expectedTotalSupply: bigint + let expectedTotalAssets: bigint before(async () => { + // Current state: + // Total assets = 75(staker A) + 25(staker B) + 100(yield) + // Total shares = 75 + 25 = 100 await afterSimulatingYieldSnapshot.restore() sharesBefore = await acre.balanceOf(stakerA.address) availableToRedeemBefore = await acre.previewRedeem(sharesBefore) + totalAssetsBefore = await acre.totalAssets() + totalSupplyBefore = await acre.totalSupply() - tbtc.mint(stakerA.address, stakerA.amountToStake) + tbtc.mint(stakerA.address, newAmountToStake) - await acre.stake(stakerA.amountToStake, stakerA.address, referral) + expectedSharesToMint = + (newAmountToStake * (totalSupplyBefore + 1n)) / + (totalAssetsBefore + 1n) + + await acre.stake(newAmountToStake, stakerA.address, referral) + + // State after stake: + // Total assets = 75(staker A) + 25(staker B) + 100(yield) + 20(staker + // A) = 220 + // Total shares = 75 + 25 + 10 = 110 + expectedTotalAssets = totalAssetsBefore + newAmountToStake + expectedTotalSupply = totalSupplyBefore + expectedSharesToMint }) it("should receive more shares", async () => { const shares = await acre.balanceOf(stakerA.address) - const availableToRedeem = await acre.previewRedeem(shares) expect(shares).to.be.greaterThan(sharesBefore) + expect(shares).to.be.eq(sharesBefore + expectedSharesToMint) + }) + + it("should be able to redeem more tokens than before", async () => { + const shares = await acre.balanceOf(stakerA.address) + const availableToRedeem = await acre.previewRedeem(shares) + + // Expected amount w/o rounding: 85 * 220 / 110 = 170 + // Expected amount w/ support for rounding: 169999999999999999999 in + // tBTC token precision. + const expectedTotalAssetsAvailableToRedeem = + (shares * (expectedTotalAssets + 1n)) / (expectedTotalSupply + 1n) + expect(availableToRedeem).to.be.greaterThan(availableToRedeemBefore) + expect(availableToRedeem).to.be.eq( + expectedTotalAssetsAvailableToRedeem, + ) }) }) }) From 3a312f7784118c5afbd941b0cd3527b7acf27c73 Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Tue, 5 Dec 2023 12:01:32 +0100 Subject: [PATCH 38/60] Add documentation comments for `Acre` contract --- core/contracts/Acre.sol | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/core/contracts/Acre.sol b/core/contracts/Acre.sol index 2cac7beed..bb8f357e9 100644 --- a/core/contracts/Acre.sol +++ b/core/contracts/Acre.sol @@ -3,6 +3,13 @@ pragma solidity ^0.8.21; import "@openzeppelin/contracts/token/ERC20/extensions/ERC4626.sol"; +/// @title Acre +/// @notice Implementation of the ERR-4626 tokenized vault standard. ERC-4626 is +/// a standard to optimize and unify the technical parameters of +/// yield-bearing vaults. This contract allows the minting and burning +/// of shares, represented as standard ERC20 token, in exchange for tBTC +/// tokens. +/// @dev ERC-4626 standard extends the ERC-20 token. contract Acre is ERC4626 { event Staked(bytes32 indexed referral, uint256 assets, uint256 shares); From 277dc1ed2d857c8bf88fc944494d02022464be9d Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Tue, 5 Dec 2023 15:33:45 +0100 Subject: [PATCH 39/60] Rename `Staked` event We decided to rename this event to `StakeReferral` and remove the `shares` param. --- core/contracts/Acre.sol | 4 ++-- core/test/Acre.test.ts | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/core/contracts/Acre.sol b/core/contracts/Acre.sol index bb8f357e9..59bf4b69d 100644 --- a/core/contracts/Acre.sol +++ b/core/contracts/Acre.sol @@ -11,7 +11,7 @@ import "@openzeppelin/contracts/token/ERC20/extensions/ERC4626.sol"; /// tokens. /// @dev ERC-4626 standard extends the ERC-20 token. contract Acre is ERC4626 { - event Staked(bytes32 indexed referral, uint256 assets, uint256 shares); + event StakeReferral(bytes32 indexed referral, uint256 assets); constructor( IERC20 tbtc @@ -32,7 +32,7 @@ contract Acre is ERC4626 { // TODO: revisit the type of referral. uint256 shares = deposit(assets, receiver); - emit Staked(referral, assets, shares); + emit StakeReferral(referral, assets); return shares; } diff --git a/core/test/Acre.test.ts b/core/test/Acre.test.ts index 369f80704..3c0934ca4 100644 --- a/core/test/Acre.test.ts +++ b/core/test/Acre.test.ts @@ -82,10 +82,10 @@ describe("Acre", () => { ) }) - it("should emit Staked event", () => { + it("should emit StakeReferral event", () => { expect(tx) - .to.emit(acre, "Staked") - .withArgs(referral, amountToStake, expectedReceivedShares) + .to.emit(acre, "StakeReferral") + .withArgs(referral, amountToStake) }) it("should mint stBTC tokens", async () => { From eb91b4d6a126d5c358e415e1bdf4bcc438afa31c Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Tue, 5 Dec 2023 15:36:00 +0100 Subject: [PATCH 40/60] Update the `stake` fn Emit the `StakeReferral` event only if the referral is not empty value. Actually, we need this data when the referral is present so it doesn't make sense to emit this event with empty referral value. --- core/contracts/Acre.sol | 4 +++- core/test/Acre.test.ts | 4 ++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/core/contracts/Acre.sol b/core/contracts/Acre.sol index 59bf4b69d..7f2bdb019 100644 --- a/core/contracts/Acre.sol +++ b/core/contracts/Acre.sol @@ -32,7 +32,9 @@ contract Acre is ERC4626 { // TODO: revisit the type of referral. uint256 shares = deposit(assets, receiver); - emit StakeReferral(referral, assets); + if (referral != bytes32(0)) { + emit StakeReferral(referral, assets); + } return shares; } diff --git a/core/test/Acre.test.ts b/core/test/Acre.test.ts index 3c0934ca4..c3902c301 100644 --- a/core/test/Acre.test.ts +++ b/core/test/Acre.test.ts @@ -120,6 +120,10 @@ describe("Acre", () => { it("should not revert", async () => { await expect(tx).to.be.not.reverted }) + + it("should not emit the StakeReferral event", async () => { + await expect(tx).to.not.emit(acre, "StakeReferral") + }) }) context( From fbb2085d5465051361eb42715421cce23292d312 Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Tue, 5 Dec 2023 15:40:08 +0100 Subject: [PATCH 41/60] Update `stake` fn docs Clarify under the `dev` tag that the amount of the assets has to be pre-approved in the tBTC contract. --- core/contracts/Acre.sol | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/core/contracts/Acre.sol b/core/contracts/Acre.sol index 7f2bdb019..84f33722c 100644 --- a/core/contracts/Acre.sol +++ b/core/contracts/Acre.sol @@ -19,7 +19,8 @@ contract Acre is ERC4626 { /// @notice Stakes a given amount of tBTC token and mints shares to a /// receiver. - /// @dev This function calls `deposit` function from `ERC4626` contract. + /// @dev This function calls `deposit` function from `ERC4626` contract. The + /// amount of the assets has to be pre-approved in the tBTC contract. /// @param assets Approved amount for the transfer and stake. /// @param receiver The address to which the shares will be minted. /// @param referral Data used for referral program. From 48eeed3c2b655bc57cb15786691237c07126ae27 Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Tue, 5 Dec 2023 15:44:26 +0100 Subject: [PATCH 42/60] Rename variable in Acre unit tests Since we have `staker2` let's rename `staker` to `staker1`. --- core/test/Acre.test.ts | 68 ++++++++++++++++++++++-------------------- 1 file changed, 35 insertions(+), 33 deletions(-) diff --git a/core/test/Acre.test.ts b/core/test/Acre.test.ts index c3902c301..a16e00930 100644 --- a/core/test/Acre.test.ts +++ b/core/test/Acre.test.ts @@ -11,29 +11,29 @@ import type { TestERC20, Acre } from "../typechain" import { to1e18 } from "./utils" async function acreFixture() { - const [staker, staker2] = await ethers.getSigners() + const [staker1, staker2] = await ethers.getSigners() const Token = await ethers.getContractFactory("TestERC20") const tbtc = await Token.deploy() const amountToMint = to1e18(100000) - tbtc.mint(staker, amountToMint) + tbtc.mint(staker1, amountToMint) tbtc.mint(staker2, amountToMint) const Acre = await ethers.getContractFactory("Acre") const acre = await Acre.deploy(await tbtc.getAddress()) - return { acre, tbtc, staker, staker2 } + return { acre, tbtc, staker1, staker2 } } describe("Acre", () => { let acre: Acre let tbtc: TestERC20 - let staker: HardhatEthersSigner + let staker1: HardhatEthersSigner let staker2: HardhatEthersSigner before(async () => { - ;({ acre, tbtc, staker, staker2 } = await loadFixture(acreFixture)) + ;({ acre, tbtc, staker1, staker2 } = await loadFixture(acreFixture)) }) describe("Staking", () => { @@ -61,20 +61,20 @@ describe("Acre", () => { beforeEach(async () => { await tbtc - .connect(staker) + .connect(staker1) .approve(await acre.getAddress(), amountToStake) tx = await acre - .connect(staker) - .stake(amountToStake, staker.address, referral) + .connect(staker1) + .stake(amountToStake, staker1.address, referral) }) it("should emit Deposit event", () => { expect(tx).to.emit(acre, "Deposit").withArgs( // Caller. - staker.address, + staker1.address, // Receiver. - staker.address, + staker1.address, // Staked tokens. amountToStake, // Received shares. @@ -91,7 +91,7 @@ describe("Acre", () => { it("should mint stBTC tokens", async () => { await expect(tx).to.changeTokenBalances( acre, - [staker.address], + [staker1.address], [amountToStake], ) }) @@ -99,7 +99,7 @@ describe("Acre", () => { it("should transfer tBTC tokens", async () => { await expect(tx).to.changeTokenBalances( tbtc, - [staker.address, acre], + [staker1.address, acre], [-amountToStake, amountToStake], ) }) @@ -110,11 +110,11 @@ describe("Acre", () => { let tx: ContractTransactionResponse beforeEach(async () => { - await tbtc.connect(staker).approve(await acre.getAddress(), 1) + await tbtc.connect(staker1).approve(await acre.getAddress(), 1) tx = await acre - .connect(staker) - .stake(1, staker.address, emptyReferral) + .connect(staker1) + .stake(1, staker1.address, emptyReferral) }) it("should not revert", async () => { @@ -132,15 +132,15 @@ describe("Acre", () => { const amountToStake = to1e18(10) beforeEach(async () => { await tbtc - .connect(staker) + .connect(staker1) .approve(await acre.getAddress(), amountToStake) }) it("should revert", async () => { await expect( acre - .connect(staker) - .stake(amountToStake + 1n, staker.address, referral), + .connect(staker1) + .stake(amountToStake + 1n, staker1.address, referral), ).to.be.reverted }) }, @@ -151,13 +151,15 @@ describe("Acre", () => { beforeEach(async () => { await tbtc - .connect(staker) + .connect(staker1) .approve(await acre.getAddress(), amountToStake) }) it("should not revert", async () => { await expect( - acre.connect(staker).stake(amountToStake, staker.address, referral), + acre + .connect(staker1) + .stake(amountToStake, staker1.address, referral), ).to.not.be.reverted }) }) @@ -167,13 +169,13 @@ describe("Acre", () => { beforeEach(async () => { await tbtc - .connect(staker) + .connect(staker1) .approve(await acre.getAddress(), amountToStake) }) it("should revert", async () => { await expect( - acre.connect(staker).stake(amountToStake, ZeroAddress, referral), + acre.connect(staker1).stake(amountToStake, ZeroAddress, referral), ).to.be.revertedWithCustomError(acre, "ERC20InvalidReceiver") }) }) @@ -185,19 +187,19 @@ describe("Acre", () => { beforeEach(async () => { await tbtc - .connect(staker) + .connect(staker1) .approve(await acre.getAddress(), amountToStake) await acre - .connect(staker) - .stake(amountToStake, staker.address, referral) + .connect(staker1) + .stake(amountToStake, staker1.address, referral) }) it("should revert", async () => { await expect( acre - .connect(staker) - .stake(amountToStake, staker.address, referral), + .connect(staker1) + .stake(amountToStake, staker1.address, referral), ).to.be.revertedWithCustomError(acre, "ERC20InsufficientAllowance") }) }, @@ -216,24 +218,24 @@ describe("Acre", () => { let afterSimulatingYieldSnapshot: SnapshotRestorer before(async () => { - const stakerAmountToStake = to1e18(75) + const staker1AmountToStake = to1e18(75) const staker2AmountToStake = to1e18(25) // Infinite approval for staking contract. await tbtc - .connect(staker) + .connect(staker1) .approve(await acre.getAddress(), ethers.MaxUint256) await tbtc .connect(staker2) .approve(await acre.getAddress(), ethers.MaxUint256) // Mint tokens. - await tbtc.connect(staker).mint(staker.address, stakerAmountToStake) + await tbtc.connect(staker1).mint(staker1.address, staker1AmountToStake) await tbtc.connect(staker2).mint(staker2.address, staker2AmountToStake) stakerA = { - signer: staker, - address: staker.address, - amountToStake: stakerAmountToStake, + signer: staker1, + address: staker1.address, + amountToStake: staker1AmountToStake, } stakerB = { signer: staker2, From f7003b81bf8637e84ffe43f4aa3fc8b3c68c6af7 Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Tue, 5 Dec 2023 15:50:30 +0100 Subject: [PATCH 43/60] Rename variable in `acreFixture` `Token` -> `TestERC20` to be consistent. --- 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 a16e00930..dff30aab1 100644 --- a/core/test/Acre.test.ts +++ b/core/test/Acre.test.ts @@ -12,8 +12,8 @@ import { to1e18 } from "./utils" async function acreFixture() { const [staker1, staker2] = await ethers.getSigners() - const Token = await ethers.getContractFactory("TestERC20") - const tbtc = await Token.deploy() + const TestERC20 = await ethers.getContractFactory("TestERC20") + const tbtc = await TestERC20.deploy() const amountToMint = to1e18(100000) From 076a29cd15221243abeca5175dc4635872229226 Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Tue, 5 Dec 2023 15:56:05 +0100 Subject: [PATCH 44/60] Update the `acreFixture` fn Let's deploy the contracts first and then perform the minting. --- core/test/Acre.test.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/core/test/Acre.test.ts b/core/test/Acre.test.ts index dff30aab1..9e167f32e 100644 --- a/core/test/Acre.test.ts +++ b/core/test/Acre.test.ts @@ -12,17 +12,17 @@ import { to1e18 } from "./utils" async function acreFixture() { const [staker1, staker2] = await ethers.getSigners() + const TestERC20 = await ethers.getContractFactory("TestERC20") const tbtc = await TestERC20.deploy() - const amountToMint = to1e18(100000) + const Acre = await ethers.getContractFactory("Acre") + const acre = await Acre.deploy(await tbtc.getAddress()) + const amountToMint = to1e18(100000) tbtc.mint(staker1, amountToMint) tbtc.mint(staker2, amountToMint) - const Acre = await ethers.getContractFactory("Acre") - const acre = await Acre.deploy(await tbtc.getAddress()) - return { acre, tbtc, staker1, staker2 } } From 961580b996da7f7cc4fbf9861984d9eb226cb0c7 Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Tue, 5 Dec 2023 15:57:02 +0100 Subject: [PATCH 45/60] Update the test name It's worth to match the test name with the function name. --- 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 9e167f32e..01b4be062 100644 --- a/core/test/Acre.test.ts +++ b/core/test/Acre.test.ts @@ -36,7 +36,7 @@ describe("Acre", () => { ;({ acre, tbtc, staker1, staker2 } = await loadFixture(acreFixture)) }) - describe("Staking", () => { + describe("stake", () => { const referral = ethers.encodeBytes32String("referral") let snapshot: SnapshotRestorer From 9674f5e5a72c9fbf36831f0ff141bbdcd40cf4e6 Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Tue, 5 Dec 2023 16:12:16 +0100 Subject: [PATCH 46/60] Rename file `TestToken` -> `TestERC20`. The file name should match the contract name. --- core/contracts/test/{TestToken.sol => TestERC20.sol} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename core/contracts/test/{TestToken.sol => TestERC20.sol} (100%) diff --git a/core/contracts/test/TestToken.sol b/core/contracts/test/TestERC20.sol similarity index 100% rename from core/contracts/test/TestToken.sol rename to core/contracts/test/TestERC20.sol From 9737ed8f443c22685c04c80a118684360587e1fc Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Tue, 5 Dec 2023 16:26:17 +0100 Subject: [PATCH 47/60] Avoid infinite approvals in unit tests --- core/test/Acre.test.ts | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/core/test/Acre.test.ts b/core/test/Acre.test.ts index 01b4be062..5964b3b8d 100644 --- a/core/test/Acre.test.ts +++ b/core/test/Acre.test.ts @@ -220,13 +220,13 @@ describe("Acre", () => { before(async () => { const staker1AmountToStake = to1e18(75) const staker2AmountToStake = to1e18(25) - // Infinite approval for staking contract. + await tbtc .connect(staker1) - .approve(await acre.getAddress(), ethers.MaxUint256) + .approve(await acre.getAddress(), staker1AmountToStake) await tbtc .connect(staker2) - .approve(await acre.getAddress(), ethers.MaxUint256) + .approve(await acre.getAddress(), staker2AmountToStake) // Mint tokens. await tbtc.connect(staker1).mint(staker1.address, staker1AmountToStake) @@ -401,6 +401,10 @@ describe("Acre", () => { tbtc.mint(stakerA.address, newAmountToStake) + await tbtc + .connect(stakerA.signer) + .approve(await acre.getAddress(), newAmountToStake) + expectedSharesToMint = (newAmountToStake * (totalSupplyBefore + 1n)) / (totalAssetsBefore + 1n) From d33c5527735da7d58690a67b791546a39557983b Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Tue, 5 Dec 2023 16:40:33 +0100 Subject: [PATCH 48/60] Get rid of unnecessary `Staker` type in unit tests --- core/test/Acre.test.ts | 85 ++++++++++++++++-------------------------- 1 file changed, 32 insertions(+), 53 deletions(-) diff --git a/core/test/Acre.test.ts b/core/test/Acre.test.ts index 5964b3b8d..c574240da 100644 --- a/core/test/Acre.test.ts +++ b/core/test/Acre.test.ts @@ -207,20 +207,12 @@ describe("Acre", () => { }) context("when there are two stakers, A and B ", () => { - type Staker = { - signer: HardhatEthersSigner - address: string - amountToStake: bigint - } - let stakerA: Staker - let stakerB: Staker + const staker1AmountToStake = to1e18(75) + const staker2AmountToStake = to1e18(25) let afterStakesSnapshot: SnapshotRestorer let afterSimulatingYieldSnapshot: SnapshotRestorer before(async () => { - const staker1AmountToStake = to1e18(75) - const staker2AmountToStake = to1e18(25) - await tbtc .connect(staker1) .approve(await acre.getAddress(), staker1AmountToStake) @@ -231,17 +223,6 @@ describe("Acre", () => { // Mint tokens. await tbtc.connect(staker1).mint(staker1.address, staker1AmountToStake) await tbtc.connect(staker2).mint(staker2.address, staker2AmountToStake) - - stakerA = { - signer: staker1, - address: staker1.address, - amountToStake: staker1AmountToStake, - } - stakerB = { - signer: staker2, - address: staker2.address, - amountToStake: staker2AmountToStake, - } }) context( @@ -255,15 +236,15 @@ describe("Acre", () => { it("should stake tokens correctly", async () => { await expect( acre - .connect(stakerA.signer) - .stake(stakerA.amountToStake, stakerA.address, referral), + .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(stakerA.address) + const shares = await acre.balanceOf(staker1.address) - expect(shares).to.eq(stakerA.amountToStake) + expect(shares).to.eq(staker1AmountToStake) }) }) @@ -271,15 +252,15 @@ describe("Acre", () => { it("should stake tokens correctly", async () => { await expect( acre - .connect(stakerB.signer) - .stake(stakerB.amountToStake, stakerB.address, referral), + .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(stakerB.address) + const shares = await acre.balanceOf(staker2.address) - expect(shares).to.eq(stakerB.amountToStake) + expect(shares).to.eq(staker2AmountToStake) }) }) }, @@ -293,15 +274,13 @@ describe("Acre", () => { it("the total assets amount should be equal to all staked tokens", async () => { const totalAssets = await acre.totalAssets() - expect(totalAssets).to.eq( - stakerA.amountToStake + stakerB.amountToStake, - ) + expect(totalAssets).to.eq(staker1AmountToStake + staker2AmountToStake) }) }) context("when vault earns yield", () => { - let stakerASharesBefore: bigint - let stakerBSharesBefore: bigint + let staker1SharesBefore: bigint + let staker2SharesBefore: bigint let vaultYield: bigint let expectedTotalAssets: bigint let expectedTotalSupply: bigint @@ -309,16 +288,16 @@ describe("Acre", () => { before(async () => { await afterStakesSnapshot.restore() - stakerASharesBefore = await acre.balanceOf(stakerA.address) - stakerBSharesBefore = await acre.balanceOf(stakerB.address) + staker1SharesBefore = await acre.balanceOf(staker1.address) + staker2SharesBefore = await acre.balanceOf(staker2.address) vaultYield = to1e18(100) // Staker A shares = 75 // Staker B shares = 25 // Total assets = 75(staker A) + 25(staker B) + 100(yield) expectedTotalAssets = - stakerA.amountToStake + stakerB.amountToStake + vaultYield + staker1AmountToStake + staker2AmountToStake + vaultYield // Total shares = 75 + 25 = 100 - expectedTotalSupply = stakerA.amountToStake + stakerB.amountToStake + expectedTotalSupply = staker1AmountToStake + staker2AmountToStake // Simulating yield returned from strategies. The vault now contains // more tokens than deposited which causes the exchange rate to @@ -332,21 +311,21 @@ describe("Acre", () => { it("the vault should hold more assets", async () => { expect(await acre.totalAssets()).to.be.eq( - stakerA.amountToStake + stakerB.amountToStake + vaultYield, + staker1AmountToStake + staker2AmountToStake + vaultYield, ) }) it("the staker's shares should be the same", async () => { - expect(await acre.balanceOf(stakerA.address)).to.be.eq( - stakerASharesBefore, + expect(await acre.balanceOf(staker1.address)).to.be.eq( + staker1SharesBefore, ) - expect(await acre.balanceOf(stakerB.address)).to.be.eq( - stakerBSharesBefore, + expect(await acre.balanceOf(staker2.address)).to.be.eq( + staker2SharesBefore, ) }) it("the staker A should be able to redeem more tokens than before", async () => { - const shares = await acre.balanceOf(stakerA.address) + const shares = await acre.balanceOf(staker1.address) const availableAssetsToRedeem = await acre.previewRedeem(shares) // Expected amount w/o rounding: 75 * 200 / 100 = 150 @@ -356,13 +335,13 @@ describe("Acre", () => { (shares * (expectedTotalAssets + 1n)) / (expectedTotalSupply + 1n) expect(availableAssetsToRedeem).to.be.greaterThan( - stakerA.amountToStake, + staker1AmountToStake, ) expect(availableAssetsToRedeem).to.be.eq(expectedAssetsToRedeem) }) it("the staker B should be able to redeem more tokens than before", async () => { - const shares = await acre.balanceOf(stakerB.address) + const shares = await acre.balanceOf(staker2.address) const availableAssetsToRedeem = await acre.previewRedeem(shares) // Expected amount w/o rounding: 25 * 200 / 100 = 50 @@ -372,7 +351,7 @@ describe("Acre", () => { (shares * (expectedTotalAssets + 1n)) / (expectedTotalSupply + 1n) expect(availableAssetsToRedeem).to.be.greaterThan( - stakerB.amountToStake, + staker2AmountToStake, ) expect(availableAssetsToRedeem).to.be.eq(expectedAssetsToRedeem) }) @@ -394,22 +373,22 @@ describe("Acre", () => { // Total shares = 75 + 25 = 100 await afterSimulatingYieldSnapshot.restore() - sharesBefore = await acre.balanceOf(stakerA.address) + sharesBefore = await acre.balanceOf(staker1.address) availableToRedeemBefore = await acre.previewRedeem(sharesBefore) totalAssetsBefore = await acre.totalAssets() totalSupplyBefore = await acre.totalSupply() - tbtc.mint(stakerA.address, newAmountToStake) + tbtc.mint(staker1.address, newAmountToStake) await tbtc - .connect(stakerA.signer) + .connect(staker1) .approve(await acre.getAddress(), newAmountToStake) expectedSharesToMint = (newAmountToStake * (totalSupplyBefore + 1n)) / (totalAssetsBefore + 1n) - await acre.stake(newAmountToStake, stakerA.address, referral) + await acre.stake(newAmountToStake, staker1.address, referral) // State after stake: // Total assets = 75(staker A) + 25(staker B) + 100(yield) + 20(staker @@ -420,14 +399,14 @@ describe("Acre", () => { }) it("should receive more shares", async () => { - const shares = await acre.balanceOf(stakerA.address) + const shares = await acre.balanceOf(staker1.address) expect(shares).to.be.greaterThan(sharesBefore) expect(shares).to.be.eq(sharesBefore + expectedSharesToMint) }) it("should be able to redeem more tokens than before", async () => { - const shares = await acre.balanceOf(stakerA.address) + const shares = await acre.balanceOf(staker1.address) const availableToRedeem = await acre.previewRedeem(shares) // Expected amount w/o rounding: 85 * 220 / 110 = 170 From eef74bc541400fca0dc2d08d04d626f4037e62bf Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Tue, 5 Dec 2023 16:50:58 +0100 Subject: [PATCH 49/60] Hardcode expected values in unit tests To avoid a false-positive result where both contracts and tests calculations are wrong. --- core/test/Acre.test.ts | 39 +++++++++------------------------------ 1 file changed, 9 insertions(+), 30 deletions(-) diff --git a/core/test/Acre.test.ts b/core/test/Acre.test.ts index c574240da..c46b6ef4d 100644 --- a/core/test/Acre.test.ts +++ b/core/test/Acre.test.ts @@ -282,22 +282,17 @@ describe("Acre", () => { let staker1SharesBefore: bigint let staker2SharesBefore: bigint let vaultYield: bigint - let expectedTotalAssets: bigint - let expectedTotalSupply: bigint before(async () => { + // Current state: + // Staker A shares = 75 + // Staker B shares = 25 + // Total assets = 75(staker A) + 25(staker B) + 100(yield) await afterStakesSnapshot.restore() staker1SharesBefore = await acre.balanceOf(staker1.address) staker2SharesBefore = await acre.balanceOf(staker2.address) vaultYield = to1e18(100) - // Staker A shares = 75 - // Staker B shares = 25 - // Total assets = 75(staker A) + 25(staker B) + 100(yield) - expectedTotalAssets = - staker1AmountToStake + staker2AmountToStake + vaultYield - // Total shares = 75 + 25 = 100 - expectedTotalSupply = staker1AmountToStake + staker2AmountToStake // Simulating yield returned from strategies. The vault now contains // more tokens than deposited which causes the exchange rate to @@ -331,8 +326,7 @@ describe("Acre", () => { // Expected amount w/o rounding: 75 * 200 / 100 = 150 // Expected amount w/ support for rounding: 149999999999999999999 in // tBTC token precision. - const expectedAssetsToRedeem = - (shares * (expectedTotalAssets + 1n)) / (expectedTotalSupply + 1n) + const expectedAssetsToRedeem = 149999999999999999999n expect(availableAssetsToRedeem).to.be.greaterThan( staker1AmountToStake, @@ -347,8 +341,7 @@ describe("Acre", () => { // Expected amount w/o rounding: 25 * 200 / 100 = 50 // Expected amount w/ support for rounding: 49999999999999999999 in // tBTC token precision. - const expectedAssetsToRedeem = - (shares * (expectedTotalAssets + 1n)) / (expectedTotalSupply + 1n) + const expectedAssetsToRedeem = 49999999999999999999n expect(availableAssetsToRedeem).to.be.greaterThan( staker2AmountToStake, @@ -359,13 +352,9 @@ describe("Acre", () => { context("when staker A stakes more tokens", () => { const newAmountToStake = to1e18(20) + const expectedSharesToMint = to1e18(10) let sharesBefore: bigint let availableToRedeemBefore: bigint - let totalAssetsBefore: bigint - let totalSupplyBefore: bigint - let expectedSharesToMint: bigint - let expectedTotalSupply: bigint - let expectedTotalAssets: bigint before(async () => { // Current state: @@ -375,8 +364,6 @@ describe("Acre", () => { sharesBefore = await acre.balanceOf(staker1.address) availableToRedeemBefore = await acre.previewRedeem(sharesBefore) - totalAssetsBefore = await acre.totalAssets() - totalSupplyBefore = await acre.totalSupply() tbtc.mint(staker1.address, newAmountToStake) @@ -384,18 +371,11 @@ describe("Acre", () => { .connect(staker1) .approve(await acre.getAddress(), newAmountToStake) - expectedSharesToMint = - (newAmountToStake * (totalSupplyBefore + 1n)) / - (totalAssetsBefore + 1n) - - await acre.stake(newAmountToStake, staker1.address, referral) - // State after stake: // Total assets = 75(staker A) + 25(staker B) + 100(yield) + 20(staker // A) = 220 // Total shares = 75 + 25 + 10 = 110 - expectedTotalAssets = totalAssetsBefore + newAmountToStake - expectedTotalSupply = totalSupplyBefore + expectedSharesToMint + await acre.stake(newAmountToStake, staker1.address, referral) }) it("should receive more shares", async () => { @@ -412,8 +392,7 @@ describe("Acre", () => { // Expected amount w/o rounding: 85 * 220 / 110 = 170 // Expected amount w/ support for rounding: 169999999999999999999 in // tBTC token precision. - const expectedTotalAssetsAvailableToRedeem = - (shares * (expectedTotalAssets + 1n)) / (expectedTotalSupply + 1n) + const expectedTotalAssetsAvailableToRedeem = 169999999999999999999n expect(availableToRedeem).to.be.greaterThan(availableToRedeemBefore) expect(availableToRedeem).to.be.eq( From 8a243cf30d65b13c2f964c99b6db5fee036c4def Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Tue, 5 Dec 2023 16:58:37 +0100 Subject: [PATCH 50/60] Remove unnecessary check in unit test We validate the exact value in the next line so this check seems redundant. --- core/test/Acre.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/core/test/Acre.test.ts b/core/test/Acre.test.ts index c46b6ef4d..863d1800e 100644 --- a/core/test/Acre.test.ts +++ b/core/test/Acre.test.ts @@ -381,7 +381,6 @@ describe("Acre", () => { it("should receive more shares", async () => { const shares = await acre.balanceOf(staker1.address) - expect(shares).to.be.greaterThan(sharesBefore) expect(shares).to.be.eq(sharesBefore + expectedSharesToMint) }) From b812107a2e8c22649758f7fcec68b075fef3e333 Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Tue, 5 Dec 2023 17:01:48 +0100 Subject: [PATCH 51/60] Rename test context name `when staking` -> `when staking as a first staker`. --- 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 863d1800e..445cf0618 100644 --- a/core/test/Acre.test.ts +++ b/core/test/Acre.test.ts @@ -40,7 +40,7 @@ describe("Acre", () => { const referral = ethers.encodeBytes32String("referral") let snapshot: SnapshotRestorer - context("when staking", () => { + context("when staking as first staker", () => { beforeEach(async () => { snapshot = await takeSnapshot() }) From 910a3992e6cf68d056abcfca6fe1f1599896f273 Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Tue, 5 Dec 2023 17:03:43 +0100 Subject: [PATCH 52/60] Update `without referral` test case Define a variable `amountToStake` instead of passing number directly to functions - to improve readability. --- core/test/Acre.test.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/core/test/Acre.test.ts b/core/test/Acre.test.ts index 445cf0618..d043adf8d 100644 --- a/core/test/Acre.test.ts +++ b/core/test/Acre.test.ts @@ -106,15 +106,18 @@ describe("Acre", () => { }) context("without referral", () => { + const amountToStake = to1e18(10) const emptyReferral = ethers.encodeBytes32String("") let tx: ContractTransactionResponse beforeEach(async () => { - await tbtc.connect(staker1).approve(await acre.getAddress(), 1) + await tbtc + .connect(staker1) + .approve(await acre.getAddress(), amountToStake) tx = await acre .connect(staker1) - .stake(1, staker1.address, emptyReferral) + .stake(amountToStake, staker1.address, emptyReferral) }) it("should not revert", async () => { From a868dcec1dc4e2483357397e6a9fc78fbaeb933b Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Tue, 5 Dec 2023 17:16:56 +0100 Subject: [PATCH 53/60] Improve test case Since the variable is named `amountToStake`, the amount we pass to `stake` function should be exactly `amountToStake`. Here we add a new variable `approvedAmount` which will be approved in the `before` hook and we use this value when defining the `amountToStake` which is equal `amountToStake + 1`. This will imporve readability. --- core/test/Acre.test.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/core/test/Acre.test.ts b/core/test/Acre.test.ts index d043adf8d..14d6db66b 100644 --- a/core/test/Acre.test.ts +++ b/core/test/Acre.test.ts @@ -132,18 +132,20 @@ describe("Acre", () => { context( "when amount to stake is greater than the approved amount", () => { - const amountToStake = to1e18(10) + const approvedAmount = to1e18(10) + const amountToStake = approvedAmount + 1n + beforeEach(async () => { await tbtc .connect(staker1) - .approve(await acre.getAddress(), amountToStake) + .approve(await acre.getAddress(), approvedAmount) }) it("should revert", async () => { await expect( acre .connect(staker1) - .stake(amountToStake + 1n, staker1.address, referral), + .stake(amountToStake, staker1.address, referral), ).to.be.reverted }) }, From 567b52ca96eb50eae82a35f339b685d81200f197 Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Tue, 5 Dec 2023 17:17:34 +0100 Subject: [PATCH 54/60] Make sure the tx is reverted with expected reason --- 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 14d6db66b..5d9c70f99 100644 --- a/core/test/Acre.test.ts +++ b/core/test/Acre.test.ts @@ -146,7 +146,7 @@ describe("Acre", () => { acre .connect(staker1) .stake(amountToStake, staker1.address, referral), - ).to.be.reverted + ).to.be.revertedWithCustomError(tbtc, "ERC20InsufficientAllowance") }) }, ) From 4889a0355677f39359fbc434899b084a021191a3 Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Tue, 5 Dec 2023 17:18:58 +0100 Subject: [PATCH 55/60] Use dedicated variable in unit test We should use `expectedReceivedShares` while checking `stBTC` balance after staking. --- 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 5d9c70f99..0d13690fd 100644 --- a/core/test/Acre.test.ts +++ b/core/test/Acre.test.ts @@ -92,7 +92,7 @@ describe("Acre", () => { await expect(tx).to.changeTokenBalances( acre, [staker1.address], - [amountToStake], + [expectedReceivedShares], ) }) From 9cac49c8d9be916656bc0bf546fbdfeb1af4f467 Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Tue, 5 Dec 2023 18:00:56 +0100 Subject: [PATCH 56/60] Update staking unit tests Since the ERC4626 deposit function has different caller and receiver roles, let's test them here. This will reflect the flow for tBTC minting and staking in one transaction, where an external contract will call the stake function in the name of the staker. --- core/test/Acre.test.ts | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/core/test/Acre.test.ts b/core/test/Acre.test.ts index 0d13690fd..051732352 100644 --- a/core/test/Acre.test.ts +++ b/core/test/Acre.test.ts @@ -58,23 +58,28 @@ describe("Acre", () => { const expectedReceivedShares = amountToStake let tx: ContractTransactionResponse + let tbtcHolder: HardhatEthersSigner + let receiver: HardhatEthersSigner beforeEach(async () => { + tbtcHolder = staker1 + receiver = staker2 + await tbtc - .connect(staker1) + .connect(tbtcHolder) .approve(await acre.getAddress(), amountToStake) tx = await acre - .connect(staker1) - .stake(amountToStake, staker1.address, referral) + .connect(tbtcHolder) + .stake(amountToStake, receiver.address, referral) }) it("should emit Deposit event", () => { expect(tx).to.emit(acre, "Deposit").withArgs( // Caller. - staker1.address, + tbtcHolder.address, // Receiver. - staker1.address, + receiver.address, // Staked tokens. amountToStake, // Received shares. @@ -91,7 +96,7 @@ describe("Acre", () => { it("should mint stBTC tokens", async () => { await expect(tx).to.changeTokenBalances( acre, - [staker1.address], + [receiver.address], [expectedReceivedShares], ) }) @@ -99,7 +104,7 @@ describe("Acre", () => { it("should transfer tBTC tokens", async () => { await expect(tx).to.changeTokenBalances( tbtc, - [staker1.address, acre], + [tbtcHolder.address, acre], [-amountToStake, amountToStake], ) }) From 88ee4d6f9ad8462ea90ffb3fed9ce0ad34246b14 Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Tue, 5 Dec 2023 18:27:44 +0100 Subject: [PATCH 57/60] Complicate the math in test scenario Complicate the math by using `50` instead of `100` to use a different value than `100` which we already have in `totalShares` (25+75=100). --- core/test/Acre.test.ts | 38 ++++++++++++++++++++------------------ 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/core/test/Acre.test.ts b/core/test/Acre.test.ts index 051732352..c858bc43e 100644 --- a/core/test/Acre.test.ts +++ b/core/test/Acre.test.ts @@ -297,12 +297,12 @@ describe("Acre", () => { // Current state: // Staker A shares = 75 // Staker B shares = 25 - // Total assets = 75(staker A) + 25(staker B) + 100(yield) + // Total assets = 75(staker A) + 25(staker B) + 50(yield) await afterStakesSnapshot.restore() staker1SharesBefore = await acre.balanceOf(staker1.address) staker2SharesBefore = await acre.balanceOf(staker2.address) - vaultYield = to1e18(100) + vaultYield = to1e18(50) // Simulating yield returned from strategies. The vault now contains // more tokens than deposited which causes the exchange rate to @@ -333,10 +333,10 @@ describe("Acre", () => { const shares = await acre.balanceOf(staker1.address) const availableAssetsToRedeem = await acre.previewRedeem(shares) - // Expected amount w/o rounding: 75 * 200 / 100 = 150 - // Expected amount w/ support for rounding: 149999999999999999999 in + // Expected amount w/o rounding: 75 * 150 / 100 = 112.5 + // Expected amount w/ support for rounding: 112499999999999999999 in // tBTC token precision. - const expectedAssetsToRedeem = 149999999999999999999n + const expectedAssetsToRedeem = 112499999999999999999n expect(availableAssetsToRedeem).to.be.greaterThan( staker1AmountToStake, @@ -348,10 +348,10 @@ describe("Acre", () => { const shares = await acre.balanceOf(staker2.address) const availableAssetsToRedeem = await acre.previewRedeem(shares) - // Expected amount w/o rounding: 25 * 200 / 100 = 50 - // Expected amount w/ support for rounding: 49999999999999999999 in + // Expected amount w/o rounding: 25 * 150 / 100 = 37.5 + // Expected amount w/ support for rounding: 37499999999999999999 in // tBTC token precision. - const expectedAssetsToRedeem = 49999999999999999999n + const expectedAssetsToRedeem = 37499999999999999999n expect(availableAssetsToRedeem).to.be.greaterThan( staker2AmountToStake, @@ -362,14 +362,16 @@ describe("Acre", () => { context("when staker A stakes more tokens", () => { const newAmountToStake = to1e18(20) - const expectedSharesToMint = to1e18(10) + // 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 before(async () => { - // Current state: - // Total assets = 75(staker A) + 25(staker B) + 100(yield) - // Total shares = 75 + 25 = 100 await afterSimulatingYieldSnapshot.restore() sharesBefore = await acre.balanceOf(staker1.address) @@ -382,9 +384,9 @@ describe("Acre", () => { .approve(await acre.getAddress(), newAmountToStake) // State after stake: - // Total assets = 75(staker A) + 25(staker B) + 100(yield) + 20(staker - // A) = 220 - // Total shares = 75 + 25 + 10 = 110 + // 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) }) @@ -398,10 +400,10 @@ describe("Acre", () => { const shares = await acre.balanceOf(staker1.address) const availableToRedeem = await acre.previewRedeem(shares) - // Expected amount w/o rounding: 85 * 220 / 110 = 170 - // Expected amount w/ support for rounding: 169999999999999999999 in + // 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 = 169999999999999999999n + const expectedTotalAssetsAvailableToRedeem = 132499999999999999999n expect(availableToRedeem).to.be.greaterThan(availableToRedeemBefore) expect(availableToRedeem).to.be.eq( From 275028608474e5e288c4be7c39b0e849a6f97697 Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Wed, 6 Dec 2023 13:33:07 +0100 Subject: [PATCH 58/60] Improve Acre contract docs comment Add short description what is the purpose of Acre system. --- core/contracts/Acre.sol | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/core/contracts/Acre.sol b/core/contracts/Acre.sol index 84f33722c..0e1cf640f 100644 --- a/core/contracts/Acre.sol +++ b/core/contracts/Acre.sol @@ -4,12 +4,16 @@ pragma solidity ^0.8.21; import "@openzeppelin/contracts/token/ERC20/extensions/ERC4626.sol"; /// @title Acre -/// @notice Implementation of the ERR-4626 tokenized vault standard. ERC-4626 is -/// a standard to optimize and unify the technical parameters of -/// yield-bearing vaults. This contract allows the minting and burning -/// of shares, represented as standard ERC20 token, in exchange for tBTC -/// tokens. -/// @dev ERC-4626 standard extends the ERC-20 token. +/// @notice This contract implements the ERC-4626 tokenized vault standard. By +/// staking tBTC, users acquire a liquid staking token called stBTC, +/// commonly referred to as "shares". The staked tBTC is securely +/// deposited into Acre's vaults, where it generates yield over time. +/// Users have the flexibility to redeem stBTC, enabling them to +/// withdraw their staked tBTC along with the accrued yield. +/// @dev ERC-4626 is a standard to optimize and unify the technical parameters +/// 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 { event StakeReferral(bytes32 indexed referral, uint256 assets); From ed73109db8c39dd10cdcc5ddedbcb5e5b086ee5c Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Wed, 6 Dec 2023 13:38:24 +0100 Subject: [PATCH 59/60] Remove unnecessary check expectation in test The equality check is enough given we hardcode the expected values. --- core/test/Acre.test.ts | 6 ------ 1 file changed, 6 deletions(-) diff --git a/core/test/Acre.test.ts b/core/test/Acre.test.ts index c858bc43e..ab9fcb0a2 100644 --- a/core/test/Acre.test.ts +++ b/core/test/Acre.test.ts @@ -338,9 +338,6 @@ describe("Acre", () => { // tBTC token precision. const expectedAssetsToRedeem = 112499999999999999999n - expect(availableAssetsToRedeem).to.be.greaterThan( - staker1AmountToStake, - ) expect(availableAssetsToRedeem).to.be.eq(expectedAssetsToRedeem) }) @@ -353,9 +350,6 @@ describe("Acre", () => { // tBTC token precision. const expectedAssetsToRedeem = 37499999999999999999n - expect(availableAssetsToRedeem).to.be.greaterThan( - staker2AmountToStake, - ) expect(availableAssetsToRedeem).to.be.eq(expectedAssetsToRedeem) }) }) From 68b81d0c540b54b1d69bce767a6bbe0d9641abdc Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Wed, 6 Dec 2023 13:49:52 +0100 Subject: [PATCH 60/60] Remove unnecessary check If the call reverts it will throw an exception in `beforeEach` hook for `await acre.connect(staker1).stake(...)`. --- core/test/Acre.test.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/core/test/Acre.test.ts b/core/test/Acre.test.ts index ab9fcb0a2..74e764ca0 100644 --- a/core/test/Acre.test.ts +++ b/core/test/Acre.test.ts @@ -125,10 +125,6 @@ describe("Acre", () => { .stake(amountToStake, staker1.address, emptyReferral) }) - it("should not revert", async () => { - await expect(tx).to.be.not.reverted - }) - it("should not emit the StakeReferral event", async () => { await expect(tx).to.not.emit(acre, "StakeReferral") })