Anyone can mint despite having collateral or not

[0fprod]

Let's say Alice wants to mint 10 dsc for the first time. So she can call this function because it is public

 /*
     * @param amountDscToMint: The amount of DSC you want to mint
     * You can only mint DSC if you hav enough collateral
     */
    function mintDsc(uint256 amountDscToMint) public moreThanZero(amountDscToMint) nonReentrant {
        s_DSCMinted[msg.sender] += amountDscToMint;
        revertIfHealthFactorIsBroken(msg.sender);
        bool minted = i_dsc.mint(msg.sender, amountDscToMint);

        if (minted != true) {
            revert DSCEngine__MintFailed();
        }
    }

Then this gets called

function revertIfHealthFactorIsBroken(address user) internal view {
        uint256 userHealthFactor = _healthFactor(user);
        if (userHealthFactor < MIN_HEALTH_FACTOR) {
            revert DSCEngine__BreaksHealthFactor(userHealthFactor);
        }
    }

Then this. At this point we assume totalDscMinted and collateralValueInUsd are 0 because its her first time, Alice never deposited collateral.

function _healthFactor(address user) private view returns (uint256) {
        (uint256 totalDscMinted, uint256 collateralValueInUsd) = _getAccountInformation(user);
        return _calculateHealthFactor(totalDscMinted, collateralValueInUsd);
    }

And then this one. So, since totalDscMinted is 0 we would always return a very high number.

 function _calculateHealthFactor(uint256 totalDscMinted, uint256 collateralValueInUsd)
        internal
        pure
        returns (uint256)
    {
        if (totalDscMinted == 0) return type(uint256).max;
        uint256 collateralAdjustedForThreshold = (collateralValueInUsd * LIQUIDATION_THRESHOLD) / LIQUIDATION_PRECISION;
        return (collateralAdjustedForThreshold * PRECISION) / totalDscMinted;
    }

That high number is never going evaluate this expression as true (userHealthFactor < MIN_HEALTH_FACTOR). So, going back to the first function call the tx is not going to be reverted despite Alice having 0 as collateral.

Isnt this a mistake? Or my reasoning is wrong?

Thanks

[ibourn]

Hello, I think you skip a step in your workflow.

Your reasoning assumes that totlaDscMinted is 0. But is it really correct to assume that ?

What state does totalDscMinted really correspond to ? What does the _getAccountInformation function return ?

[0fprod]

Oh Thank you @ibourn ! I totally missed this s_DSCMinted[msg.sender] += amountDscToMint; in the first function call haha, it changes everything 😄

In my version of the contract I was doing it the other way around. First I was calling the dsc.mint and then the mutation of the mapping that holds dsc's per user based on the bool returned 😅