ERC20Wrapper underlying variable is public

[ernestognw]

📝 Details

For ERC20Wrapper, the Upgradeable version's underlying variable is public and the immutable is removed.

We don't want that since child contracts should not be able to override it.

It's then needed to replace the public visibility of the underlying variable in the ERC20Wrapper contract and create a public view function that replaces its access.

[kimanikelly]

@ernestognw @frangio

Changes made in https://github.com/kimanikelly/openzeppelin-contracts/tree/issue-4026

Are there any other details I'm missing to complete this task, if so please let me know, so I can make those changes and submit a PR, thanks.

Removed the public visibility from IERC20 immutable _underlying; and refactored it to include the _ due to linting errors.

Implemented the underlying() public view function to return the value of the _underlying variable

A test for underlying was already included in test/token/ERC20/extensions/ERC20Wrapper.test.js but after the refactor the call targets the new public view function instead of the previous public underlying variable

[ernestognw]

@kimanikelly Thanks!

Feel free to open a PR for this, it doesn't require major test changes since it's already covered as you mention, and maybe only a Changeset entry.

I think we all agree on the implementation of this so we can add comments to your PR

[kimanikelly]

@ernestognw I went ahead and created the PR #4029. All tests/checks have passed and are currently pending on a reviewer, thanks