Informational - Minor nitpicks

[kitty-the-kat]

General nitpicks with comments, naming, and typos in the contracts.

Technical Details

1. Consider naming withdrawalQueue(uint256 i) to withdrawalQueueAt(uint256 i) and withdrawalQueue() to fullWithdrawalQueue().
2. Consider clarifying the strategy return value is an address, not a Strategy struct.
3. getStrategyDebt() and getStrategyAssets() return the totalDebt of a strategy and might be improved with more similar names to avoid confusion over debt vs. assets.
4. NatSpec is incomplete for some functions, such as missing return value descriptions for beforeWithdraw()/excessDebt/_excessDebt and no NatSpec for _removeStrategy() in GVault
5. depositIntoTrancheForCaller() is missing a comment that _token_index of 3 or greater is 3CRV
6. Typo: adapetr -> adapter
7. Typo: Apporve -> Approve
8. Typo: CHIANLINK_FACTOR -> CHAINLINK_FACTOR
9. Typo: add_liquididty -> add_liquidity
10. Typo: strategies -> strategy's and same here
11. Typo: enstimated -> estimated
12. Typo: excluding and profits -> excluding profits
13. Typo: srategy -> strategy
14. Typo: do generate -> to generate
15. Typo: unledrying -> underlying
16. Typo: beneth -> beneath and here
17. Typo: baring -> bearing
18. Typo: extensoin -> extension
19. Typo: their -> there
20. Typo: underlyng -> underlying
21. Typo: prive -> price
22. Typo: it -> its
23. Typo: utiliszation -> utilisation
24. Typo: _tranchTokens -> _trancheTokens
25. Typo: between to underlying -> between the underlying
26. Typo: amount of price -> amount of yield token
27. Typo: and ontermiadry -> an intermediary
28. Typo: amount of transform (unclear what this means)
29. Typo: setUtilizationThreshold() (note utilization with a 'z') sets the variable utilisationThreshold (note utilisation with 's') and there is a function utilization() (note utilization with a 'z')
30. Typo: experiene -> experience
31. Improve precision by changing ((_lockedProfit / _releaseTime) * _timeSinceLastReport) to ((_lockedProfit * _timeSinceLastReport) / _releaseTime) to match Vault.vy approach
32. Fix this comment that references a non-existent emergencyExit() function
33. The emergency boolean function argument is missing a NatSpec comment as is _calcFactor() in GTranche and _loss in PnL
34. The debt variable is not used for any purpose. It may be better to simply compare debtPayment to the value of _excessDebt(msg.sender) to replace the safeMath in this line.
35. Inaccurate NatSpec for withdraw()'s _amount (better would be "asset quantity needed by Vault if not holding enough asset balance") and missing NatSpec for return values
36. Replace PnL magic numbers with constants. For example, replace 10000 with utilisationThreshold.
37. Junior Tranche is branded as GVT token, so this comment should replace PWRD with GVT
38. Consider a better name than "controller" or "ctrl" in GToken for the GTranche address, because the word "controller" does not appear anywhere in GTranche
39. Assets in Convex are not locked and therefore are not used to vote in reward distribution. There are potential downsides to this approach and this choice should be documented somewhere in Gro's documentation. The strategy that ConvexStrategy was inspired by does lock some tokens for voting.
40. _claimableRewards() in ConvexStrategy does not return a value if MIN_REWARD_SELL_AMOUNT is not met and this if statement is not entered
41. Consider replacing slippage in divestAll() with the baseSlippage value used elsewhere because baseSlippage can be modified by the owner unlike slippage
42. estimatedTotalAssets() should replace _estimatedTotalAssets(true) with _estimatedTotalAssets(false) because the rewards return value is not needed
43. Incomplete NatSpec for factor in _calcTrancheValue() and factor elsewhere in GTranche
44. This return is redundant, the named return values would be returned properly without this line
45. The SafeMath OpenZeppelin import in GToken is redundant because solidity 0.8.10 is used. The contract should be updated accordingly.
46. The Ownable import in GToken is redundant because the import of Whitelist.sol includes Ownable already
47. _calcTokenAmount() can remove the _deposit boolean function argument because it is never used for anything useful in the function
48. The NatSpec in CurveOracle uses the term "yield token" to mostly refer to 3CRV while FixedTokensCurve NatSpec uses "yield token" to mostly refer to GVault shares. Consider terms that more clearly differentiate the tokens.
49. Remove unused _tranche bool function argument from _calcTrancheValue()
50. Remove if (_factor == 0) logic from _calcTrancheValue() because this can never happen based on current contract logic
51. lastDistribution could be uint32 instead of uint64
52. Consider renaming _calcFees() to _gainSubFees()

Impact

Informational.

Recommendation

Consider fixing nitpicks.

[kitty-the-kat]

Acknowledged - Fixed a considerable amount of the nitpicks