ERC20Burnable Not Required

[mgroovyank]

While creating DecentralizedStablecoin.sol, Patrick implements ERC20Burnable contract. Is that really required? He could have just implemented ERC20 contract directly. There is also no need to override burn function of ERC20Burnable contract as well that way.

Here is how, I think it could've been:

    contract DecentralizedStablecoin is **ERC20**, Ownable {
    error DecentralizedStablecoin__BalanceMustBeMoreThanZero();
    error DecentralizedStablecoin__BurnAmountExceedsBalance();

    constructor(
        address owner
    ) ERC20("DecentralizedStablecoin", "DSC") Ownable(owner) {}

    function burn(uint256 amount) public onlyOwner {
        uint256 balance = balanceOf(msg.sender);
        if (amount <= 0) {
            revert DecentralizedStablecoin__BalanceMustBeMoreThanZero();
        }
        if (amount > balance) {
            revert DecentralizedStablecoin__BurnAmountExceedsBalance();
        }
        **_burn(msg.sender, amount);**
    }
}

What do you think about this approach? Am I missing anything?

[EngrPips]

Hello @mgroovyank, The ERC20Burnable contract allows an ERC20 token to have an additional burning mechanism which we needed in the DecentralizedStablecoin So if you replace the ERC20Burnable contract with an ordinary ERC20 contract then you won't be able to burn the token except if you implement a native burning mechanism in your contract but why do that when you can just inherit from ERC20Burnable and make use of the functionality.

[FrankezeCode]

@EngrPips the _burn function is in the ERC20 contract , the ERCBurnable contract inherited it from the ERC20 ,
What @mgroovyank did was to call it directly , . But If you call it directly you will miss other functionality in the ERCBurnable contract like burnFrom function.

@EngrPips just to clarify, I think he would also be able to burn token with the contract, just that he would be limited to just _burn function.

Thanks , I hope you guys understand my point

[EngrPips]

Hello @FrankezeCode, you are correct as Openzeppelin implemented a burn functionality in their ERC20 contract, but what I was actually pointing out is that the standard interface of ERC20 does not have a burn functionality in it. The below code is the Openzeppelin interface for ERC20

// SPDX-License-Identifier: MIT
// OpenZeppelin Contracts (last updated v5.0.0) (token/ERC20/IERC20.sol)

pragma solidity ^0.8.20;

/**
 * @dev Interface of the ERC20 standard as defined in the EIP.
 */
interface IERC20 {
    /**
     * @dev Emitted when `value` tokens are moved from one account (`from`) to
     * another (`to`).
     *
     * Note that `value` may be zero.
     */
    event Transfer(address indexed from, address indexed to, uint256 value);

    /**
     * @dev Emitted when the allowance of a `spender` for an `owner` is set by
     * a call to {approve}. `value` is the new allowance.
     */
    event Approval(address indexed owner, address indexed spender, uint256 value);

    /**
     * @dev Returns the value of tokens in existence.
     */
    function totalSupply() external view returns (uint256);

    /**
     * @dev Returns the value of tokens owned by `account`.
     */
    function balanceOf(address account) external view returns (uint256);

    /**
     * @dev Moves a `value` amount of tokens from the caller's account to `to`.
     *
     * Returns a boolean value indicating whether the operation succeeded.
     *
     * Emits a {Transfer} event.
     */
    function transfer(address to, uint256 value) external returns (bool);

    /**
     * @dev Returns the remaining number of tokens that `spender` will be
     * allowed to spend on behalf of `owner` through {transferFrom}. This is
     * zero by default.
     *
     * This value changes when {approve} or {transferFrom} are called.
     */
    function allowance(address owner, address spender) external view returns (uint256);

    /**
     * @dev Sets a `value` amount of tokens as the allowance of `spender` over the
     * caller's tokens.
     *
     * Returns a boolean value indicating whether the operation succeeded.
     *
     * IMPORTANT: Beware that changing an allowance with this method brings the risk
     * that someone may use both the old and the new allowance by unfortunate
     * transaction ordering. One possible solution to mitigate this race
     * condition is to first reduce the spender's allowance to 0 and set the
     * desired value afterwards:
     * https://github.com/ethereum/EIPs/issues/20#issuecomment-263524729
     *
     * Emits an {Approval} event.
     */
    function approve(address spender, uint256 value) external returns (bool);

    /**
     * @dev Moves a `value` amount of tokens from `from` to `to` using the
     * allowance mechanism. `value` is then deducted from the caller's
     * allowance.
     *
     * Returns a boolean value indicating whether the operation succeeded.
     *
     * Emits a {Transfer} event.
     */
    function transferFrom(address from, address to, uint256 value) external returns (bool);
}

[FrankezeCode]

Okay , I understand 👍

[EngrPips]

While creating DecentralizedStablecoin.sol, Patrick implements ERC20Burnable contract. Is that really required? He could have just implemented ERC20 contract directly. There is also no need to override burn function of ERC20Burnable contract as well that way.

Here is how, I think it could've been:

    contract DecentralizedStablecoin is **ERC20**, Ownable {
    error DecentralizedStablecoin__BalanceMustBeMoreThanZero();
    error DecentralizedStablecoin__BurnAmountExceedsBalance();

    constructor(
        address owner
    ) ERC20("DecentralizedStablecoin", "DSC") Ownable(owner) {}

    function burn(uint256 amount) public onlyOwner {
        uint256 balance = balanceOf(msg.sender);
        if (amount <= 0) {
            revert DecentralizedStablecoin__BalanceMustBeMoreThanZero();
        }
        if (amount > balance) {
            revert DecentralizedStablecoin__BurnAmountExceedsBalance();
        }
        **_burn(msg.sender, amount);**
    }
}

What do you think about this approach? Am I missing anything?

So I look more into the difference between ERC20 contract and ERC20Burnable contract and I see that the _burn function inside of the ERC20 contract is not virtual which means you can not override It, so if you have some contract where you will like to do some stuff before actually burning the token you won't have the privilege to do so.

ERC20 BURN

/**
     * @dev Destroys a `value` amount of tokens from `account`, lowering the total supply.
     * Relies on the `_update` mechanism.
     *
     * Emits a {Transfer} event with `to` set to the zero address.
     *
     * NOTE: This function is not virtual, {_update} should be overridden instead
     */
    function _burn(address account, uint256 value) internal {
        if (account == address(0)) {
            revert ERC20InvalidSender(address(0));
        }
        _update(account, address(0), value);
    }

You will need to instead override the _update function instead, but for the ERC20Burnable burn function you can actually directly override the burn function.

ERC20Burnable BURN

/**
     * @dev Destroys a `value` amount of tokens from the caller.
     *
     * See {ERC20-_burn}.
     */
    function burn(uint256 value) public virtual {
        _burn(_msgSender(), value);
    }

So I think it is just a flexibility difference between the two contracts.

[mgroovyank]

@EngrPips @FrankezeCode Thanks for your effort to provide clarifications. What I understand is, it is a personal choice to use ERC20Burnable contract. We all know the benefits of using it. If I don't use it, I'll have to implement a native burning mechanism in my contract. This seems fine to me, just that using ERC20Burnable makes it explicit for anyone to understand that the token is burnable.

[EngrPips]

This makes sense. Keep crushing it.