- High: 0
- Medium: 1
- Low: 1
- Gas/Info: 1
The DSCEngine.sol contract may face functionality issues if it is deployed with certain ERC20 tokens as approved collateral. These tokens, which do not return a boolean on their transfer methods (e.g. USDT), will cause multiple functions in DSCEngine.sol to fail consistently.
During the contract deployment, there is no check in the constructor to ensure that the approved collateral tokens strictly adhere to the ERC20 standard. Consequently, it is possible to deploy the contract with tokens that do not return a boolean value on transfer methods, leading to subsequent failures in some of the contract's key functions.
The functions depositCollateral and _redeemCollateral in the DSCEngine.sol contract will not operate as expected when dealing with ERC20 tokens that do not return a boolean on their transfer functions. This could significantly impair the contract's core functionality.
Manual review
Consider using the SafeERC20 library from Open Zeppelin and call safeTransfer or safeTransferFrom when transferring ERC20 tokens
All .sol files in src & script are currently using a floating pragma, ^0.8.18, which allows for potential inconsistencies and vulnerabilities due to differences between Solidity compiler versions. It is possible these contracts get deployed with an outdated compiler version that might introduce bugs that negatively affect the stablecoin system.
Manual review
Contracts should be deployed with the same compiler version they were tested with. Fix all pragmas to 0.8.19.
https://github.com/Cyfrin/2023-07-foundry-defi-stablecoin/blob/main/src/DSCEngine.sol#L181
This comment on the redeemCollateral function is misleading
* @notice If you have DSC minted, you will not be able to redeem until you burn your DSC
The redeemCollateral function does not directly require the user to burn DSC to redeem their collateral. Instead, it checks whether the operation would break the health factor. In cases where a user has a high collateralization ratio, they may redeem some of their collateral without burning DSC while keeping their health factor above the threshold. Therefore, the NatSpec comment may inaccurately represent the redeemCollateral functionality under certain conditions.
PoC: Add this test to DSCEngineTest.t.sol and it passes
function testCanRedeemCollateralWithSomeDSCMintedAndNotBurnDSC() public {
//user deposits a large amout of weth and mints a small amount of dsc
vm.startPrank(user);
ERC20Mock(weth).approve(address(dsce), 1000);
dsce.depositCollateralAndMintDsc(weth, 1000, 1);
//user redeems some collateral without burning any dsc
dsce.redeemCollateral(weth, 10);
vm.stopPrank;
}
This comment can lead users and auditors to misunderstand how the function works.
Manual Review
Remove the NatSpec line or further clarify that the redeemCollateral function may revert if the user has too much DSC minted and will need to burn DSC before calling the function again.