SafeMath library is only effective with uint256 types:
using SafeMath for uint16;
Also, uint16 does not make any sense, nor give any improvements here:
uint16 stratReward = IBathHouse(_bathHouse).getBPSToStrats();
function getBPSToStrats() external view returns (uint8);
Consider storing stratReward in uint256 and removing using SafeMath for uint16.
timeDelay and all the functions related to it are not used in any meaningful way. Depending on the intentions, consider either removing it or implementing it where intended.
/// @notice A variable time delay after which a strategist must return funds to the Bath Token
uint256 public timeDelay;
/// @notice Admin-only function to set a Bath Token's timeDelay
function setBathTokenMarket(address bathToken, address newMarket)
From the whitepaper: "BathTokens can be redeemed for the underlying tokens at any time".
However, there exists a reserve ratio, that is initially set to 50%, meaning not all the liquidity is available to redeem at any time. Please keep your users informed and aware of this.
From the whitepaper: FeeBPS "The fee is currently set to zero but could change in the near future"
feeBPS = 3; //Fee set to 3 BPS initially
No need for assembly here, can get it from block.chainid:
assembly {
chainId := chainid()
}
The current best practice is to use safe ERC20 library for token interactions (safeApprove and safeTransfer), e.g.:
take(bytes32(offerId), uint128(offers[offerId].pay_amt)); //We take the whole offer
take(bytes32(offerId), uint128(baux)); //We take the portion of the offer that we need
Make sure that large values will not be truncated due to explicit casting and utilize safe casts where possible.
The array of bonusTokens only grows, elements can't be removed. Consider introducing a reasonable upper limit or remove function, otherwise it may grow so large to that calls to distributeBonusTokenRewards will start reverting due to block gas limitations.
/// @notice Array of Bonus ERC-20 tokens that are given as liquidity incentives to pool withdrawers
address[] public bonusTokens;
/// @notice Admin-only function to add a bonus token to bonusTokens for pool incentives
function setBonusToken(address newBonusERC20) external onlyBathHouse {
bonusTokens.push(newBonusERC20);
}
for (uint256 index = 0; index < bonusTokens.length; index++)
Lack of re-entrancy protection, e.g. function strategistBootyClaim first transfers tokens and only then updates the state. Tokens may contain transfer hooks that can be used to exploit re-entrancy. Either make sure you 100% trust the callers (usually strategists) or implement a re-entrancy guard.
In tailOff a strategist can choose any _stratUtil target. A malicious target can drain the tokens. Consider having a whitelist, at least temporary, and you can later disable it if everything goes smoothly.
Contracts use Solidity version that does not protect from overflow / underflow by default, however, there are regular math operations that could be exploited, e.g.:
SafeMath library is only effective with uint256 types:
Also, uint16 does not make any sense, nor give any improvements here:
Consider storing stratReward in uint256 and removing using SafeMath for uint16.
timeDelay and all the functions related to it are not used in any meaningful way. Depending on the intentions, consider either removing it or implementing it where intended.
BathHouse imports the same contract twice:
Misleading comment:
From the whitepaper: "BathTokens can be redeemed for the underlying tokens at any time". However, there exists a reserve ratio, that is initially set to 50%, meaning not all the liquidity is available to redeem at any time. Please keep your users informed and aware of this.
From the whitepaper: FeeBPS "The fee is currently set to zero but could change in the near future"
No need for assembly here, can get it from block.chainid:
The current best practice is to use safe ERC20 library for token interactions (safeApprove and safeTransfer), e.g.:
https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/token/ERC20/utils/SafeERC20.sol
Consider using SafeCast library:
Also, there are many unchecked casts, e.g.:
Make sure that large values will not be truncated due to explicit casting and utilize safe casts where possible.
The array of bonusTokens only grows, elements can't be removed. Consider introducing a reasonable upper limit or remove function, otherwise it may grow so large to that calls to distributeBonusTokenRewards will start reverting due to block gas limitations.
Lack of re-entrancy protection, e.g. function strategistBootyClaim first transfers tokens and only then updates the state. Tokens may contain transfer hooks that can be used to exploit re-entrancy. Either make sure you 100% trust the callers (usually strategists) or implement a re-entrancy guard.
In tailOff a strategist can choose any _stratUtil target. A malicious target can drain the tokens. Consider having a whitelist, at least temporary, and you can later disable it if everything goes smoothly.
Contracts use Solidity version that does not protect from overflow / underflow by default, however, there are regular math operations that could be exploited, e.g.:
Or:
Consider using SafeMath operations everywhere.