ERC20PresetMinterPauser.sol _beforeTokenTransfer() Hook Utility

Support Contracts
1

Hi,
I’m implementing an ERC20 token getting inspired by ERC20PresetMinterPauser.sol.

I’m not inheriting the preset cause I would like to inherit Ownable.sol add extra functions (like permit()) and have everything in one smart contract.

My question is do I need to include the following function?

function _beforeTokenTransfer(address from, address to, uint256 amount) internal virtual override(ERC20, ERC20Pausable) {
      super._beforeTokenTransfer(from, to, amount);
  }

I’m learning about hooks and it’s not completely clear to me the utility of this particular function.

So, If my understanding is correct, it should override both ERC20.sol and ERC20Pausable _beforeTokenTransfer() hooks with this one that should call the one in ERC20Pausable:

function _beforeTokenTransfer(address from, address to, uint256 amount) internal virtual override {
        super._beforeTokenTransfer(from, to, amount);

        require(!paused(), "ERC20Pausable: token transfer while paused");
    }

That require that the smart contract is not paused, otherwise it will revert _mint(), _burn() and _transfer(), right?

Also, If I should include this function, should I add the attribute “virtual” to it, allowing other contracts to override it? isn’t it unsafe?

Cause here specify to always add the virtual attribute to the hook when overriding:
https://docs.openzeppelin.com/contracts/3.x/extending-contracts#rules_of_hooks

Thanks!

:computer: Environment

Truffle v5.1.33 (core: 5.1.33)
Solidity - 0.6.11 (solc-js)
Node v12.18.2
Web3.js v1.2.1

:memo:Details

:1234: Code to reproduce

1 Like
2

Hi @naszam,

Glad to hear that you are inspired by the ERC20 preset.

As an aside, rather than inherit from Ownable you may want to consider using AccessControl, see Role-Based Access Control

As for whether you need to override _beforeTokenTransfer, you only need to override if you are inheriting from multiple contracts that define this function or if you want to extend this hook.

If you are inheriting from ERC20Pausable and ERC20Burnable then you will need to override as is done in ERC20PresetMinterPauser. If you leave it out, the compiler will give you an error.

If you want to allow someone to create a contract that extends your contract then you should make the function virtual. If you don’t want to support extending, then don’t make it virtual.

1 Like
3

Hi @abcoathup,

Thanks!

Regarding Ownable, I think it’s useful to define the owner of the contract, even if I agree with you, Role-Based Access Control is the way to go for future developments, design patterns.

Currenlty, I see an hybrid solution, having Ownable for deployer/owner of the smart contract and Roles for specific actions in the contract, like Admin, Minter, …

I see why I have to override the hook, my doubt was more around the virtual attribute, how can other contracts change the behaviour of the token once on mainnet, seems kinda a security issue.
Thanks!

1 Like
4

Hi @naszam,

I’m curious what functionality you would want an owner to have that couldn’t be a specific role?

As for the virtual attribute, it isn’t used to impact behaviour of a deployed contract, it is for a contract inheriting the source code of your contract to be able to extend functionality using the hook. If you don’t want to support this, then you don’t need to make the function virtual.

1 Like
5

Hi @abcoathup,

Here is the source code:

Thanks for clarifying the usage of the virtual attribute! Now make sense :sweat_smile:

1 Like
6

Hi @naszam,

Thanks for sharing your contract. Though it looks like you are only using owner to set roles in the constructor, where you could use the contract deployer with msg.sender so assume that you plan to use it for something else otherwise I am not sure I see the point.

If you need to prove you deployed the contract you can always sign a message from the account that deployed the contract. e.g. for a market place or a block explorer.

1 Like
7

Hi @abcoathup,

I think keeping track of the owner/deployer is a good practise and I don’t see other viable options.
Regarding the signature, I’m not sure to get what you mean but definitely something to consider eventually.
Thanks!

1 Like
8

Hi @naszam,

To prove that I was the deployer of a contract a centralized service asked me to sign a message using the private key of the account which deployed the contract, which I could only do if I controlled the address.

Etherscan is an example does this if you want to set/change token information: https://info.etherscan.com/what-is-verify-address-ownership/

1 Like