Cyfrin / foundry-full-course-cu

GNU General Public License v3.0
2.88k stars 671 forks source link

OpenZeppelin's Ownable contract updated its constructor #288

Closed ChefAharoni closed 11 months ago

ChefAharoni commented 1 year ago

The following question was raised in discussions:

Discussed in https://github.com/Cyfrin/foundry-full-course-f23/discussions/286

Originally posted by **Glow-in-the-dark** July 8, 2023 in DecentralisedStableCoin.sol, it inherits the 1) ERC20Burnable, which inherits the ERC20, and also the 2) Ownable contract. However, the constructor of the DecentralizedStableCoin contract, only seems to initiate the constructor of the ERC20() , but not the Ownable() contract, which also have a constructor. ```solidity contract DecentralizedStableCoin is ERC20Burnable, Ownable { error DecentralizedStableCoin__MustBeMoreThanZero(); error DecentralizedStableCoin__BurnAmountExceedsBalance(); error DecentralizedStableCoin__NotZeroAddress(); constructor () ERC20 ("DecentralizedStableCoin","DSC") { } ``` why is it not something like: ```solidity constructor () ERC20("DecentralizedStableCoin","DSC") Ownable(0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266){ } ``` since under "Ownable.sol", there is a constructor, which is: ``` constructor(address initialOwner) { _transferOwnership(initialOwner); } ``` My underrstanding is that the child contract inheriting the parent contract, will need to also include the parent's constructor. Whis is the example: https://github.com/Cyfrin/foundry-defi-stablecoin-f23/blob/main/src/DecentralizedStableCoin.sol it inherits both parents, but only instantiate the ERC20 token, and does not include the Ownable's constructor.

There was an update to OpenZeppelin's code two months ago that changed from the code that's used in the course, to what's shown above. Since the update, address of the owner must be entered manually through the constructor parameter.
I suggest noting this change on YouTube, and I'll also note this in the Decentralized coin repo.

image
PatrickAlphaC commented 1 year ago

Great question! And you pretty much got it though.

In the version we used, the constructor looks like this:

constructor() {
        _transferOwnership(_msgSender());
    }

With no parameters. Due to this, solidity is smart enough to not require us put that in our DecentralizedStableCoin constructor. Yes, if that had parameters, we'd need to put it in our constructor.

PatrickAlphaC commented 11 months ago

Closing as solved