Cyfrin / foundry-defi-stablecoin-cu

250 stars 117 forks source link

Updating to open-zeppelin 4.9.2 #15

Closed 0xtotem closed 1 year ago

0xtotem commented 1 year ago

With the latest OZ library version 4.9.2 the HelperConfig.s.sol is not working as the constructor signature for ERC20Mock has changed.

To fix it we need (unless I miss something) our own ERC20Mock token:

contract ERC20Mock is ERC20 {
    constructor(string memory name, string memory symbol) ERC20(name, symbol) {}

    function mint(address account, uint256 amount) external {
        _mint(account, amount);
    }

    function burn(address account, uint256 amount) external {
        _burn(account, amount);
    }
}

And update the getOrCreateAnvilEthConfig method:

    function getOrCreateAnvilEthConfig() public returns (NetworkConfig memory anvilNetworkConfig) {
        // Check to see if we set an active network config
        if (activeNetworkConfig.wethUsdPriceFeed != address(0)) {
            return activeNetworkConfig;
        }

        vm.startBroadcast();
        MockV3Aggregator ethUsdPriceFeed = new MockV3Aggregator(
            DECIMALS,
            ETH_USD_PRICE
        );
        ERC20Mock wethMock = new ERC20Mock("WETH", "WETH");
        wethMock.mint(msg.sender, 1000e8);

        MockV3Aggregator btcUsdPriceFeed = new MockV3Aggregator(
            DECIMALS,
            BTC_USD_PRICE
        );
        ERC20Mock wbtcMock = new ERC20Mock("WBTC", "WBTC");
        wbtcMock.mint(msg.sender, 1000e8);
        vm.stopBroadcast();

        anvilNetworkConfig = NetworkConfig({
            wethUsdPriceFeed: address(ethUsdPriceFeed), // ETH / USD
            weth: address(wethMock),
            wbtcUsdPriceFeed: address(btcUsdPriceFeed),
            wbtc: address(wbtcMock),
            deployerKey: DEFAULT_ANVIL_PRIVATE_KEY
        });
    }
PatrickAlphaC commented 1 year ago

Thanks! We are not using the newest OZ version in this package, though. I should have told people in the video to download the specific version :/

megabyte0x commented 1 year ago

@PatrickAlphaC It would be great if you could mention the open-zeppelin version in the Readme.

@0xtotem I don't think we require to make a Mock ERC20, it's still there just without the parameters we require. So, IMO the only update we require is to add the mint function.

PatrickAlphaC commented 1 year ago

Makes sense! Would you be interested in making the PR for that?

ShayanShamsi commented 1 year ago

Hey @PatrickAlphaC. I just created a PR and saw this issue afterward. My PR adds the openzeppelin-contracts version in the README as @megabyte0x suggested.

PatrickAlphaC commented 1 year ago

Fixed in https://github.com/Cyfrin/foundry-defi-stablecoin-f23/pull/19