Code-quality improvements
These following notes on code quality are not directly security related.
Remove unused receive() handlers and companion recoverEth
The vault implementations (HourglassStableVault, HourglassStableVaultFrax, HourglassStableVaultKYC) expose an empty receive() hook:
receive() external payable { }Each contract also wires an admin-only recoverEth helper purely to drain whatever slips in via the passive handler. Because the vaults do not intentionally accept native coin deposits and the ecosystem already exposes targeted recovery flows on the bridge contract, this pattern just introduces maintenance surface without delivering functionality. We recommend dropping both the receive() function and the paired recoverEth entry points so unexpected ETH transfers revert outright.
Force full Spark withdrawal during recovery
Note that HourglassStableVault.withdrawFromSparkRecovery forwards the caller-provided amount to _withdrawFromSpark:
function withdrawFromSparkRecovery(uint256 amount) external onlyMode(OperationalMode.Recovery) nonReentrant {
_withdrawFromSpark(amount);
}Allowing arbitrary partial withdrawals makes the recovery path dependent on user-provided inputs when the goal is to unwind the entire Spark position. Because _withdrawFromSpark(0) already translates to "withdraw everything", consider hardcoding 0 (or otherwise ignoring the argument) so recovery mode always brings all liquidity back into the vault for subsequent share redemptions.
Drop redundant unchecked increment in KYC batch loop
In HourglassStableVaultKYC.batchSetKycStatus, the loop increments i inside an unchecked block:
for (uint256 i = 0; i < length;) {
address user = users[i];
if (user == address(0)) revert ZeroAddress();
_setKycStatusWithAccounting(user, status);
unchecked {
++i;
}
}Since the project targets Solidity ^0.8.29, the compiler already elides redundant overflow checks for simple ++i increments (per the 0.8.22 release). Removing the manual unchecked block makes the loop clearer without changing gas costs.
Remove unreachable DepositCapNotSet branch
Note that HourglassStableVaultDepositCapManager._getMaxDepositAgainstCap guards against an unset cap:
if (_depositCap == 0) revert DepositCapNotSet();However, _depositCap is always initialized through _setDepositCap, which rejects zero values:
if (newCap == 0 || newCap < currentDeposits) revert InvalidDepositCap();Because the zero check can never fire, the DepositCapNotSet revert is effectively dead code. We suggest removing this code.
Use forceApprove for USDT allowances
HourglassStableVault's constructor currently performs a vanilla approve call against USDT:
USDT.approve(address(SPARK_LEND_USDT_POOL), type(uint256).max);Because USDT mandates zeroing allowances before reapproval, switch to OZ’s SafeERC20.forceApprove, which wraps that reset logic. Update the constructor to:
USDT.forceApprove(address(SPARK_LEND_USDT_POOL), type(uint256).max);Consider using a merkle tree for KYC status
The current KYC status management in HourglassStableVaultKYC relies on a mapping and batch updates, which will be gas-intensive for large user bases. Consider implementing a merkle tree-based approach↗ for KYC status verification. However, note that this would require relatively large code changes to implement.