Fixed mint bug and small improvements

.prettierignore

@@ -3,3 +3,5 @@ artifacts
 cache
 coverage*
 gasReporterOutput.json
+typechain
+typechain-types

Readme.md

@@ -36,31 +36,38 @@
 1. refactoring: scripts/helper.ts
 1. add comments to unified test scripts
 1. improve unified test performance
+1. write our own version of safeApprove: https://github.com/OpenZeppelin/openzeppelin-contracts/issues/2219
 
-## Releasing a new strategy
+## Versioning of strategies
 
 The following section explains how the versioning of the strategies needs to be done in order to
 always be able to retrieve the source code of the deployed strategies, even if the strategy was not verified on the blockchain. Even if the strategy was verified, we would still need a way to easily find the git commit from which the deployed strategy was built.
 
-### Git
+### Version number
+
+Each strategy has a version number in semver format: https://semver.org/. This version number has to be part of the git tag.
+
+The most important thing is to bump the major version number at each breaking changes for example if the current verstion was 2.3.1, and there was a breaking change, then the new version should be 3.0.0
+
+## Post strategy release tasks
+
+### Git tagging
 
 Currently we have a very simple git workflow. New strategies are released from the main branch
-and tagged by part of the 'name' of the strategy. For example if a new version of the cash strategy is released, and the current 'name' property in the source code of that strategy has the value of **block42.cash_strategy.cash_strategy_v1.0.2**, then the following git commands has to be executed.
+and tagged by part of the 'name' of the strategy. For example if a new version of the cash strategy is released, and the current 'name' property in the source code of that strategy has the value of **brokkr.cash_strategy.cash_strategy_v1.0.2**, then the following git commands has to be executed.
 
 ```bash
 git checkout main
 git pull
 # the tag name will be the the string following the last dot of the name, in the example the name was
-# block42.cash_strategy.cash_strategy_v1.0.2
+# brokkr.cash_strategy.cash_strategy_v1.0.2
 git tag -a cash_strategy_v1.0.2
 # the command above will open a text editor, please write down how this version is different from the previuos one
 git push origin cash_strategy_v1.0.2
 ```
 
 If you release multiple strategies at once, please create the tags for all strategies.
 
-### Versioning of strategies
-
-Each strategy has a version number in semver format: https://semver.org/. This version number has to be part of the git tag.
+### Verifying deployed contracts on Snowtrace
 
-The most important thing is to bump the major version number at each breaking changes for example if the current verstion was 2.3.1, and there was a breaking change, then the new version should be 3.0.0
+Both the logic contract and the proxy contract should be verified after deploying them.

contracts/common/bases/PortfolioBaseUpgradeable.sol

@@ -216,41 +216,38 @@ abstract contract PortfolioBaseUpgradeable is
         if (amount == 0) revert ZeroAmountDeposited();
 
         // check investment limits
-        uint256 totalEquity;
+        // the underlying defi protocols might take fees, but for limit check we can safely ignore it
+        uint256 equityValuationBeforeInvestment = getEquityValuation(
+            true,
+            false
+        );
         uint256 userEquity;
         uint256 investmentTokenSupply = getInvestmentTokenSupply();
         if (investmentTokenSupply != 0) {
-            totalEquity = getEquityValuation(true, false);
-
             uint256 investmentTokenBalance = getInvestmentTokenBalanceOf(
                 investmentTokenReceiver
             );
             userEquity =
-                (totalEquity * investmentTokenBalance) /
+                (equityValuationBeforeInvestment * investmentTokenBalance) /
                 investmentTokenSupply;
         }
-        checkTotalInvestmentLimit(amount, totalEquity);
+        checkTotalInvestmentLimit(amount, equityValuationBeforeInvestment);
         checkInvestmentLimitPerAddress(amount, userEquity);
 
+        // transfering deposit tokens from the user
         depositToken.safeTransferFrom(_msgSender(), address(this), amount);
-        uint256 equity = getEquityValuation(true, false);
-        uint256 investmentTokenTotalSupply = getInvestmentTokenSupply();
-        investmentToken.mint(
-            investmentTokenReceiver,
-            InvestableLib.calculateMintAmount(
-                equity,
-                amount,
-                investmentTokenTotalSupply
-            )
-        );
+
+        // 1. emitting event for portfolios at the higher level first
+        // 2. emitting the deposit amount versus the actual invested amount
         emit Deposit(_msgSender(), investmentTokenReceiver, amount);
+
         for (uint256 i = 0; i < investableDescs.length; i++) {
             uint256 embeddedAmount = (amount *
                 investableDescs[i].allocationPercentage) /
                 Math.SHORT_FIXED_DECIMAL_FACTOR /
                 100;
             if (embeddedAmount == 0) continue;
-            depositToken.safeApprove(
+            depositToken.approve(
                 address(investableDescs[i].investable),
                 embeddedAmount
             );
@@ -260,6 +257,26 @@ abstract contract PortfolioBaseUpgradeable is
                 params
             );
         }
+
+        // calculating the actual amount invested into the defi protocol
+        uint256 equityValuationAfterInvestment = getEquityValuation(
+            true,
+            false
+        );
+        uint256 actualInvested = equityValuationAfterInvestment -
+            equityValuationBeforeInvestment;
+        if (actualInvested == 0) revert ZeroAmountInvested();
+
+        // minting should be based on the actual amount invested versus the deposited amount
+        // to take defi fees and losses into consideration
+        investmentToken.mint(
+            investmentTokenReceiver,
+            InvestableLib.calculateMintAmount(
+                equityValuationBeforeInvestment,
+                actualInvested,
+                investmentTokenSupply
+            )
+        );
     }
 
     function withdraw(
@@ -277,7 +294,7 @@ abstract contract PortfolioBaseUpgradeable is
                 .getInvestmentTokenBalanceOf(address(this)) * amount) /
                 investmentTokenSupply;
             if (embeddedTokenAmountToBurn == 0) continue;
-            embeddedInvestable.getInvestmentToken().safeApprove(
+            embeddedInvestable.getInvestmentToken().approve(
                 address(embeddedInvestable),
                 embeddedTokenAmountToBurn
             );
@@ -349,7 +366,7 @@ abstract contract PortfolioBaseUpgradeable is
                         address(this)
                     ) * targetInvestableEquities[i]) /
                     currentInvestableEquities[i];
-                embeddedInvestable.getInvestmentToken().safeApprove(
+                embeddedInvestable.getInvestmentToken().approve(
                     address(embeddedInvestable),
                     withdrawAmount
                 );

contracts/common/bases/StrategyBaseUpgradeable.sol

@@ -104,35 +104,52 @@ abstract contract StrategyBaseUpgradeable is
         if (amount == 0) revert ZeroAmountDeposited();
 
         // check investment limits
-        uint256 totalEquity;
+        // the underlying defi protocols might take fees, but for limit check we can safely ignore it
+        uint256 equityValuationBeforeInvestment = getEquityValuation(
+            true,
+            false
+        );
         uint256 userEquity;
         uint256 investmentTokenSupply = getInvestmentTokenSupply();
         if (investmentTokenSupply != 0) {
-            totalEquity = getEquityValuation(true, false);
-
             uint256 investmentTokenBalance = getInvestmentTokenBalanceOf(
                 investmentTokenReceiver
             );
             userEquity =
-                (totalEquity * investmentTokenBalance) /
+                (equityValuationBeforeInvestment * investmentTokenBalance) /
                 investmentTokenSupply;
         }
-        checkTotalInvestmentLimit(amount, totalEquity);
+        checkTotalInvestmentLimit(amount, equityValuationBeforeInvestment);
         checkInvestmentLimitPerAddress(amount, userEquity);
 
+        // transfering deposit tokens from the user
         depositToken.safeTransferFrom(_msgSender(), address(this), amount);
 
-        uint256 equity = getEquityValuation(true, false);
-        uint256 investmentTokenTotalSupply = getInvestmentTokenSupply();
+        // investing into the underlying defi protocol
+        _deposit(amount, params);
+
+        // calculating the actual amount invested into the defi protocol
+        uint256 equityValuationAfterInvestment = getEquityValuation(
+            true,
+            false
+        );
+
+        uint256 actualInvested = equityValuationAfterInvestment -
+            equityValuationBeforeInvestment;
+        if (actualInvested == 0) revert ZeroAmountInvested();
+
+        // minting should be based on the actual amount invested versus the deposited amount
+        // to take defi fees and losses into consideration
         investmentToken.mint(
             investmentTokenReceiver,
             InvestableLib.calculateMintAmount(
-                equity,
-                amount,
-                investmentTokenTotalSupply
+                equityValuationBeforeInvestment,
+                actualInvested,
+                investmentTokenSupply
             )
         );
-        _deposit(amount, params);
+
+        // emitting the deposit amount versus the actual invested amount
         emit Deposit(_msgSender(), investmentTokenReceiver, amount);
     }
 

contracts/common/interfaces/IInvestable.sol

@@ -8,6 +8,7 @@ import "./IInvestmentToken.sol";
 
 interface IInvestable is IAum, IFee {
     error ZeroAmountDeposited();
+    error ZeroAmountInvested();
     error ZeroAmountWithdrawn();
 
     event Deposit(

contracts/portfolios/percentageAllocation/PercentageAllocation.sol

@@ -11,12 +11,12 @@ contract PercentageAllocation is
 {
     // solhint-disable-next-line const-name-snakecase
     string public constant name =
-        "block42.percentage_allocation_portfolio.percentage_allocation_portfolio_v1.0.0";
+        "brokkr.percentage_allocation_portfolio.percentage_allocation_portfolio_v1.0.1";
     // solhint-disable-next-line const-name-snakecase
     string public constant humanReadableName =
         "Percentage allocation portfolio";
     // solhint-disable-next-line const-name-snakecase
-    string public constant version = "1.0.0";
+    string public constant version = "1.0.1";
 
     /// @custom:oz-upgrades-unsafe-allow constructor
     constructor() {

contracts/strategies/cash/Cash.sol

@@ -8,11 +8,11 @@ import "@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol";
 
 contract Cash is UUPSUpgradeable, StrategyOwnablePausableBaseUpgradeable {
     // solhint-disable-next-line const-name-snakecase
-    string public constant name = "block42.cash_strategy.cash_strategy_v1.0.0";
+    string public constant name = "brokkr.cash_strategy.cash_strategy_v1.0.1";
     // solhint-disable-next-line const-name-snakecase
     string public constant humanReadableName = "Cash strategy";
     // solhint-disable-next-line const-name-snakecase
-    string public constant version = "1.0.0";
+    string public constant version = "1.0.1";
 
     /// @custom:oz-upgrades-unsafe-allow constructor
     constructor() {

contracts/strategies/stargate/Readme.md

@@ -8,8 +8,10 @@ This strategy invests USDC into USDC or USDT pool of Stargate on Avalanche.
 
 ## High priority
 
-1. solve the delta credit/liqidity issue
+1. Solve the delta credit/liqidity issue.
 
 ## Medium priority
 
+1. Expose 2 methods to be able to change the swapping path for deposit and reap reward.
+
 ## Low priority

contracts/strategies/stargate/Stargate.sol

@@ -18,11 +18,11 @@ contract Stargate is UUPSUpgradeable, StrategyOwnablePausableBaseUpgradeable {
 
     // solhint-disable-next-line const-name-snakecase
     string public constant name =
-        "brokkr.stargate_strategy.stargate_strategy_v1.0.0";
+        "brokkr.stargate_strategy.stargate_strategy_v1.0.1";
     // solhint-disable-next-line const-name-snakecase
     string public constant humanReadableName = "Stargate Strategy";
     // solhint-disable-next-line const-name-snakecase
-    string public constant version = "1.0.0";
+    string public constant version = "1.0.1";
 
     /// @custom:oz-upgrades-unsafe-allow constructor
     constructor() {
@@ -168,10 +168,9 @@ contract Stargate is UUPSUpgradeable, StrategyOwnablePausableBaseUpgradeable {
 
         strategyStorage.lpStaking.deposit(strategyStorage.farmId, 0);
 
-        address[] memory path = new address[](3);
+        address[] memory path = new address[](2);
         path[0] = address(strategyStorage.stgToken);
-        path[1] = address(InvestableLib.WAVAX);
-        path[2] = address(depositToken);
+        path[1] = address(depositToken);
 
         swapExactTokensForTokens(
             swapService,

contracts/strategies/template/Template.sol

@@ -12,7 +12,7 @@ import "@openzeppelin/contracts-upgradeable/token/ERC20/utils/SafeERC20Upgradeab
 contract Template is StrategyOwnablePausableBaseUpgradeable {
     // solhint-disable-next-line const-name-snakecase
     string public constant name =
-        "block42.template_strategy.<insert git label here>";
+        "brokkr.template_strategy.<insert git label here>";
     // solhint-disable-next-line const-name-snakecase
     string public constant humanReadableName = "Template strategy";
     // solhint-disable-next-line const-name-snakecase

contracts/strategies/traderjoe/TraderJoe.sol

@@ -17,11 +17,11 @@ contract TraderJoe is UUPSUpgradeable, StrategyOwnablePausableBaseUpgradeable {
 
     // solhint-disable-next-line const-name-snakecase
     string public constant name =
-        "brokkr.traderjoe_strategy.traderjoe_strategy_v1.0.0";
+        "brokkr.traderjoe_strategy.traderjoe_strategy_v1.0.1";
     // solhint-disable-next-line const-name-snakecase
     string public constant humanReadableName = "TraderJoe Strategy";
     // solhint-disable-next-line const-name-snakecase
-    string public constant version = "1.0.0";
+    string public constant version = "1.0.1";
 
     /// @custom:oz-upgrades-unsafe-allow constructor
     constructor() {

contracts/tests/MockPortfolio.sol

@@ -6,7 +6,7 @@ import "../common/bases/PortfolioOwnableBaseUpgradeable.sol";
 contract MockPortfolio is PortfolioOwnableBaseUpgradeable {
     // solhint-disable-next-line const-name-snakecase
     string public constant name =
-        "block42.mock_portfolio.<insert git label here>";
+        "brokkr.mock_portfolio.<insert git label here>";
     // solhint-disable-next-line const-name-snakecase
     string public constant humanReadableName = "Mock portfolio";
     // solhint-disable-next-line const-name-snakecase

contracts/tests/MockStrategy.sol

@@ -13,7 +13,7 @@ contract MockStrategy is
 {
     // solhint-disable-next-line const-name-snakecase
     string public constant name =
-        "block42.mock_strategy.<insert git label here>";
+        "brokkr.mock_strategy.<insert git label here>";
     // solhint-disable-next-line const-name-snakecase
     string public constant humanReadableName = "Mock strategy";
     // solhint-disable-next-line const-name-snakecase

contracts/tests/portfolios/percentageAllocation/PercentageAllocationV2.sol

@@ -11,7 +11,7 @@ contract PercentageAllocationV2 is
 {
     // solhint-disable-next-line const-name-snakecase
     string public constant name =
-        "block42.percentage_allocation_portfolio.percentage_allocation_portfolio_v2.0.0";
+        "brokkr.percentage_allocation_portfolio.percentage_allocation_portfolio_v2.0.0";
     // solhint-disable-next-line const-name-snakecase
     string public constant humanReadableName =
         "Percentage allocation portfolio";

contracts/tests/strategies/cash/CashV2.sol

@@ -8,7 +8,7 @@ import "@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol";
 
 contract CashV2 is UUPSUpgradeable, StrategyOwnablePausableBaseUpgradeable {
     // solhint-disable-next-line const-name-snakecase
-    string public constant name = "block42.cash_strategy.cash_strategy_initial";
+    string public constant name = "brokkr.cash_strategy.cash_strategy_initial";
     // solhint-disable-next-line const-name-snakecase
     string public constant humanReadableName = "Cash strategy";
     // solhint-disable-next-line const-name-snakecase

hardhat.config.ts

@@ -63,6 +63,9 @@ const config: HardhatUserConfig = {
     enabled: process.env.REPORT_GAS !== undefined,
     currency: "USD",
   },
+  mocha: {
+    timeout: 90000,
+  },
   // contractSizer: {
   //   alphaSort: true,
   //   runOnCompile: true,

package.json

@@ -3,7 +3,7 @@
   "version": "1.0.0",
   "description": "template for project setup",
   "main": "index.js",
-  "author": "block42",
+  "author": "brokkr",
   "license": "MIT",
   "devDependencies": {
     "@aave/core-v3": "^1.13.1",

test/portfolio/Unified.test.ts

@@ -1,5 +1,5 @@
-import { ethers, network } from "hardhat"
 import { takeSnapshot } from "@nomicfoundation/hardhat-network-helpers"
+import { ethers, network } from "hardhat"
 import { TokenAddrs, WhaleAddrs } from "../shared/addresses"
 import { getTokenContract } from "../shared/contracts"
 import { testAllocations } from "./UnifiedAllocations.test"

test/portfolio/UnifiedAllocations.test.ts

@@ -54,7 +54,7 @@ export function testAllocations() {
       )
     })
 
-    it("should success when the sum of target investable allocations equals to 100% and the length of target investable allocations equals to the length of investables", async function () {
+    it("should succeed when the sum of target investable allocations equals to 100% and the length of target investable allocations equals to the length of investables", async function () {
       const investableLength = (await this.portfolio.getInvestables()).length
       let allocations: number[] = [100000]
       for (let i = 1; i < investableLength; i++) {