Cyfrin / foundry-defi-stablecoin-cu

241 stars 107 forks source link

Invariant test causes `redeemCollateral` to throw `[Revert] DSCEngine__HealthFactorBelowMinimum(412043441151367101)` #82

Closed Ed-Marcavage closed 3 months ago

Ed-Marcavage commented 3 months ago

When I implemented, redeem, deposit, and mintDsc in my invariant test (around video 22 of the section), my redeemCollateral handler started to break because of a health factor < 1.

Did anyone else experience this? I didn't see any similar issues reported, so I may have missed an aspect of the tutorial (I double-checked my code with this repo, and all seems correct from what I checked so far).

However, the code below fixed the issue by avoiding calls to redeem that would redeem too much collateral—lowering the HF below 1.

If this is a valid bug, please let me know, and I'd be happy to open a PR.

    function redeemCollateral(uint256 collateralSeed, uint256 amountCollateral) public {
        ERC20Mock collateral = _getCollateralFromSeed(collateralSeed);
        uint256 maxCollateral = dscEngine.getCollateralBalanceOfUser(msg.sender, address(collateral));

        amountCollateral = bound(amountCollateral, 0, maxCollateral);
        //vm.prank(msg.sender);
        if (amountCollateral == 0) {
            return;
        }
+        uint256 collateralUSD = dsce.getAccountCollateralValue(msg.sender);
+        uint256 dscValue = dsce.getDSCMinted(msg.sender);
+        uint256 amountCollateralUSD = dsce.getUsdValue(
+            address(collateral),
+           amountCollateral
+        );
+   if (
+        dsce.calculateHealthFactor(
+             dscValue,
+              collateralUSD - amountCollateralUSD
+           ) < 1e18
+        ) {
+            return;
+        }
        vm.prank(msg.sender);
        dscEngine.redeemCollateral(address(collateral), amountCollateral);
    }

Here are my logs before the fix: image image

PatrickAlphaC commented 3 months ago

Yes! This is great! I'd like to leave the codebase as such tho so other people can find this too though :)