code review comments

Author: aalavandhan

contracts/ACash.sol

@@ -31,11 +31,20 @@ contract ACash is ERC20, Initializable, Ownable {
     // calculates bond price
     IPricingStrategy public pricingStrategy;
 
+    // Yield applied on each tranche
+    // tranche yields is specific to the parent bond's class identified by its config hash
+    // a bond's class is the combination of the {collateralToken, trancheRatios}
+    // specified as a fixed point number with YIELD_DECIMALS
+    mapping(bytes32 => uint256[]) private _trancheYields;
+
     // bondQueue is a queue of Bonds, which have an associated number of seniority-based tranches.
     AddressQueue.Queue public bondQueue;
 
-    // system only keeps bonds which mature further out than the `tolarableBondMaturiy` in the queue
-    uint256 private _tolarableBondMaturiy;
+    // the minimum maturity time in seconds for a bond below which it gets removed from the bond queue
+    uint256 private _minQueueMaturiySec;
+
+    // the maximum maturity time in seconds for a bond above which it can't get added into the bond queue
+    uint256 private _maxQueueMaturiySec;
 
     //---- ERC-20 parameters
     uint8 private immutable _decimals;
@@ -44,6 +53,9 @@ contract ACash is ERC20, Initializable, Ownable {
     // They can only be rolled over and not burnt
     mapping(ITranche => bool) trancheIcebox;
 
+    // constants
+    uint256 public constant YIELD_DECIMALS = 6;
+
     constructor(
         string memory name,
         string memory symbol,
@@ -84,11 +96,16 @@ contract ACash is ERC20, Initializable, Ownable {
 
         uint256 mintAmt = 0;
         for (uint256 i = 0; i < trancheCount; i++) {
+            uint256 trancheYield = _trancheYields[bondIssuer.configHash(mintingBond)][i];
+            if(trancheYield == 0){
+                continue;
+            }
+
             (ITranche t, ) = mintingBond.tranches(i);
             t.safeTransferFrom(_msgSender(), address(this), trancheAmts[i]);
 
             // get bond price, ie amount of SPOT for trancheAmts[i] amount of t tranches
-            mintAmt += pricingStrategy.getTranchePrice(t, trancheAmts[i]);
+            mintAmt += (pricingStrategy.getTranchePrice(t, trancheAmts[i]) * trancheYield) / (10**YIELD_DECIMALS);
         }
 
         int256 fee = feeStrategy.computeMintFee(mintAmt);
@@ -102,8 +119,9 @@ contract ACash is ERC20, Initializable, Ownable {
     // push new bond into the queue
     function advanceMintBond(IBondController newBond) public {
         require(address(newBond) != bondQueue.head(), "New bond already in queue");
-        require(bondIssuer.isInstance(address(newBond)), "Expect new bond to be minted by the minter");
-        require(newBond.maturityDate() > tolarableBondMaturiyDate(), "New bond matures too soon");
+        require(bondIssuer.isInstance(newBond), "Expect new bond to be minted by the minter");
+        require(newBond.maturityDate() > minQueueMaturityDate(), "New bond matures too soon");
+        require(newBond.maturityDate() <= maxQueueMaturityDate(), "New bond matures too late");
 
         bondQueue.enqueue(address(newBond));
     }
@@ -114,7 +132,7 @@ contract ACash is ERC20, Initializable, Ownable {
         while (true) {
             IBondController latestBond = IBondController(bondQueue.tail());
 
-            if (address(latestBond) == address(0) || latestBond.maturityDate() > tolarableBondMaturiyDate()) {
+            if (address(latestBond) == address(0) || latestBond.maturityDate() > minQueueMaturityDate()) {
                 break;
             }
 
@@ -168,12 +186,21 @@ contract ACash is ERC20, Initializable, Ownable {
         feeStrategy = feeStrategy_;
     }
 
-    function setTolarableBondMaturiy(uint256 tolarableBondMaturiy) external onlyOwner {
-        _tolarableBondMaturiy = tolarableBondMaturiy;
+    function setTolarableBondMaturiy(uint256 minQueueMaturiySec, uint256 maxQueueMaturiySec) external onlyOwner {
+        _minQueueMaturiySec = minQueueMaturiySec;
+        _maxQueueMaturiySec = maxQueueMaturiySec;
+    }
+
+    function setTrancheYields(bytes32 configHash, uint256[] memory yields) external onlyOwner {
+        _trancheYields[configHash] = yields;
+    }
+
+    function minQueueMaturityDate() public view returns (uint256) {
+        return block.timestamp + _minQueueMaturiySec;
     }
 
-    function tolarableBondMaturiyDate() public view returns (uint256) {
-        return block.timestamp + _tolarableBondMaturiy;
+    function maxQueueMaturityDate() public view returns (uint256) {
+        return block.timestamp + _maxQueueMaturiySec;
     }
 
     /*

contracts/BondIssuer.sol

@@ -2,6 +2,7 @@ pragma solidity ^0.8.0;
 
 import { IBondFactory } from "./interfaces/button-wood/IBondFactory.sol";
 import { IBondIssuer } from "./interfaces/IBondIssuer.sol";
+import { IBondController } from "./interfaces/button-wood/IBondController.sol";
 
 // A minter periodically mints a specified class of bonds or config
 // a config is uniquely identified by {collateralToken, trancheRatios and duration}
@@ -22,9 +23,10 @@ contract BondIssuer is IBondIssuer {
     uint256 public lastIssueWindowTimestamp;
 
     IBondIssuer.BondConfig public config;
+    bytes32 private _configHash;
 
     // mapping of minted bonds
-    mapping(address => bool) mintedBonds;
+    mapping(IBondController => bool) public issuedBonds;
 
     constructor(
         IBondFactory bondFactory_,
@@ -37,16 +39,24 @@ contract BondIssuer is IBondIssuer {
         minIssueTimeInterval = minIssueTimeInterval_; // 1 week
         issueWindowOffset = issueWindowOffset_; // 7200, 2AM UTC
         bondDuration = bondDuration_; // 4 weeks
+
         config = config_;
+        _configHash = keccak256(abi.encode(config_.collateralToken, config_.trancheRatios));
 
         lastIssueWindowTimestamp = 0;
     }
 
     // checks if bond has been minted using this minter
-    function isInstance(address bond) external view override returns (bool) {
-        return mintedBonds[bond];
+    function isInstance(IBondController bond) external view override returns (bool) {
+        return issuedBonds[bond];
+    }
+
+    // returns the config hash of a given bond if issued by this issuer
+    function configHash(IBondController bond) external view override returns (bytes32) {
+        return issuedBonds[bond] ? _configHash : bytes32(0);
     }
 
+    // issues new bond
     function issue() external override {
         require(
             lastIssueWindowTimestamp + minIssueTimeInterval < block.timestamp,
@@ -61,7 +71,7 @@ contract BondIssuer is IBondIssuer {
             lastIssueWindowTimestamp + bondDuration
         );
 
-        mintedBonds[bond] = true;
+        issuedBonds[IBondController(bond)] = true;
 
         emit BondIssued(bond);
     }

contracts/FeeStrategy.sol

@@ -18,8 +18,8 @@ contract FeeStrategy is Ownable, IFeeStrategy {
 
     // expected mint token to be have the same number of decimals as the fee token
     function computeMintFee(uint256 mintAmt) external view override returns (int256) {
-       # TODO use https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/utils/math/SafeCast.sol
-       # Todo: check all math for overflow reverts 
+        // TODO use https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/utils/math/SafeCast.sol
+        // Todo: check all math for overflow reverts
         return (int256(mintAmt) * mintFeePct) / int256(10**PCT_DECIMALS);
     }
 

contracts/PricingStrategy.sol

@@ -7,24 +7,17 @@ import { IPricingStrategy } from "./interfaces/IPricingStrategy.sol";
 import { IBondIssuer } from "./interfaces/IBondIssuer.sol";
 
 contract PricingStrategy is Ownable, IPricingStrategy {
-    uint256 public constant PCT_DECIMALS = 6;
     uint256 public constant PRICE_DECIMALS = 18;
 
-    // todo: add setters
-    // tranche yields is specific to the parent bond's class identified by its config hash
-    // a bond's class is the combination of the {collateralToken, trancheRatios}
-    mapping(bytes32 => uint256[]) private _trancheYields;
-
     struct TrancheConfig {
         IBondController bond;
         uint256[] trancheRatios;
         uint256 seniorityIDX;
     }
 
-    // tranche_price => yield * price_fn(tranche) * tranche_amount
+    // tranche_price => price_fn(tranche) * tranche_amount
     function getTranchePrice(ITranche t, uint256 trancheAmt) external view override returns (uint256) {
-        uint256 yieldFactor = (getTrancheYield(t) * computeTranchePrice(t)) / (10**PRICE_DECIMALS);
-        return (yieldFactor * trancheAmt) / (10**PCT_DECIMALS);
+        return (computeTranchePrice(t) * trancheAmt) / (10**PRICE_DECIMALS);
     }
 
     // Tranche pricing function goes here:
@@ -34,11 +27,6 @@ contract PricingStrategy is Ownable, IPricingStrategy {
         return (10**PRICE_DECIMALS);
     }
 
-    function getTrancheYield(ITranche t) private view returns (uint256) {
-        TrancheConfig memory c = getTrancheConfig(t);
-        return _trancheYields[keccak256(abi.encode(c.bond.collateralToken(), c.trancheRatios))][c.seniorityIDX];
-    }
-
     // NOTE: this is very gas intensive
     // rebuilding the tranche's pricing parameters though the parent bond
     // Alternatively the bond issuer can map the tranche to these parameters for efficient recovery

contracts/interfaces/IBondIssuer.sol

@@ -1,3 +1,5 @@
+import { IBondController } from "./button-wood/IBondController.sol";
+
 interface IBondIssuer {
     struct BondConfig {
         address collateralToken;
@@ -8,5 +10,7 @@ interface IBondIssuer {
 
     function issue() external;
 
-    function isInstance(address bond) external view returns (bool);
+    function isInstance(IBondController bond) external view returns (bool);
+
+    function configHash(IBondController bond) external view returns (bytes32);
 }