From c84eb3c91a1c3a8fb25f3c1d329506bfcc3b219b Mon Sep 17 00:00:00 2001 From: James Earle Date: Mon, 23 Dec 2024 15:55:35 -0500 Subject: [PATCH 1/7] stake and claim working using native token, just need unstake --- contracts/staking/ERC20/IStakingERC20.sol | 13 ++-- contracts/staking/ERC20/StakingERC20.sol | 29 +++++++-- contracts/staking/ERC721/StakingERC721.sol | 7 ++- contracts/staking/IStakingBase.sol | 40 ++++++++++++ contracts/staking/StakingBase.sol | 12 +++- test/helpers/staking/defaults.ts | 17 +++-- test/staking20.test.ts | 73 +++++++++++++++++++++- 7 files changed, 168 insertions(+), 23 deletions(-) diff --git a/contracts/staking/ERC20/IStakingERC20.sol b/contracts/staking/ERC20/IStakingERC20.sol index a261ae6..815c142 100644 --- a/contracts/staking/ERC20/IStakingERC20.sol +++ b/contracts/staking/ERC20/IStakingERC20.sol @@ -34,15 +34,20 @@ interface IStakingERC20 { */ error UnstakeMoreThanStake(); - function stakeWithLock(uint256 amount, uint256 lockDuration) external; + /** + * @notice Revert when the user is staking an amount inequal to the amount given + */ + error InsufficientValue(); + + function stakeWithLock(uint256 amount, uint256 lockDuration) external payable; - function stakeWithoutLock(uint256 amount) external; + function stakeWithoutLock(uint256 amount) external payable; function claim() external; - function unstake(uint256 amount, bool exit) external; + function unstake(uint256 amount, bool exit) external payable; - function unstakeLocked(uint256 amount, bool exit) external; + function unstakeLocked(uint256 amount, bool exit) external payable; function getRemainingLockTime() external view returns (uint256); diff --git a/contracts/staking/ERC20/StakingERC20.sol b/contracts/staking/ERC20/StakingERC20.sol index 9ffcda1..dcd0c72 100644 --- a/contracts/staking/ERC20/StakingERC20.sol +++ b/contracts/staking/ERC20/StakingERC20.sol @@ -27,6 +27,11 @@ contract StakingERC20 is StakingBase, IStakingERC20 { ) StakingBase(_config) {} + // We must be able to receive in the case that the + // `stakingToken` is the chain's native token + receive() external payable {} + fallback() external payable {} + /** * @notice Stake an amount of ERC20 with a lock period By locking, * a user cannot access their funds until the lock period is over, but they @@ -38,7 +43,7 @@ contract StakingERC20 is StakingBase, IStakingERC20 { * @param amount The amount to stake * @param lockDuration The duration of the lock period */ - function stakeWithLock(uint256 amount, uint256 lockDuration) external override { + function stakeWithLock(uint256 amount, uint256 lockDuration) payable external override { if (lockDuration < config.minimumLockTime) { revert LockTimeTooShort(); } @@ -52,7 +57,7 @@ contract StakingERC20 is StakingBase, IStakingERC20 { * * @param amount The amount to stake */ - function stakeWithoutLock(uint256 amount) external override { + function stakeWithoutLock(uint256 amount) payable external override { _stake(amount, 0); } @@ -72,7 +77,7 @@ contract StakingERC20 is StakingBase, IStakingERC20 { * * @param amount The amount to withdraw */ - function unstake(uint256 amount, bool exit) external override { + function unstake(uint256 amount, bool exit) external payable override { _unstake(amount, false, exit); } @@ -83,7 +88,7 @@ contract StakingERC20 is StakingBase, IStakingERC20 { * @param amount The amount to withdraw * @param exit Boolean if user wants to forfeit rewards */ - function unstakeLocked(uint256 amount, bool exit) public override { + function unstakeLocked(uint256 amount, bool exit) external payable override { _unstake(amount, true, exit); } @@ -110,14 +115,26 @@ contract StakingERC20 is StakingBase, IStakingERC20 { revert ZeroValue(); } + // If stakingToken is gas token, `msg.value` must equal `amount` + if (config.stakingToken == address(0)) { + if (msg.value != amount) { + revert InsufficientValue(); + } + } + Staker storage staker = stakers[msg.sender]; _coreStake(staker, amount, lockDuration); totalStaked += amount; - // Transfers user's funds to this contract - IERC20(config.stakingToken).safeTransferFrom(msg.sender, address(this), amount); + // Transfers user's funds to this contract + if (config.stakingToken != address(0)) { + IERC20(config.stakingToken).safeTransferFrom(msg.sender, address(this), amount); + } + + // IERC20(config.stakingToken).safeTransferFrom(msg.sender, address(this), amount); + // Mint the user's stake as a representative token IERC20MintableBurnable(config.stakeRepToken).mint(msg.sender, amount); diff --git a/contracts/staking/ERC721/StakingERC721.sol b/contracts/staking/ERC721/StakingERC721.sol index f8406eb..e95e3da 100644 --- a/contracts/staking/ERC721/StakingERC721.sol +++ b/contracts/staking/ERC721/StakingERC721.sol @@ -39,7 +39,12 @@ contract StakingERC721 is StakingBase, IStakingERC721 { Config memory config ) StakingBase(config) - {} + { + if (_config.stakingToken.code.length == 0) { + revert InitializedWithZero(); + } + + } /** * @notice Stake one or more ERC721 tokens with a lock period diff --git a/contracts/staking/IStakingBase.sol b/contracts/staking/IStakingBase.sol index 1af914b..2d3f5ad 100644 --- a/contracts/staking/IStakingBase.sol +++ b/contracts/staking/IStakingBase.sol @@ -77,6 +77,26 @@ interface IStakingBase { uint256 indexed rewards ); + /** + * @notice Emit when `reqwardsPerPeriod` is set + * @param owner The address of the contract owner + * @param rewardsPerPeriod The new rewards per period value + */ + event RewardsPerPeriodSet( + address indexed owner, + uint256 indexed rewardsPerPeriod + ); + + /** + * @notice Emit when the period length is set + * @param owner The address of the contract owner + * @param periodLength The new period length value + */ + event PeriodLengthSet( + address indexed owner, + uint256 indexed periodLength + ); + /** * @notice Emit when the multiplier is set * @param owner The address of the contract owner @@ -97,6 +117,26 @@ interface IStakingBase { uint256 indexed minimumLockTime ); + /** + * @notice Emit when the minimum rewards multiplier is set + * @param owner The address of the contract owner + * @param minimumRewardsMultiplier The new minimum rewards multiplier + */ + event MinimumRewardsMultiplierSet( + address indexed owner, + uint256 indexed minimumRewardsMultiplier + ); + + /** + * @notice Emit when the maximum rewards multiplier is set + * @param owner The address of the contract owner + * @param maximumRewardsMultiplier The new maximum rewards multiplier + */ + event MaximumRewardsMultiplierSet( + address indexed owner, + uint256 indexed maximumRewardsMultiplier + ); + /** * @notice Emit incoming stake is not valid */ diff --git a/contracts/staking/StakingBase.sol b/contracts/staking/StakingBase.sol index 725dd3f..70fa794 100644 --- a/contracts/staking/StakingBase.sol +++ b/contracts/staking/StakingBase.sol @@ -35,8 +35,6 @@ contract StakingBase is Ownable, ReentrancyGuard, IStakingBase { Config memory _config ) Ownable(_config.contractOwner) { if ( - _config.stakingToken.code.length == 0 || - address(_config.rewardsToken).code.length == 0 || _config.rewardsPerPeriod == 0 || _config.periodLength == 0 ) revert InitializedWithZero(); @@ -68,6 +66,7 @@ contract StakingBase is Ownable, ReentrancyGuard, IStakingBase { */ function setRewardsPerPeriod(uint256 _rewardsPerPeriod) public override onlyOwner { config.rewardsPerPeriod = _rewardsPerPeriod; + emit RewardsPerPeriodSet(owner(), _rewardsPerPeriod); } /** @@ -78,6 +77,7 @@ contract StakingBase is Ownable, ReentrancyGuard, IStakingBase { */ function setPeriodLength(uint256 _periodLength) public override onlyOwner { config.periodLength = _periodLength; + emit PeriodLengthSet(owner(), _periodLength); } /** @@ -99,6 +99,7 @@ contract StakingBase is Ownable, ReentrancyGuard, IStakingBase { */ function setMinimumRewardsMultiplier(uint256 _minimumRewardsMultiplier) public override onlyOwner { config.minimumRewardsMultiplier = _minimumRewardsMultiplier; + emit MinimumRewardsMultiplierSet(owner(), _minimumRewardsMultiplier); } /** @@ -109,6 +110,7 @@ contract StakingBase is Ownable, ReentrancyGuard, IStakingBase { */ function setMaximumRewardsMultiplier(uint256 _maximumRewardsMultiplier) public override onlyOwner { config.maximumRewardsMultiplier = _maximumRewardsMultiplier; + emit MaximumRewardsMultiplierSet(owner(), _maximumRewardsMultiplier); } /** @@ -281,7 +283,11 @@ contract StakingBase is Ownable, ReentrancyGuard, IStakingBase { if (rewards == 0) revert ZeroRewards(); - config.rewardsToken.safeTransfer(msg.sender, rewards); + if (address(config.rewardsToken) == address(0)) { + payable(msg.sender).transfer(rewards); + } else { + config.rewardsToken.safeTransfer(msg.sender, rewards); + } emit Claimed(msg.sender, rewards); } diff --git a/test/helpers/staking/defaults.ts b/test/helpers/staking/defaults.ts index 8e5b2ec..7c34d47 100644 --- a/test/helpers/staking/defaults.ts +++ b/test/helpers/staking/defaults.ts @@ -7,6 +7,8 @@ import { BaseConfig, } from "./types"; +import * as hre from "hardhat"; + import { DEFAULT_PERIOD_LENGTH_ERC721, PRECISION_DIVISOR, @@ -21,15 +23,15 @@ import { import { SignerWithAddress } from "@nomicfoundation/hardhat-ethers/signers"; export const createDefaultStakingConfig = async ( - rewardsERC20 : MockERC20, contractOwner : SignerWithAddress, + rewardsERC20 ?: MockERC20, erc721 ?: MockERC721, stakeERC20 ?: MockERC20, stakeRepERC20 ?: ZeroVotingERC20, stakeRepERC721 ?: ZeroVotingERC721, ) => { const config : Partial = { - rewardsToken: await rewardsERC20.getAddress(), + rewardsToken: rewardsERC20 ? await rewardsERC20.getAddress() : hre.ethers.ZeroAddress, minimumLockTime: DEFAULT_MINIMUM_LOCK, divisor: PRECISION_DIVISOR, lockedDivisor: LOCKED_PRECISION_DIVISOR, @@ -45,14 +47,17 @@ export const createDefaultStakingConfig = async ( config.stakeRepToken = await (stakeRepERC721 as ZeroVotingERC721).getAddress(); return config as BaseConfig; - } else if (stakeERC20) { - config.stakingToken = await stakeERC20.getAddress(); + } else { + if (stakeERC20) { + config.stakingToken = await stakeERC20.getAddress(); + } else { + config.stakingToken = hre.ethers.ZeroAddress; + } + config.rewardsPerPeriod = DEFAULT_REWARDS_PER_PERIOD_ERC20; config.periodLength = DEFAULT_PERIOD_LENGTH_ERC20; config.stakeRepToken = await (stakeRepERC20 as ZeroVotingERC20).getAddress(); return config as BaseConfig; } - - throw new Error("No valid staking token provided"); }; diff --git a/test/staking20.test.ts b/test/staking20.test.ts index c8b1f0d..1e355ff 100644 --- a/test/staking20.test.ts +++ b/test/staking20.test.ts @@ -30,7 +30,6 @@ import { DAY_IN_SECONDS, calcLockedRewards, calcTotalUnlockedRewards, - calcTotalLockedRewards, calcStakeRewards, DEFAULT_MINIMUM_LOCK, } from "./helpers/staking"; @@ -77,8 +76,8 @@ describe("StakingERC20", () => { stakeRepToken = await stakeRepFactory.deploy("VotingToken", "VTKN", owner); config = await createDefaultStakingConfig( - rewardsToken, owner, + rewardsToken, undefined, stakeToken, stakeRepToken, @@ -127,6 +126,74 @@ describe("StakingERC20", () => { }); describe("#getContractRewardsBalance", () => { + it.only("native token flow", async () => { + // When neither erc20 or erc721 specified it will assume erc20 with native token + // same with rewards + config = await createDefaultStakingConfig( + owner, + undefined, + undefined, + undefined, + stakeRepToken, + ); + + const localStakingFactory = await hre.ethers.getContractFactory("StakingERC20"); + const localContract = await localStakingFactory.deploy(config) as StakingERC20; + + await stakeRepToken.connect(owner).grantRole(await stakeRepToken.MINTER_ROLE(), await localContract.getAddress()); + await stakeRepToken.connect(owner).grantRole(await stakeRepToken.BURNER_ROLE(), await localContract.getAddress()); + + // Provide rewards funding in native token + await owner.sendTransaction({ + to: await localContract.getAddress(), + value: hre.ethers.parseEther("9999"), + }) + + const userBalBefore = await hre.ethers.provider.getBalance(stakerA.address); + const contrBalBefore = await hre.ethers.provider.getBalance(await localContract.getAddress()); + + + + const stakeTx = await localContract.connect(stakerA).stakeWithoutLock( + DEFAULT_STAKED_AMOUNT, + { + value: DEFAULT_STAKED_AMOUNT, + } + ); + const stakedAt = BigInt(await time.latest()); + + const receipt = await stakeTx.wait(); + + const userBalAfter = await hre.ethers.provider.getBalance(stakerA.address); + const contrBalAfter = await hre.ethers.provider.getBalance(await localContract.getAddress()); + + expect(userBalAfter).to.eq(userBalBefore - DEFAULT_STAKED_AMOUNT - (receipt!.gasUsed * receipt!.gasPrice)); + expect(contrBalAfter).to.eq(contrBalBefore + DEFAULT_STAKED_AMOUNT); + + await time.increase(DEFAULT_LOCK / 2n); + + const rewardsBefore = await hre.ethers.provider.getBalance(stakerA.address); + const contrRewardsBalBefore = await hre.ethers.provider.getBalance(await localContract.getAddress()); + + + const claimTx = await localContract.connect(stakerA).claim(); + const claimedAt = BigInt(await time.latest()); + + const claimReceipt = await claimTx.wait(); + + const rewardsAfter = await hre.ethers.provider.getBalance(stakerA.address); + const contrRewardsBalAfter = await hre.ethers.provider.getBalance(await localContract.getAddress()); + + const stakeRewards = calcStakeRewards( + DEFAULT_STAKED_AMOUNT, + claimedAt - stakedAt, + false, + config + ); + + expect(rewardsAfter).to.eq(rewardsBefore + stakeRewards - (claimReceipt!.gasUsed * claimReceipt!.gasPrice)); + expect(contrRewardsBalAfter).to.eq(contrRewardsBalBefore - stakeRewards); + }); it("Allows a user to see the total rewards remaining in a pool", async () => { const rewardsInPool = await contract.getContractRewardsBalance(); const poolBalance = await rewardsToken.balanceOf(await contract.getAddress()); @@ -1287,8 +1354,8 @@ describe("StakingERC20", () => { await reset(); const localConfig = await createDefaultStakingConfig( - rewardsToken, owner, + rewardsToken, undefined, stakeToken, stakeRepToken From 2fe97d68be3b3749a6d32f2253b752892829b13a Mon Sep 17 00:00:00 2001 From: James Earle Date: Fri, 27 Dec 2024 13:22:53 -0500 Subject: [PATCH 2/7] full flow for using native token, some updates to the helper code in testing, all erc20 passing --- contracts/staking/ERC20/StakingERC20.sol | 27 ++- contracts/staking/ERC721/StakingERC721.sol | 6 +- contracts/staking/IStakingBase.sol | 4 +- contracts/staking/StakingBase.sol | 24 +- test/helpers/staking/defaults.ts | 78 ++++++ test/staking20.test.ts | 270 ++++++++++++++------- 6 files changed, 292 insertions(+), 117 deletions(-) diff --git a/contracts/staking/ERC20/StakingERC20.sol b/contracts/staking/ERC20/StakingERC20.sol index dcd0c72..9317d3c 100644 --- a/contracts/staking/ERC20/StakingERC20.sol +++ b/contracts/staking/ERC20/StakingERC20.sol @@ -7,7 +7,6 @@ import { IStakingERC20 } from "./IStakingERC20.sol"; import { StakingBase } from "../StakingBase.sol"; import { IERC20MintableBurnable } from "../../types/IERC20MintableBurnable.sol"; - /** * @title StakingERC20 * @notice A staking contract for ERC20 tokens @@ -131,9 +130,7 @@ contract StakingERC20 is StakingBase, IStakingERC20 { // Transfers user's funds to this contract if (config.stakingToken != address(0)) { IERC20(config.stakingToken).safeTransferFrom(msg.sender, address(this), amount); - } - - // IERC20(config.stakingToken).safeTransferFrom(msg.sender, address(this), amount); + } // the `else` case is handled by including `recieve` above to accept `msg.value` // Mint the user's stake as a representative token IERC20MintableBurnable(config.stakeRepToken).mint(msg.sender, amount); @@ -167,11 +164,12 @@ contract StakingERC20 is StakingBase, IStakingERC20 { staker.lastTimestampLocked = 0; staker.unlockedTimestamp = 0; } else if (_getRemainingLockTime(staker) > 0) { - // if still locked and not exiting, revert + // if still locked revert revert TimeLockNotPassed(); } else { - // If claims happen after lock period has passed, the lastTimestamp is more accurate + // If claims happen after lock period has passed, the lastTimestampLocked is more accurate // but if they don't happen, then lastTimestampLocked may still be the original stake timestamp + // and we should use `unlockedTimestamp` instead. // so we have to calculate which is more recent before calculating rewards uint256 mostRecentTimestamp = _mostRecentTimestamp(staker); @@ -244,14 +242,17 @@ contract StakingERC20 is StakingBase, IStakingERC20 { if (rewards > 0) { // Transfer the user's rewards // Will fail if the contract does not have funding for this - config.rewardsToken.safeTransfer(msg.sender, rewards); + // If rewards address is `0x0` we use the chain's native token + _transferToken(config.rewardsToken, rewards); + emit Claimed(msg.sender, rewards); } totalStaked -= amount; // Return the user's initial stake - IERC20(config.stakingToken).safeTransfer(msg.sender, amount); + _transferToken(config.stakingToken, amount); + // Burn the user's stake representative token IERC20MintableBurnable(config.stakeRepToken).burn(msg.sender, amount); @@ -265,9 +266,15 @@ contract StakingERC20 is StakingBase, IStakingERC20 { * return the balance of the rewards token minus the total staked amount */ function _getContractRewardsBalance() internal view override returns (uint256) { - uint256 balance = super._getContractRewardsBalance(); + uint256 balance; + + if (address(config.rewardsToken) == address(0)) { + balance = address(this).balance; + } else { + balance = IERC20(config.rewardsToken).balanceOf(address(this)); + } - if (address(config.rewardsToken) == config.stakingToken) { + if (config.rewardsToken == config.stakingToken) { return balance - totalStaked; } diff --git a/contracts/staking/ERC721/StakingERC721.sol b/contracts/staking/ERC721/StakingERC721.sol index e95e3da..ddd3f46 100644 --- a/contracts/staking/ERC721/StakingERC721.sol +++ b/contracts/staking/ERC721/StakingERC721.sol @@ -40,7 +40,7 @@ contract StakingERC721 is StakingBase, IStakingERC721 { ) StakingBase(config) { - if (_config.stakingToken.code.length == 0) { + if (config.stakingToken.code.length == 0) { revert InitializedWithZero(); } @@ -306,8 +306,8 @@ contract StakingERC721 is StakingBase, IStakingERC721 { if (!exit) { // Transfer the user's rewards - // Will fail if the contract does not have funding - config.rewardsToken.safeTransfer(msg.sender, rewards); + _transferToken(config.rewardsToken, rewards); + emit Claimed(msg.sender, rewards); } diff --git a/contracts/staking/IStakingBase.sol b/contracts/staking/IStakingBase.sol index 2d3f5ad..7848d44 100644 --- a/contracts/staking/IStakingBase.sol +++ b/contracts/staking/IStakingBase.sol @@ -47,7 +47,7 @@ interface IStakingBase { struct Config { address stakingToken; address contractOwner; - IERC20 rewardsToken; + address rewardsToken; address stakeRepToken; uint256 rewardsPerPeriod; uint256 periodLength; @@ -201,7 +201,7 @@ interface IStakingBase { function getStakingToken() external view returns(address); - function getRewardsToken() external view returns(IERC20); + function getRewardsToken() external view returns(address); function getStakeRepToken() external view returns (address); diff --git a/contracts/staking/StakingBase.sol b/contracts/staking/StakingBase.sol index 70fa794..032eb0f 100644 --- a/contracts/staking/StakingBase.sol +++ b/contracts/staking/StakingBase.sol @@ -53,7 +53,7 @@ contract StakingBase is Ownable, ReentrancyGuard, IStakingBase { // Do not send empty transfer if (balance == 0) revert InsufficientContractBalance(); - config.rewardsToken.safeTransfer(owner(), balance); + _transferToken(config.rewardsToken, balance); emit LeftoverRewardsWithdrawn(owner(), balance); } @@ -156,7 +156,7 @@ contract StakingBase is Ownable, ReentrancyGuard, IStakingBase { /** * @notice Get the rewards token address */ - function getRewardsToken() public view override returns (IERC20) { + function getRewardsToken() public view override returns (address) { return config.rewardsToken; } @@ -283,11 +283,7 @@ contract StakingBase is Ownable, ReentrancyGuard, IStakingBase { if (rewards == 0) revert ZeroRewards(); - if (address(config.rewardsToken) == address(0)) { - payable(msg.sender).transfer(rewards); - } else { - config.rewardsToken.safeTransfer(msg.sender, rewards); - } + _transferToken(config.rewardsToken, rewards); emit Claimed(msg.sender, rewards); } @@ -380,6 +376,18 @@ contract StakingBase is Ownable, ReentrancyGuard, IStakingBase { * @dev Get the rewards balance of this contract */ function _getContractRewardsBalance() internal view virtual returns (uint256) { - return config.rewardsToken.balanceOf(address(this)); + return IERC20(config.rewardsToken).balanceOf(address(this)); + } + + /** + * @dev Transfer funds to a recipient after deciding whether to use + * native or ERC20 tokens + */ + function _transferToken(address token, uint256 amount) internal { + if (token == address(0)) { + payable(msg.sender).transfer(amount); + } else { + IERC20(token).safeTransfer(msg.sender, amount); + } } } diff --git a/test/helpers/staking/defaults.ts b/test/helpers/staking/defaults.ts index 7c34d47..16e2ceb 100644 --- a/test/helpers/staking/defaults.ts +++ b/test/helpers/staking/defaults.ts @@ -1,6 +1,7 @@ import { MockERC721, MockERC20, ZeroVotingERC20, ZeroVotingERC721, + StakingERC20, } from "../../../typechain"; import { @@ -19,6 +20,7 @@ import { DEFAULT_MINIMUM_LOCK, DEFAULT_MINIMUM_RM, DEFAULT_MAXIMUM_RM, + INIT_BALANCE, } from "../constants"; import { SignerWithAddress } from "@nomicfoundation/hardhat-ethers/signers"; @@ -61,3 +63,79 @@ export const createDefaultStakingConfig = async ( return config as BaseConfig; } }; + +export const getDefaultERC721Setup = async () => { + // TODO impl +}; + +export const getDefaultERC20Setup = async ( + owner : SignerWithAddress, + rewardsToken : MockERC20, + stakeToken : MockERC20, + stakeRepToken : ZeroVotingERC20, +): Promise<[StakingERC20, BaseConfig]> => { + const config = await createDefaultStakingConfig( + owner, + rewardsToken, + undefined, + stakeToken, + stakeRepToken + ); + + const stakingFactory = await hre.ethers.getContractFactory("StakingERC20"); + + const contract = await stakingFactory.deploy(config) as StakingERC20; + + await stakeRepToken.connect(owner).grantRole(await stakeRepToken.MINTER_ROLE(), await contract.getAddress()); + await stakeRepToken.connect(owner).grantRole(await stakeRepToken.BURNER_ROLE(), await contract.getAddress()); + + return [contract, config]; +} + +// todo `getmodified erc20 setup` + +export const getNativeSetup = async ( + owner : SignerWithAddress, + stakeRepToken : ZeroVotingERC20, +) => { + const config = await createDefaultStakingConfig( + owner, + undefined, + undefined, + undefined, + stakeRepToken, + ); + + const localStakingFactory = await hre.ethers.getContractFactory("StakingERC20"); + const localContract = await localStakingFactory.deploy(config) as StakingERC20; + + await stakeRepToken.connect(owner).grantRole(await stakeRepToken.MINTER_ROLE(), await localContract.getAddress()); + await stakeRepToken.connect(owner).grantRole(await stakeRepToken.BURNER_ROLE(), await localContract.getAddress()); + + // Provide rewards funding in native token + await owner.sendTransaction({ + to: await localContract.getAddress(), + value: hre.ethers.parseEther("9999"), + }); + + return localContract; +} + +// TODO modify with types for possibly using erc721 +export const fundAndApprove = async ( + owner : SignerWithAddress, + addresses : Array, + stakeToken : MockERC20, + contractAddress : string, + amount ?: bigint, +) => { + for (let i = 0; i < addresses.length; i++) { + await stakeToken.connect(owner).transfer( + addresses[i].address, amount ?? INIT_BALANCE + ); + + await stakeToken.connect(addresses[i]).approve( + contractAddress, amount ?? INIT_BALANCE + ); + } +} diff --git a/test/staking20.test.ts b/test/staking20.test.ts index 1e355ff..198bcd9 100644 --- a/test/staking20.test.ts +++ b/test/staking20.test.ts @@ -1,7 +1,7 @@ import * as hre from "hardhat"; import { expect } from "chai"; import { SignerWithAddress } from "@nomicfoundation/hardhat-ethers/signers"; -import { time } from "@nomicfoundation/hardhat-network-helpers"; +import { setBalance, time } from "@nomicfoundation/hardhat-network-helpers"; import { MockERC20, StakingERC20, ZeroVotingERC20, @@ -32,6 +32,9 @@ import { calcTotalUnlockedRewards, calcStakeRewards, DEFAULT_MINIMUM_LOCK, + getNativeSetup, + getDefaultERC20Setup, + fundAndApprove, } from "./helpers/staking"; @@ -75,125 +78,63 @@ describe("StakingERC20", () => { rewardsToken = await mockERC20Factory.deploy("WilderWorld", "WW"); stakeRepToken = await stakeRepFactory.deploy("VotingToken", "VTKN", owner); - config = await createDefaultStakingConfig( + const ownerBal = await hre.ethers.provider.getBalance(owner.address); + + // Give the owner ample funds for transfers in native token case + await setBalance(owner.address, INIT_BALANCE * 10n); + + [contract, config] = await getDefaultERC20Setup( owner, rewardsToken, - undefined, stakeToken, stakeRepToken, - ); - - contract = await stakingFactory.deploy(config) as StakingERC20; - - await stakeRepToken.connect(owner).grantRole(await stakeRepToken.MINTER_ROLE(), await contract.getAddress()); - await stakeRepToken.connect(owner).grantRole(await stakeRepToken.BURNER_ROLE(), await contract.getAddress()); + ) // Give each user funds to stake - await stakeToken.connect(owner).transfer( - stakerA.address, - INIT_BALANCE - ); - - await stakeToken.connect(owner).transfer( - stakerB.address, - INIT_BALANCE - ); - - await stakeToken.connect(owner).transfer( - stakerC.address, - INIT_BALANCE - ); - - await stakeToken.connect(owner).transfer( - stakerD.address, - INIT_BALANCE - ); - - await stakeToken.connect(owner).transfer( - stakerF.address, - INIT_BALANCE + await fundAndApprove( + owner, + [ + stakerA, + stakerB, + stakerC, + stakerD, + stakerF, + ], + stakeToken, + await contract.getAddress(), ); - - // Approve staking contract to spend staker funds - await stakeToken.connect(stakerA).approve(await contract.getAddress(), hre.ethers.MaxUint256); - await stakeToken.connect(stakerB).approve(await contract.getAddress(), hre.ethers.MaxUint256); - await stakeToken.connect(stakerC).approve(await contract.getAddress(), hre.ethers.MaxUint256); - await stakeToken.connect(stakerD).approve(await contract.getAddress(), hre.ethers.MaxUint256); - await stakeToken.connect(stakerF).approve(await contract.getAddress(), hre.ethers.MaxUint256); }; await reset(); }); describe("#getContractRewardsBalance", () => { - it.only("native token flow", async () => { - // When neither erc20 or erc721 specified it will assume erc20 with native token - // same with rewards - config = await createDefaultStakingConfig( - owner, - undefined, - undefined, - undefined, - stakeRepToken, - ); - - const localStakingFactory = await hre.ethers.getContractFactory("StakingERC20"); - const localContract = await localStakingFactory.deploy(config) as StakingERC20; - - await stakeRepToken.connect(owner).grantRole(await stakeRepToken.MINTER_ROLE(), await localContract.getAddress()); - await stakeRepToken.connect(owner).grantRole(await stakeRepToken.BURNER_ROLE(), await localContract.getAddress()); + it("it accounts for balance when rewards and stake are same token", async () => { + const localContract = await getNativeSetup(owner, stakeRepToken); // Provide rewards funding in native token await owner.sendTransaction({ to: await localContract.getAddress(), value: hre.ethers.parseEther("9999"), - }) + }); - const userBalBefore = await hre.ethers.provider.getBalance(stakerA.address); - const contrBalBefore = await hre.ethers.provider.getBalance(await localContract.getAddress()); - - - - const stakeTx = await localContract.connect(stakerA).stakeWithoutLock( - DEFAULT_STAKED_AMOUNT, + const stakeAmount = DEFAULT_STAKED_AMOUNT + await localContract.connect(stakerA).stakeWithoutLock( + stakeAmount, { - value: DEFAULT_STAKED_AMOUNT, + value: stakeAmount, } ); - const stakedAt = BigInt(await time.latest()); - - const receipt = await stakeTx.wait(); - const userBalAfter = await hre.ethers.provider.getBalance(stakerA.address); - const contrBalAfter = await hre.ethers.provider.getBalance(await localContract.getAddress()); + const totalStaked = await localContract.totalStaked(); + expect(totalStaked).to.eq(stakeAmount); - expect(userBalAfter).to.eq(userBalBefore - DEFAULT_STAKED_AMOUNT - (receipt!.gasUsed * receipt!.gasPrice)); - expect(contrBalAfter).to.eq(contrBalBefore + DEFAULT_STAKED_AMOUNT); - - await time.increase(DEFAULT_LOCK / 2n); - - const rewardsBefore = await hre.ethers.provider.getBalance(stakerA.address); - const contrRewardsBalBefore = await hre.ethers.provider.getBalance(await localContract.getAddress()); + const contrRewardsBal = await hre.ethers.provider.getBalance(await localContract.getAddress()); + const contrRewardsBalFromContract = await localContract.getContractRewardsBalance(); - - const claimTx = await localContract.connect(stakerA).claim(); - const claimedAt = BigInt(await time.latest()); - - const claimReceipt = await claimTx.wait(); - - const rewardsAfter = await hre.ethers.provider.getBalance(stakerA.address); - const contrRewardsBalAfter = await hre.ethers.provider.getBalance(await localContract.getAddress()); - - const stakeRewards = calcStakeRewards( - DEFAULT_STAKED_AMOUNT, - claimedAt - stakedAt, - false, - config - ); - - expect(rewardsAfter).to.eq(rewardsBefore + stakeRewards - (claimReceipt!.gasUsed * claimReceipt!.gasPrice)); - expect(contrRewardsBalAfter).to.eq(contrRewardsBalBefore - stakeRewards); + expect(contrRewardsBalFromContract).to.eq(contrRewardsBal - totalStaked); }); + it("Allows a user to see the total rewards remaining in a pool", async () => { const rewardsInPool = await contract.getContractRewardsBalance(); const poolBalance = await rewardsToken.balanceOf(await contract.getAddress()); @@ -458,6 +399,10 @@ describe("StakingERC20", () => { expect(stakerData.owedRewards).to.eq(0n); }); + it("Fails when using native token and `amount` does not equal `msg.value`", async () => { + + }); + it("Fails when the staker locks for less than the minimum lock time", async () => { await expect( contract.connect(stakerA).stakeWithLock(DEFAULT_STAKED_AMOUNT, DEFAULT_MINIMUM_LOCK - 1n) @@ -1349,6 +1294,143 @@ describe("StakingERC20", () => { }); describe("Different configs", async () => { + it("Stakes, claims, partially and fully unstakes when stake and reward token are chain token", async () => { + // When neither erc20 or erc721 specified it will assume erc20 with native token + // same with rewards + config = await createDefaultStakingConfig( + owner, + undefined, + undefined, + undefined, + stakeRepToken, + ); + + const localStakingFactory = await hre.ethers.getContractFactory("StakingERC20"); + const localContract = await localStakingFactory.deploy(config) as StakingERC20; + + await stakeRepToken.connect(owner).grantRole(await stakeRepToken.MINTER_ROLE(), await localContract.getAddress()); + await stakeRepToken.connect(owner).grantRole(await stakeRepToken.BURNER_ROLE(), await localContract.getAddress()); + + // Provide rewards funding in native token + await owner.sendTransaction({ + to: await localContract.getAddress(), + value: hre.ethers.parseEther("9999"), + }) + + const userBalBefore = await hre.ethers.provider.getBalance(stakerA.address); + const contrBalBefore = await hre.ethers.provider.getBalance(await localContract.getAddress()); + + // #stake + const stakeAmount = DEFAULT_STAKED_AMOUNT + const stakeTx = await localContract.connect(stakerA).stakeWithoutLock( + stakeAmount, + { + value: stakeAmount, + } + ); + const stakedAt = BigInt(await time.latest()); + + const receipt = await stakeTx.wait(); + + const userBalAfter = await hre.ethers.provider.getBalance(stakerA.address); + const contrBalAfter = await hre.ethers.provider.getBalance(await localContract.getAddress()); + + expect(userBalAfter).to.eq(userBalBefore - stakeAmount - (receipt!.gasUsed * receipt!.gasPrice)); + expect(contrBalAfter).to.eq(contrBalBefore + stakeAmount); + + await time.increase(DEFAULT_LOCK / 2n); + + const rewardsBefore = await hre.ethers.provider.getBalance(stakerA.address); + const contrRewardsBalBefore = await hre.ethers.provider.getBalance(await localContract.getAddress()); + const contrRewardsBalBeforeFromContract = await localContract.getContractRewardsBalance(); + + // If `stakingToken` and `rewardsToken` are the same, we subtract the difference when checking + // the balance. Verify this here. + expect(contrRewardsBalBeforeFromContract).to.eq(contrRewardsBalBefore - await localContract.totalStaked()); + + // #claim + const claimTx = await localContract.connect(stakerA).claim(); + const claimedAt = BigInt(await time.latest()); + + const claimReceipt = await claimTx.wait(); + + const rewardsAfter = await hre.ethers.provider.getBalance(stakerA.address); + const contrRewardsBalAfter = await hre.ethers.provider.getBalance(await localContract.getAddress()); + const contrRewardsBalAfterFromContract = await localContract.getContractRewardsBalance(); + + const totalStaked = await localContract.totalStaked(); + expect(contrRewardsBalAfterFromContract).to.eq(contrRewardsBalAfter - totalStaked); + + const stakeRewards = calcStakeRewards( + DEFAULT_STAKED_AMOUNT, + claimedAt - stakedAt, + false, + config + ); + + expect(rewardsAfter).to.eq( + rewardsBefore + stakeRewards - (claimReceipt!.gasUsed * claimReceipt!.gasPrice) + ); + expect(contrRewardsBalAfter).to.eq(contrRewardsBalBefore - stakeRewards); + + await time.increase(DEFAULT_LOCK / 2n); + + // Partial unstake + const stakerData = await localContract.stakers(stakerA.address); + const unstakeAmount = stakerData.amountStaked / 2n; + + const rewardsBeforeUnstake = await hre.ethers.provider.getBalance(stakerA.address); + + // partial #unstake + const partialUnstakeTx = await localContract.connect(stakerA).unstake(unstakeAmount, false); + const unstakedAt = BigInt(await time.latest()); + + const partialUnstakeReceipt = await partialUnstakeTx.wait(); + + const stakerDataAfter = await localContract.stakers(stakerA.address); + + const rewardsAfterUnstake = await hre.ethers.provider.getBalance(stakerA.address); + + const stakeRewardsUnstake = calcStakeRewards( + unstakeAmount, + unstakedAt - claimedAt, + false, + config + ); + + expect(stakerDataAfter.amountStaked).to.eq(stakeAmount - unstakeAmount); + expect(rewardsAfterUnstake).to.eq( + rewardsBeforeUnstake + unstakeAmount + stakeRewardsUnstake + - (partialUnstakeReceipt!.gasPrice * partialUnstakeReceipt!.gasUsed) + ); + + // # full unstake + const fullUnstakeTx = await localContract.connect(stakerA).unstake(stakerDataAfter.amountStaked, false); + const fullUnstakedAt = BigInt(await time.latest()); + + const stakerDataAfterFull = await localContract.stakers(stakerA.address); + + const stakeRewardsFullnstake = calcStakeRewards( + stakerDataAfter.amountStaked, + fullUnstakedAt - unstakedAt, + false, + config + ); + + const rewardsAfterFullUnstake = await hre.ethers.provider.getBalance(stakerA.address); + + const fullUnstakeReceipt = await fullUnstakeTx.wait(); + + expect(await localContract.totalStaked()).to.eq(0n); + expect(stakerDataAfterFull.amountStaked).to.eq(0n); + expect(stakerDataAfterFull.owedRewards).to.eq(0n); + + expect(rewardsAfterFullUnstake).to.eq( + rewardsAfterUnstake + stakerDataAfter.amountStaked + stakeRewardsFullnstake + - (fullUnstakeReceipt!.gasPrice * fullUnstakeReceipt!.gasUsed) + ); + }); + it("Stakes, claims, and unstakes correctly with an entirely different config", async () => { // Even though we are manipulating the config here we still reset to be sure all token balances are what we expect await reset(); From d2474e1c2e3e30fd54ae90015fa34b25819464db Mon Sep 17 00:00:00 2001 From: James Earle Date: Fri, 27 Dec 2024 15:23:30 -0500 Subject: [PATCH 3/7] update 721 and associated tests --- contracts/staking/ERC20/StakingERC20.sol | 5 --- contracts/staking/StakingBase.sol | 5 +++ test/helpers/staking/defaults.ts | 39 ++++++++++++++---- test/staking20.test.ts | 31 +++----------- test/staking721.test.ts | 52 ++++++++++++------------ 5 files changed, 67 insertions(+), 65 deletions(-) diff --git a/contracts/staking/ERC20/StakingERC20.sol b/contracts/staking/ERC20/StakingERC20.sol index 9317d3c..101e221 100644 --- a/contracts/staking/ERC20/StakingERC20.sol +++ b/contracts/staking/ERC20/StakingERC20.sol @@ -26,11 +26,6 @@ contract StakingERC20 is StakingBase, IStakingERC20 { ) StakingBase(_config) {} - // We must be able to receive in the case that the - // `stakingToken` is the chain's native token - receive() external payable {} - fallback() external payable {} - /** * @notice Stake an amount of ERC20 with a lock period By locking, * a user cannot access their funds until the lock period is over, but they diff --git a/contracts/staking/StakingBase.sol b/contracts/staking/StakingBase.sol index 032eb0f..e659970 100644 --- a/contracts/staking/StakingBase.sol +++ b/contracts/staking/StakingBase.sol @@ -41,6 +41,11 @@ contract StakingBase is Ownable, ReentrancyGuard, IStakingBase { config = _config; } + // We must be able to receive in the case that the + // `stakingToken` is the chain's native token + receive() external payable {} + fallback() external payable {} + /** * @notice Emergency function for the contract owner to withdraw leftover rewards * in case of an abandoned contract. diff --git a/test/helpers/staking/defaults.ts b/test/helpers/staking/defaults.ts index 16e2ceb..550dc81 100644 --- a/test/helpers/staking/defaults.ts +++ b/test/helpers/staking/defaults.ts @@ -2,6 +2,7 @@ import { MockERC721, MockERC20, ZeroVotingERC20, ZeroVotingERC721, StakingERC20, + StakingERC721, } from "../../../typechain"; import { @@ -64,10 +65,6 @@ export const createDefaultStakingConfig = async ( } }; -export const getDefaultERC721Setup = async () => { - // TODO impl -}; - export const getDefaultERC20Setup = async ( owner : SignerWithAddress, rewardsToken : MockERC20, @@ -92,9 +89,7 @@ export const getDefaultERC20Setup = async ( return [contract, config]; } -// todo `getmodified erc20 setup` - -export const getNativeSetup = async ( +export const getNativeSetupERC20 = async ( owner : SignerWithAddress, stakeRepToken : ZeroVotingERC20, ) => { @@ -121,7 +116,35 @@ export const getNativeSetup = async ( return localContract; } -// TODO modify with types for possibly using erc721 +export const getNativeSetupERC721 = async ( + owner : SignerWithAddress, + stakeToken : MockERC721, + stakeRepToken : ZeroVotingERC721 +) => { + const config = await createDefaultStakingConfig( + owner, + undefined, + stakeToken, + undefined, + undefined, + stakeRepToken, + ); + + const stakingFactory = await hre.ethers.getContractFactory("StakingERC721"); + const contract = await stakingFactory.deploy(config) as StakingERC721; + + await stakeRepToken.connect(owner).grantRole(await stakeRepToken.MINTER_ROLE(), await contract.getAddress()); + await stakeRepToken.connect(owner).grantRole(await stakeRepToken.BURNER_ROLE(), await contract.getAddress()); + + // Provide rewards funding in native token + await owner.sendTransaction({ + to: await contract.getAddress(), + value: hre.ethers.parseEther("9999"), + }); + + return contract; +} + export const fundAndApprove = async ( owner : SignerWithAddress, addresses : Array, diff --git a/test/staking20.test.ts b/test/staking20.test.ts index 198bcd9..2e10e66 100644 --- a/test/staking20.test.ts +++ b/test/staking20.test.ts @@ -32,7 +32,7 @@ import { calcTotalUnlockedRewards, calcStakeRewards, DEFAULT_MINIMUM_LOCK, - getNativeSetup, + getNativeSetupERC20, getDefaultERC20Setup, fundAndApprove, } from "./helpers/staking"; @@ -70,7 +70,6 @@ describe("StakingERC20", () => { ] = await hre.ethers.getSigners(); const mockERC20Factory = await hre.ethers.getContractFactory("MockERC20"); - const stakingFactory = await hre.ethers.getContractFactory("StakingERC20"); const stakeRepFactory = await hre.ethers.getContractFactory("ZeroVotingERC20"); reset = async () => { @@ -78,8 +77,6 @@ describe("StakingERC20", () => { rewardsToken = await mockERC20Factory.deploy("WilderWorld", "WW"); stakeRepToken = await stakeRepFactory.deploy("VotingToken", "VTKN", owner); - const ownerBal = await hre.ethers.provider.getBalance(owner.address); - // Give the owner ample funds for transfers in native token case await setBalance(owner.address, INIT_BALANCE * 10n); @@ -110,7 +107,7 @@ describe("StakingERC20", () => { describe("#getContractRewardsBalance", () => { it("it accounts for balance when rewards and stake are same token", async () => { - const localContract = await getNativeSetup(owner, stakeRepToken); + const localContract = await getNativeSetupERC20(owner, stakeRepToken); // Provide rewards funding in native token await owner.sendTransaction({ @@ -1293,30 +1290,14 @@ describe("StakingERC20", () => { }); }); - describe("Different configs", async () => { + describe("Other configs", async () => { it("Stakes, claims, partially and fully unstakes when stake and reward token are chain token", async () => { - // When neither erc20 or erc721 specified it will assume erc20 with native token - // same with rewards - config = await createDefaultStakingConfig( + // When neither erc20 or erc721 specified we assume erc20 with native token + const localContract = await getNativeSetupERC20( owner, - undefined, - undefined, - undefined, - stakeRepToken, + stakeRepToken ); - const localStakingFactory = await hre.ethers.getContractFactory("StakingERC20"); - const localContract = await localStakingFactory.deploy(config) as StakingERC20; - - await stakeRepToken.connect(owner).grantRole(await stakeRepToken.MINTER_ROLE(), await localContract.getAddress()); - await stakeRepToken.connect(owner).grantRole(await stakeRepToken.BURNER_ROLE(), await localContract.getAddress()); - - // Provide rewards funding in native token - await owner.sendTransaction({ - to: await localContract.getAddress(), - value: hre.ethers.parseEther("9999"), - }) - const userBalBefore = await hre.ethers.provider.getBalance(stakerA.address); const contrBalBefore = await hre.ethers.provider.getBalance(await localContract.getAddress()); diff --git a/test/staking721.test.ts b/test/staking721.test.ts index 3a56709..5631c9a 100644 --- a/test/staking721.test.ts +++ b/test/staking721.test.ts @@ -24,6 +24,7 @@ import { DEFAULT_MINIMUM_LOCK, DEFAULT_MINIMUM_RM, DEFAULT_MAXIMUM_RM, + getNativeSetupERC721, } from "./helpers/staking"; import { FAILED_INNER_CALL_ERR, @@ -73,17 +74,6 @@ describe("StakingERC721", () => { let unstakedAtA : bigint; let unstakedAtB : bigint; - let secondUnstakedAt : bigint; - - let balanceAtStakeOneA : bigint; - let balanceAtStakeTwoA : bigint; - - let balanceAtStakeOneB : bigint; - let balanceAtStakeTwoB : bigint; - - let durationOne : bigint; - let durationTwo : bigint; - // Default token ids const tokenIdA = 1n; const tokenIdB = 2n; @@ -130,8 +120,8 @@ describe("StakingERC721", () => { stakingTokenAddress = await stakingToken.getAddress(); config = await createDefaultStakingConfig( - rewardToken, owner, + rewardToken, stakingToken, undefined, undefined, @@ -166,10 +156,6 @@ describe("StakingERC721", () => { await stakingToken.connect(owner).mint(owner.address, unStakedTokenId); - - // fails to mint? - // await stakingToken.mint(notStaker.address, unStakedTokenId); - await stakingToken.connect(stakerA).approve(await stakingERC721.getAddress(), tokenIdA); await stakingToken.connect(stakerA).approve(await stakingERC721.getAddress(), tokenIdB); await stakingToken.connect(stakerA).approve(await stakingERC721.getAddress(), tokenIdC); @@ -312,9 +298,6 @@ describe("StakingERC721", () => { const supplyAfter = await stakeRepToken.totalSupply(); - // Get balance of sNFTs - balanceAtStakeOneA = await stakeRepToken.balanceOf(stakerA.address); - const stakerData = await stakingERC721.connect(stakerA).nftStakers(stakerA.address); const stakes = await stakingERC721.connect(stakerA).getStakedTokenIds(); @@ -345,9 +328,6 @@ describe("StakingERC721", () => { const supplyAfter = await stakeRepToken.totalSupply(); - // Get balance of sNFTs - balanceAtStakeOneA = await stakeRepToken.balanceOf(stakerB.address); - const stakerData = await stakingERC721.connect(stakerB).nftStakers(stakerB.address); const stakes = await stakingERC721.connect(stakerB).getStakedTokenIds(); @@ -377,8 +357,6 @@ describe("StakingERC721", () => { const supplyAfter = await stakeRepToken.totalSupply(); - balanceAtStakeTwoA = await stakeRepToken.balanceOf(stakerA.address); - const stakerData = await stakingERC721.connect(stakerA).nftStakers(stakerA.address); // const amountStaked = await stakingERC721.connect(stakerA).getAmountStaked(); @@ -413,11 +391,8 @@ describe("StakingERC721", () => { const supplyAfter = await stakeRepToken.totalSupply(); - balanceAtStakeTwoA = await stakeRepToken.balanceOf(stakerB.address); - const stakerData = await stakingERC721.connect(stakerB).nftStakers(stakerB.address); - // const amountStaked = await stakingERC721.connect(stakerB).getAmountStaked(); const tokenIds = await stakingERC721.connect(stakerB).getStakedTokenIds(); @@ -1152,6 +1127,29 @@ describe("StakingERC721", () => { }); describe("Other configs", () => { + it("Can set the rewards token as native token", async () => { + const localContract : StakingERC721 = await getNativeSetupERC721( + owner, + stakingToken, + stakeRepToken + ) + + await stakingToken.connect(stakerA).approve(await localContract.getAddress(), tokenIdA); + + await localContract.connect(stakerA).stakeWithoutLock([tokenIdA], [emptyUri]); + const stakedAt = BigInt(await time.latest()); + + await time.increase(DEFAULT_LOCK / 2n); + + await localContract.connect(stakerA).claim(); + const claimedAt = BigInt(await time.latest()); + + await time.increase(DEFAULT_LOCK / 2n); + + await localContract.connect(stakerA).unstake([tokenIdA], false); + const unstakedAt = BigInt(await time.latest()); + }); + it ("Can't use the StakingERC721 contract when an IERC20 is the staking token", async () => { const localConfig = { stakingToken: await rewardToken.getAddress(), From a54794b71068f33e300b443e5e1c38efe69ea966 Mon Sep 17 00:00:00 2001 From: James Earle Date: Mon, 30 Dec 2024 17:20:17 -0500 Subject: [PATCH 4/7] erc721 tests update, contract modification to add staked mapping to avoid gas for iterating array when unstaking --- contracts/mock/tokens/MockERC20.sol | 3 +- contracts/staking/ERC20/StakingERC20.sol | 4 +- contracts/staking/ERC721/IStakingERC721.sol | 10 +- contracts/staking/ERC721/StakingERC721.sol | 22 +- contracts/staking/StakingBase.sol | 23 +- test/helpers/constants.ts | 5 +- test/helpers/staking/defaults.ts | 34 +- test/staking721.test.ts | 381 +++++++++++++++++--- 8 files changed, 402 insertions(+), 80 deletions(-) diff --git a/contracts/mock/tokens/MockERC20.sol b/contracts/mock/tokens/MockERC20.sol index fdedb39..5affd5a 100644 --- a/contracts/mock/tokens/MockERC20.sol +++ b/contracts/mock/tokens/MockERC20.sol @@ -7,8 +7,9 @@ import { ERC20Mod } from "./ERC20Mod.sol"; /* solhint-disable */ contract MockERC20 is ERC20Mod { constructor(string memory name, string memory symbol) ERC20Mod(name, symbol) { - _mint(msg.sender, 9000000000000 * 10 ** 18); + _mint(msg.sender, 999999999999999999 * 10 ** 18); } + function mint(address to, uint256 amount) public virtual { _mint(to, amount); } diff --git a/contracts/staking/ERC20/StakingERC20.sol b/contracts/staking/ERC20/StakingERC20.sol index 101e221..414db49 100644 --- a/contracts/staking/ERC20/StakingERC20.sol +++ b/contracts/staking/ERC20/StakingERC20.sol @@ -238,7 +238,7 @@ contract StakingERC20 is StakingBase, IStakingERC20 { // Transfer the user's rewards // Will fail if the contract does not have funding for this // If rewards address is `0x0` we use the chain's native token - _transferToken(config.rewardsToken, rewards); + _transferAmount(config.rewardsToken, rewards); emit Claimed(msg.sender, rewards); } @@ -246,7 +246,7 @@ contract StakingERC20 is StakingBase, IStakingERC20 { totalStaked -= amount; // Return the user's initial stake - _transferToken(config.stakingToken, amount); + _transferAmount(config.stakingToken, amount); // Burn the user's stake representative token IERC20MintableBurnable(config.stakeRepToken).burn(msg.sender, amount); diff --git a/contracts/staking/ERC721/IStakingERC721.sol b/contracts/staking/ERC721/IStakingERC721.sol index d1665b2..b4d8eb1 100644 --- a/contracts/staking/ERC721/IStakingERC721.sol +++ b/contracts/staking/ERC721/IStakingERC721.sol @@ -15,7 +15,15 @@ interface IStakingERC721 is IERC721Receiver, IStakingBase { */ struct NFTStaker { Staker stake; - uint256[] tokenIds; + + // A) have array of token ids AND `staked` mapping + // This way we can mark tokens as `unstaked` without iterating + // `tokenIds` array each time. + // B) we can just remove the `unstakeAll` option because the front end could do this + // Considering we don't yet have a subgraph for this it might be tricky + uint256[] tokenIds; // use sNFT as proof of ownership of stake, and `amountStaked(locked)` as quantity + // TODO look at gas costs for this and see if off chain tids is a better solution (with a subgraph) + mapping(uint256 tokenId => bool staked) staked; mapping(uint256 tokenId => bool locked) locked; } diff --git a/contracts/staking/ERC721/StakingERC721.sol b/contracts/staking/ERC721/StakingERC721.sol index ddd3f46..eb66cf0 100644 --- a/contracts/staking/ERC721/StakingERC721.sol +++ b/contracts/staking/ERC721/StakingERC721.sol @@ -171,8 +171,9 @@ contract StakingERC721 is StakingBase, IStakingERC721 { tokenIds[i] ); - // Add to array and to mapping for indexing in unstake + // Add to array and to mapping for when unstaking nftStaker.tokenIds.push(tokenIds[i]); + nftStaker.staked[tokenIds[i]] = true; nftStaker.locked[tokenIds[i]] = lockDuration > 0; // Mint user sNFT @@ -188,6 +189,10 @@ contract StakingERC721 is StakingBase, IStakingERC721 { } function _unstakeMany(uint256[] memory _tokenIds, bool exit) internal { + // its possible that token IDs that are already unstaked are passed here + // because removing them from the users tokenIds[] would be gas expensive + // and so burning will fail with `non-existent token` error + // so we check if the token is owned by the user and if not, skip it NFTStaker storage nftStaker = nftStakers[msg.sender]; uint256 rewards; @@ -203,7 +208,8 @@ contract StakingERC721 is StakingBase, IStakingERC721 { uint256 i; for (i; i < _tokenIds.length;) { if ( - IERC721(config.stakeRepToken).ownerOf(_tokenIds[i]) == address(0) + nftStaker.staked[_tokenIds[i]] == false + || IERC721(config.stakeRepToken).ownerOf(_tokenIds[i]) == address(0) || IERC721(config.stakeRepToken).ownerOf(_tokenIds[i]) != msg.sender ) { // Either the list of tokenIds contains a non-existent token @@ -241,9 +247,8 @@ contract StakingERC721 is StakingBase, IStakingERC721 { if (!rewardsGivenLocked) { rewards += nftStaker.stake.owedRewardsLocked; - // set to 0 so they don't get it again in future calls + // can only get this once per tx, not each loop, so set to 0 nftStaker.stake.owedRewardsLocked = 0; - rewardsGivenLocked = true; } } @@ -254,6 +259,7 @@ contract StakingERC721 is StakingBase, IStakingERC721 { // Unstake if they are passed their lock time or exiting _unstake(_tokenIds[i]); --nftStaker.stake.amountStakedLocked; + nftStaker.staked[_tokenIds[i]] = false; isAction = true; } else { // stake is locked and cannot be unstaked @@ -281,6 +287,7 @@ contract StakingERC721 is StakingBase, IStakingERC721 { _unstake(_tokenIds[i]); --nftStaker.stake.amountStaked; + nftStaker.staked[_tokenIds[i]] = false; isAction = true; } @@ -306,7 +313,7 @@ contract StakingERC721 is StakingBase, IStakingERC721 { if (!exit) { // Transfer the user's rewards - _transferToken(config.rewardsToken, rewards); + _transferAmount(config.rewardsToken, rewards); emit Claimed(msg.sender, rewards); } @@ -314,12 +321,11 @@ contract StakingERC721 is StakingBase, IStakingERC721 { // If a complete withdrawal, delete the staker struct for this user as well if (nftStaker.stake.amountStaked == 0 && nftStaker.stake.amountStakedLocked == 0) { delete nftStakers[msg.sender]; - } else if (nftStaker.stake.amountStaked != 0) { + } else if (nftStaker.stake.amountStaked != 0 && nftStaker.stake.amountStakedLocked == 0) { nftStaker.stake.amountStakedLocked = 0; nftStaker.stake.lastTimestampLocked = 0; nftStaker.stake.unlockedTimestamp = 0; - nftStaker.stake.lockDuration = 0; - } else { + } else if (nftStaker.stake.amountStaked == 0 && nftStaker.stake.amountStakedLocked != 0) { nftStaker.stake.amountStaked = 0; nftStaker.stake.lastTimestamp = 0; } diff --git a/contracts/staking/StakingBase.sol b/contracts/staking/StakingBase.sol index e659970..2612dfd 100644 --- a/contracts/staking/StakingBase.sol +++ b/contracts/staking/StakingBase.sol @@ -8,7 +8,7 @@ import { SafeERC20 } from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.s import { IStakingBase } from "./IStakingBase.sol"; import { ReentrancyGuard } from "@openzeppelin/contracts/utils/ReentrancyGuard.sol"; - +import { console } from "hardhat/console.sol"; /** * @title StakingBase * @notice A set of common elements that are used in any Staking contract @@ -58,7 +58,7 @@ contract StakingBase is Ownable, ReentrancyGuard, IStakingBase { // Do not send empty transfer if (balance == 0) revert InsufficientContractBalance(); - _transferToken(config.rewardsToken, balance); + _transferAmount(config.rewardsToken, balance); emit LeftoverRewardsWithdrawn(owner(), balance); } @@ -126,7 +126,6 @@ contract StakingBase is Ownable, ReentrancyGuard, IStakingBase { * @param locked Boolean if the stake is locked */ function getStakeRewards(uint256 amount, uint256 timeDuration, bool locked) public override view returns (uint256) { - uint256 rewardsMultiplier = locked ? _calcRewardsMultiplier(timeDuration) : 1; return _getStakeRewards( @@ -288,7 +287,7 @@ contract StakingBase is Ownable, ReentrancyGuard, IStakingBase { if (rewards == 0) revert ZeroRewards(); - _transferToken(config.rewardsToken, rewards); + _transferAmount(config.rewardsToken, rewards); emit Claimed(msg.sender, rewards); } @@ -373,7 +372,7 @@ contract StakingBase is Ownable, ReentrancyGuard, IStakingBase { function _calcRewardsMultiplier(uint256 lock) internal view returns (uint256) { return config.minimumRewardsMultiplier + (config.maximumRewardsMultiplier - config.minimumRewardsMultiplier) - * (lock ) + * (lock) / config.periodLength; } @@ -381,15 +380,25 @@ contract StakingBase is Ownable, ReentrancyGuard, IStakingBase { * @dev Get the rewards balance of this contract */ function _getContractRewardsBalance() internal view virtual returns (uint256) { - return IERC20(config.rewardsToken).balanceOf(address(this)); + if (config.rewardsToken == address(0)) { + return address(this).balance; + } else { + return IERC20(config.rewardsToken).balanceOf(address(this)); + } } /** * @dev Transfer funds to a recipient after deciding whether to use * native or ERC20 tokens + * + * We give `token` as an argument here because in ERC721 it is always the + * reward token to transfer, but in ERC20 it could be either staking or rewards + * token and we won't know which to check. */ - function _transferToken(address token, uint256 amount) internal { + function _transferAmount(address token, uint256 amount) internal { if (token == address(0)) { + if (address(this).balance < amount) revert InsufficientContractBalance(); + payable(msg.sender).transfer(amount); } else { IERC20(token).safeTransfer(msg.sender, amount); diff --git a/test/helpers/constants.ts b/test/helpers/constants.ts index 31f86b0..ca4d7cf 100644 --- a/test/helpers/constants.ts +++ b/test/helpers/constants.ts @@ -5,12 +5,13 @@ export const DAY_IN_SECONDS = 86400n; export const DEFAULT_REWARDS_PER_PERIOD_ERC20 = 50n; export const DEFAULT_PERIOD_LENGTH_ERC20 = 365n * DAY_IN_SECONDS; -export const DEFAULT_REWARDS_PER_PERIOD_ERC721 = ethers.parseEther("1500"); -export const DEFAULT_PERIOD_LENGTH_ERC721 = DAY_IN_SECONDS; +export const DEFAULT_REWARDS_PER_PERIOD_ERC721 = ethers.parseEther("1000000"); +export const DEFAULT_PERIOD_LENGTH_ERC721 = 365n * DAY_IN_SECONDS; export const DEFAULT_LOCK = 365n * DAY_IN_SECONDS; export const DEFAULT_MINIMUM_LOCK = 30n * DAY_IN_SECONDS; +// 1.0 and 10.0, respectively export const DEFAULT_MINIMUM_RM = 100n; export const DEFAULT_MAXIMUM_RM = 1000n; diff --git a/test/helpers/staking/defaults.ts b/test/helpers/staking/defaults.ts index 550dc81..f302f3d 100644 --- a/test/helpers/staking/defaults.ts +++ b/test/helpers/staking/defaults.ts @@ -102,18 +102,16 @@ export const getNativeSetupERC20 = async ( ); const localStakingFactory = await hre.ethers.getContractFactory("StakingERC20"); - const localContract = await localStakingFactory.deploy(config) as StakingERC20; + const contract = await localStakingFactory.deploy(config) as StakingERC20; + const contractAddress = await contract.getAddress(); - await stakeRepToken.connect(owner).grantRole(await stakeRepToken.MINTER_ROLE(), await localContract.getAddress()); - await stakeRepToken.connect(owner).grantRole(await stakeRepToken.BURNER_ROLE(), await localContract.getAddress()); + await stakeRepToken.connect(owner).grantRole(await stakeRepToken.MINTER_ROLE(), contractAddress); + await stakeRepToken.connect(owner).grantRole(await stakeRepToken.BURNER_ROLE(), contractAddress); // Provide rewards funding in native token - await owner.sendTransaction({ - to: await localContract.getAddress(), - value: hre.ethers.parseEther("9999"), - }); + fundRewards(contractAddress); - return localContract; + return contract; } export const getNativeSetupERC721 = async ( @@ -132,19 +130,18 @@ export const getNativeSetupERC721 = async ( const stakingFactory = await hre.ethers.getContractFactory("StakingERC721"); const contract = await stakingFactory.deploy(config) as StakingERC721; + const contractAddress = await contract.getAddress(); - await stakeRepToken.connect(owner).grantRole(await stakeRepToken.MINTER_ROLE(), await contract.getAddress()); - await stakeRepToken.connect(owner).grantRole(await stakeRepToken.BURNER_ROLE(), await contract.getAddress()); + await stakeRepToken.connect(owner).grantRole(await stakeRepToken.MINTER_ROLE(), contractAddress); + await stakeRepToken.connect(owner).grantRole(await stakeRepToken.BURNER_ROLE(), contractAddress); // Provide rewards funding in native token - await owner.sendTransaction({ - to: await contract.getAddress(), - value: hre.ethers.parseEther("9999"), - }); + await fundRewards(contractAddress) return contract; } +// Fund users and approve for an amount export const fundAndApprove = async ( owner : SignerWithAddress, addresses : Array, @@ -162,3 +159,12 @@ export const fundAndApprove = async ( ); } } + + +const fundRewards = async (contractAddress : string) => { + await hre.network.provider.send("hardhat_setBalance", [ + contractAddress, + `0x${hre.ethers.parseEther("999999999").toString()}`, + ] +); +} \ No newline at end of file diff --git a/test/staking721.test.ts b/test/staking721.test.ts index 5631c9a..cb7e9a6 100644 --- a/test/staking721.test.ts +++ b/test/staking721.test.ts @@ -109,8 +109,6 @@ describe("StakingERC721", () => { const mockERC20Factory = await hre.ethers.getContractFactory("MockERC20"); rewardToken = await mockERC20Factory.deploy("MEOW", "MEOW"); - rewardTokenAddress = await rewardToken.getAddress(); - const mockERC721Factory = await hre.ethers.getContractFactory("MockERC721"); stakingToken = await mockERC721Factory.deploy("WilderWheels", "WW", baseUri); @@ -130,31 +128,32 @@ describe("StakingERC721", () => { const stakingFactory = await hre.ethers.getContractFactory("StakingERC721"); stakingERC721 = await stakingFactory.deploy(config); - stakingERC721Address = await stakingERC721.getAddress(); + + const stakingERC721Address = await stakingERC721.getAddress(); await stakeRepToken.connect(owner).grantRole(await stakeRepToken.MINTER_ROLE(), stakingERC721Address); await stakeRepToken.connect(owner).grantRole(await stakeRepToken.BURNER_ROLE(), stakingERC721Address); // Give staking contract balance to pay rewards - await rewardToken.connect(owner).transfer( - await stakingERC721.getAddress(), - hre.ethers.parseEther("8000000000000") - ); - - await stakingToken.connect(owner).mint(stakerA.address, tokenIdA); - await stakingToken.connect(owner).mint(stakerA.address, tokenIdB); - await stakingToken.connect(owner).mint(stakerA.address, tokenIdC); + await rewardToken.connect(owner).mint( + stakingERC721Address, + hre.ethers.parseEther("999999999") + ) - await stakingToken.connect(owner).mint(stakerB.address, tokenIdD); - await stakingToken.connect(owner).mint(stakerB.address, tokenIdE); - await stakingToken.connect(owner).mint(stakerB.address, tokenIdF); - await stakingToken.connect(owner).mint(stakerB.address, tokenIdG); + await stakingToken.mint(stakerA.address, tokenIdA); + await stakingToken.mint(stakerA.address, tokenIdB); + await stakingToken.mint(stakerA.address, tokenIdC); - await stakingToken.connect(owner).mint(stakerC.address, tokenIdH); - await stakingToken.connect(owner).mint(stakerC.address, tokenIdI); - await stakingToken.connect(owner).mint(stakerC.address, tokenIdJ); + await stakingToken.mint(stakerB.address, tokenIdD); + await stakingToken.mint(stakerB.address, tokenIdE); + await stakingToken.mint(stakerB.address, tokenIdF); + await stakingToken.mint(stakerB.address, tokenIdG); + + await stakingToken.mint(stakerC.address, tokenIdH); + await stakingToken.mint(stakerC.address, tokenIdI); + await stakingToken.mint(stakerC.address, tokenIdJ); - await stakingToken.connect(owner).mint(owner.address, unStakedTokenId); + await stakingToken.mint(owner.address, unStakedTokenId); await stakingToken.connect(stakerA).approve(await stakingERC721.getAddress(), tokenIdA); await stakingToken.connect(stakerA).approve(await stakingERC721.getAddress(), tokenIdB); @@ -373,7 +372,6 @@ describe("StakingERC721", () => { expect(stakerData.amountStaked).to.eq(stakes.length); expect(stakes[0]).to.eq(tokenIdA); // Is unchanged - expect(stakerData.lockDuration).to.eq(0); expect(stakerData.lastTimestamp).to.eq(secondStakedAtA); }); @@ -847,13 +845,10 @@ describe("StakingERC721", () => { config ); - // Rounding error in Solidity, so we `-1n` here, can't calculate the exact value - // because TypeScript handles the math better. - expect(balanceAfter).to.eq(balanceBefore + lockedStakeRewards + interimTimeRewards - 1n); + expect(balanceAfter).to.eq(balanceBefore + lockedStakeRewards + interimTimeRewards); const stakerDataAfter = await stakingERC721.nftStakers(stakerA.address); - // User has regained their NFTs and the SNFT was burned expect(await stakingToken.balanceOf(stakerA.address)).to.eq(2); // unstaked two expect(await stakeRepToken.balanceOf(stakerA.address)).to.eq(1); // one remains staked @@ -864,7 +859,6 @@ describe("StakingERC721", () => { // Because `unstake` also claims, the non-locked timestamp is updated as well expect(stakerDataAfter.lastTimestamp).to.eq(stakedAtUnlocked); expect(stakerDataAfter.lastTimestampLocked).to.eq(0n); - expect(stakerDataAfter.lockDuration).to.eq(0n); expect(stakerDataAfter.amountStaked).to.eq(stakerDataBefore.amountStaked); expect(stakerDataAfter.amountStakedLocked).to.eq(stakerDataBefore.amountStakedLocked - 2n); expect(stakerDataAfter.owedRewards).to.eq(0n); @@ -894,17 +888,25 @@ describe("StakingERC721", () => { }); it("Fails to unstake when token id is invalid", async () => { - // It will revert with the "ZeroRewards" error before it has a chance to check the token id + await time.increase(DEFAULT_LOCK); + + // if unstake with a valid and not valid token, the not valid one is skipped + // so it doesnt revert, but doesnt do anything with the extra either + // Desired? Or should we revert when we find this instead? + // try { + // await stakingERC721.connect(stakerA).unstake([tokenIdA, unmintedTokenId], false) + // } catch(e) { + // console.log((e as Error).message); + // } await expect( stakingERC721.connect(stakerA).unstake([unmintedTokenId], false) - ).to.be.revertedWithCustomError(stakeRepToken, NONEXISTENT_TOKEN_ERR); + ).to.be.revertedWithCustomError(stakingERC721, INVALID_UNSTAKE_ERR); // Confirm with `exit` await expect( stakingERC721.connect(stakerA).unstake([unmintedTokenId], true) - ).to.be.revertedWithCustomError(stakeRepToken, NONEXISTENT_TOKEN_ERR) - .withArgs(unmintedTokenId); + ).to.be.revertedWithCustomError(stakingERC721, INVALID_UNSTAKE_ERR) }); it("Fails to unstake when caller is not the owner of the SNFT", async () => { @@ -920,11 +922,10 @@ describe("StakingERC721", () => { }); it("Fails to unstake when token id is not staked", async () => { - // If the a token is not staked, the relevant SNFT does not exist and so we can't unstake it + // If the a token is not staked, the relevant does not exist and so we can't unstake it await expect( stakingERC721.connect(stakerA).unstake([unStakedTokenId], false) - ).to.be.revertedWithCustomError(stakingToken, NONEXISTENT_TOKEN_ERR) - .withArgs(unStakedTokenId); + ).to.be.revertedWithCustomError(stakingERC721, INVALID_UNSTAKE_ERR) }); }); @@ -940,8 +941,7 @@ describe("StakingERC721", () => { // as if the owner has already exited await expect( stakingERC721.connect(stakerB).unstake([unmintedTokenId], true) - ).to.be.revertedWithCustomError(stakeRepToken, NONEXISTENT_TOKEN_ERR) - .withArgs(unmintedTokenId); + ).to.be.revertedWithCustomError(stakingERC721, INVALID_UNSTAKE_ERR) }); it("Allows the user to remove their stake within the timelock period without rewards", async () => { @@ -1062,9 +1062,10 @@ describe("StakingERC721", () => { const latest = BigInt(await time.latest()); - const futureExpectedRewardsA = calcTotalUnlockedRewards( - [latest - stakedAt], - [await stakeRepToken.balanceOf(stakerA.address)], + const futureExpectedRewardsA = calcStakeRewards( + await stakeRepToken.balanceOf(stakerA.address), + latest - stakedAt, + false, config ); @@ -1135,19 +1136,285 @@ describe("StakingERC721", () => { ) await stakingToken.connect(stakerA).approve(await localContract.getAddress(), tokenIdA); + await stakingToken.connect(stakerA).approve(await localContract.getAddress(), tokenIdB); - await localContract.connect(stakerA).stakeWithoutLock([tokenIdA], [emptyUri]); + await localContract.connect(stakerA).stakeWithoutLock([tokenIdA, tokenIdB], [emptyUri, emptyUri]); const stakedAt = BigInt(await time.latest()); - await time.increase(DEFAULT_LOCK / 2n); + await time.increase(DEFAULT_LOCK / 5n); + + const rewardsBefore = await hre.ethers.provider.getBalance(stakerA.address); - await localContract.connect(stakerA).claim(); + const claimTx = await localContract.connect(stakerA).claim(); const claimedAt = BigInt(await time.latest()); - await time.increase(DEFAULT_LOCK / 2n); + const stakerData = await localContract.nftStakers(stakerA.address); + + const claimReceipt = await claimTx.wait(); + + const claimRewards = calcStakeRewards( + stakerData.amountStaked, + claimedAt - stakedAt, + false, + config + ); + + const rewardsAfterClaim = await hre.ethers.provider.getBalance(stakerA.address); + + expect(rewardsAfterClaim).to.eq( + rewardsBefore + claimRewards - (claimReceipt!.gasUsed * claimReceipt!.gasPrice) + ); + + expect(stakerData.owedRewards).to.eq(0n); + expect(stakerData.lastTimestamp).to.eq(claimedAt); + expect(stakerData.amountStaked).to.eq(2n); // Unchanged after claim + + // No locked stake + expect(stakerData.amountStakedLocked).to.eq(0n); + expect(stakerData.owedRewardsLocked).to.eq(0n); + expect(stakerData.lastTimestampLocked).to.eq(0n); + + await time.increase(DEFAULT_LOCK / 5n); + + // Partial unstake + const partialUnstakeTx = await localContract.connect(stakerA).unstake([tokenIdA], false); + const partialUnstakedAt = BigInt(await time.latest()); + + const partialUnstakeReceipt = await partialUnstakeTx.wait(); + const stakerDataAfter = await localContract.nftStakers(stakerA.address); + + const rewardsAfterUnstake = await hre.ethers.provider.getBalance(stakerA.address); + + const unstakeRewards = calcStakeRewards( + stakerDataAfter.amountStaked, + partialUnstakedAt - claimedAt, + false, + config + ); + + expect(rewardsAfterUnstake).to.eq( + rewardsAfterClaim + unstakeRewards - (partialUnstakeReceipt!.gasUsed * partialUnstakeReceipt!.gasPrice) + ); + + expect(stakerDataAfter.owedRewards).to.eq(0n); + expect(stakerDataAfter.lastTimestamp).to.eq(partialUnstakedAt); + expect(stakerDataAfter.amountStaked).to.eq(1n); // Decrement after partial unstake + + // No locked stake + expect(stakerDataAfter.amountStakedLocked).to.eq(0n); + expect(stakerDataAfter.owedRewardsLocked).to.eq(0n); + expect(stakerDataAfter.lastTimestampLocked).to.eq(0n); + + // Full unstake + await time.increase(DEFAULT_LOCK / 5n); + + const fullUnstakeTx = await localContract.connect(stakerA).unstakeAll(false); + const fullUnstakedAt = BigInt(await time.latest()); + + const fullUnstakeReceipt = await fullUnstakeTx.wait(); + + const rewardsAfterFullUnstake = await hre.ethers.provider.getBalance(stakerA.address); + + const fullUnstakeRewards = calcStakeRewards( + stakerDataAfter.amountStaked, + fullUnstakedAt - partialUnstakedAt, + false, + config + ); + + expect(rewardsAfterFullUnstake).to.eq( + rewardsAfterUnstake + fullUnstakeRewards - (fullUnstakeReceipt!.gasUsed * fullUnstakeReceipt!.gasPrice) + ); + + const stakerDataFinal = await localContract.nftStakers(stakerA.address); + + // Delete staker struct on full unstake + expect(stakerDataFinal.owedRewards).to.eq(0n); + expect(stakerDataFinal.lastTimestamp).to.eq(0n); + expect(stakerDataFinal.amountStaked).to.eq(0n); + + // No locked stake + expect(stakerDataFinal.amountStakedLocked).to.eq(0n); + expect(stakerDataFinal.owedRewardsLocked).to.eq(0n); + expect(stakerDataFinal.lastTimestampLocked).to.eq(0n); + }); + + it("Can set the rewards token as native token when locking", async () => { + await reset(); + + const localContract : StakingERC721 = await getNativeSetupERC721( + owner, + stakingToken, + stakeRepToken + ) + + await stakingToken.connect(stakerA).approve(await localContract.getAddress(), tokenIdA); + await stakingToken.connect(stakerA).approve(await localContract.getAddress(), tokenIdB); + await stakingToken.connect(stakerA).approve(await localContract.getAddress(), tokenIdC); + + await localContract.connect(stakerA).stakeWithLock([tokenIdA, tokenIdB], [emptyUri, emptyUri], DEFAULT_LOCK); + const stakedAtLocked = BigInt(await time.latest()); + + await localContract.connect(stakerA).stakeWithoutLock([tokenIdC], [emptyUri]); + const stakedAt = BigInt(await time.latest()); + + const dataAfterStakes = await localContract.nftStakers(stakerA.address); + + // Pre calculated lock stake value + const lockedRewards = calcStakeRewards( + dataAfterStakes.amountStakedLocked, + DEFAULT_LOCK, + true, + config + ); + + expect(dataAfterStakes.amountStaked).to.eq(1n); + expect(dataAfterStakes.amountStakedLocked).to.eq(2n); + expect(dataAfterStakes.lastTimestamp).to.eq(stakedAt); + expect(dataAfterStakes.lastTimestampLocked).to.eq(stakedAtLocked); + expect(dataAfterStakes.unlockedTimestamp).to.eq(stakedAtLocked + DEFAULT_LOCK); + expect(dataAfterStakes.owedRewards).to.eq(0n); + expect(dataAfterStakes.owedRewardsLocked).to.eq(lockedRewards); + + await time.increase(DEFAULT_LOCK / 3n); + + const rewardsBefore = await hre.ethers.provider.getBalance(stakerA.address); + const rewardsBeforeContr = await hre.ethers.provider.getBalance(await localContract.getAddress()); + + // claim + const claimTx = await localContract.connect(stakerA).claim(); + const claimedAt = BigInt(await time.latest()); + + const claimReceipt = await claimTx.wait(); + const stakerData = await localContract.nftStakers(stakerA.address); + + const claimRewards = calcStakeRewards( + stakerData.amountStaked, + claimedAt - stakedAt, + false, + config + ); + + const rewardsAfterClaim = await hre.ethers.provider.getBalance(stakerA.address); + + // Confirm we don't receive rewards from stake that is still locked when calling `claim` + expect(rewardsAfterClaim).to.eq( + rewardsBefore + claimRewards - (claimReceipt!.gasUsed * claimReceipt!.gasPrice) + ); + + expect(stakerData.owedRewards).to.eq(0n); + expect(stakerData.lastTimestamp).to.eq(claimedAt); + expect(stakerData.amountStaked).to.eq(dataAfterStakes.amountStaked); // Unchanged after claim + expect(stakerData.amountStakedLocked).to.eq(dataAfterStakes.amountStakedLocked); // Unchanged after claim + expect(stakerData.owedRewardsLocked).to.eq(lockedRewards); + expect(stakerData.lastTimestampLocked).to.eq(stakedAtLocked); - await localContract.connect(stakerA).unstake([tokenIdA], false); - const unstakedAt = BigInt(await time.latest()); + // Move time forward to unlock the stake + await time.increase(DEFAULT_LOCK); + + // Claim again + const fullClaimTx = await localContract.connect(stakerA).claim(); + const fullClaimedAt = BigInt(await time.latest()); + + const fullClaimReceipt = await fullClaimTx.wait(); + const dataAfterFullClaim = await localContract.nftStakers(stakerA.address); + + const unlockedRewards = calcStakeRewards( + stakerData.amountStaked, + fullClaimedAt - claimedAt, + false, + config + ); + + const interimRewards = calcStakeRewards( + stakerData.amountStakedLocked, + fullClaimedAt - stakerData.unlockedTimestamp, + false, + config + ); + + const rewardsAfterFullClaim = await hre.ethers.provider.getBalance(stakerA.address); + + const calcedVal = rewardsAfterClaim + lockedRewards + interimRewards + unlockedRewards - (fullClaimReceipt!.gasUsed * fullClaimReceipt!.gasPrice) + expect(rewardsAfterFullClaim).to.eq( + rewardsAfterClaim + lockedRewards + interimRewards + unlockedRewards - (fullClaimReceipt!.gasUsed * fullClaimReceipt!.gasPrice) + ); + + expect(dataAfterFullClaim.owedRewards).to.eq(0n); + expect(dataAfterFullClaim.owedRewardsLocked).to.eq(0n); + expect(dataAfterFullClaim.lastTimestamp).to.eq(fullClaimedAt); + expect(dataAfterFullClaim.lastTimestampLocked).to.eq(fullClaimedAt); + expect(dataAfterFullClaim.amountStaked).to.eq(dataAfterStakes.amountStaked); + expect(dataAfterFullClaim.amountStakedLocked).to.eq(dataAfterStakes.amountStakedLocked); + + // Partial unstake + const partialUnstakeTx = await localContract.connect(stakerA).unstake([tokenIdA], false); + const partialUnstakedAt = BigInt(await time.latest()); + + const partialUnstakeReceipt = await partialUnstakeTx.wait(); + + const dataAfterPartialUnstake = await localContract.nftStakers(stakerA.address); + + const rewardsAfterUnstake = await hre.ethers.provider.getBalance(stakerA.address); + + const unstakeRewards = calcStakeRewards( + 1n, + partialUnstakedAt - fullClaimedAt, + false, + config + ); + + expect(rewardsAfterUnstake).to.eq( + rewardsAfterFullClaim + unstakeRewards - (partialUnstakeReceipt!.gasUsed * partialUnstakeReceipt!.gasPrice) + ); + + expect(dataAfterPartialUnstake.owedRewards).to.eq(0n); + expect(dataAfterPartialUnstake.lastTimestamp).to.eq(fullClaimedAt); + expect(dataAfterPartialUnstake.lastTimestampLocked).to.eq(partialUnstakedAt); // shouldnt be 0 + expect(dataAfterPartialUnstake.amountStaked).to.eq(1n); // Decrement after partial unstake + expect(dataAfterPartialUnstake.amountStakedLocked).to.eq(1n); + expect(dataAfterPartialUnstake.owedRewardsLocked).to.eq(0n); + + // Full unstake + await time.increase(DEFAULT_LOCK / 5n); + + const fullUnstakeTx = await localContract.connect(stakerA).unstakeAll(false); + const fullUnstakedAt = BigInt(await time.latest()); + + const fullUnstakeReceipt = await fullUnstakeTx.wait(); + + const rewardsAfterFullUnstake = await hre.ethers.provider.getBalance(stakerA.address); + + const fullInterimRewards = calcStakeRewards( + dataAfterPartialUnstake.amountStakedLocked, + fullUnstakedAt - partialUnstakedAt, + false, + config + ); + + const fullUnstakeRewards = calcStakeRewards( + dataAfterPartialUnstake.amountStaked, + fullUnstakedAt - fullClaimedAt, + false, + config + ); + + expect(rewardsAfterFullUnstake).to.eq( + rewardsAfterUnstake + fullUnstakeRewards + fullInterimRewards - (fullUnstakeReceipt!.gasUsed * fullUnstakeReceipt!.gasPrice) + ); + + const stakerDataFinal = await localContract.nftStakers(stakerA.address); + + // Delete staker struct on full unstake + expect(stakerDataFinal.owedRewards).to.eq(0n); + expect(stakerDataFinal.lastTimestamp).to.eq(0n); + expect(stakerDataFinal.amountStaked).to.eq(0n); + + // No locked stake + expect(stakerDataFinal.owedRewardsLocked).to.eq(0n); + expect(stakerDataFinal.amountStakedLocked).to.eq(0n); + expect(stakerDataFinal.lastTimestampLocked).to.eq(0n); + expect(stakerDataFinal.unlockedTimestamp).to.eq(0n); }); it ("Can't use the StakingERC721 contract when an IERC20 is the staking token", async () => { @@ -1308,16 +1575,40 @@ describe("StakingERC721", () => { }); describe("Utility functions + ZeroVotingERC721 standard functions", () => { - it("Calculates the users rewards multiplier when they lock based on their lock time", async () => { + it("Calculates the users rewards when they lock based on their lock time", async () => { await reset(); - const timeDuration = DAY_IN_SECONDS * 10n; + const timeDuration = DEFAULT_LOCK; const unlocked = await stakingERC721.connect(owner).getStakeRewards(1n, timeDuration, false); const locked = await stakingERC721.connect(owner).getStakeRewards(1n, timeDuration, true); + const stakeValue = calcStakeRewards( + 1n, + timeDuration, + true, + config + ) + + expect(stakeValue).to.eq(locked); + console.log("Locked rewards: ", locked.toString()); console.log("Unlocked rewards: ", unlocked.toString()); expect(locked).to.be.gt(unlocked); + + // This also double checks that the helper method to calc rewards returns + // the same values as if using the contract methods + await stakingToken.connect(stakerA).approve(await stakingERC721.getAddress(), tokenIdB); + await stakingERC721.connect(stakerA).stakeWithoutLock([tokenIdB], [emptyUri]); + + await time.increase(DEFAULT_LOCK - 1n); + + const rewardsBefore = await rewardToken.balanceOf(stakerA.address); + + await stakingERC721.connect(stakerA).claim(); + + const rewardsAfter = await rewardToken.balanceOf(stakerA.address); + + expect(rewardsAfter).to.eq(rewardsBefore + unlocked); }); it("#setBaseURI() should set the base URI", async () => { From 83347994d0d5d98f4c5576c375bc09054ef9c8a0 Mon Sep 17 00:00:00 2001 From: James Earle Date: Mon, 30 Dec 2024 17:27:36 -0500 Subject: [PATCH 5/7] remove console from stakingbase --- contracts/staking/StakingBase.sol | 1 - 1 file changed, 1 deletion(-) diff --git a/contracts/staking/StakingBase.sol b/contracts/staking/StakingBase.sol index 2612dfd..950c519 100644 --- a/contracts/staking/StakingBase.sol +++ b/contracts/staking/StakingBase.sol @@ -8,7 +8,6 @@ import { SafeERC20 } from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.s import { IStakingBase } from "./IStakingBase.sol"; import { ReentrancyGuard } from "@openzeppelin/contracts/utils/ReentrancyGuard.sol"; -import { console } from "hardhat/console.sol"; /** * @title StakingBase * @notice A set of common elements that are used in any Staking contract From 98b391c3b766f88a544e416b900821eaf7c7e22d Mon Sep 17 00:00:00 2001 From: James Earle Date: Tue, 31 Dec 2024 10:38:52 -0500 Subject: [PATCH 6/7] fix lint --- contracts/staking/ERC20/StakingERC20.sol | 4 +- contracts/staking/ERC721/StakingERC721.sol | 2 - contracts/staking/IStakingBase.sol | 5 +- contracts/staking/StakingBase.sol | 41 ++++----- test/helpers/errors.ts | 1 + test/helpers/staking/defaults.ts | 25 +++--- test/staking-base.test.ts | 5 +- test/staking20.test.ts | 33 +++----- test/staking721.test.ts | 99 +++++----------------- 9 files changed, 76 insertions(+), 139 deletions(-) diff --git a/contracts/staking/ERC20/StakingERC20.sol b/contracts/staking/ERC20/StakingERC20.sol index 414db49..8374fd0 100644 --- a/contracts/staking/ERC20/StakingERC20.sol +++ b/contracts/staking/ERC20/StakingERC20.sol @@ -37,7 +37,7 @@ contract StakingERC20 is StakingBase, IStakingERC20 { * @param amount The amount to stake * @param lockDuration The duration of the lock period */ - function stakeWithLock(uint256 amount, uint256 lockDuration) payable external override { + function stakeWithLock(uint256 amount, uint256 lockDuration) external payable override { if (lockDuration < config.minimumLockTime) { revert LockTimeTooShort(); } @@ -51,7 +51,7 @@ contract StakingERC20 is StakingBase, IStakingERC20 { * * @param amount The amount to stake */ - function stakeWithoutLock(uint256 amount) payable external override { + function stakeWithoutLock(uint256 amount) external payable override { _stake(amount, 0); } diff --git a/contracts/staking/ERC721/StakingERC721.sol b/contracts/staking/ERC721/StakingERC721.sol index eb66cf0..912a052 100644 --- a/contracts/staking/ERC721/StakingERC721.sol +++ b/contracts/staking/ERC721/StakingERC721.sol @@ -4,8 +4,6 @@ pragma solidity 0.8.26; import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; import { IERC721 } from "@openzeppelin/contracts/token/ERC721/IERC721.sol"; import { SafeERC20 } from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; -import { ERC721 } from "@openzeppelin/contracts/token/ERC721/ERC721.sol"; -import { ERC721URIStorage } from "@openzeppelin/contracts/token/ERC721/extensions/ERC721URIStorage.sol"; import { IStakingERC721 } from "./IStakingERC721.sol"; import { StakingBase } from "../StakingBase.sol"; import { IERC721MintableBurnableURIStorage } from "../../types/IERC721MintableBurnableURIStorage.sol"; diff --git a/contracts/staking/IStakingBase.sol b/contracts/staking/IStakingBase.sol index 7848d44..96c66ae 100644 --- a/contracts/staking/IStakingBase.sol +++ b/contracts/staking/IStakingBase.sol @@ -1,7 +1,6 @@ // SPDX-License-Identifier: MIT pragma solidity 0.8.26; -import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; /** * @title IStakingBase @@ -185,6 +184,10 @@ interface IStakingBase { */ error InitializedWithZero(); + receive() external payable; + + fallback() external payable; + function withdrawLeftoverRewards() external; function setRewardsPerPeriod(uint256 _rewardsPerPeriod) external; diff --git a/contracts/staking/StakingBase.sol b/contracts/staking/StakingBase.sol index 950c519..0d5d360 100644 --- a/contracts/staking/StakingBase.sol +++ b/contracts/staking/StakingBase.sol @@ -42,8 +42,9 @@ contract StakingBase is Ownable, ReentrancyGuard, IStakingBase { // We must be able to receive in the case that the // `stakingToken` is the chain's native token - receive() external payable {} - fallback() external payable {} + receive() external override payable {} + + fallback() external override payable {} /** * @notice Emergency function for the contract owner to withdraw leftover rewards @@ -291,6 +292,24 @@ contract StakingBase is Ownable, ReentrancyGuard, IStakingBase { emit Claimed(msg.sender, rewards); } + /** + * @dev Transfer funds to a recipient after deciding whether to use + * native or ERC20 tokens + * + * We give `token` as an argument here because in ERC721 it is always the + * reward token to transfer, but in ERC20 it could be either staking or rewards + * token and we won't know which to check. + */ + function _transferAmount(address token, uint256 amount) internal { + if (token == address(0)) { + if (address(this).balance < amount) revert InsufficientContractBalance(); + + payable(msg.sender).transfer(amount); + } else { + IERC20(token).safeTransfer(msg.sender, amount); + } + } + /** * @dev Calculate the time remaining for a staker's lock. Return 0 if no locked funds or if passed lock time * @param staker The staker to get the lock time for @@ -385,22 +404,4 @@ contract StakingBase is Ownable, ReentrancyGuard, IStakingBase { return IERC20(config.rewardsToken).balanceOf(address(this)); } } - - /** - * @dev Transfer funds to a recipient after deciding whether to use - * native or ERC20 tokens - * - * We give `token` as an argument here because in ERC721 it is always the - * reward token to transfer, but in ERC20 it could be either staking or rewards - * token and we won't know which to check. - */ - function _transferAmount(address token, uint256 amount) internal { - if (token == address(0)) { - if (address(this).balance < amount) revert InsufficientContractBalance(); - - payable(msg.sender).transfer(amount); - } else { - IERC20(token).safeTransfer(msg.sender, amount); - } - } } diff --git a/test/helpers/errors.ts b/test/helpers/errors.ts index 51769b6..0b15895 100644 --- a/test/helpers/errors.ts +++ b/test/helpers/errors.ts @@ -18,6 +18,7 @@ export const OPERATOR_NOT_ASSIGNED_ERR = "OperatorNotAssigned"; // StakingERC20 export const UNEQUAL_UNSTAKE_ERR = "UnstakeMoreThanStake"; export const ZERO_REWARDS_ERR = "ZeroRewards"; +export const INSUFFICIENT_VALUE_ERR = "InsufficientValue"; // StakingERC721 export const TIME_LOCK_NOT_PASSED_ERR = "TimeLockNotPassed"; diff --git a/test/helpers/staking/defaults.ts b/test/helpers/staking/defaults.ts index f302f3d..6cd7838 100644 --- a/test/helpers/staking/defaults.ts +++ b/test/helpers/staking/defaults.ts @@ -70,7 +70,7 @@ export const getDefaultERC20Setup = async ( rewardsToken : MockERC20, stakeToken : MockERC20, stakeRepToken : ZeroVotingERC20, -): Promise<[StakingERC20, BaseConfig]> => { +) : Promise<[StakingERC20, BaseConfig]> => { const config = await createDefaultStakingConfig( owner, rewardsToken, @@ -87,7 +87,7 @@ export const getDefaultERC20Setup = async ( await stakeRepToken.connect(owner).grantRole(await stakeRepToken.BURNER_ROLE(), await contract.getAddress()); return [contract, config]; -} +}; export const getNativeSetupERC20 = async ( owner : SignerWithAddress, @@ -109,10 +109,10 @@ export const getNativeSetupERC20 = async ( await stakeRepToken.connect(owner).grantRole(await stakeRepToken.BURNER_ROLE(), contractAddress); // Provide rewards funding in native token - fundRewards(contractAddress); + await fundRewards(contractAddress); return contract; -} +}; export const getNativeSetupERC721 = async ( owner : SignerWithAddress, @@ -136,10 +136,10 @@ export const getNativeSetupERC721 = async ( await stakeRepToken.connect(owner).grantRole(await stakeRepToken.BURNER_ROLE(), contractAddress); // Provide rewards funding in native token - await fundRewards(contractAddress) + await fundRewards(contractAddress); return contract; -} +}; // Fund users and approve for an amount export const fundAndApprove = async ( @@ -149,16 +149,17 @@ export const fundAndApprove = async ( contractAddress : string, amount ?: bigint, ) => { - for (let i = 0; i < addresses.length; i++) { + for (const address of addresses) { await stakeToken.connect(owner).transfer( - addresses[i].address, amount ?? INIT_BALANCE + address, + amount ?? INIT_BALANCE ); - await stakeToken.connect(addresses[i]).approve( + await stakeToken.connect(address).approve( contractAddress, amount ?? INIT_BALANCE ); } -} +}; const fundRewards = async (contractAddress : string) => { @@ -166,5 +167,5 @@ const fundRewards = async (contractAddress : string) => { contractAddress, `0x${hre.ethers.parseEther("999999999").toString()}`, ] -); -} \ No newline at end of file + ); +}; \ No newline at end of file diff --git a/test/staking-base.test.ts b/test/staking-base.test.ts index e936372..00d505e 100644 --- a/test/staking-base.test.ts +++ b/test/staking-base.test.ts @@ -9,7 +9,6 @@ import { OWNABLE_UNAUTHORIZED_ERR } from "./helpers/errors"; describe("StakingBase Unit Tests", () => { let owner : SignerWithAddress; let user : SignerWithAddress; - let mockAcc : SignerWithAddress; let initialConfig : BaseConfig; @@ -19,7 +18,7 @@ describe("StakingBase Unit Tests", () => { let mockErc3 : MockERC20; before(async () => { - [owner, user, mockAcc] = await hre.ethers.getSigners(); + [owner, user] = await hre.ethers.getSigners(); const erc20Fact = await hre.ethers.getContractFactory("MockERC20"); mockErc1 = await erc20Fact.deploy("reward", "symbol"); @@ -29,8 +28,8 @@ describe("StakingBase Unit Tests", () => { const fact = await hre.ethers.getContractFactory("StakingBase"); initialConfig = await createDefaultStakingConfig( - mockErc1, owner, + mockErc1, undefined, mockErc2, mockErc3 as ZeroVotingERC20, diff --git a/test/staking20.test.ts b/test/staking20.test.ts index 2e10e66..c0b55cd 100644 --- a/test/staking20.test.ts +++ b/test/staking20.test.ts @@ -16,6 +16,7 @@ import { LOCK_TOO_SHORT_ERR, INSUFFICIENT_CONTRACT_BALANCE_ERR, NOT_FULL_EXIT_ERR, + INSUFFICIENT_VALUE_ERR, } from "./helpers/errors"; import { WITHDRAW_EVENT, @@ -85,7 +86,7 @@ describe("StakingERC20", () => { rewardsToken, stakeToken, stakeRepToken, - ) + ); // Give each user funds to stake await fundAndApprove( @@ -115,7 +116,7 @@ describe("StakingERC20", () => { value: hre.ethers.parseEther("9999"), }); - const stakeAmount = DEFAULT_STAKED_AMOUNT + const stakeAmount = DEFAULT_STAKED_AMOUNT; await localContract.connect(stakerA).stakeWithoutLock( stakeAmount, { @@ -287,7 +288,6 @@ describe("StakingERC20", () => { ); await contract.connect(stakerA).stakeWithLock(addedStake, addedStakeLock); - const latest = BigInt(await time.latest()); const stakerDataAfter = await contract.stakers(stakerA.address); // The time in between stake A and B should be rewarded at rate 1.0 @@ -397,7 +397,13 @@ describe("StakingERC20", () => { }); it("Fails when using native token and `amount` does not equal `msg.value`", async () => { - + await expect( + contract.connect(stakerA).stakeWithoutLock(DEFAULT_STAKED_AMOUNT, + { + value: DEFAULT_STAKED_AMOUNT - 1n, + } + ) + ).to.be.revertedWithCustomError(contract, INSUFFICIENT_VALUE_ERR); }); it("Fails when the staker locks for less than the minimum lock time", async () => { @@ -592,7 +598,6 @@ describe("StakingERC20", () => { await reset(); await contract.connect(stakerA).stakeWithLock(DEFAULT_STAKED_AMOUNT, DEFAULT_LOCK); - const stakedAt = BigInt(await time.latest()); await time.increase(DEFAULT_LOCK / 4n); @@ -1047,8 +1052,6 @@ describe("StakingERC20", () => { ); await contract.connect(stakerA).unstake(DEFAULT_STAKED_AMOUNT, true); - const unstakedAt = BigInt(await time.latest()); - const rewardsBalanceAfter = await rewardsToken.balanceOf(stakerA.address); // should receive no rewards @@ -1181,7 +1184,6 @@ describe("StakingERC20", () => { describe("#getTotalPendingRewards", () => { let stakedAt : bigint; - let stakedAtLocked : bigint; it("Allows the user to view the total pending rewards when passed lock time", async () => { await reset(); @@ -1190,7 +1192,6 @@ describe("StakingERC20", () => { stakedAt = BigInt(await time.latest()); await contract.connect(stakerA).stakeWithLock(DEFAULT_STAKED_AMOUNT, DEFAULT_LOCK); - stakedAtLocked = BigInt(await time.latest()); const stakerData = await contract.stakers(stakerA.address); @@ -1217,12 +1218,6 @@ describe("StakingERC20", () => { const timeIncrease = 67n; - const unlockedStakeValue = await contract.getStakeRewards( - DEFAULT_STAKED_AMOUNT, - BigInt(await time.latest()) - stakedAt, - true - ); - // Time lock + arbitrary additional time await time.increase(DEFAULT_LOCK + timeIncrease); @@ -1302,7 +1297,7 @@ describe("StakingERC20", () => { const contrBalBefore = await hre.ethers.provider.getBalance(await localContract.getAddress()); // #stake - const stakeAmount = DEFAULT_STAKED_AMOUNT + const stakeAmount = DEFAULT_STAKED_AMOUNT; const stakeTx = await localContract.connect(stakerA).stakeWithoutLock( stakeAmount, { @@ -1325,7 +1320,7 @@ describe("StakingERC20", () => { const contrRewardsBalBefore = await hre.ethers.provider.getBalance(await localContract.getAddress()); const contrRewardsBalBeforeFromContract = await localContract.getContractRewardsBalance(); - // If `stakingToken` and `rewardsToken` are the same, we subtract the difference when checking + // If `stakingToken` and `rewardsToken` are the same, we subtract the difference when checking // the balance. Verify this here. expect(contrRewardsBalBeforeFromContract).to.eq(contrRewardsBalBefore - await localContract.totalStaked()); @@ -1522,7 +1517,6 @@ describe("StakingERC20", () => { describe("Events", () => { let stakedAt : bigint; - let stakedAtLocked : bigint; let claimedAt : bigint; let unstakedAt : bigint; @@ -1542,8 +1536,6 @@ describe("StakingERC20", () => { contract.connect(stakerF).stakeWithLock(DEFAULT_STAKED_AMOUNT, DEFAULT_LOCK) ).to.emit(contract, STAKED_EVENT) .withArgs(stakerF.address, DEFAULT_STAKED_AMOUNT, DEFAULT_LOCK); - - stakedAtLocked = BigInt(await time.latest()); }); it("Emits a Claimed event when a user claims rewards", async () => { @@ -1604,7 +1596,6 @@ describe("StakingERC20", () => { it("Emits an Unstaked event when unstaking locked funds passed the lock period", async () => { await contract.connect(stakerA).stakeWithLock(DEFAULT_STAKED_AMOUNT, DEFAULT_LOCK); - stakedAtLocked = BigInt(await time.latest()); await time.increase(DEFAULT_LOCK); diff --git a/test/staking721.test.ts b/test/staking721.test.ts index cb7e9a6..e59aa7b 100644 --- a/test/staking721.test.ts +++ b/test/staking721.test.ts @@ -19,7 +19,6 @@ import { calcTotalUnlockedRewards, PRECISION_DIVISOR, LOCKED_PRECISION_DIVISOR, - DAY_IN_SECONDS, calcStakeRewards, DEFAULT_MINIMUM_LOCK, DEFAULT_MINIMUM_RM, @@ -30,7 +29,6 @@ import { FAILED_INNER_CALL_ERR, FUNCTION_SELECTOR_ERR, ZERO_INIT_ERR, - NON_TRANSFERRABLE_ERR, INCORRECT_OWNER_ERR, NONEXISTENT_TOKEN_ERR, INSUFFICIENT_APPROVAL_721_ERR, OWNABLE_UNAUTHORIZED_ERR, @@ -53,10 +51,6 @@ describe("StakingERC721", () => { let stakingToken : MockERC721; let stakeRepToken : ZeroVotingERC721; - let stakingERC721Address : string; - let rewardTokenAddress : string; - let stakingTokenAddress : string; - // We don't use `PoolConfig` anymore on the contracts but for convenience in testing // we can leave this type where it is let config : BaseConfig; @@ -115,8 +109,6 @@ describe("StakingERC721", () => { const stakeRepFactory = await hre.ethers.getContractFactory("ZeroVotingERC721"); stakeRepToken = await stakeRepFactory.deploy("VotingToken", "VNFT", "1.0", baseUri, owner.address); - stakingTokenAddress = await stakingToken.getAddress(); - config = await createDefaultStakingConfig( owner, rewardToken, @@ -138,7 +130,7 @@ describe("StakingERC721", () => { await rewardToken.connect(owner).mint( stakingERC721Address, hre.ethers.parseEther("999999999") - ) + ); await stakingToken.mint(stakerA.address, tokenIdA); await stakingToken.mint(stakerA.address, tokenIdB); @@ -148,7 +140,7 @@ describe("StakingERC721", () => { await stakingToken.mint(stakerB.address, tokenIdE); await stakingToken.mint(stakerB.address, tokenIdF); await stakingToken.mint(stakerB.address, tokenIdG); - + await stakingToken.mint(stakerC.address, tokenIdH); await stakingToken.mint(stakerC.address, tokenIdI); await stakingToken.mint(stakerC.address, tokenIdJ); @@ -184,10 +176,6 @@ describe("StakingERC721", () => { expect(await stakeRepToken.balanceOf(stakerA.address)).to.eq(1); expect(await stakeRepToken.balanceOf(stakerB.address)).to.eq(1); - // now call to claim - const stakerABalanceBefore = await rewardToken.balanceOf(stakerA.address); - const stakerBBalanceBefore = await rewardToken.balanceOf(stakerB.address); - await stakingERC721.connect(stakerA).claim(); claimedAtA = BigInt(await time.latest()); @@ -448,14 +436,12 @@ describe("StakingERC721", () => { await stakingERC721.connect(stakerB).stakeWithLock([tokenIdD], [emptyUri], DEFAULT_LOCK); const stakerDataBefore = await stakingERC721.connect(stakerB).nftStakers(stakerB.address); - const stakedAtFirst = BigInt(await time.latest()); const timeIncrease = 100n; await time.increase(timeIncrease); // Existing lock is `DEFAULT_LOCK - 100s` now, so the incoming lock will be smaller and it should not update await stakingERC721.connect(stakerB).stakeWithLock([tokenIdE], [emptyUri], DEFAULT_LOCK / 2n); - const stakedAtSecond = BigInt(await time.latest()); const stakerDataAfter = await stakingERC721.connect(stakerB).nftStakers(stakerB.address); @@ -531,9 +517,6 @@ describe("StakingERC721", () => { await reset(); await stakingERC721.connect(stakerB).stakeWithLock([tokenIdG], [emptyUri], DEFAULT_LOCK); - const stakedAt = BigInt(await time.latest()); - - const tokenIds = await stakingERC721.connect(stakerB).getStakedTokenIds(); const initPendingRewards = await stakingERC721.connect(stakerB).getPendingRewards(); @@ -541,7 +524,6 @@ describe("StakingERC721", () => { expect(initPendingRewards).to.eq(0); await time.increase(DEFAULT_LOCK / 2n); - const latest = BigInt(await time.latest()); const stakerData = await stakingERC721.nftStakers(stakerB.address); @@ -591,8 +573,6 @@ describe("StakingERC721", () => { const balanceBefore = await rewardToken.balanceOf(stakerA.address); - const pendingRewards = await stakingERC721.connect(stakerA).getPendingRewards(); - await stakingERC721.connect(stakerA).claim(); const claimedAt = BigInt(await time.latest()); @@ -759,21 +739,6 @@ describe("StakingERC721", () => { config ); - const expectedLockedStakeValue = calcStakeRewards( - stakerDataBefore.amountStakedLocked, - DEFAULT_LOCK, - true, - config, - stakerDataBefore.rewardsMultiplier - ); - - const expectedLockedInterimRewards = calcStakeRewards( - stakerDataBefore.amountStakedLocked, - unstakedAt - stakerDataBefore.unlockedTimestamp, - false, - config - ); - const balanceAfter = await rewardToken.balanceOf(stakerA.address); expect(balanceAfter).to.eq(balanceBefore + expectedUnlocked); @@ -831,13 +796,6 @@ describe("StakingERC721", () => { stakerDataBefore.rewardsMultiplier ); - const unlockedStakeRewards = calcStakeRewards( - stakerDataBefore.amountStaked, - unstakedAt - stakedAtUnlocked, - false, - config - ); - const interimTimeRewards = calcStakeRewards( stakerDataBefore.amountStakedLocked, unstakedAt - stakerDataBefore.unlockedTimestamp, @@ -890,15 +848,9 @@ describe("StakingERC721", () => { it("Fails to unstake when token id is invalid", async () => { await time.increase(DEFAULT_LOCK); - // if unstake with a valid and not valid token, the not valid one is skipped - // so it doesnt revert, but doesnt do anything with the extra either - // Desired? Or should we revert when we find this instead? - // try { - // await stakingERC721.connect(stakerA).unstake([tokenIdA, unmintedTokenId], false) - // } catch(e) { - // console.log((e as Error).message); - // } - + // If unstake with two tokens, one valid and one not valid token, + // the not valid one is skipped not reverted. Nothing happens, so at + // worst the user wastes some gas await expect( stakingERC721.connect(stakerA).unstake([unmintedTokenId], false) ).to.be.revertedWithCustomError(stakingERC721, INVALID_UNSTAKE_ERR); @@ -906,7 +858,7 @@ describe("StakingERC721", () => { // Confirm with `exit` await expect( stakingERC721.connect(stakerA).unstake([unmintedTokenId], true) - ).to.be.revertedWithCustomError(stakingERC721, INVALID_UNSTAKE_ERR) + ).to.be.revertedWithCustomError(stakingERC721, INVALID_UNSTAKE_ERR); }); it("Fails to unstake when caller is not the owner of the SNFT", async () => { @@ -925,7 +877,7 @@ describe("StakingERC721", () => { // If the a token is not staked, the relevant does not exist and so we can't unstake it await expect( stakingERC721.connect(stakerA).unstake([unStakedTokenId], false) - ).to.be.revertedWithCustomError(stakingERC721, INVALID_UNSTAKE_ERR) + ).to.be.revertedWithCustomError(stakingERC721, INVALID_UNSTAKE_ERR); }); }); @@ -941,7 +893,7 @@ describe("StakingERC721", () => { // as if the owner has already exited await expect( stakingERC721.connect(stakerB).unstake([unmintedTokenId], true) - ).to.be.revertedWithCustomError(stakingERC721, INVALID_UNSTAKE_ERR) + ).to.be.revertedWithCustomError(stakingERC721, INVALID_UNSTAKE_ERR); }); it("Allows the user to remove their stake within the timelock period without rewards", async () => { @@ -1108,8 +1060,6 @@ describe("StakingERC721", () => { await time.increase(DEFAULT_LOCK / 5n); - const balanceBefore = await rewardToken.balanceOf(stakerA.address); - const latest = BigInt(await time.latest()); const futureExpectedRewards = calcTotalUnlockedRewards( [latest - stakedAt], @@ -1130,17 +1080,17 @@ describe("StakingERC721", () => { describe("Other configs", () => { it("Can set the rewards token as native token", async () => { const localContract : StakingERC721 = await getNativeSetupERC721( - owner, + owner, stakingToken, stakeRepToken - ) + ); await stakingToken.connect(stakerA).approve(await localContract.getAddress(), tokenIdA); await stakingToken.connect(stakerA).approve(await localContract.getAddress(), tokenIdB); await localContract.connect(stakerA).stakeWithoutLock([tokenIdA, tokenIdB], [emptyUri, emptyUri]); const stakedAt = BigInt(await time.latest()); - + await time.increase(DEFAULT_LOCK / 5n); const rewardsBefore = await hre.ethers.provider.getBalance(stakerA.address); @@ -1243,10 +1193,10 @@ describe("StakingERC721", () => { await reset(); const localContract : StakingERC721 = await getNativeSetupERC721( - owner, + owner, stakingToken, stakeRepToken - ) + ); await stakingToken.connect(stakerA).approve(await localContract.getAddress(), tokenIdA); await stakingToken.connect(stakerA).approve(await localContract.getAddress(), tokenIdB); @@ -1279,7 +1229,6 @@ describe("StakingERC721", () => { await time.increase(DEFAULT_LOCK / 3n); const rewardsBefore = await hre.ethers.provider.getBalance(stakerA.address); - const rewardsBeforeContr = await hre.ethers.provider.getBalance(await localContract.getAddress()); // claim const claimTx = await localContract.connect(stakerA).claim(); @@ -1294,7 +1243,7 @@ describe("StakingERC721", () => { false, config ); - + const rewardsAfterClaim = await hre.ethers.provider.getBalance(stakerA.address); // Confirm we don't receive rewards from stake that is still locked when calling `claim` @@ -1335,9 +1284,9 @@ describe("StakingERC721", () => { const rewardsAfterFullClaim = await hre.ethers.provider.getBalance(stakerA.address); - const calcedVal = rewardsAfterClaim + lockedRewards + interimRewards + unlockedRewards - (fullClaimReceipt!.gasUsed * fullClaimReceipt!.gasPrice) expect(rewardsAfterFullClaim).to.eq( - rewardsAfterClaim + lockedRewards + interimRewards + unlockedRewards - (fullClaimReceipt!.gasUsed * fullClaimReceipt!.gasPrice) + rewardsAfterClaim + lockedRewards + interimRewards + unlockedRewards + - (fullClaimReceipt!.gasUsed * fullClaimReceipt!.gasPrice) ); expect(dataAfterFullClaim.owedRewards).to.eq(0n); @@ -1350,7 +1299,7 @@ describe("StakingERC721", () => { // Partial unstake const partialUnstakeTx = await localContract.connect(stakerA).unstake([tokenIdA], false); const partialUnstakedAt = BigInt(await time.latest()); - + const partialUnstakeReceipt = await partialUnstakeTx.wait(); const dataAfterPartialUnstake = await localContract.nftStakers(stakerA.address); @@ -1400,7 +1349,8 @@ describe("StakingERC721", () => { ); expect(rewardsAfterFullUnstake).to.eq( - rewardsAfterUnstake + fullUnstakeRewards + fullInterimRewards - (fullUnstakeReceipt!.gasUsed * fullUnstakeReceipt!.gasPrice) + rewardsAfterUnstake + fullUnstakeRewards + fullInterimRewards + - (fullUnstakeReceipt!.gasUsed * fullUnstakeReceipt!.gasPrice) ); const stakerDataFinal = await localContract.nftStakers(stakerA.address); @@ -1505,13 +1455,10 @@ describe("StakingERC721", () => { // To be sure balance check isn't the failure, we give balance of many NFTs so the // number is similar - const pendingRewardsTotal = await localStakingERC721.connect(stakerA).getPendingRewards(); const pendingRewards = await localStakingERC721.connect(stakerA).getPendingRewards(); - const bal = await stakingToken.balanceOf(await localStakingERC721.getAddress()); - // Provide enough rewards so the contract passes the "No rewards balance" error - // 14 is offset for number already in existence, and 100 is a buffer for amount over the rewards we need + // 14 is offset for number of tokens already minted, and 100 is a buffer for amount over the rewards we need for (let i = 14; i < pendingRewards + 100n; i++) { await stakingToken.connect(stakerA).mint(await localStakingERC721.getAddress(), i); } @@ -1557,10 +1504,6 @@ describe("StakingERC721", () => { await time.increase(DEFAULT_LOCK / 3n); - const rewardsBalance = await rewardToken.balanceOf(await localStakingERC721.getAddress()); - - // TODO debug, why is this passing below but we show 0 balance for rewards when check? - // this should revert but doesnt const rewardsInPool = await localStakingERC721.getContractRewardsBalance(); expect(rewardsInPool).to.eq(0); @@ -1587,7 +1530,7 @@ describe("StakingERC721", () => { timeDuration, true, config - ) + ); expect(stakeValue).to.eq(locked); From 1d9d6d1428a9becc054800d570fe688df6604d23 Mon Sep 17 00:00:00 2001 From: James Earle Date: Tue, 31 Dec 2024 10:47:03 -0500 Subject: [PATCH 7/7] fix test for amount != msg.value, use correct local config contract --- test/staking20.test.ts | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/test/staking20.test.ts b/test/staking20.test.ts index c0b55cd..badb227 100644 --- a/test/staking20.test.ts +++ b/test/staking20.test.ts @@ -396,16 +396,6 @@ describe("StakingERC20", () => { expect(stakerData.owedRewards).to.eq(0n); }); - it("Fails when using native token and `amount` does not equal `msg.value`", async () => { - await expect( - contract.connect(stakerA).stakeWithoutLock(DEFAULT_STAKED_AMOUNT, - { - value: DEFAULT_STAKED_AMOUNT - 1n, - } - ) - ).to.be.revertedWithCustomError(contract, INSUFFICIENT_VALUE_ERR); - }); - it("Fails when the staker locks for less than the minimum lock time", async () => { await expect( contract.connect(stakerA).stakeWithLock(DEFAULT_STAKED_AMOUNT, DEFAULT_MINIMUM_LOCK - 1n) @@ -1407,6 +1397,21 @@ describe("StakingERC20", () => { ); }); + it("Fails when using native token and `amount` does not equal `msg.value`", async () => { + const localContract = await getNativeSetupERC20( + owner, + stakeRepToken + ); + + await expect( + localContract.connect(stakerA).stakeWithoutLock(DEFAULT_STAKED_AMOUNT, + { + value: DEFAULT_STAKED_AMOUNT - 1n, + } + ) + ).to.be.revertedWithCustomError(contract, INSUFFICIENT_VALUE_ERR); + }); + it("Stakes, claims, and unstakes correctly with an entirely different config", async () => { // Even though we are manipulating the config here we still reset to be sure all token balances are what we expect await reset();