BondDepository Code Style Findings

BDY-01C: Improper Error Messages

Description:

The linked error messages state that the bond has concluded, however, the same error will arise if the capacity is simply exceeded rather than 0 and would otherwise succeed with a payout / _amount less than the limit.

Example:

Copy
184// ensure there is remaining capacity for bond
185if (info.capacityIsPayout) {
186  // capacity in payout terms
187  require(info.capacity >= payout, "Bond concluded");
188  info.capacity = info.capacity.sub(payout);
189} else {
190  // capacity in principal terms
191  require(info.capacity >= _amount, "Bond concluded");
192  info.capacity = info.capacity.sub(_amount);
193}

Recommendation:

We advise the error message to be more descriptive to aid users in adjusting the variables correctly to ensure their action succeeds.

Alleviation:

The Olympus DAO team considered this exhibit but opted not to apply any remediation for it.

BDY-02C: Inexistent Error Messages

Description:

The linked require checks contain no descriptive error messages.

Example:

Copy
67constructor(address _OHM, address _treasury) {
68  require(_OHM != address(0));
69  OHM = IERC20(_OHM);
70  require(_treasury != address(0));
71  treasury = ITreasury(_treasury);
72}

Recommendation:

We advise them to be set so to aid in the debugging of the application and to also enable more accurate validation of the require condition purposes.

Alleviation:

The Olympus DAO team considered this exhibit but opted not to apply any remediation for it.

BDY-03C: Potentially Misleading USD Evaluation

Description:

The on-chain USD evaluation of a bond is solely used for events, however, it appears to be incorrect as its comments indicate that a DAI value is calculated whereas the principal's paired assets are simply used with no relation to the actual DAI token.

Example:

Copy
343/**
344 * @notice converts bond price to DAI value
345 * @param _BID uint
346 * @return price_ uint
347 */
348function bondPriceInUSD(uint256 _BID) public view returns (uint256 price_) {
349  Bond memory bond = bonds[_BID];
350  if (address(bond.calculator) != address(0)) {
351    price_ = bondPrice(_BID).mul(bond.calculator.markdown(address(bond.principal))).div(100);
352  } else {
353    price_ = bondPrice(_BID).mul(10**IERC20Metadata(address(bond.principal)).decimals()).div(100);
354  }
355}

Recommendation:

We advise either the code or the comments surrounding it to be revised to better illustrate the function's purpose.

Alleviation:

The Olympus DAO team considered this exhibit but opted not to apply any remediation for it.

BDY-04C: Redundant Explicit Zero-Value Assignment

Description:

The addBond function redundantly assigns the Terms memory terms pointer to an uninitialized Terms entry.

Example:

Copy
76/**
77 * @notice creates a new bond type
78 * @param _principal address
79 * @param _calculator address
80 * @param _capacity uint
81 * @param _capacityIsPayout bool
82 */
83function addBond(
84  address _principal,
85  address _calculator,
86  uint256 _capacity,
87  bool _capacityIsPayout
88) external onlyGuardian returns (uint256 id_) {
89  Terms memory terms = Terms({controlVariable: 0, fixedTerm: false, vestingTerm: 0, expiration: 0, conclusion: 0, minimumPrice: 0, maxPayout: 0, maxDebt: 0});
90
91  bonds[IDs.length] = Bond({principal: IERC20(_principal), calculator: IBondingCalculator(_calculator), terms: terms, termsSet: false, totalDebt: 0, lastDecay: block.number, capacity: _capacity, capacityIsPayout: _capacityIsPayout});
92
93  id_ = IDs.length;
94  IDs.push(_principal);
95}

Recommendation:

We advise the assignment to be omitted and the pointer to be used in the next statement directly as it points to an uninitialized Terms struct when declared.

Alleviation:

The Olympus DAO team considered this exhibit but opted not to apply any remediation for it.