Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Staking #52

Merged
merged 65 commits into from
Dec 7, 2023
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
65 commits
Select commit Hold shift + click to select a range
05030cf
Add OpenZeppelin contracts
r-czajkowski Nov 24, 2023
c80b77c
Update solhint rules
r-czajkowski Nov 24, 2023
3ebb88b
Initial implementation of the main Acre contract
r-czajkowski Nov 24, 2023
c81cfd5
Add initial implementation of the stake function.
r-czajkowski Nov 24, 2023
e754aca
Add `@thesis/solidity-contracts` dependency
r-czajkowski Nov 27, 2023
f36c65c
Add support for receive approval pattern to Acre
r-czajkowski Nov 27, 2023
14c14e1
Add test token contract
r-czajkowski Nov 27, 2023
02c2b3b
Add basic unit tests for staking
r-czajkowski Nov 27, 2023
a9b6c0c
Remove unnecessary `super` keyword
r-czajkowski Nov 28, 2023
a4f9f87
Update the name of the ERC4626 token
r-czajkowski Nov 28, 2023
028319f
Remove unnecessary undersocre for `_token`
r-czajkowski Nov 28, 2023
f1a126b
Update the `stake` function
r-czajkowski Nov 28, 2023
684d667
Update the Acre unit tests
r-czajkowski Nov 28, 2023
829c3ed
Rename param in `stake` function
r-czajkowski Nov 28, 2023
ffaaba6
Update `Acre` unit tests
r-czajkowski Nov 28, 2023
142c71f
Merge branch 'main' into token-vault
r-czajkowski Nov 28, 2023
b8a5b96
Fix typo
r-czajkowski Nov 30, 2023
6132a7b
Add more unit tests for staking flow
r-czajkowski Nov 30, 2023
3a3cb35
Add `Staked` event
r-czajkowski Nov 30, 2023
e319a62
Merge branch 'main' into token-vault
r-czajkowski Nov 30, 2023
6837e74
Revert "Add support for receive approval pattern to Acre"
r-czajkowski Nov 30, 2023
c960102
Remove unit tests for receive approval pattern
r-czajkowski Nov 30, 2023
c3c674d
Merge branch 'main' into token-vault
r-czajkowski Nov 30, 2023
58d2ddd
Rename variable in fixture
r-czajkowski Nov 30, 2023
05b2091
Update unit test for staking
r-czajkowski Nov 30, 2023
a77b1a4
Rename variable
r-czajkowski Nov 30, 2023
4c9eeec
Fix typo
r-czajkowski Nov 30, 2023
249fb11
Rename variable in tests
r-czajkowski Nov 30, 2023
20086c6
Update staking flow unit tests
r-czajkowski Nov 30, 2023
63aac1b
Use unnamed return in the `stake` fn
r-czajkowski Dec 4, 2023
e923f5e
Leave TODO in the `stake` fn
r-czajkowski Dec 4, 2023
5403ac3
Rename constructor parameter in Acre
r-czajkowski Dec 4, 2023
6511fef
Remove `@thesis/solidity-contracts` dependency
r-czajkowski Dec 4, 2023
84bdf8e
Add unit test heleprs
r-czajkowski Dec 4, 2023
ce16fba
Add more unit tests for staking
r-czajkowski Dec 4, 2023
c3a2bd8
Update solhint config
r-czajkowski Dec 4, 2023
b20259e
Add more stakers to the Acre test fixture
r-czajkowski Dec 4, 2023
300b2a9
Rename variable in Acre unit tests
r-czajkowski Dec 5, 2023
44b3f85
Update comment in Acre unit tests
r-czajkowski Dec 5, 2023
b84ee1b
Update staking flow unit tests
r-czajkowski Dec 5, 2023
3a312f7
Add documentation comments for `Acre` contract
r-czajkowski Dec 5, 2023
05fb49d
Merge branch 'main' into token-vault
r-czajkowski Dec 5, 2023
277dc1e
Rename `Staked` event
r-czajkowski Dec 5, 2023
eb91b4d
Update the `stake` fn
r-czajkowski Dec 5, 2023
fbb2085
Update `stake` fn docs
r-czajkowski Dec 5, 2023
48eeed3
Rename variable in Acre unit tests
r-czajkowski Dec 5, 2023
f7003b8
Rename variable in `acreFixture`
r-czajkowski Dec 5, 2023
076a29c
Update the `acreFixture` fn
r-czajkowski Dec 5, 2023
961580b
Update the test name
r-czajkowski Dec 5, 2023
9674f5e
Rename file
r-czajkowski Dec 5, 2023
9737ed8
Avoid infinite approvals in unit tests
r-czajkowski Dec 5, 2023
d33c552
Get rid of unnecessary `Staker` type in unit tests
r-czajkowski Dec 5, 2023
eef74bc
Hardcode expected values in unit tests
r-czajkowski Dec 5, 2023
8a243cf
Remove unnecessary check in unit test
r-czajkowski Dec 5, 2023
b812107
Rename test context name
r-czajkowski Dec 5, 2023
910a399
Update `without referral` test case
r-czajkowski Dec 5, 2023
a868dce
Improve test case
r-czajkowski Dec 5, 2023
567b52c
Make sure the tx is reverted with expected reason
r-czajkowski Dec 5, 2023
4889a03
Use dedicated variable in unit test
r-czajkowski Dec 5, 2023
9cac49c
Update staking unit tests
r-czajkowski Dec 5, 2023
88ee4d6
Complicate the math in test scenario
r-czajkowski Dec 5, 2023
2750286
Improve Acre contract docs comment
r-czajkowski Dec 6, 2023
ed73109
Remove unnecessary check expectation in test
r-czajkowski Dec 6, 2023
68b81d0
Remove unnecessary check
r-czajkowski Dec 6, 2023
e55dad2
Merge branch 'main' into token-vault
r-czajkowski Dec 6, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion core/.solhint.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
{
"extends": "thesis",
"plugins": [],
"rules": {}
"rules": {
"func-visibility": ["off", { "ignoreConstructors": true }]
}
nkuba marked this conversation as resolved.
Show resolved Hide resolved
}
48 changes: 44 additions & 4 deletions core/contracts/Acre.sol
Original file line number Diff line number Diff line change
@@ -1,9 +1,49 @@
// 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";
import "@thesis/solidity-contracts/contracts/token/IReceiveApproval.sol";

contract Acre {
// TODO: add your implementation
contract Acre is ERC4626, IReceiveApproval {
constructor(IERC20 _token) ERC4626(_token) ERC20("Staking BTC", "stBTC") {}
dimpar marked this conversation as resolved.
Show resolved Hide resolved

/// @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,
dimpar marked this conversation as resolved.
Show resolved Hide resolved
bytes calldata extraData
) external override {
require(_token == super.asset(), "Unrecognized token");
dimpar marked this conversation as resolved.
Show resolved Hide resolved
// TODO: Decide on the format of the `extradata` variable.
require(extraData.length == 32, "Corrupted stake data");

dimpar marked this conversation as resolved.
Show resolved Hide resolved
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.
nkuba marked this conversation as resolved.
Show resolved Hide resolved
/// @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
nkuba marked this conversation as resolved.
Show resolved Hide resolved
) external returns (uint256 shares) {
dimpar marked this conversation as resolved.
Show resolved Hide resolved
require(referrer != bytes32(0), "Referrer can not be empty");
nkuba marked this conversation as resolved.
Show resolved Hide resolved

return super.deposit(assets, receiver);
dimpar marked this conversation as resolved.
Show resolved Hide resolved
}
}
31 changes: 31 additions & 0 deletions core/contracts/test/TestToken.sol
nkuba marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -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 {
nkuba marked this conversation as resolved.
Show resolved Hide resolved
nkuba marked this conversation as resolved.
Show resolved Hide resolved
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;
}
}
4 changes: 4 additions & 0 deletions core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -55,5 +55,9 @@
"ts-node": "^10.9.1",
"typechain": "^8.3.2",
"typescript": "^5.2.2"
},
"dependencies": {
"@openzeppelin/contracts": "^5.0.0",
"@thesis/solidity-contracts": "github:thesis/solidity-contracts#c315b9d"
nkuba marked this conversation as resolved.
Show resolved Hide resolved
}
}
117 changes: 115 additions & 2 deletions core/test/Acre.test.ts
Original file line number Diff line number Diff line change
@@ -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 }
dimpar marked this conversation as resolved.
Show resolved Hide resolved
}

describe("Acre", () => {
describe("Add your tests", () => {
it("Should validate something", async () => {})
let acre: Acre
let token: TestToken
nkuba marked this conversation as resolved.
Show resolved Hide resolved
let tokenHolder: HardhatEthersSigner

beforeEach(async () => {
;({ acre, token, tokenHolder } = await loadFixture(acreFixture))
})

describe("Staking", () => {
nkuba marked this conversation as resolved.
Show resolved Hide resolved
const referrer = ethers.encodeBytes32String("referrer")

context("when staking via Acre contract", () => {
Copy link
Member

Choose a reason for hiding this comment

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

Can we add more test cases:

  • when amountToStake is zero,
  • when amountToStake is one,
  • when amountToStake is greater than the approved amount,
  • when receiver is address zero,
  • when staker approved amountToStake, staked it and tries to stake again, without another approval.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

when amountToStake is zero,

The ERC46626 from OZ seems to allow that. Shouldn't we add this validation to the deposit function as a general rule?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

when amountToStake is zero,

The ERC46626 from OZ seems to allow that. Shouldn't we add this validation to the deposit function as a general rule?

We should introduce a minimum deposit limit, we will add it in #59.

beforeEach(async () => {
// Infinite approval for staking contract.
await token
.connect(tokenHolder)
.approve(await acre.getAddress(), ethers.MaxUint256)
dimpar marked this conversation as resolved.
Show resolved Hide resolved
nkuba marked this conversation as resolved.
Show resolved Hide resolved
})

it("should stake tokens and receive shares", async () => {
const tokenHolderAddress = tokenHolder.address
dimpar marked this conversation as resolved.
Show resolved Hide resolved
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("")
dimpar marked this conversation as resolved.
Show resolved Hide resolved

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")
})
},
)
})
})
16 changes: 16 additions & 0 deletions core/yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -874,6 +874,16 @@
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"
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"
Expand Down Expand Up @@ -1107,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"
Expand Down
Loading