From 59f7cb3b3aab893e941d95df522dc1927eb63450 Mon Sep 17 00:00:00 2001
From: pinkiebell <40266861+pinkiebell@users.noreply.github.com>
Date: Mon, 8 Feb 2021 16:53:39 +0100
Subject: [PATCH 1/8] add initial test harness for harvest

---
 contracts/mocks/DummyStrategyMock.sol |  55 ++++++++++++
 test/HelloWorld.js                    |  51 +++++++++++
 test/harness/harvest.js               | 118 ++++++++++++++++++++++++++
 3 files changed, 224 insertions(+)
 create mode 100644 contracts/mocks/DummyStrategyMock.sol
 create mode 100644 test/HelloWorld.js
 create mode 100644 test/harness/harvest.js

diff --git a/contracts/mocks/DummyStrategyMock.sol b/contracts/mocks/DummyStrategyMock.sol
new file mode 100644
index 00000000..8ee81ec1
--- /dev/null
+++ b/contracts/mocks/DummyStrategyMock.sol
@@ -0,0 +1,55 @@
+// SPDX-License-Identifier: MIT
+pragma solidity 0.6.12;
+import "../interfaces/IStrategy.sol";
+import "@boringcrypto/boring-solidity/contracts/libraries/BoringMath.sol";
+import "@boringcrypto/boring-solidity/contracts/libraries/BoringERC20.sol";
+
+// solhint-disable not-rely-on-time
+
+contract DummyStrategyMock is IStrategy {
+    using BoringMath for uint256;
+    using BoringERC20 for IERC20;
+
+    IERC20 private immutable token;
+    address private immutable bentoBox;
+
+    int256 public _harvestProfit;
+
+    modifier onlyBentoBox() {
+        require(msg.sender == bentoBox, "Ownable: caller is not the owner");
+        _;
+    }
+
+    constructor(address bentoBox_, IERC20 token_) public {
+        bentoBox = bentoBox_;
+        token = token_;
+    }
+
+    function setHarvestProfit(int256 val) public {
+        _harvestProfit = val;
+    }
+
+    // Send the assets to the Strategy and call skim to invest them
+    function skim(uint256) external override onlyBentoBox {
+        return;
+    }
+
+    // Harvest any profits made converted to the asset and pass them to the caller
+    function harvest(
+        uint256 /*balance*/
+    ) external override onlyBentoBox returns (int256 amountAdded) {
+        amountAdded = _harvestProfit;
+    }
+
+    // Withdraw assets. The returned amount can differ from the requested amount due to rounding or if the request was more than there is.
+    function withdraw(uint256 amount) external override onlyBentoBox returns (uint256 actualAmount) {
+        actualAmount = amount;
+    }
+
+    // Withdraw all assets in the safest way possible. This shouldn't fail.
+    function exit(
+        uint256 /*balance*/
+    ) external override onlyBentoBox returns (int256 amountAdded) {
+        amountAdded = _harvestProfit;
+    }
+}
diff --git a/test/HelloWorld.js b/test/HelloWorld.js
new file mode 100644
index 00000000..d891b842
--- /dev/null
+++ b/test/HelloWorld.js
@@ -0,0 +1,51 @@
+const assert = require("assert")
+const {
+    getBigNumber,
+    prepare,
+    setMasterContractApproval,
+    deploymentsFixture,
+} = require("./utilities")
+
+describe("HelloWorld", function () {
+    const APPROVAL_AMOUNT = 1000
+
+    before(async function () {
+        await prepare(this, [
+            "HelloWorld",
+            "ReturnFalseERC20Mock",
+        ])
+    })
+
+    it("Setup", async function () {
+        await deploymentsFixture(this, async (cmd) => {
+            await cmd.addToken("tokenA", "Token A", "A", 18, this.ReturnFalseERC20Mock)
+        })
+
+        await this.HelloWorld.new("helloWorld", this.bentoBox.address, this.tokenA.address)
+    })
+
+    it("should reject deposit: no token- nor master contract approval", async function () {
+        await assert.rejects(this.helloWorld.deposit(APPROVAL_AMOUNT))
+    })
+
+    it("approve BentoBox", async function () {
+        await this.tokenA.approve(this.bentoBox.address, getBigNumber(APPROVAL_AMOUNT, await this.tokenA.decimals()))
+    })
+
+    it("should reject deposit: user approved, master contract not approved", async function () {
+        await assert.rejects(this.helloWorld.deposit(APPROVAL_AMOUNT))
+    })
+
+    it("approve master contract", async function () {
+        await setMasterContractApproval(this.bentoBox, this.alice, this.alice, this.alicePrivateKey, this.helloWorld.address, true)
+    })
+
+    it("should allow deposit", async function () {
+        await this.helloWorld.deposit(APPROVAL_AMOUNT)
+        assert.equal((await this.helloWorld.balance()).toString(), APPROVAL_AMOUNT.toString())
+    })
+
+    it("should allow withdraw", async function () {
+        await this.helloWorld.withdraw()
+    })
+})
diff --git a/test/harness/harvest.js b/test/harness/harvest.js
new file mode 100644
index 00000000..e74fc3fe
--- /dev/null
+++ b/test/harness/harvest.js
@@ -0,0 +1,118 @@
+const { ethers } = require("hardhat")
+const assert = require("assert")
+const { getBigNumber, advanceTime, prepare, setMasterContractApproval, deploymentsFixture } = require("../utilities")
+const { LendingPair } = require("../utilities/lendingpair")
+
+async function depositHelper(bentoBox, token, wallet, amount) {
+    await bentoBox.deposit(token.address, wallet.address, wallet.address, getBigNumber(amount, await token.decimals()), 0)
+}
+
+async function withdrawHelper(bentoBox, token, wallet, amount) {
+    await bentoBox.withdraw(token.address, wallet.address, wallet.address, getBigNumber(amount, await token.decimals()), 0)
+}
+
+describe("Harvest with DummyStrategyMock", function () {
+    const APPROVAL_AMOUNT = 1000000
+    const DEPOSIT_AMOUNT = 1000
+    let strategyBalance = 0
+
+    before(async function () {
+        await prepare(this, ["ReturnFalseERC20Mock", "DummyStrategyMock"])
+    })
+
+    it("Setup", async function () {
+        await deploymentsFixture(this, async (cmd) => {
+            await cmd.addToken("tokenA", "Token A", "A", 18, this.ReturnFalseERC20Mock)
+        })
+
+        await this.DummyStrategyMock.new("dummyStrategy", this.bentoBox.address, this.tokenA.address)
+        await this.tokenA.approve(this.bentoBox.address, getBigNumber(APPROVAL_AMOUNT, await this.tokenA.decimals()))
+    })
+
+    it("should allow adding of balances to the BentoBox", async function () {
+        await depositHelper(this.bentoBox, this.tokenA, this.alice, DEPOSIT_AMOUNT)
+    })
+
+    it("check balances of BentoBox", async function () {
+        assert.equal(
+            (await this.bentoBox.balanceOf(this.tokenA.address, this.alice.address)).toString(),
+            getBigNumber(DEPOSIT_AMOUNT, await this.tokenA.decimals()).toString(),
+            "should match deposit amount"
+        )
+
+        assert.equal(
+            (await this.tokenA.balanceOf(this.bentoBox.address)).toString(),
+            getBigNumber(DEPOSIT_AMOUNT, await this.tokenA.decimals()).toString(),
+            "should match deposit amount"
+        )
+    })
+
+    it("set harvest profit on strategy mock", async function () {
+        await this.dummyStrategy.setHarvestProfit(DEPOSIT_AMOUNT)
+    })
+
+    it("harvest without strategy set - revert", async function () {
+        await assert.rejects(this.bentoBox.harvest(this.tokenA.address, true, 0))
+    })
+
+    it("set strategy and target percentage", async function () {
+        await this.bentoBox.setStrategy(this.tokenA.address, this.dummyStrategy.address)
+        await advanceTime(1209600, ethers)
+        await this.bentoBox.setStrategy(this.tokenA.address, this.dummyStrategy.address)
+
+        await this.bentoBox.setStrategyTargetPercentage(this.tokenA.address, 95)
+    })
+
+    it("harvest", async function () {
+        await this.bentoBox.harvest(this.tokenA.address, true, 1)
+        strategyBalance++
+    })
+
+    it("set harvest profit on strategy mock to zero", async function () {
+        await this.dummyStrategy.setHarvestProfit(0)
+    })
+
+    it("harvest", async function () {
+        await this.bentoBox.harvest(this.tokenA.address, true, 1)
+        strategyBalance++
+    })
+
+    it("check balance of dummy strategy contract", async function () {
+        assert.equal(
+            (await this.tokenA.balanceOf(this.dummyStrategy.address)).toString(),
+            strategyBalance.toString(),
+            "should match DEPOSIT_AMOUNT - (2x profit drain)"
+        )
+    })
+
+    it("set harvest profit on strategy mock negative", async function () {
+        await this.dummyStrategy.setHarvestProfit(-DEPOSIT_AMOUNT)
+    })
+
+    it("set strategy percentage to zero", async function () {
+        await this.bentoBox.setStrategyTargetPercentage(this.tokenA.address, 0)
+    })
+
+    it("harvest", async function () {
+        await this.bentoBox.harvest(this.tokenA.address, true, 1)
+    })
+
+    it("set target percentage positive and set re-apply strategy contract", async function () {
+        await this.dummyStrategy.setHarvestProfit(DEPOSIT_AMOUNT)
+        await this.bentoBox.setStrategy(this.tokenA.address, this.dummyStrategy.address)
+        await advanceTime(1209600, ethers)
+        await this.bentoBox.setStrategy(this.tokenA.address, this.dummyStrategy.address)
+    })
+
+    it("harvest", async function () {
+        await this.bentoBox.harvest(this.tokenA.address, true, 1)
+    })
+
+    it("should allow withdraw from BentoBox", async function () {
+        await withdrawHelper(this.bentoBox, this.tokenA, this.alice, DEPOSIT_AMOUNT - strategyBalance)
+    })
+
+    it("harvest", async function () {
+        await this.bentoBox.harvest(this.tokenA.address, true, 1)
+    })
+})

From 405ef2b87824f3713b1b89097e17d2289c95b02a Mon Sep 17 00:00:00 2001
From: pinkiebell <40266861+pinkiebell@users.noreply.github.com>
Date: Mon, 8 Feb 2021 16:57:19 +0100
Subject: [PATCH 2/8] bentoBox: comment-out possibly not needed balance update

The `data.balance` assignment is updated in the `balance < 0` case.
But looking further down, then `data.balance` looks like it should only be updated if `bool balance == true` .

To elaborate, if the first harvest is negative we hit a underflow at this line too.
---
 contracts/BentoBox.sol | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/contracts/BentoBox.sol b/contracts/BentoBox.sol
index 8caf323d..0695ecb3 100644
--- a/contracts/BentoBox.sol
+++ b/contracts/BentoBox.sol
@@ -420,12 +420,14 @@ contract BentoBox is MasterContractManager, BoringBatchable {
             uint256 sub = uint256(-balanceChange);
             totalElastic = totalElastic.sub(sub);
             totals[token].elastic = totalElastic.to128();
-            data.balance = data.balance.sub(sub.to128());
+            // XXX: probably wrong place?
+            // data.balance = data.balance.sub(sub.to128());
             emit LogStrategyLoss(token, sub);
         }
 
         if (balance) {
             uint256 targetBalance = totalElastic.mul(data.targetPercentage) / 100;
+            // if data.balance == targetBalance there is nothing to update
             if (data.balance < targetBalance) {
                 uint256 amountOut = targetBalance.sub(data.balance);
                 if (maxChangeAmount != 0 && amountOut > maxChangeAmount) {

From 13a503967bbaf3f737005027ec440bee26ec7a6b Mon Sep 17 00:00:00 2001
From: pinkiebell <40266861+pinkiebell@users.noreply.github.com>
Date: Mon, 8 Feb 2021 16:58:00 +0100
Subject: [PATCH 3/8] fix coverage for SimpleSLPOracle

---
 test/oracles/SimpleSLPOracle.js | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/test/oracles/SimpleSLPOracle.js b/test/oracles/SimpleSLPOracle.js
index db9fcf76..d7c1a330 100644
--- a/test/oracles/SimpleSLPOracle.js
+++ b/test/oracles/SimpleSLPOracle.js
@@ -41,12 +41,14 @@ describe("SimpleSLPOracle", function () {
         describe("name", function () {
             it("should get name", async function () {
                 expect(await this.oracleF.name(this.oracleData)).to.be.equal("SushiSwap TWAP")
+                expect(await this.oracleB.name(this.oracleData)).to.be.equal("SushiSwap TWAP")
             })
         })
 
         describe("symbol", function () {
             it("should get symbol", async function () {
                 expect(await this.oracleF.symbol(this.oracleData)).to.be.equal("S")
+                expect(await this.oracleB.symbol(this.oracleData)).to.be.equal("S")
             })
         })
 

From cace19faa2e3bfffec03b3db3b1d72139e729b50 Mon Sep 17 00:00:00 2001
From: pinkiebell <40266861+pinkiebell@users.noreply.github.com>
Date: Mon, 8 Feb 2021 17:17:11 +0100
Subject: [PATCH 4/8] fix after merge

---
 contracts/mocks/DummyStrategyMock.sol | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/contracts/mocks/DummyStrategyMock.sol b/contracts/mocks/DummyStrategyMock.sol
index 8ee81ec1..dfeae5f2 100644
--- a/contracts/mocks/DummyStrategyMock.sol
+++ b/contracts/mocks/DummyStrategyMock.sol
@@ -36,7 +36,8 @@ contract DummyStrategyMock is IStrategy {
 
     // Harvest any profits made converted to the asset and pass them to the caller
     function harvest(
-        uint256 /*balance*/
+        uint256, /*balance*/
+        address /*sender*/
     ) external override onlyBentoBox returns (int256 amountAdded) {
         amountAdded = _harvestProfit;
     }

From 9c100c22baee59bbcbed7038d8f128602ce3070c Mon Sep 17 00:00:00 2001
From: pinkiebell <40266861+pinkiebell@users.noreply.github.com>
Date: Mon, 8 Feb 2021 18:32:50 +0100
Subject: [PATCH 5/8] test/oracles: add missing call to peek for oracleB

---
 test/oracles/SimpleSLPOracle.js | 1 +
 1 file changed, 1 insertion(+)

diff --git a/test/oracles/SimpleSLPOracle.js b/test/oracles/SimpleSLPOracle.js
index d7c1a330..73c71b3c 100644
--- a/test/oracles/SimpleSLPOracle.js
+++ b/test/oracles/SimpleSLPOracle.js
@@ -55,6 +55,7 @@ describe("SimpleSLPOracle", function () {
         describe("peek", function () {
             it("should return false on first peek", async function () {
                 expect((await this.oracleF.peek(this.oracleData))[1]).to.equal("0")
+                expect((await this.oracleB.peek(this.oracleData))[1]).to.equal("0")
             })
 
             it("should get price even when time since last update is longer than period", async function () {

From cbdce27a9114c19d58300569e62ead9fbe3dfd82 Mon Sep 17 00:00:00 2001
From: pinkiebell <40266861+pinkiebell@users.noreply.github.com>
Date: Tue, 9 Feb 2021 08:33:56 +0100
Subject: [PATCH 6/8] fix, cleanup, comment

---
 contracts/BentoBox.sol                |  5 +--
 contracts/mocks/DummyStrategyMock.sol | 26 ++++++++++---
 test/harness/harvest.js               | 54 +++++++++++++++++----------
 3 files changed, 57 insertions(+), 28 deletions(-)

diff --git a/contracts/BentoBox.sol b/contracts/BentoBox.sol
index f25d7bca..2350d8d9 100644
--- a/contracts/BentoBox.sol
+++ b/contracts/BentoBox.sol
@@ -63,7 +63,7 @@ contract BentoBox is MasterContractManager, BoringBatchable {
     struct StrategyData {
         uint64 strategyStartDate;
         uint64 targetPercentage;
-        uint128 balance;
+        uint128 balance; // the actual balance of the strategy that can differ from `totals[token]`
     }
 
     // ******************************** //
@@ -420,8 +420,7 @@ contract BentoBox is MasterContractManager, BoringBatchable {
             uint256 sub = uint256(-balanceChange);
             totalElastic = totalElastic.sub(sub);
             totals[token].elastic = totalElastic.to128();
-            // XXX: probably wrong place?
-            // data.balance = data.balance.sub(sub.to128());
+            data.balance = data.balance.sub(sub.to128());
             emit LogStrategyLoss(token, sub);
         }
 
diff --git a/contracts/mocks/DummyStrategyMock.sol b/contracts/mocks/DummyStrategyMock.sol
index dfeae5f2..33802eaf 100644
--- a/contracts/mocks/DummyStrategyMock.sol
+++ b/contracts/mocks/DummyStrategyMock.sol
@@ -36,21 +36,37 @@ contract DummyStrategyMock is IStrategy {
 
     // Harvest any profits made converted to the asset and pass them to the caller
     function harvest(
-        uint256, /*balance*/
+        uint256 balance,
         address /*sender*/
     ) external override onlyBentoBox returns (int256 amountAdded) {
-        amountAdded = _harvestProfit;
+        if (_harvestProfit > 0) {
+            amountAdded = int256(balance) + _harvestProfit;
+        } else {
+            amountAdded = int256(balance) - _harvestProfit;
+        }
     }
 
     // Withdraw assets. The returned amount can differ from the requested amount due to rounding or if the request was more than there is.
     function withdraw(uint256 amount) external override onlyBentoBox returns (uint256 actualAmount) {
-        actualAmount = amount;
+        actualAmount = token.balanceOf(address(this));
+        if (amount > actualAmount) {
+            actualAmount = amount;
+        }
+        token.safeTransfer(msg.sender, actualAmount);
     }
 
     // Withdraw all assets in the safest way possible. This shouldn't fail.
     function exit(
-        uint256 /*balance*/
+        uint256 balance
     ) external override onlyBentoBox returns (int256 amountAdded) {
-        amountAdded = _harvestProfit;
+        uint256 moneyToBeTransferred = token.balanceOf(address(this));
+        amountAdded = int256(moneyToBeTransferred) - int256(balance);
+        // that is here to reach some branches in BentoBox
+        if (_harvestProfit > 0) {
+            amountAdded += _harvestProfit;
+        } else {
+            amountAdded -= _harvestProfit;
+        }
+        token.safeTransfer(msg.sender, moneyToBeTransferred);
     }
 }
diff --git a/test/harness/harvest.js b/test/harness/harvest.js
index e74fc3fe..aef21abc 100644
--- a/test/harness/harvest.js
+++ b/test/harness/harvest.js
@@ -1,7 +1,6 @@
-const { ethers } = require("hardhat")
 const assert = require("assert")
+const { ethers } = require("hardhat")
 const { getBigNumber, advanceTime, prepare, setMasterContractApproval, deploymentsFixture } = require("../utilities")
-const { LendingPair } = require("../utilities/lendingpair")
 
 async function depositHelper(bentoBox, token, wallet, amount) {
     await bentoBox.deposit(token.address, wallet.address, wallet.address, getBigNumber(amount, await token.decimals()), 0)
@@ -14,7 +13,7 @@ async function withdrawHelper(bentoBox, token, wallet, amount) {
 describe("Harvest with DummyStrategyMock", function () {
     const APPROVAL_AMOUNT = 1000000
     const DEPOSIT_AMOUNT = 1000
-    let strategyBalance = 0
+    const HARVEST_MAX_AMOUNT = 3
 
     before(async function () {
         await prepare(this, ["ReturnFalseERC20Mock", "DummyStrategyMock"])
@@ -47,8 +46,8 @@ describe("Harvest with DummyStrategyMock", function () {
         )
     })
 
-    it("set harvest profit on strategy mock", async function () {
-        await this.dummyStrategy.setHarvestProfit(DEPOSIT_AMOUNT)
+    it("set harvest profit on strategy mock to zero", async function () {
+        await this.dummyStrategy.setHarvestProfit(0)
     })
 
     it("harvest without strategy set - revert", async function () {
@@ -64,24 +63,26 @@ describe("Harvest with DummyStrategyMock", function () {
     })
 
     it("harvest", async function () {
-        await this.bentoBox.harvest(this.tokenA.address, true, 1)
-        strategyBalance++
+        await this.bentoBox.harvest(this.tokenA.address, true, HARVEST_MAX_AMOUNT)
     })
 
-    it("set harvest profit on strategy mock to zero", async function () {
-        await this.dummyStrategy.setHarvestProfit(0)
+    it("set harvest profit on strategy mock positive", async function () {
+        await this.dummyStrategy.setHarvestProfit(DEPOSIT_AMOUNT)
     })
 
-    it("harvest", async function () {
-        await this.bentoBox.harvest(this.tokenA.address, true, 1)
-        strategyBalance++
+    it("harvest profit", async function () {
+        await this.bentoBox.harvest(this.tokenA.address, true, HARVEST_MAX_AMOUNT)
+    })
+
+    it("harvest profit", async function () {
+        await this.bentoBox.harvest(this.tokenA.address, true, HARVEST_MAX_AMOUNT)
     })
 
     it("check balance of dummy strategy contract", async function () {
         assert.equal(
-            (await this.tokenA.balanceOf(this.dummyStrategy.address)).toString(),
-            strategyBalance.toString(),
-            "should match DEPOSIT_AMOUNT - (2x profit drain)"
+            (await this.bentoBox.strategyData(this.tokenA.address)).balance.toString(),
+            (HARVEST_MAX_AMOUNT * 3).toString(),
+            "should match"
         )
     })
 
@@ -94,7 +95,7 @@ describe("Harvest with DummyStrategyMock", function () {
     })
 
     it("harvest", async function () {
-        await this.bentoBox.harvest(this.tokenA.address, true, 1)
+        await this.bentoBox.harvest(this.tokenA.address, true, HARVEST_MAX_AMOUNT)
     })
 
     it("set target percentage positive and set re-apply strategy contract", async function () {
@@ -105,14 +106,27 @@ describe("Harvest with DummyStrategyMock", function () {
     })
 
     it("harvest", async function () {
-        await this.bentoBox.harvest(this.tokenA.address, true, 1)
+        await this.bentoBox.harvest(this.tokenA.address, true, HARVEST_MAX_AMOUNT)
     })
 
-    it("should allow withdraw from BentoBox", async function () {
-        await withdrawHelper(this.bentoBox, this.tokenA, this.alice, DEPOSIT_AMOUNT - strategyBalance)
+    it("should allow withdraw original deposit amount from BentoBox", async function () {
+        await withdrawHelper(this.bentoBox, this.tokenA, this.alice, DEPOSIT_AMOUNT)
     })
 
     it("harvest", async function () {
-        await this.bentoBox.harvest(this.tokenA.address, true, 1)
+        await this.bentoBox.harvest(this.tokenA.address, true, HARVEST_MAX_AMOUNT)
+    })
+
+    it("check token balances of contracts", async function () {
+        assert.equal(
+            (await this.tokenA.balanceOf(this.bentoBox.address)).toString(),
+            "0",
+            "bentoBox"
+        )
+        assert.equal(
+            (await this.tokenA.balanceOf(this.dummyStrategy.address)).toString(),
+            "0",
+            "dummyStrategy"
+        )
     })
 })

From ef4772e65fddc2a9938e4d583b8a40c09e375e81 Mon Sep 17 00:00:00 2001
From: pinkiebell <40266861+pinkiebell@users.noreply.github.com>
Date: Tue, 9 Feb 2021 08:52:03 +0100
Subject: [PATCH 7/8] lint

---
 contracts/mocks/DummyStrategyMock.sol |  4 +---
 test/harness/harvest.js               | 12 ++----------
 2 files changed, 3 insertions(+), 13 deletions(-)

diff --git a/contracts/mocks/DummyStrategyMock.sol b/contracts/mocks/DummyStrategyMock.sol
index 33802eaf..af62d8a9 100644
--- a/contracts/mocks/DummyStrategyMock.sol
+++ b/contracts/mocks/DummyStrategyMock.sol
@@ -56,9 +56,7 @@ contract DummyStrategyMock is IStrategy {
     }
 
     // Withdraw all assets in the safest way possible. This shouldn't fail.
-    function exit(
-        uint256 balance
-    ) external override onlyBentoBox returns (int256 amountAdded) {
+    function exit(uint256 balance) external override onlyBentoBox returns (int256 amountAdded) {
         uint256 moneyToBeTransferred = token.balanceOf(address(this));
         amountAdded = int256(moneyToBeTransferred) - int256(balance);
         // that is here to reach some branches in BentoBox
diff --git a/test/harness/harvest.js b/test/harness/harvest.js
index aef21abc..8d76c46c 100644
--- a/test/harness/harvest.js
+++ b/test/harness/harvest.js
@@ -118,15 +118,7 @@ describe("Harvest with DummyStrategyMock", function () {
     })
 
     it("check token balances of contracts", async function () {
-        assert.equal(
-            (await this.tokenA.balanceOf(this.bentoBox.address)).toString(),
-            "0",
-            "bentoBox"
-        )
-        assert.equal(
-            (await this.tokenA.balanceOf(this.dummyStrategy.address)).toString(),
-            "0",
-            "dummyStrategy"
-        )
+        assert.equal((await this.tokenA.balanceOf(this.bentoBox.address)).toString(), "0", "bentoBox")
+        assert.equal((await this.tokenA.balanceOf(this.dummyStrategy.address)).toString(), "0", "dummyStrategy")
     })
 })

From 6f3141048ec1ccdaa4ba55700c0093879ba56e8e Mon Sep 17 00:00:00 2001
From: pinkiebell <40266861+pinkiebell@users.noreply.github.com>
Date: Tue, 9 Feb 2021 10:09:06 +0100
Subject: [PATCH 8/8] BentoBox.sol: natspec docs

---
 contracts/BentoBox.sol | 62 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 62 insertions(+)

diff --git a/contracts/BentoBox.sol b/contracts/BentoBox.sol
index 12c2b547..114ade45 100644
--- a/contracts/BentoBox.sol
+++ b/contracts/BentoBox.sol
@@ -135,6 +135,11 @@ contract BentoBox is MasterContractManager, BoringBatchable {
     // *** PUBLIC FUNCTIONS *** //
     // ************************ //
 
+    /// @dev Helper function to represent an `amount` of `token` in shares.
+    /// @param token The ERC-20 token.
+    /// @param amount The `token` amount.
+    /// @param roundUp If the result `share` should be rounded up.
+    /// @return share The token amount represented in shares.
     function toShare(
         IERC20 token,
         uint256 amount,
@@ -143,6 +148,11 @@ contract BentoBox is MasterContractManager, BoringBatchable {
         share = totals[token].toBase(amount, roundUp);
     }
 
+    /// @dev Helper function represent shares back into the `token` amount.
+    /// @param token The ERC-20 token.
+    /// @param share The amount of shares.
+    /// @param roundUp If the result should be rounded up.
+    /// @return amount The share amount back into native representation.
     function toAmount(
         IERC20 token,
         uint256 share,
@@ -151,6 +161,14 @@ contract BentoBox is MasterContractManager, BoringBatchable {
         amount = totals[token].toElastic(share, roundUp);
     }
 
+    /// @notice Deposit an amount of `token` represented in either `amount` or `share`.
+    /// @param token_ The ERC-20 token to deposit.
+    /// @param from which account to pull the tokens.
+    /// @param to which account to push the tokens.
+    /// @param amount Token amount in native representation to deposit.
+    /// @param share Token amount represented in shares to deposit. Takes precedence over `amount`.
+    /// @return amountOut The amount deposited.
+    /// @return shareOut The deposited amount repesented in shares.
     function deposit(
         IERC20 token_,
         address from,
@@ -208,6 +226,12 @@ contract BentoBox is MasterContractManager, BoringBatchable {
         shareOut = share;
     }
 
+    /// @notice Withdraws an amount of `token` from a user account.
+    /// @param token_ The ERC-20 token to withdraw.
+    /// @param from which user to pull the tokens.
+    /// @param to which user to push the tokens.
+    /// @param amount of tokens. Either one of `amount` or `share` needs to be supplied.
+    /// @param share Like above, but `share` takes precedence over `amount`.
     function withdraw(
         IERC20 token_,
         address from,
@@ -253,6 +277,11 @@ contract BentoBox is MasterContractManager, BoringBatchable {
         shareOut = share;
     }
 
+    /// @notice Transfer shares from a user account to another one.
+    /// @param token The ERC-20 token to transfer.
+    /// @param from which user to pull the tokens.
+    /// @param to which user to push the tokens.
+    /// @param share The amount of `token` in shares.
     // Clones of master contracts can transfer from any account that has approved them
     // F3 - Can it be combined with another similar function?
     // F3: This isn't combined with transferMultiple for gas optimization
@@ -272,6 +301,11 @@ contract BentoBox is MasterContractManager, BoringBatchable {
         emit LogTransfer(token, from, to, share);
     }
 
+    /// @notice Transfer shares from a user account to multiple other ones.
+    /// @param token The ERC-20 token to transfer.
+    /// @param from which user to pull the tokens.
+    /// @param tos The receivers of the tokens.
+    /// @param shares The amount of `token` in shares for each receiver in `tos`.
     // F3 - Can it be combined with another similar function?
     // F3: This isn't combined with transfer for gas optimization
     function transferMultiple(
@@ -295,6 +329,12 @@ contract BentoBox is MasterContractManager, BoringBatchable {
         balanceOf[token][from] = balanceOf[token][from].sub(totalAmount);
     }
 
+    /// @notice Flashloan ability.
+    /// @param borrower The address of the contract that implements and conforms to `IFlashBorrower` and handles the flashloan.
+    /// @param receiver Address of the token receiver.
+    /// @param token The address of the token to receive.
+    /// @param amount of the tokens to receive.
+    /// @param data The calldata to pass to the `borrower` contract.
     // F5 - Checks-Effects-Interactions pattern followed? (SWC-107)
     // F5: Not possible to follow this here, reentrancy has been reviewed
     // F6 - Check for front-running possibilities, such as the approve function (SWC-114)
@@ -315,6 +355,12 @@ contract BentoBox is MasterContractManager, BoringBatchable {
         emit LogFlashLoan(address(borrower), token, amount, fee, receiver);
     }
 
+    /// @notice Support for batched flashloans. Useful to request multiple different `tokens` in a single transaction.
+    /// @param borrower The address of the contract that implements and conforms to `IBatchFlashBorrower` and handles the flashloan.
+    /// @param receivers An array of the token receivers. A one-to-one mapping with `tokens` and `amounts`.
+    /// @param tokens The addresses of the tokens.
+    /// @param amounts of the tokens for each receiver.
+    /// @param data The calldata to pass to the `borrower` contract.
     // F5 - Checks-Effects-Interactions pattern followed? (SWC-107)
     // F5: Not possible to follow this here, reentrancy has been reviewed
     // F6 - Check for front-running possibilities, such as the approve function (SWC-114)
@@ -345,6 +391,10 @@ contract BentoBox is MasterContractManager, BoringBatchable {
         }
     }
 
+    /// @notice Sets the target percentage of the strategy for `token`.
+    /// @dev Only the owner of this contract is allowed to change this.
+    /// @param token The address of the token that maps to a strategy to change.
+    /// @param targetPercentage_ The new target in percent. Must be lesser or equal to `MAX_TARGET_PERCENTAGE`.
     function setStrategyTargetPercentage(IERC20 token, uint64 targetPercentage_) public onlyOwner {
         // Checks
         require(targetPercentage_ <= MAX_TARGET_PERCENTAGE, "StrategyManager: Target too high");
@@ -354,6 +404,12 @@ contract BentoBox is MasterContractManager, BoringBatchable {
         emit LogStrategyTargetPercentage(token, targetPercentage_);
     }
 
+    /// @notice Sets the contract address of a new strategy that conforms to `IStrategy` for `token`.
+    /// Must be called twice with the same arguments.
+    /// A new strategy becomes pending first and can be activated once `STRATEGY_DELAY` is over.
+    /// @dev Only the owner of this contract is allowed to change this.
+    /// @param token The address of the token that maps to a strategy to change.
+    /// @param newStrategy The address of the contract that conforms to `IStrategy`.
     // F5 - Checks-Effects-Interactions pattern followed? (SWC-107)
     // F5: Total amount is updated AFTER interaction. But strategy is under our control.
     // C4 - Use block.timestamp only for long intervals (SWC-116)
@@ -393,6 +449,12 @@ contract BentoBox is MasterContractManager, BoringBatchable {
         }
     }
 
+    /// @notice The actual process of yield farming. Executes the strategy of `token`.
+    /// Optionally does housekeeping if `balance` is true.
+    /// `maxChangeAmount` is relevant for skimming or withdrawing if `balance` is true.
+    /// @param token The address of the token for which a strategy is deployed.
+    /// @param balance True if housekeeping should be done.
+    /// @param maxChangeAmount The maximum amount for either pulling or pushing from/to the `IStrategy` contract.
     // F5 - Checks-Effects-Interactions pattern followed? (SWC-107)
     // F5: Total amount is updated AFTER interaction. But strategy is under our control.
     // F5: Not followed to prevent reentrancy issues with flashloans and BentoBox skims?