Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make view and pure functions virtual #2473

Merged
merged 40 commits into from
Jan 26, 2021

Conversation

Amxx
Copy link
Collaborator

@Amxx Amxx commented Jan 18, 2021

Fixes #2154


Note that in many cases overriding a function will not imply that other functions in the contract will automatically adapt to the overridden definitions. For example, overriding ERC20.balanceOf does not guarantee that that balance is available for transferring. However, it will generaly be possible to obtain the desired behavior by overriding other functions in the contract, in this case ERC20._transfer. Understand that this limitation was necessary to keep the contracts simple and consistent, but it means that the responsibility to ensure correctness when functions are overriden will be on the users and not on the OpenZeppelin maintainers. People who wish to override should consult the source code to fully understand the impact it will have, and should consider whether they need to override additional functions to achieve the desired behavior. As always, feel free to ask for help in the OpenZeppelin Forum and we'll be happy to help.

@frangio frangio marked this pull request as draft January 18, 2021 15:23
@frangio frangio changed the title Making all view and pure function virtual Making all view and pure functions virtual Jan 18, 2021
Copy link
Contributor

@frangio frangio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are all my opinions but I got tired of writing "I think...". 😛

contracts/access/AccessControl.sol Outdated Show resolved Hide resolved
contracts/access/Ownable.sol Show resolved Hide resolved
contracts/GSN/GSNRecipientERC20Fee.sol Outdated Show resolved Hide resolved
contracts/GSN/GSNRecipientERC20Fee.sol Outdated Show resolved Hide resolved
contracts/access/TimelockController.sol Show resolved Hide resolved
contracts/token/ERC777/ERC777.sol Show resolved Hide resolved
contracts/token/ERC777/ERC777.sol Show resolved Hide resolved
contracts/token/ERC777/ERC777.sol Show resolved Hide resolved
contracts/token/ERC721/ERC721.sol Show resolved Hide resolved
contracts/token/ERC721/ERC721.sol Show resolved Hide resolved
@Amxx Amxx force-pushed the feature/virtualview branch 2 times, most recently from 592c71e to 6ba0ea6 Compare January 25, 2021 10:01
@frangio frangio marked this pull request as ready for review January 26, 2021 15:07
@frangio frangio changed the title Making all view and pure functions virtual Make view and pure functions virtual Jan 26, 2021
@frangio frangio merged commit 18c7efe into OpenZeppelin:master Jan 26, 2021
@Amxx Amxx deleted the feature/virtualview branch January 26, 2021 17:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Virtual view functions
2 participants