StrategyBase - Inheritance-Related Issues
A. The `StrategyBase` contract defines `view` functions that, given an amount of shares, return the equivalent amount of tokens (sharesToUnderlyingView) and vice versa (underlyingToSharesView). These two functions also have non-view counterparts: `sharesToUnderlying` and `underlyingToShares`, and their NatSpec documentation explicitly states that they should be allowed to make state changes. Given the scope of this engagement, it is unclear if these non-view versions are needed, but assuming they are, this does currently not work as intended.\\nFirst, the interface `IStrategy` declares `underlyingToShares` as `view` (unlike sharesToUnderlying). This means overriding this function in derived contracts is impossible without the `view` modifier. Hence, in `StrategyBase` - which implements the `IStrategy` interface - this (virtual) function is (and has to be) `view`. The same applies to overridden versions of this function in contracts inherited from `StrategyBase`.\\n```\\n/\\*\\*\\n \\* @notice Used to convert an amount of underlying tokens to the equivalent amount of shares in this strategy.\\n \\* @notice In contrast to `underlyingToSharesView`, this function \\*\\*may\\*\\* make state modifications\\n \\* @param amountUnderlying is the amount of `underlyingToken` to calculate its conversion into strategy shares\\n \\* @dev Implementation for these functions in particular may vary signifcantly for different strategies\\n \\*/\\nfunction underlyingToShares(uint256 amountUnderlying) external view returns (uint256);\\n```\\n\\n```\\n/\\*\\*\\n \\* @notice Used to convert an amount of underlying tokens to the equivalent amount of shares in this strategy.\\n \\* @notice In contrast to `underlyingToSharesView`, this function \\*\\*may\\*\\* make state modifications\\n \\* @param amountUnderlying is the amount of `underlyingToken` to calculate its conversion into strategy shares\\n \\* @dev Implementation for these functions in particular may vary signifcantly for different strategies\\n \\*/\\nfunction underlyingToShares(uint256 amountUnderlying) external view virtual returns (uint256) {\\n    return underlyingToSharesView(amountUnderlying);\\n}\\n```\\n\\nAs mentioned above, the `sharesToUnderlying` function does not have the `view` modifier in the interface `IStrategy`. However, the overridden (and virtual) version in `StrategyBase` does, which means again that overriding this function in contracts inherited from `StrategyBase` is impossible without the `view` modifier.\\n```\\n/\\*\\*\\n \\* @notice Used to convert a number of shares to the equivalent amount of underlying tokens for this strategy.\\n \\* @notice In contrast to `sharesToUnderlyingView`, this function \\*\\*may\\*\\* make state modifications\\n \\* @param amountShares is the amount of shares to calculate its conversion into the underlying token\\n \\* @dev Implementation for these functions in particular may vary signifcantly for different strategies\\n \\*/\\nfunction sharesToUnderlying(uint256 amountShares) public view virtual override returns (uint256) {\\n    return sharesToUnderlyingView(amountShares);\\n}\\n```\\n\\nB. The `initialize` function in the `StrategyBase` contract is not virtual, which means the name will not be available in derived contracts (unless with different parameter types). It also has the `initializer` modifier, which is unavailable in concrete strategies inherited from `StrategyBase`.
A. If state-changing versions of the conversion functions are needed, the `view` modifier has to be removed from `IStrategy.underlyingToShares`, `StrategyBase.underlyingToShares`, and `StrategyBase.sharesToUnderlying`. They should be removed entirely from the interface and base contract if they're not needed.\\nB. Consider making the `StrategyBase` contract `abstract`, maybe give the `initialize` function a more specific name such as `_initializeStrategyBase`, change its visibility to `internal`, and use the `onlyInitializing` modifier instead of `initializer`.
```\\n/\\*\\*\\n \\* @notice Used to convert an amount of underlying tokens to the equivalent amount of shares in this strategy.\\n \\* @notice In contrast to `underlyingToSharesView`, this function \\*\\*may\\*\\* make state modifications\\n \\* @param amountUnderlying is the amount of `underlyingToken` to calculate its conversion into strategy shares\\n \\* @dev Implementation for these functions in particular may vary signifcantly for different strategies\\n \\*/\\nfunction underlyingToShares(uint256 amountUnderlying) external view returns (uint256);\\n```\\n