Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

No deposit fee #117

Closed
wants to merge 10 commits into from
Closed

No deposit fee #117

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
ee4426b
Add deposit fee exemption for EOAs with Smol Joes
cryptofish7 Sep 2, 2022
80e9db9
Add test for deposit fee exemption for EOAs with Smol Joes
cryptofish7 Sep 2, 2022
e68cd09
Modify deploy script for sJOE
cryptofish7 Sep 2, 2022
b9048ef
Add Louis' nits
cryptofish7 Sep 3, 2022
1d5a73b
Init implementation
cryptofish7 Sep 3, 2022
8df35d7
Change setter to initializer
cryptofish7 Sep 3, 2022
b005318
Change init function on deploy script
cryptofish7 Sep 3, 2022
a468913
Add fixes
cryptofish7 Sep 13, 2022
138c840
Fix conditional
cryptofish7 Sep 15, 2022
fd88471
Make smolJoes var public
cryptofish7 Sep 15, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 30 additions & 4 deletions contracts/StableJoeStaking.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";
import "@openzeppelin/contracts-upgradeable/math/SafeMathUpgradeable.sol";
import "@openzeppelin/contracts-upgradeable/proxy/Initializable.sol";
import "@openzeppelin/contracts-upgradeable/token/ERC20/IERC20Upgradeable.sol";
import "@openzeppelin/contracts-upgradeable/token/ERC721/IERC721Upgradeable.sol";
import "@openzeppelin/contracts-upgradeable/token/ERC20/SafeERC20Upgradeable.sol";

/**
Expand Down Expand Up @@ -66,6 +67,9 @@ contract StableJoeStaking is Initializable, OwnableUpgradeable {
/// @dev Info of each user that stakes JOE
mapping(address => UserInfo) private userInfo;

/// @dev Smol Joes contract address
IERC721Upgradeable smolJoes;
Copy link
Contributor

Choose a reason for hiding this comment

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

NIT: Mark this variable explicitly as public (default if not specified) to match the style of other variables.


/// @notice Emitted when a user deposits JOE
event Deposit(address indexed user, uint256 amount, uint256 fee);

Expand All @@ -87,6 +91,9 @@ contract StableJoeStaking is Initializable, OwnableUpgradeable {
/// @notice Emitted when owner removes a token from the reward tokens list
event RewardTokenRemoved(address token);

/// @notice Emitted when Smol Joes address is changed
event SmolJoesChanged(IERC721Upgradeable newSmolJoes, IERC721Upgradeable oldSmolJoes);

/**
* @notice Initialize a new StableJoeStaking contract
* @dev This contract needs to receive an ERC20 `_rewardToken` in order to distribute them
Expand All @@ -95,20 +102,24 @@ contract StableJoeStaking is Initializable, OwnableUpgradeable {
* @param _joe The address of the JOE token
* @param _feeCollector The address where deposit fees will be sent
* @param _depositFeePercent The deposit fee percent, scalled to 1e18, e.g. 3% is 3e16
* @param _smolJoes The address of ERC721 Smol Joes NFT
*/
function initialize(
IERC20Upgradeable _rewardToken,
IERC20Upgradeable _joe,
address _feeCollector,
uint256 _depositFeePercent
uint256 _depositFeePercent,
IERC721Upgradeable _smolJoes
Copy link
Contributor

Choose a reason for hiding this comment

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

Add a setter for smolJoe's address as we can't use initialize during an upgrade as the contract has already been initialized.

) external initializer {
__Ownable_init();
require(address(_rewardToken) != address(0), "StableJoeStaking: reward token can't be address(0)");
require(address(_joe) != address(0), "StableJoeStaking: joe can't be address(0)");
require(address(_smolJoes) != address(0), "StableJoeStaking: smol joes can't be address(0)");
require(_feeCollector != address(0), "StableJoeStaking: fee collector can't be address(0)");
require(_depositFeePercent <= 5e17, "StableJoeStaking: max deposit fee can't be greater than 50%");

joe = _joe;
smolJoes = _smolJoes;
depositFeePercent = _depositFeePercent;
feeCollector = _feeCollector;

Expand All @@ -125,7 +136,11 @@ contract StableJoeStaking is Initializable, OwnableUpgradeable {
function deposit(uint256 _amount) external {
UserInfo storage user = userInfo[_msgSender()];

uint256 _fee = _amount.mul(depositFeePercent).div(DEPOSIT_FEE_PERCENT_PRECISION);
uint256 _fee;
// Only EOAs holding Smol Joes are exempt from paying the deposit fee
if (smolJoes.balanceOf(_msgSender()) == 0 || _msgSender() != tx.origin) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This will revert on smolJoes being address 0, (this will be the case during upgrade)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I check that it's not address(0) when I initialize it though?

Copy link
Contributor

@jummy123 jummy123 Sep 9, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I believe is talking about the case between when the contract is upgraded and the initializeSmalJoes function is called (which may be a very small amount of time). You could do this in a backwards compatible way by changing this to
if (address(smolJoes) != address(0) && smolJoes.balanceOf(_msgSender()) == 0 || _msgSender() != tx.origin) this works due to the short circuiting behaviour of && and ||

_fee = _amount.mul(depositFeePercent).div(DEPOSIT_FEE_PERCENT_PRECISION);
}
uint256 _amountMinusFee = _amount.sub(_fee);

uint256 _previousAmount = user.amount;
Expand Down Expand Up @@ -153,8 +168,8 @@ contract StableJoeStaking is Initializable, OwnableUpgradeable {
}

internalJoeBalance = internalJoeBalance.add(_amountMinusFee);
joe.safeTransferFrom(_msgSender(), feeCollector, _fee);
joe.safeTransferFrom(_msgSender(), address(this), _amountMinusFee);
if (_fee > 0) joe.safeTransferFrom(_msgSender(), feeCollector, _fee);
if (_amountMinusFee > 0) joe.safeTransferFrom(_msgSender(), address(this), _amountMinusFee);
emit Deposit(_msgSender(), _amountMinusFee, _fee);
}

Expand Down Expand Up @@ -224,6 +239,17 @@ contract StableJoeStaking is Initializable, OwnableUpgradeable {
emit DepositFeeChanged(_depositFeePercent, oldFee);
}

/**
* @notice Set the Smol Joes address
* @param _newSmolJoes The new Smol Joes contract address
*/
function setSmolJoes(IERC721Upgradeable _newSmolJoes) external onlyOwner {
require(address(_newSmolJoes) != address(0), "StableJoeStaking: smol joes can't be address(0)");
IERC721Upgradeable oldSmolJoes = smolJoes;
smolJoes = _newSmolJoes;
emit SmolJoesChanged(_newSmolJoes, oldSmolJoes);
}

/**
* @notice View function to see pending reward token on frontend
* @param _user The address of the user
Expand Down
16 changes: 16 additions & 0 deletions contracts/mocks/ERC721Mock.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// SPDX-License-Identifier: MIT

pragma solidity 0.6.12;

import "@openzeppelin/contracts/token/ERC721/ERC721.sol";

contract ERC721Mock is ERC721 {
uint256 tokenOwners;

constructor(string memory name, string memory symbol) public ERC721(name, symbol) {}

function mint(address _account) external {
_mint(_account, tokenOwners);
tokenOwners += 1;
}
}
22 changes: 22 additions & 0 deletions contracts/mocks/StableJoeVaultMock.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// SPDX-License-Identifier: MIT

pragma solidity 0.7.6;

import "@openzeppelin/contracts/token/ERC20/ERC20.sol";
import "../StableJoeStaking.sol";

contract StableJoeVaultMock {
IERC20 public immutable joe;
StableJoeStaking public immutable sJoe;

constructor(IERC20 _joe, StableJoeStaking _sJoe) public {
joe = _joe;
sJoe = _sJoe;
}

function deposit(uint256 _amount) external {
joe.transferFrom(msg.sender, address(this), _amount);
joe.approve(address(sJoe), _amount);
sJoe.deposit(_amount);
}
}
36 changes: 30 additions & 6 deletions deploy/StableJoeStaking.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,12 @@ module.exports = async function ({ getNamedAccounts, deployments }) {
const { deploy, catchUnknownSigner } = deployments;
const { deployer } = await getNamedAccounts();

let rewardToken, joeAddress, feeCollector, depositFeePercent, proxyOwner;
let rewardToken,
joeAddress,
feeCollector,
depositFeePercent,
smolJoes,
proxyOwner;

const chainId = await getChainId();
if (chainId == 4) {
Expand All @@ -12,6 +17,7 @@ module.exports = async function ({ getNamedAccounts, deployments }) {
rewardToken = "0x9Ad6C38BE94206cA50bb0d90783181662f0Cfa10";
feeCollector = "0x2fbB61a10B96254900C03F1644E9e1d2f5E76DD2";
depositFeePercent = 0;
smolJoes = "0xce347E069B68C53A9ED5e7DA5952529cAF8ACCd4"; // use an ERC20 as placeholder for now
proxyOwner = deployer.address;
} else if (chainId == 43114 || chainId == 31337) {
// avalanche mainnet or hardhat network addresses
Expand All @@ -20,25 +26,43 @@ module.exports = async function ({ getNamedAccounts, deployments }) {
rewardToken = "0xB97EF9Ef8734C71904D8002F8b6Bc66Dd9c48a6E";
feeCollector = "0x2fbB61a10B96254900C03F1644E9e1d2f5E76DD2";
depositFeePercent = 0;
smolJoes = "0xC70DF87e1d98f6A531c8E324C9BCEC6FC82B5E8d";
proxyOwner = "0x2fbB61a10B96254900C03F1644E9e1d2f5E76DD2";
}

await catchUnknownSigner(
deploy("StableJoeStaking", {
await catchUnknownSigner(async () => {
const sJoe = await deploy("StableJoeStaking", {
from: deployer,
proxy: {
owner: proxyOwner,
proxyContract: "OpenZeppelinTransparentProxy",
execute: {
init: {
methodName: "initialize",
args: [rewardToken, joeAddress, feeCollector, depositFeePercent],
args: [
rewardToken,
joeAddress,
feeCollector,
depositFeePercent,
smolJoes,
],
},
},
},
Copy link
Contributor

@0x0Louis 0x0Louis Sep 2, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

For safety reasons, don't forget to initialize the implementation even if it's useless for transparent proxies.

log: true,
})
);
});
if (sJoe.newlyDeployed) {
console.log("Initializing implementation for safe measure...");
const impl = await ethers.getContract("StableJoeStaking_Implementation");
await impl.initialize(
rewardToken,
joeAddress,
feeCollector,
depositFeePercent,
smolJoes
);
}
});
};

module.exports.tags = ["StableJoeStaking"];
Expand Down
48 changes: 48 additions & 0 deletions test/StableJoeStaking.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,39 +9,54 @@ describe("Stable Joe Staking", function () {
this.StableJoeStakingCF = await ethers.getContractFactory(
"StableJoeStaking"
);
this.StableJoeVaultCF = await ethers.getContractFactory(
"StableJoeVaultMock"
);
this.JoeTokenCF = await ethers.getContractFactory("JoeToken");
this.SmolJoesCF = await ethers.getContractFactory("ERC721Mock");

this.signers = await ethers.getSigners();
this.dev = this.signers[0];
this.alice = this.signers[1];
this.bob = this.signers[2];
this.carol = this.signers[3];
this.dylan = this.signers[4];
this.joeMaker = this.signers[4];
this.penaltyCollector = this.signers[5];
});

beforeEach(async function () {
this.rewardToken = await this.JoeTokenCF.deploy();
this.joe = await this.JoeTokenCF.deploy();
this.smolJoes = await this.SmolJoesCF.deploy("Smol Joes", "SMOL JOES");

await this.joe.mint(this.alice.address, ethers.utils.parseEther("1000"));
await this.joe.mint(this.bob.address, ethers.utils.parseEther("1000"));
await this.joe.mint(this.carol.address, ethers.utils.parseEther("1000"));
await this.joe.mint(this.dylan.address, ethers.utils.parseEther("1000"));
await this.rewardToken.mint(
this.joeMaker.address,
ethers.utils.parseEther("1000000")
); // 1_000_000 tokens

await this.smolJoes.mint(this.dylan.address);

this.stableJoeStaking = await hre.upgrades.deployProxy(
this.StableJoeStakingCF,
[
this.rewardToken.address,
this.joe.address,
this.penaltyCollector.address,
ethers.utils.parseEther("0.03"),
this.smolJoes.address,
]
);

this.stableJoeVault = await this.StableJoeVaultCF.deploy(
this.joe.address,
this.stableJoeStaking.address
);

await this.joe
.connect(this.alice)
.approve(
Expand All @@ -60,6 +75,16 @@ describe("Stable Joe Staking", function () {
this.stableJoeStaking.address,
ethers.utils.parseEther("100000")
);
await this.joe
.connect(this.dylan)
.approve(
this.stableJoeStaking.address,
ethers.utils.parseEther("100000")
);

await this.joe
.connect(this.dylan)
.approve(this.stableJoeVault.address, ethers.utils.parseEther("100000"));
});

describe("should allow deposits and withdraws", function () {
Expand Down Expand Up @@ -672,6 +697,29 @@ describe("Stable Joe Staking", function () {
);
});

it("should allow EOAs with Smol Joes to be exempt from paying deposit fee", async function () {
await this.stableJoeStaking
.connect(this.dylan)
.deposit(ethers.utils.parseEther("100"));
Copy link
Contributor

@jummy123 jummy123 Nov 28, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

There doesn't look to be a bug there but for fuller test coverage you should also deposit via the vault before transferring the ERC721 to it.

expect(
await this.joe.balanceOf(this.stableJoeStaking.address)
).to.be.equal(ethers.utils.parseEther("100"));

// Transfer Smol Joe to vault contract and deposit via vault
await this.smolJoes
.connect(this.dylan)
.transferFrom(this.dylan.address, this.stableJoeVault.address, 0);
await this.stableJoeVault
.connect(this.dylan)
.deposit(ethers.utils.parseEther("100"));
expect(
await this.joe.balanceOf(this.stableJoeStaking.address)
).to.be.equal(ethers.utils.parseEther("197"));
expect(
await this.joe.balanceOf(this.penaltyCollector.address)
).to.be.equal(ethers.utils.parseEther("3"));
});

it("should allow emergency withdraw", async function () {
await this.stableJoeStaking
.connect(this.alice)
Expand Down