Cyfrin / foundry-defi-stablecoin-cu

249 stars 117 forks source link

Update DSCEngine.sol to ensure contract is not initialized without valid collateral. #86

Closed RamanSB closed 4 months ago

RamanSB commented 4 months ago

Hey @PatrickAlphaC - currently there is nothing stopping the deployer from deploying the contract with an empty address[] for both validCollateral and priceFeeds, this check is not sufficient:

if (tokenAddresses.length != priceFeedAddresses.length) { // 0 == 0 will pass this check.
            revert DSCEngine__TokenAddressesAndPriceFeedAddressesAmountsDontMatch(); 
}

This will prevent the deployer from erroneously deploying the DSCEngine without any form of valid collateral, in essence preventing the below script from successfully deploying:

// SDPX-License-Identifier: MIT

pragma solidity ^0.8.26;

import {Script} from "forge-std/Script.sol";
import {DSCEngine} from "../src/DSCEngine.sol";

contract DeployDSCEngine is Script {
    function run() external returns (DSCEngine) {
        vm.startBroadcast();
        address[] memory validCollaterals;
        address[] memory priceFeeds;
        DSCEngine dscEngine = new DSCEngine(validCollaterals, priceFeeds);
        vm.stopBroadcast();
        return dscEngine;
    }
}
RamanSB commented 4 months ago

Alternatively with my own code (I've used a different set of variable names so apologies), we can define a custom error instead of re-using the modifier if that makes things clearer for students?

  error DSCEngine__RequireAtleastOneFormOfCollateral();

  constructor(
        address[] memory validCollateral,
        address[] memory priceFeeds,
        address dscAddress
    ) moreThanZero(validCollateral.length) {
        if (validCollateral.length != priceFeeds.length) {
            revert DSCEngine__TokenAddressesAndPriceFeedAddressesMustBeSameLength();
        }
        if (!(validCollateral.length > 0)) {
            revert DSCEngine__RequireAtleastOneFormOfCollateral();
        }
    }
PatrickAlphaC commented 4 months ago

This would be a great addition!

This was also called out in the audit report (you can see it in the audit folder).

I'm going to close this for now though to keep the code as similar to the video.