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

Add ERC721 Wrapper #3863

Merged
merged 38 commits into from
Feb 9, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
36f15d1
Feature: ERC721 Wrapper
ernestognw Dec 7, 2022
50cceb1
Add PR link
ernestognw Dec 7, 2022
e0c02e6
Added tests
ernestognw Dec 10, 2022
42a681a
Update version
ernestognw Dec 12, 2022
73e498b
Add batching
ernestognw Dec 16, 2022
83f3a0a
Update wording
ernestognw Dec 19, 2022
b76c741
Check ownership or approval in `withdrawTo`
ernestognw Jan 4, 2023
8d8788d
Support minting on `onERC721Received` check
ernestognw Jan 4, 2023
4fdb735
Merge branch 'master' into feature/erc721-wrapper
ernestognw Jan 4, 2023
89d13d8
Fix `onERC721Received` param location and ran prettier
ernestognw Jan 4, 2023
bf1dbb7
Applied prettier to tests
ernestognw Jan 4, 2023
4348d23
Switched to hardhat-exposed features
ernestognw Jan 4, 2023
59a7491
Remove ERC721WrapperMock
ernestognw Jan 4, 2023
cd5c4b9
Cache sloads
Amxx Jan 4, 2023
265d596
Fix lint
ernestognw Jan 9, 2023
aa7f791
Update ERC721.behavior.js
Amxx Jan 10, 2023
7c35759
add test: onERC721Received to another account
Amxx Jan 10, 2023
75074df
Add extra comment
ernestognw Jan 24, 2023
3d0438c
Merge branch 'master' into feature/erc721-wrapper
ernestognw Jan 24, 2023
77fcda8
Merge branch 'master' into feature/erc721-wrapper
ernestognw Jan 24, 2023
3cc752c
Add changeset
ernestognw Jan 24, 2023
24ac3dc
Accept suggestion
ernestognw Jan 24, 2023
935e84b
Fix: Add reentrancy note
ernestognw Jan 24, 2023
f9ee65e
Remove warning
ernestognw Jan 24, 2023
0adf63e
Update docs/modules/ROOT/pages/governance.adoc
ernestognw Jan 24, 2023
16daba8
Remove unnecesary file
ernestognw Feb 3, 2023
1695aaa
Move `underlying` to private and complete branch coverage tests
ernestognw Feb 3, 2023
fb22e8d
Add magic value
ernestognw Feb 3, 2023
3f5b0f3
Switch to bytes12 for magic value
ernestognw Feb 3, 2023
17a5475
Lint
ernestognw Feb 3, 2023
24b1033
Change magic value check
ernestognw Feb 3, 2023
1fd1094
Merge branch 'master' into feature/erc721-wrapper
ernestognw Feb 4, 2023
ca26854
Make magic value public
ernestognw Feb 7, 2023
29f31f7
Missing change
ernestognw Feb 7, 2023
d51374b
Merge branch 'master' into feature/erc721-wrapper
ernestognw Feb 7, 2023
92d650b
Merge branch 'master' into feature/erc721-wrapper
Amxx Feb 8, 2023
b49c187
Make `onERC721Received` more strict with the magic value
ernestognw Feb 8, 2023
e6eb16d
Merge branch 'master' into feature/erc721-wrapper
ernestognw Feb 8, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/early-oranges-raise.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'openzeppelin-solidity': minor
---

`ERC721Wrapper`: add a new extension of the `ERC721` token which wraps an underlying token. Deposit and withdraw guarantee that the ownership of each token is backed by a corresponding underlying token with the same identifier.
3 changes: 3 additions & 0 deletions contracts/token/ERC721/README.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ Additionally there are a few of other extensions:
* {ERC721Royalty}: A way to signal royalty information following ERC2981.
* {ERC721Pausable}: A primitive to pause contract operation.
* {ERC721Burnable}: A way for token holders to burn their own tokens.
* {ERC721Wrapper}: Wrapper to create an ERC721 backed by another ERC721, with deposit and withdraw methods. Useful in conjunction with {ERC721Votes}.

NOTE: This core set of contracts is designed to be unopinionated, allowing developers to access the internal functions in ERC721 (such as <<ERC721-_mint-address-uint256-,`_mint`>>) and expose them as external functions in the way they prefer. On the other hand, xref:ROOT:erc721.adoc#Presets[ERC721 Presets] (such as {ERC721PresetMinterPauserAutoId}) are designed using opinionated patterns to provide developers with ready to use, deployable contracts.

Expand Down Expand Up @@ -59,6 +60,8 @@ NOTE: This core set of contracts is designed to be unopinionated, allowing devel

{{ERC721Royalty}}

{{ERC721Wrapper}}

== Presets

These contracts are preconfigured combinations of the above features. They can be used through inheritance or as models to copy and paste their source code.
Expand Down
102 changes: 102 additions & 0 deletions contracts/token/ERC721/extensions/ERC721Wrapper.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
// SPDX-License-Identifier: MIT

pragma solidity ^0.8.0;

import "../ERC721.sol";
import "../utils/ERC721Holder.sol";

/**
* @dev Extension of the ERC721 token contract to support token wrapping.
*
* Users can deposit and withdraw an "underlying token" and receive a "wrapped token" with a matching tokenId. This is useful
* in conjunction with other modules. For example, combining this wrapping mechanism with {ERC721Votes} will allow the
* wrapping of an existing "basic" ERC721 into a governance token.
*
* _Available since v4.9.0_
*/
abstract contract ERC721Wrapper is ERC721, ERC721Holder {
IERC721 private immutable _underlying;

// Kept as bytes12 so it can be packed with an address
// Equal to 0xb125e89df18e2ceac5fd2fa8
bytes12 public constant WRAPPER_ACCEPT_MAGIC = bytes12(keccak256("WRAPPER_ACCEPT_MAGIC"));
Amxx marked this conversation as resolved.
Show resolved Hide resolved

constructor(IERC721 underlyingToken) {
_underlying = underlyingToken;
}

/**
* @dev Allow a user to deposit underlying tokens and mint the corresponding tokenIds.
*/
function depositFor(address account, uint256[] memory tokenIds) public virtual returns (bool) {
bytes memory data = abi.encodePacked(WRAPPER_ACCEPT_MAGIC, account);

uint256 length = tokenIds.length;
for (uint256 i = 0; i < length; ++i) {
underlying().safeTransferFrom(_msgSender(), address(this), tokenIds[i], data);
}

return true;
}

/**
* @dev Allow a user to burn wrapped tokens and withdraw the corresponding tokenIds of the underlying tokens.
*/
function withdrawTo(address account, uint256[] memory tokenIds) public virtual returns (bool) {
ernestognw marked this conversation as resolved.
Show resolved Hide resolved
uint256 length = tokenIds.length;
for (uint256 i = 0; i < length; ++i) {
uint256 tokenId = tokenIds[i];
require(_isApprovedOrOwner(_msgSender(), tokenId), "ERC721Wrapper: caller is not token owner or approved");
_burn(tokenId);
// Checks were already performed at this point, and there's no way to retake ownership or approval from
// the wrapped tokenId after this point, so it's safe to remove the reentrancy check for the next line.
// slither-disable-next-line reentrancy-no-eth
underlying().safeTransferFrom(address(this), account, tokenId);
}

return true;
}

/**
* @dev Overrides {IERC721Receiver-onERC721Received} to allow minting on direct ERC721 transfers to
* this contract.
*
* In case there's data attached, it validates that the sender is aware of this contract's existence and behavior
* by checking a magic value (`WRAPPER_ACCEPT_MAGIC`) in the first 12 bytes. If it also matches, the rest 20
* bytes are used as an address to send the tokens to.
*
* WARNING: Doesn't work with unsafe transfers (eg. {IERC721-transferFrom}). Use {ERC721Wrapper-_recover}
* for recovering in that scenario.
*/
function onERC721Received(
address,
address from,
uint256 tokenId,
bytes memory data
) public override returns (bytes4) {
require(address(underlying()) == _msgSender(), "ERC721Wrapper: caller is not underlying");
if (data.length > 0) {
require(data.length == 32 && WRAPPER_ACCEPT_MAGIC == bytes12(data), "ERC721Wrapper: Invalid data format");
from = address(bytes20(bytes32(data) << 96));
}
_safeMint(from, tokenId);
return IERC721Receiver.onERC721Received.selector;
}

/**
* @dev Mint a wrapped token to cover any underlyingToken that would have been transferred by mistake. Internal
* function that can be exposed with access control if desired.
*/
function _recover(address account, uint256 tokenId) internal virtual returns (uint256) {
require(underlying().ownerOf(tokenId) == address(this), "ERC721Wrapper: wrapper is not token owner");
_safeMint(account, tokenId);
return tokenId;
}

/**
* @dev Returns the underlying token.
*/
function underlying() public view virtual returns (IERC721) {
return _underlying;
}
}
2 changes: 1 addition & 1 deletion docs/modules/ROOT/pages/governance.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ contract MyToken is ERC20, ERC20Permit, ERC20Votes, ERC20Wrapper {
}
```

NOTE: Voting power could be determined in different ways: multiple ERC20 tokens, ERC721 tokens, sybil resistant identities, etc. All of these options are potentially supported by writing a custom Votes module for your Governor. The only other source of voting power available in OpenZeppelin Contracts currently is xref:api:token/ERC721.adoc#ERC721Votes[`ERC721Votes`].
NOTE:The only other source of voting power available in OpenZeppelin Contracts currently is xref:api:token/ERC721.adoc#ERC721Votes[`ERC721Votes`]. ERC721 tokens that don't provide this functionality can be wrapped into a voting tokens using a combination of xref:api:token/ERC721.adoc#ERC721Votes[`ERC721Votes`] and xref:api:token/ERC721Wrapper.adoc#ERC721Wrapper[`ERC721Wrapper`].

=== Governor

Expand Down
5 changes: 3 additions & 2 deletions test/token/ERC721/ERC721.behavior.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ const { ZERO_ADDRESS } = constants;
const { shouldSupportInterfaces } = require('../../utils/introspection/SupportsInterface.behavior');

const ERC721ReceiverMock = artifacts.require('ERC721ReceiverMock');
const NonERC721ReceiverMock = artifacts.require('CallReceiverMock');

const Error = ['None', 'RevertWithMessage', 'RevertWithoutMessage', 'Panic'].reduce(
(acc, entry, idx) => Object.assign({ [entry]: idx }, acc),
Expand Down Expand Up @@ -316,7 +317,7 @@ function shouldBehaveLikeERC721(errorPrefix, owner, newOwner, approved, anotherA

describe('to a contract that does not implement the required function', function () {
it('reverts', async function () {
const nonReceiver = this.token;
ernestognw marked this conversation as resolved.
Show resolved Hide resolved
const nonReceiver = await NonERC721ReceiverMock.new();
await expectRevert(
this.token.safeTransferFrom(owner, nonReceiver.address, tokenId, { from: owner }),
'ERC721: transfer to non ERC721Receiver implementer',
Expand Down Expand Up @@ -392,7 +393,7 @@ function shouldBehaveLikeERC721(errorPrefix, owner, newOwner, approved, anotherA

context('to a contract that does not implement the required function', function () {
it('reverts', async function () {
const nonReceiver = this.token;
const nonReceiver = await NonERC721ReceiverMock.new();
await expectRevert(
this.token.$_safeMint(nonReceiver.address, tokenId),
'ERC721: transfer to non ERC721Receiver implementer',
Expand Down