Informational Issues

[0xMilenov]

	Issue	Instances	Gas Savings
[N-01]	The risks of relying on assert() in solidity contracts	1	None
[N-02]	Prefer string.concat() or bytes.concat() to abi.encodePacked	1	None
[N-03]	Recommendation to use uint48 for time-centric variables	3	None
[N-04]	Unresolved TODO comments in code	1	None
[N-05]	Use of override is unnecessary	2	None

[N-01] The risks of relying on assert() in solidity contracts

In Solidity, the assert() function is meant to handle conditions that should never be false, and if they are, it points to a bug within the contract.

Before Solidity 0.8.0, when assert() is triggered, it consumes all of the remaining gas in a transaction, which differs from the behavior of require() and revert() that return unspent gas.

Even in versions after 0.8.0, it's prudent to minimize the use of assert(). The Solidity documentation suggests that code should never reach a state where assert() is triggered. Instead, for better clarity and understanding of contract failures, developers are advised to utilize require() statements or custom error messages.

By doing so, they can provide a more transparent and user-friendly contract, which aids in identifying and rectifying issues more efficiently.

File: contracts/StabilityPool.sol

}482:         assert(_stableCoinLossPerUnitStaked < = DECIMAL_PRECISION);
}571:         assert(newP  > 0);
}828:         assert(_debtToOffset < = _totalStableCoinDeposits);

482, 571, 828

[N-02] Prefer string.concat() or bytes.concat() to abi.encodePacked

With the release of Solidity version 0.8.4, the language introduced bytes.concat() as a clearer alternative to the abi.encodePacked(< bytes >,< bytes >) method for concatenating byte sequences.

Similarly, Solidity version 0.8.12 brought string.concat(), providing a direct means for string concatenation, sidestepping the abi.encodePacked(< str >,< str >) approach.

Incorporating these recent methods can improve code readability and align your contracts with modern Solidity practices.

File: contracts/OwnerProxy.sol

}43:             keccak256(abi.encodePacked(caller, targetAddress, targetSignature))
}72:             keccak256(abi.encodePacked(_msgSender(), target, _targetSignature))

43, 72

[N-03] Recommendation to use uint48 for time-centric variables

For time-relevant variables, uint32 might be restrictive as it ends in 2106, potentially posing challenges in the far future.

On the other hand, using excessively large types like uint256 is unnecessary. Thus, uint48 offers a balanced choice for such use cases.

File: contracts/TBANKStaking.sol

}29:     uint256 public lastFeeOperationTime;
}75:     function setInitialLastFee(uint256 _timestamp) public onlyOwner {

29, 75

[N-04] Unresolved TODO comments in code

The presence of TODO comments suggests that certain features might be incomplete or not ready for audit.

It's advisable to address these issues and remove the TODO markers from the code.

File: contracts/StabilityPool.sol

}458:         ][cachedScale]; // TODO: maybe remove and read twice?
}462:         depositSnapshots[_depositor].tokenToSArray = cachedTokenToSArray; // TODO

458, 462

[N-05] Use of override is unnecessary

Starting with Solidity version 0.8.8, using the override keyword when the function solely overrides an interface function, and the function doesn't exist in multiple base contracts, is unnecessary.

File: contracts/VaultExtraSettings.sol

}24:     ) external override onlyOwner {
}35:     ) external override onlyOwner {
}46:         override

24, 35, 46