From eecf6f65a43d606fff203acc9ddcd7a5c92c887f Mon Sep 17 00:00:00 2001 From: Aodhgan Date: Fri, 2 Jul 2021 12:04:17 -0700 Subject: [PATCH 1/8] only run unit tests in coverage --- .solcover.js | 2 +- package.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.solcover.js b/.solcover.js index b1c5565..58e6bd1 100644 --- a/.solcover.js +++ b/.solcover.js @@ -1,3 +1,3 @@ module.exports = { - skipFiles: ["test/RNGServiceMock.sol"], + skipFiles: ["test/"], }; diff --git a/package.json b/package.json index 656ae5e..7ef1cae 100644 --- a/package.json +++ b/package.json @@ -42,7 +42,7 @@ "lint": "yarn solhint 'contracts/SushiYieldSource.sol' && yarn prettier -c './**/*.js'", "format": "yarn prettier --write contracts/*.sol && yarn prettier --write test/*.js", "hint": "solhint \"contracts/SushiYieldSource.sol\"", - "coverage": "FORK_MAINNET=true yarn hardhat coverage --testfiles \"test/*.js\"", + "coverage": "OPTIMIZER_DISABLED=true hardhat coverage --testfiles \"test/unit_test.js\"", "coverage:file": "yarn hardhat coverage --testfiles", "start-fork": "FORK_MAINNET=true hardhat node --no-reset", "fork-run": "hardhat run --network localhost" From 46387470f0edd8fc96abe21df5b689d2103ad806 Mon Sep 17 00:00:00 2001 From: Aodhgan Date: Fri, 2 Jul 2021 12:20:17 -0700 Subject: [PATCH 2/8] using lint .sol files --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 7ef1cae..eb1fa10 100644 --- a/package.json +++ b/package.json @@ -39,7 +39,7 @@ "etherscan-verify": "hardhat etherscan-verify --network", "test:integration": "FORK_MAINNET=true yarn hardhat test --network hardhat", "test": "yarn hardhat test --network hardhat test/unit_test.js", - "lint": "yarn solhint 'contracts/SushiYieldSource.sol' && yarn prettier -c './**/*.js'", + "lint": "yarn solhint 'contracts/SushiYieldSource.sol'", "format": "yarn prettier --write contracts/*.sol && yarn prettier --write test/*.js", "hint": "solhint \"contracts/SushiYieldSource.sol\"", "coverage": "OPTIMIZER_DISABLED=true hardhat coverage --testfiles \"test/unit_test.js\"", From 7b16fefd931858cb9b249e35fc670e522a83a1b8 Mon Sep 17 00:00:00 2001 From: Aodhgan Date: Tue, 29 Jun 2021 13:40:23 -0700 Subject: [PATCH 3/8] use add() --- contracts/SushiYieldSource.sol | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/contracts/SushiYieldSource.sol b/contracts/SushiYieldSource.sol index 5cbacf3..eb15b02 100644 --- a/contracts/SushiYieldSource.sol +++ b/contracts/SushiYieldSource.sol @@ -11,12 +11,12 @@ import "./ISushi.sol"; /// @title A pooltogether yield source for sushi token /// @author Steffel Fenix contract SushiYieldSource is IYieldSource { - + using SafeMath for uint256; - + ISushiBar public immutable sushiBar; ISushi public immutable sushiAddr; - + mapping(address => uint256) public balances; constructor(ISushiBar _sushiBar, ISushi _sushiAddr) public { @@ -38,7 +38,7 @@ contract SushiYieldSource is IYieldSource { uint256 totalShares = sushiBar.totalSupply(); uint256 barSushiBalance = sushiAddr.balanceOf(address(sushiBar)); - return balances[addr].mul(barSushiBalance).div(totalShares); + return balances[addr].mul(barSushiBalance).div(totalShares); } /// @notice Allows assets to be supplied on other user's behalf using the `to` param. @@ -50,44 +50,44 @@ contract SushiYieldSource is IYieldSource { ISushiBar bar = sushiBar; uint256 beforeBalance = bar.balanceOf(address(this)); - + bar.enter(amount); - + uint256 afterBalance = bar.balanceOf(address(this)); uint256 balanceDiff = afterBalance.sub(beforeBalance); - + balances[to] = balances[to].add(balanceDiff); } /// @notice Redeems tokens from the yield source to the msg.sender, it burns yield bearing tokens and returns token to the sender. /// @param amount The amount of `token()` to withdraw. Denominated in `token()` as above. /// @dev The maxiumum that can be called for token() is calculated by balanceOfToken() above. - /// @return The actual amount of tokens that were redeemed. This may be different from the amount passed due to the fractional math involved. + /// @return The actual amount of tokens that were redeemed. This may be different from the amount passed due to the fractional math involved. function redeemToken(uint256 amount) public override returns (uint256) { ISushiBar bar = sushiBar; ISushi sushi = sushiAddr; uint256 totalShares = bar.totalSupply(); - if(totalShares == 0) return 0; + if (totalShares == 0) return 0; uint256 barSushiBalance = sushi.balanceOf(address(bar)); - if(barSushiBalance == 0) return 0; + if (barSushiBalance == 0) return 0; uint256 sushiBeforeBalance = sushi.balanceOf(address(this)); - uint256 requiredShares = ((amount.mul(totalShares) + totalShares)).div(barSushiBalance); - if(requiredShares == 0) return 0; - + uint256 requiredShares = ((amount.mul(totalShares).add(totalShares))).div(barSushiBalance); + if (requiredShares == 0) return 0; + uint256 requiredSharesBalance = requiredShares.sub(1); bar.leave(requiredSharesBalance); uint256 sushiAfterBalance = sushi.balanceOf(address(this)); - + uint256 sushiBalanceDiff = sushiAfterBalance.sub(sushiBeforeBalance); balances[msg.sender] = balances[msg.sender].sub(requiredSharesBalance); sushi.transfer(msg.sender, sushiBalanceDiff); - + return (sushiBalanceDiff); } From 104615db9e17af04e01627c7752e8f8ca882dd2e Mon Sep 17 00:00:00 2001 From: kamescg Date: Tue, 29 Jun 2021 16:58:33 -0700 Subject: [PATCH 4/8] update public functions to external functions --- contracts/SushiYieldSource.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/contracts/SushiYieldSource.sol b/contracts/SushiYieldSource.sol index eb15b02..bdb3863 100644 --- a/contracts/SushiYieldSource.sol +++ b/contracts/SushiYieldSource.sol @@ -44,7 +44,7 @@ contract SushiYieldSource is IYieldSource { /// @notice Allows assets to be supplied on other user's behalf using the `to` param. /// @param amount The amount of `token()` to be supplied /// @param to The user whose balance will receive the tokens - function supplyTokenTo(uint256 amount, address to) public override { + function supplyTokenTo(uint256 amount, address to) external override { sushiAddr.transferFrom(msg.sender, address(this), amount); sushiAddr.approve(address(sushiBar), amount); @@ -63,7 +63,7 @@ contract SushiYieldSource is IYieldSource { /// @param amount The amount of `token()` to withdraw. Denominated in `token()` as above. /// @dev The maxiumum that can be called for token() is calculated by balanceOfToken() above. /// @return The actual amount of tokens that were redeemed. This may be different from the amount passed due to the fractional math involved. - function redeemToken(uint256 amount) public override returns (uint256) { + function redeemToken(uint256 amount) external override returns (uint256) { ISushiBar bar = sushiBar; ISushi sushi = sushiAddr; From 680b066729ce3065695809cf0f72fec674507abe Mon Sep 17 00:00:00 2001 From: kamescg Date: Tue, 29 Jun 2021 17:03:05 -0700 Subject: [PATCH 5/8] update public functions to external functions --- contracts/SushiYieldSource.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/contracts/SushiYieldSource.sol b/contracts/SushiYieldSource.sol index bdb3863..4c79cfb 100644 --- a/contracts/SushiYieldSource.sol +++ b/contracts/SushiYieldSource.sol @@ -26,13 +26,13 @@ contract SushiYieldSource is IYieldSource { /// @notice Returns the ERC20 asset token used for deposits. /// @return The ERC20 asset token - function depositToken() public view override returns (address) { + function depositToken() external view override returns (address) { return address(sushiAddr); } /// @notice Returns the total balance (in asset tokens). This includes the deposits and interest. /// @return The underlying balance of asset tokens - function balanceOfToken(address addr) public override returns (uint256) { + function balanceOfToken(address addr) external override returns (uint256) { if (balances[addr] == 0) return 0; uint256 totalShares = sushiBar.totalSupply(); From 0d2856957c1b99c9f444ce098f2f99ac7bf7baf8 Mon Sep 17 00:00:00 2001 From: Aodhgan Date: Tue, 29 Jun 2021 18:02:47 -0700 Subject: [PATCH 6/8] added events --- contracts/SushiYieldSource.sol | 17 +++++++++++++++++ test/unit_test.js | 4 ++-- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/contracts/SushiYieldSource.sol b/contracts/SushiYieldSource.sol index 4c79cfb..b87d302 100644 --- a/contracts/SushiYieldSource.sol +++ b/contracts/SushiYieldSource.sol @@ -19,6 +19,21 @@ contract SushiYieldSource is IYieldSource { mapping(address => uint256) public balances; + /// @notice Emitted when asset tokens are redeemed from the yield source + event RedeemedToken( + address indexed from, + uint256 shares, + uint256 amount + ); + + /// @notice Emitted when asset tokens are supplied to the yield source + event SuppliedTokenTo( + address indexed from, + uint256 shares, + uint256 amount, + address indexed to + ); + constructor(ISushiBar _sushiBar, ISushi _sushiAddr) public { sushiBar = _sushiBar; sushiAddr = _sushiAddr; @@ -57,6 +72,7 @@ contract SushiYieldSource is IYieldSource { uint256 balanceDiff = afterBalance.sub(beforeBalance); balances[to] = balances[to].add(balanceDiff); + emit SuppliedTokenTo(msg.sender, balanceDiff, amount, to); } /// @notice Redeems tokens from the yield source to the msg.sender, it burns yield bearing tokens and returns token to the sender. @@ -87,6 +103,7 @@ contract SushiYieldSource is IYieldSource { balances[msg.sender] = balances[msg.sender].sub(requiredSharesBalance); sushi.transfer(msg.sender, sushiBalanceDiff); + emit RedeemedToken(msg.sender, requiredSharesBalance, amount); return (sushiBalanceDiff); } diff --git a/test/unit_test.js b/test/unit_test.js index c1433c8..10122b1 100644 --- a/test/unit_test.js +++ b/test/unit_test.js @@ -67,7 +67,7 @@ describe("SushiYieldSource", function () { it("supplyTokenTo", async function () { await sushi.connect(wallet).approve(yieldSource.address, amount); - await yieldSource.supplyTokenTo(amount, wallet.address); + expect(await yieldSource.supplyTokenTo(amount, wallet.address)).to.emit(yieldSource, "SuppliedTokenTo"); expect(await sushi.balanceOf(sushiBar.address)).to.eq(amount.mul(100)); expect(await yieldSource.callStatic.balanceOfToken(wallet.address)).to.eq( amount @@ -79,7 +79,7 @@ describe("SushiYieldSource", function () { await yieldSource.supplyTokenTo(amount, wallet.address); expect(await sushi.balanceOf(wallet.address)).to.eq(0); - await yieldSource.redeemToken(amount); + expect(await yieldSource.redeemToken(amount)).to.emit(yieldSource, "RedeemedToken"); expect(await sushi.balanceOf(wallet.address)).to.eq(amount); }); From a99e3a455a94bee1169722f73f8a4f3de63cb81b Mon Sep 17 00:00:00 2001 From: Pierrick Turelier Date: Thu, 1 Jul 2021 18:08:08 +0200 Subject: [PATCH 7/8] fix(contract): check constructor addresses --- contracts/ISushiBar.sol | 1 - contracts/SushiYieldSource.sol | 13 +++++++-- package.json | 2 +- scripts/test.js | 14 ++++----- test/integration_test.js | 2 +- test/unit_test.js | 52 ++++++++++++++++++++++++++++++---- yarn.lock | 2 +- 7 files changed, 66 insertions(+), 20 deletions(-) diff --git a/contracts/ISushiBar.sol b/contracts/ISushiBar.sol index 082f830..af512be 100644 --- a/contracts/ISushiBar.sol +++ b/contracts/ISushiBar.sol @@ -10,5 +10,4 @@ interface ISushiBar { function totalSupply() external view returns (uint256); function balanceOf(address account) external view returns (uint256); - } diff --git a/contracts/SushiYieldSource.sol b/contracts/SushiYieldSource.sol index 4c79cfb..eb7df0c 100644 --- a/contracts/SushiYieldSource.sol +++ b/contracts/SushiYieldSource.sol @@ -2,7 +2,7 @@ pragma solidity 0.6.12; -import { IYieldSource } from "@pooltogether/yield-source-interface/contracts/IYieldSource.sol"; +import "@pooltogether/yield-source-interface/contracts/IYieldSource.sol"; import "@openzeppelin/contracts/math/SafeMath.sol"; import "./ISushiBar.sol"; @@ -11,7 +11,6 @@ import "./ISushi.sol"; /// @title A pooltogether yield source for sushi token /// @author Steffel Fenix contract SushiYieldSource is IYieldSource { - using SafeMath for uint256; ISushiBar public immutable sushiBar; @@ -20,6 +19,15 @@ contract SushiYieldSource is IYieldSource { mapping(address => uint256) public balances; constructor(ISushiBar _sushiBar, ISushi _sushiAddr) public { + require( + address(_sushiBar) != address(0), + "SushiYieldSource/sushiBar-not-zero-address" + ); + require( + address(_sushiAddr) != address(0), + "SushiYieldSource/sushiAddr-not-zero-address" + ); + sushiBar = _sushiBar; sushiAddr = _sushiAddr; } @@ -90,5 +98,4 @@ contract SushiYieldSource is IYieldSource { return (sushiBalanceDiff); } - } diff --git a/package.json b/package.json index eb1fa10..32a9b24 100644 --- a/package.json +++ b/package.json @@ -21,7 +21,7 @@ "hardhat-typechain": "^0.3.5", "prettier": "^2.2.1", "prettier-plugin-solidity": "^1.0.0-beta.6", - "solidity-coverage": "^0.7.16", + "solidity-coverage": "0.7.16", "ts-generator": "^0.1.1", "ts-node": "^9.1.1", "typechain": "^4.0.3", diff --git a/scripts/test.js b/scripts/test.js index 973da31..feb0772 100644 --- a/scripts/test.js +++ b/scripts/test.js @@ -34,15 +34,15 @@ async function getYieldSourcePrizePoolProxy(tx) { } async function run() { - console.log("running fork script") + console.log("running fork script"); await hre.network.provider.request({ method: "hardhat_impersonateAccount", params: [SUSHI_HOLDER], }); - const SUSHI_TOKEN_ADDRESS = "0x6B3595068778DD592e39A122f4f5a5cF09C90fE2" - const XSUSHI_ADDRESS = "0x8798249c2E607446EfB7Ad49eC89dD1865Ff4272" + const SUSHI_TOKEN_ADDRESS = "0x6B3595068778DD592e39A122f4f5a5cF09C90fE2"; + const XSUSHI_ADDRESS = "0x8798249c2E607446EfB7Ad49eC89dD1865Ff4272"; const sushiHolder = await ethers.provider.getUncheckedSigner(SUSHI_HOLDER); const sushi = await ethers.getContractAt( @@ -50,19 +50,19 @@ async function run() { SUSHI_TOKEN_ADDRESS, sushiHolder ); - console.log("getting builder") + console.log("getting builder"); const builder = await ethers.getContractAt( "PoolWithMultipleWinnersBuilder", "0x39E2F33ff4Ad3491106B3BB15dc66EbE24e4E9C7" ); - console.log("deploying") + console.log("deploying"); SushiYieldSourceFactory = await ethers.getContractFactory("SushiYieldSource"); sushiYieldSource = await SushiYieldSourceFactory.deploy( XSUSHI_ADDRESS, - SUSHI_TOKEN_ADDRESS + SUSHI_TOKEN_ADDRESS ); - console.log("deployed SushiYieldSource at ", sushiYieldSource.address) + console.log("deployed SushiYieldSource at ", sushiYieldSource.address); const block = await ethers.provider.getBlock(); diff --git a/test/integration_test.js b/test/integration_test.js index d8c7796..1d83dd9 100644 --- a/test/integration_test.js +++ b/test/integration_test.js @@ -110,7 +110,7 @@ describe("SushiYieldSource integration", function () { { gasLimit: 9500000 } ); - const exchangeWalletAddress = "0xD551234Ae421e3BCBA99A0Da6d736074f22192FF"; + const exchangeWalletAddress = "0xF977814e90dA44bFA03b6295A0616a897441aceC"; await hre.network.provider.request({ method: "hardhat_impersonateAccount", params: [exchangeWalletAddress], diff --git a/test/unit_test.js b/test/unit_test.js index c1433c8..7571111 100644 --- a/test/unit_test.js +++ b/test/unit_test.js @@ -17,6 +17,18 @@ describe("SushiYieldSource", function () { let yieldSource; let amount; + let SushiYieldSourceContract; + + let isDeployTest = false; + + const deploySushiYieldSource = async (sushiBarAddress, sushiAddress) => { + yieldSource = await SushiYieldSourceContract.deploy( + sushiBarAddress, + sushiAddress, + overrides + ); + }; + beforeEach(async function () { [wallet, wallet2] = await ethers.getSigners(); const ERC20MintableContract = await hre.ethers.getContractFactory( @@ -33,14 +45,14 @@ describe("SushiYieldSource", function () { ); sushiBar = await SushiBarContract.deploy(sushi.address); - const SushiYieldSourceContract = await ethers.getContractFactory( + SushiYieldSourceContract = await ethers.getContractFactory( "SushiYieldSource" ); - yieldSource = await SushiYieldSourceContract.deploy( - sushiBar.address, - sushi.address, - overrides - ); + + if (!isDeployTest) { + deploySushiYieldSource(sushiBar.address, sushi.address); + } + amount = toWei("100"); await sushi.mint(wallet.address, amount); await sushi.mint(wallet2.address, amount.mul(99)); @@ -48,6 +60,34 @@ describe("SushiYieldSource", function () { await sushiBar.connect(wallet2).enter(amount.mul(99)); }); + describe("constructor()", () => { + let randomWalletAddress; + + before(() => { + isDeployTest = true; + }); + + beforeEach(() => { + randomWalletAddress = ethers.Wallet.createRandom().address; + }); + + after(() => { + isDeployTest = false; + }); + + it("should fail if sushiBar address is address 0", async () => { + await expect( + deploySushiYieldSource(ethers.constants.AddressZero, sushi.address) + ).to.be.revertedWith("SushiYieldSource/sushiBar-not-zero-address"); + }); + + it("should fail if sushi address is address 0", async () => { + await expect( + deploySushiYieldSource(sushiBar.address, ethers.constants.AddressZero) + ).to.be.revertedWith("SushiYieldSource/sushiAddr-not-zero-address"); + }); + }); + it("get token address", async function () { let address = await yieldSource.depositToken(); expect(address == sushi); diff --git a/yarn.lock b/yarn.lock index 6d17e34..bece1e8 100644 --- a/yarn.lock +++ b/yarn.lock @@ -8512,7 +8512,7 @@ solidity-comments-extractor@^0.0.4: resolved "https://registry.yarnpkg.com/solidity-comments-extractor/-/solidity-comments-extractor-0.0.4.tgz#ce420aef23641ffd0131c7d80ba85b6e1e42147e" integrity sha512-58glBODwXIKMaQ7rfcJOrWtFQMMOK28tJ0/LcB5Xhu7WtAxk4UX2fpgKPuaL41XjMp/y0gAa1MTLqk018wuSzA== -solidity-coverage@^0.7.16: +solidity-coverage@0.7.16: version "0.7.16" resolved "https://registry.yarnpkg.com/solidity-coverage/-/solidity-coverage-0.7.16.tgz#c8c8c46baa361e2817bbf275116ddd2ec90a55fb" integrity sha512-ttBOStywE6ZOTJmmABSg4b8pwwZfYKG8zxu40Nz+sRF5bQX7JULXWj/XbX0KXps3Fsp8CJXg8P29rH3W54ipxw== From 53264318d52caa24d22160403020f6d2283b5160 Mon Sep 17 00:00:00 2001 From: kamescg Date: Wed, 30 Jun 2021 00:42:31 -0700 Subject: [PATCH 8/8] reduce SLOAD calls in supplyTokenTo --- contracts/SushiYieldSource.sol | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/contracts/SushiYieldSource.sol b/contracts/SushiYieldSource.sol index bdcef73..4950b74 100644 --- a/contracts/SushiYieldSource.sol +++ b/contracts/SushiYieldSource.sol @@ -72,6 +72,10 @@ contract SushiYieldSource is IYieldSource { sushiAddr.approve(address(sushiBar), amount); ISushiBar bar = sushiBar; + + sushi.transferFrom(msg.sender, address(this), amount); + sushi.approve(address(bar), amount); + uint256 beforeBalance = bar.balanceOf(address(this)); bar.enter(amount);