From bd25a0a26f452e475c8e3e30a94ad4bc4966a76e Mon Sep 17 00:00:00 2001 From: Francisco Date: Wed, 4 Oct 2023 22:37:11 -0300 Subject: [PATCH] Fix guides for 5.0 (#4654) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Ernesto García (cherry picked from commit e12511b53eb6ec99d86acec76b0923acfa5b8da2) --- contracts/governance/utils/Votes.sol | 2 +- .../mocks/docs/ERC20WithAutoMinerReward.sol | 22 ++++++++ contracts/mocks/docs/MyContractOwnable.sol | 17 ++++++ docs/modules/ROOT/pages/access-control.adoc | 21 ++------ docs/modules/ROOT/pages/erc20-supply.adoc | 24 ++------- .../ROOT/pages/extending-contracts.adoc | 54 ------------------- test/token/ERC20/extensions/ERC4626.test.js | 4 +- 7 files changed, 50 insertions(+), 94 deletions(-) create mode 100644 contracts/mocks/docs/ERC20WithAutoMinerReward.sol create mode 100644 contracts/mocks/docs/MyContractOwnable.sol diff --git a/contracts/governance/utils/Votes.sol b/contracts/governance/utils/Votes.sol index 56c8772f95d..da23aac7d66 100644 --- a/contracts/governance/utils/Votes.sol +++ b/contracts/governance/utils/Votes.sol @@ -27,7 +27,7 @@ import {Time} from "../../utils/types/Time.sol"; * * When using this module the derived contract must implement {_getVotingUnits} (for example, make it return * {ERC721-balanceOf}), and can use {_transferVotingUnits} to track a change in the distribution of those units (in the - * previous example, it would be included in {ERC721-_beforeTokenTransfer}). + * previous example, it would be included in {ERC721-_update}). */ abstract contract Votes is Context, EIP712, Nonces, IERC5805 { using Checkpoints for Checkpoints.Trace208; diff --git a/contracts/mocks/docs/ERC20WithAutoMinerReward.sol b/contracts/mocks/docs/ERC20WithAutoMinerReward.sol new file mode 100644 index 00000000000..46be53238c1 --- /dev/null +++ b/contracts/mocks/docs/ERC20WithAutoMinerReward.sol @@ -0,0 +1,22 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.20; + +import {ERC20} from "../../token/ERC20/ERC20.sol"; + +contract ERC20WithAutoMinerReward is ERC20 { + constructor() ERC20("Reward", "RWD") { + _mintMinerReward(); + } + + function _mintMinerReward() internal { + _mint(block.coinbase, 1000); + } + + function _update(address from, address to, uint256 value) internal virtual override { + if (!(from == address(0) && to == block.coinbase)) { + _mintMinerReward(); + } + super._update(from, to, value); + } +} diff --git a/contracts/mocks/docs/MyContractOwnable.sol b/contracts/mocks/docs/MyContractOwnable.sol new file mode 100644 index 00000000000..01847c0362e --- /dev/null +++ b/contracts/mocks/docs/MyContractOwnable.sol @@ -0,0 +1,17 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.20; + +import {Ownable} from "../../access/Ownable.sol"; + +contract MyContract is Ownable { + constructor(address initialOwner) Ownable(initialOwner) {} + + function normalThing() public { + // anyone can call this normalThing() + } + + function specialThing() public onlyOwner { + // only the owner can call specialThing()! + } +} diff --git a/docs/modules/ROOT/pages/access-control.adoc b/docs/modules/ROOT/pages/access-control.adoc index a60a34388a0..2a977a2b188 100644 --- a/docs/modules/ROOT/pages/access-control.adoc +++ b/docs/modules/ROOT/pages/access-control.adoc @@ -9,24 +9,9 @@ The most common and basic form of access control is the concept of _ownership_: OpenZeppelin Contracts provides xref:api:access.adoc#Ownable[`Ownable`] for implementing ownership in your contracts. -[source,solidity] ----- -// contracts/MyContract.sol -// SPDX-License-Identifier: MIT -pragma solidity ^0.8.20; - -import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol"; - -contract MyContract is Ownable { - function normalThing() public { - // anyone can call this normalThing() - } - - function specialThing() public onlyOwner { - // only the owner can call specialThing()! - } -} ----- +```solidity +include::api:example$MyContractOwnable.sol[] +``` By default, the xref:api:access.adoc#Ownable-owner--[`owner`] of an `Ownable` contract is the account that deployed it, which is usually exactly what you want. diff --git a/docs/modules/ROOT/pages/erc20-supply.adoc b/docs/modules/ROOT/pages/erc20-supply.adoc index 44cbd73dd67..bf6e240583f 100644 --- a/docs/modules/ROOT/pages/erc20-supply.adoc +++ b/docs/modules/ROOT/pages/erc20-supply.adoc @@ -57,27 +57,13 @@ As we can see, `_mint` makes it super easy to do this correctly. [[automating-the-reward]] == Automating the Reward -So far our supply mechanism was triggered manually, but `ERC20` also allows us to extend the core functionality of the token through the xref:api:token/ERC20.adoc#ERC20-_beforeTokenTransfer-address-address-uint256-[`_beforeTokenTransfer`] hook (see xref:extending-contracts.adoc#using-hooks[Using Hooks]). +So far our supply mechanism was triggered manually, but `ERC20` also allows us to extend the core functionality of the token through the xref:api:token/ERC20.adoc#ERC20-_update-address-address-uint256-[`_update`] function. -Adding to the supply mechanism from the previous section, we can use this hook to mint a miner reward for every token transfer that is included in the blockchain. +Adding to the supply mechanism from the previous section, we can use this function to mint a miner reward for every token transfer that is included in the blockchain. -[source,solidity] ----- -contract ERC20WithAutoMinerReward is ERC20 { - constructor() ERC20("Reward", "RWD") {} - - function _mintMinerReward() internal { - _mint(block.coinbase, 1000); - } - - function _beforeTokenTransfer(address from, address to, uint256 value) internal virtual override { - if (!(from == address(0) && to == block.coinbase)) { - _mintMinerReward(); - } - super._beforeTokenTransfer(from, to, value); - } -} ----- +```solidity +include::api:example$ERC20WithAutoMinerReward.sol[] +``` [[wrapping-up]] == Wrapping Up diff --git a/docs/modules/ROOT/pages/extending-contracts.adoc b/docs/modules/ROOT/pages/extending-contracts.adoc index 1c13d6b4bfa..330d3a5bc84 100644 --- a/docs/modules/ROOT/pages/extending-contracts.adoc +++ b/docs/modules/ROOT/pages/extending-contracts.adoc @@ -68,60 +68,6 @@ The `super.revokeRole` statement at the end will invoke ``AccessControl``'s orig NOTE: The same rule is implemented and extended in xref:api:access.adoc#AccessControlDefaultAdminRules[`AccessControlDefaultAdminRules`], an extension that also adds enforced security measures for the `DEFAULT_ADMIN_ROLE`. -[[using-hooks]] -== Using Hooks - -Sometimes, in order to extend a parent contract you will need to override multiple related functions, which leads to code duplication and increased likelihood of bugs. - -For example, consider implementing safe xref:api:token/ERC20.adoc#ERC20[`ERC20`] transfers in the style of xref:api:token/ERC721.adoc#IERC721Receiver[`IERC721Receiver`]. You may think overriding xref:api:token/ERC20.adoc#ERC20-transfer-address-uint256-[`transfer`] and xref:api:token/ERC20.adoc#ERC20-transferFrom-address-address-uint256-[`transferFrom`] would be enough, but what about xref:api:token/ERC20.adoc#ERC20-_transfer-address-address-uint256-[`_transfer`] and xref:api:token/ERC20.adoc#ERC20-_mint-address-uint256-[`_mint`]? To prevent you from having to deal with these details, we introduced **hooks**. - -Hooks are simply functions that are called before or after some action takes place. They provide a centralized point to _hook into_ and extend the original behavior. - -Here's how you would implement the `IERC721Receiver` pattern in `ERC20`, using the xref:api:token/ERC20.adoc#ERC20-_beforeTokenTransfer-address-address-uint256-[`_beforeTokenTransfer`] hook: - -```solidity -pragma solidity ^0.8.20; - -import {ERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol"; - -contract ERC20WithSafeTransfer is ERC20 { - function _beforeTokenTransfer(address from, address to, uint256 amount) - internal virtual override - { - super._beforeTokenTransfer(from, to, amount); - - require(_validRecipient(to), "ERC20WithSafeTransfer: invalid recipient"); - } - - function _validRecipient(address to) private view returns (bool) { - ... - } - - ... -} -``` - -Using hooks this way leads to cleaner and safer code, without having to rely on a deep understanding of the parent's internals. - -=== Rules of Hooks - -There's a few guidelines you should follow when writing code that uses hooks in order to prevent issues. They are very simple, but do make sure you follow them: - -1. Whenever you override a parent's hook, re-apply the `virtual` attribute to the hook. That will allow child contracts to add more functionality to the hook. -2. **Always** call the parent's hook in your override using `super`. This will make sure all hooks in the inheritance tree are called: contracts like xref:api:token/ERC20.adoc#ERC20Pausable[`ERC20Pausable`] rely on this behavior. - -```solidity -contract MyToken is ERC20 { - function _beforeTokenTransfer(address from, address to, uint256 amount) - internal virtual override // Add virtual here! - { - super._beforeTokenTransfer(from, to, amount); // Call parent hook - ... - } -} -``` -That's it! Enjoy simpler code using hooks! - == Security The maintainers of OpenZeppelin Contracts are mainly concerned with the correctness and security of the code as published in the library, and the combinations of base contracts with the official extensions from the library. diff --git a/test/token/ERC20/extensions/ERC4626.test.js b/test/token/ERC20/extensions/ERC4626.test.js index 499cf0628d9..fa66785f070 100644 --- a/test/token/ERC20/extensions/ERC4626.test.js +++ b/test/token/ERC20/extensions/ERC4626.test.js @@ -88,7 +88,7 @@ contract('ERC4626', function (accounts) { const sharesForDeposit = await vault.previewDeposit(value, { from: holder }); const sharesForReenter = await vault.previewDeposit(reenterValue, { from: holder }); - // Do deposit normally, triggering the _beforeTokenTransfer hook + // Deposit normally, reentering before the internal `_update` const receipt = await vault.deposit(value, holder, { from: holder }); // Main deposit event @@ -170,7 +170,7 @@ contract('ERC4626', function (accounts) { // Price before const sharesBefore = await vault.previewDeposit(value); - // Deposit, triggering the _beforeTokenTransfer hook + // Deposit, reentering before the internal `_update` const receipt = await vault.deposit(value, holder, { from: holder }); // Price is as previewed