Add internal overrideable _flashFee in ERC20FlashMint

[nirban256]

Fixes #3331

added internal _flashFee function

added the function so that instead of flashFee function the _flashFee function can be overridden inside the ERC20FlashMint contract

PR Checklist

• Tests
• Documentation

[Amxx]

Hello @nirban256

I'm really puzzeled by this design.

    function flashFee(address token, uint256 amount) public view virtual override returns (uint256) {
        require(token == address(this), "ERC20FlashMint: wrong token");
        // silence warning about unused variable without the addition of bytecode.
        amount;
        return 0;
    }
    function _flashFee(address token, uint256 amount) internal view virtual returns (uint256) {
        uint256 fee = flashFee(token, amount);
        return fee;
    }

What are dev supposed to override? If you override the public one, you risk losing the require. If you override the internal, you risk having an inconsistency between the public value and the value used.

[Amxx]

Maybe what we want is

    function flashFee(address token, uint256 amount) public view virtual override returns (uint256) {
        require(token == address(this), "ERC20FlashMint: wrong token");
        return _flashFee(token, amount);
    }
    function _flashFee(address token, uint256 amount) internal view virtual returns (uint256) {
        // silence warning about unused variable without the addition of bytecode.
        token;
        amount;
        return 0;
    }

[nirban256]

Ok I will change this to this new implementation shortly

[frangio]

Please update ERC20FlashMintMock to override _flashFee instead of flashFee like it does currently.

[frangio]

My previous comment hasn't been addressed yet:

Please update ERC20FlashMintMock to override _flashFee instead of flashFee like it does currently.

It refers to the mock contract used for testing.

[nirban256]

Sir I have pushed a commit, let me know what are the changes required.

[frangio]

Thanks!

[nirban256]

do I need to do anything more in order to complete this pull request? @frangio