ERC20.sol private variables with no setters

OpenZep's ERC20.sol cannot efficiently be used as the basis of an upgradable (proxied) ERC20 contract because it declares name & symbol as private, and does not provide setter methods. I understand that these variables being private is in accordance to the zep style guide, but it seems counterproductive to essentially require a copy/paste of the ERC20 implementation just to be able to set these variables from an initializer vs constructor... what am I missing?

1 Like

You need to be using the Upgradeable variant of OpenZeppelin Contracts. See the docs here.

As a followup question and attempting to reframe @aathan 's question.

Even with contracts-upgradeable/token/ERC20/ERC20Upgradeable.sol, because of the visibility of variables such as _totalSupply and etc being private, it is not feasible to reuse many of the functions by deriving the contract.

If I understand it correctly, we can't have a custom implementation of mint, because the variables are not accessible.
In addition, were we to reuse the internal _mint function as suggested in https://docs.openzeppelin.com/contracts/4.x/erc20-supply#rewarding-miners, we are bound to writing logic before, during or after the _TokenTransfer hooks, but not in between, which is sometimes required.

Would appreciate your input @frangio . Thanks!

Can you give an example?


We're comfortable with this. If you're going to modify the contract so much that you need to access state variables, we encourage you to fork the code instead of using inheritance.

Thanks for the swift reply!

Sure. This is the _mint function of ERC20Upgradeable.sol

function _mint(address account, uint256 amount) internal virtual {
    require(account != address(0), "ERC20: mint to the zero address");

    _beforeTokenTransfer(address(0), account, amount);

    _totalSupply += amount;
    _balances[account] += amount;
    emit Transfer(address(0), account, amount);

    _afterTokenTransfer(address(0), account, amount);
}

Using a shared scenario, an implementation of "Rewarding Miners" https://docs.openzeppelin.com/contracts/4.x/erc20-supply#automating-the-reward

We have on the table the usage of transfer hooks, that let us validate conditions or even mutate local and external state, before token balances are edited.

Let's say this works well. But if in addition, my logic states that
a) If the miners balance is higher than 5% of supply, they won't be rewarded on mint operations
b) lost rewards can be tracked on another field (implemented on the deriving/child contract, still and ERC20), and these rewards have a different time-decay-based logic (hence not materialised balances)

Suddenly a big step back needs to be taken.

Here's where I see the conflicts with a) and b)

a) requires a different _mint logic where _beforeTokenTransfer may or may not happen or happen in a different shape (but we still need to apply the basic +=s to totalSupply and balances)
b) requires to perform another assignment, based on the result of the "_beforeTokenMint" (made up name, based on the premise above)

I'd highlight the nuances being that while _mint is internal :heavy_check_mark: and virtual :heavy_check_mark:, the main fields are private which does constrain the extensibility.

Agree on that forking is an option - I was curious to see what else was possible. Lastly I'm eager to learn, from the best practices perspective, what are the main drivers for it to be a comfortable solution. Before replying I did scan the forum and found this here:

This is to increase encapsulation, to be able to reason better about the code.

Is there anything beyond? Other benefits outweighing the cons?

Thanks again :+1:

It's what you said and also to help guide people towards proper use of the contracts.

If the balances mapping was internal, someone might inadvertently modify the balances without emitting an appropriate Transfer event and this will mess up clients, indexers, wallets, etc. This is explained in the guide that you linked so I'm sure you heard it already.

I'm not sure I fully understood the example but what prevents you from implementing it based on the internal interface (including balanceOf(...))? You can write if (balanceOf(...) < X) mint(...).