Skip to content

Commit

Permalink
Upgrades OpenZeppelin to v5 and replaces isContract calls.
Browse files Browse the repository at this point in the history
  • Loading branch information
@steven2308
steven2308 committed Dec 4, 2023
1 parent 90c99a5 commit d9015c0
Show file tree
Hide file tree
Showing 9 changed files with 48 additions and 50 deletions.
29 changes: 16 additions & 13 deletions contracts/RMRK/equippable/RMRKMinifiedEquippable.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ pragma solidity ^0.8.21;

import "@openzeppelin/contracts/token/ERC721/IERC721.sol";
import "@openzeppelin/contracts/token/ERC721/IERC721Receiver.sol";
import "@openzeppelin/contracts/utils/Address.sol";
import "@openzeppelin/contracts/utils/Context.sol";
import "@openzeppelin/contracts/utils/introspection/IERC165.sol";

Expand Down Expand Up @@ -33,7 +32,6 @@ contract RMRKMinifiedEquippable is
RMRKCore
{
using RMRKLib for uint64[];
using Address for address;

uint256 private constant _MAX_LEVELS_TO_CHECK_FOR_INHERITANCE_LOOP = 100;

Expand Down Expand Up @@ -217,15 +215,10 @@ contract RMRKMinifiedEquippable is
) public virtual onlyApprovedOrDirectOwner(tokenId) {
(address immediateOwner, uint256 parentId, ) = directOwnerOf(tokenId);
if (immediateOwner != from) revert ERC721TransferFromIncorrectOwner();
if (to == address(0)) revert ERC721TransferToTheZeroAddress();
if (to == address(this) && tokenId == destinationId)
revert RMRKNestableTransferToSelf();

// Destination contract checks:
// It seems redundant, but otherwise it would revert with no error
if (!to.isContract()) revert RMRKIsNotContract();
if (!IERC165(to).supportsInterface(type(IERC7401).interfaceId))
revert RMRKNestableTransferToNonRMRKNestableImplementer();
_checkDestination(to);
_checkForInheritanceLoop(tokenId, to, destinationId);

_beforeTokenTransfer(from, to, tokenId);
Expand Down Expand Up @@ -398,9 +391,7 @@ contract RMRKMinifiedEquippable is
bytes memory data
) internal virtual {
// It seems redundant, but otherwise it would revert with no error
if (!to.isContract()) revert RMRKIsNotContract();
if (!IERC165(to).supportsInterface(type(IERC7401).interfaceId))
revert RMRKMintToNonRMRKNestableImplementer();
_checkDestination(to);

_innerMint(to, tokenId, destinationId, data);
_sendToNFT(address(0), to, 0, destinationId, tokenId, data);
Expand Down Expand Up @@ -715,7 +706,7 @@ contract RMRKMinifiedEquippable is
uint256 tokenId,
bytes memory data
) private returns (bool) {
if (to.isContract()) {
if (to.code.length != 0) {
try
IERC721Receiver(to).onERC721Received(
_msgSender(),
Expand Down Expand Up @@ -755,7 +746,7 @@ contract RMRKMinifiedEquippable is
_requireMinted(parentId);

address childAddress = _msgSender();
if (!childAddress.isContract()) revert RMRKIsNotContract();
if (childAddress.code.length == 0) revert RMRKIsNotContract();

Child memory child = Child({
contractAddress: childAddress,
Expand Down Expand Up @@ -2069,6 +2060,18 @@ contract RMRKMinifiedEquippable is
return _equipments[tokenId][targetCatalogAddress][slotPartId];
}

/**
* @notice Checks the destination is valid for a Nest Transfer/Mint.
* @dev The destination must be a contract that implements the IERC7401 interface.
* @param to Address of the destination
*/
function _checkDestination(address to) internal view {
// It seems redundant, but otherwise it would revert with no error
if (to.code.length == 0) revert RMRKIsNotContract();
if (!IERC165(to).supportsInterface(type(IERC7401).interfaceId))
revert RMRKNestableTransferToNonRMRKNestableImplementer();
}

// HOOKS

/**
Expand Down
2 changes: 0 additions & 2 deletions contracts/RMRK/library/RMRKErrors.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,6 @@ error RMRKMaxPendingAssetsReached();
error RMRKMaxRecursiveBurnsReached(address childContract, uint256 childId);
/// Attempting to mint a number of tokens that would cause the total supply to be greater than maximum supply
error RMRKMintOverMax();
/// Attempting to mint a nested token to a smart contract that doesn't support nesting
error RMRKMintToNonRMRKNestableImplementer();
/// Attempting to mint zero tokens
error RMRKMintZero();
/// Attempting to pass complementary arrays of different lengths
Expand Down
5 changes: 1 addition & 4 deletions contracts/RMRK/multiasset/RMRKMultiAsset.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ pragma solidity ^0.8.21;

import "@openzeppelin/contracts/token/ERC721/IERC721.sol";
import "@openzeppelin/contracts/token/ERC721/IERC721Receiver.sol";
import "@openzeppelin/contracts/utils/Address.sol";
import "@openzeppelin/contracts/utils/introspection/IERC165.sol";
import "./AbstractMultiAsset.sol";
import "../core/RMRKCore.sol";
Expand All @@ -17,8 +16,6 @@ import "../library/RMRKErrors.sol";
* @notice Smart contract of the RMRK Multi asset module.
*/
contract RMRKMultiAsset is IERC165, IERC721, AbstractMultiAsset, RMRKCore {
using Address for address;

// Mapping from token ID to owner address
mapping(uint256 => address) private _owners;

Expand Down Expand Up @@ -490,7 +487,7 @@ contract RMRKMultiAsset is IERC165, IERC721, AbstractMultiAsset, RMRKCore {
uint256 tokenId,
bytes memory data
) private returns (bool) {
if (to.isContract()) {
if (to.code.length != 0) {
try
IERC721Receiver(to).onERC721Received(
_msgSender(),
Expand Down
30 changes: 16 additions & 14 deletions contracts/RMRK/nestable/RMRKNestable.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import "./IERC7401.sol";
import "../core/RMRKCore.sol";
import "@openzeppelin/contracts/token/ERC721/IERC721.sol";
import "@openzeppelin/contracts/token/ERC721/IERC721Receiver.sol";
import "@openzeppelin/contracts/utils/Address.sol";
import "@openzeppelin/contracts/utils/Context.sol";
import "@openzeppelin/contracts/utils/introspection/IERC165.sol";
import "../library/RMRKErrors.sol";
Expand All @@ -21,8 +20,6 @@ import "../library/RMRKErrors.sol";
* gas limits allow it.
*/
contract RMRKNestable is Context, IERC165, IERC721, IERC7401, RMRKCore {
using Address for address;

uint256 private constant _MAX_LEVELS_TO_CHECK_FOR_INHERITANCE_LOOP = 100;

// Mapping owner address to token count
Expand Down Expand Up @@ -288,15 +285,10 @@ contract RMRKNestable is Context, IERC165, IERC721, IERC7401, RMRKCore {
) internal virtual {
(address immediateOwner, uint256 parentId, ) = directOwnerOf(tokenId);
if (immediateOwner != from) revert ERC721TransferFromIncorrectOwner();
if (to == address(0)) revert ERC721TransferToTheZeroAddress();
if (to == address(this) && tokenId == destinationId)
revert RMRKNestableTransferToSelf();

// Destination contract checks:
// It seems redundant, but otherwise it would revert with no error
if (!to.isContract()) revert RMRKIsNotContract();
if (!IERC165(to).supportsInterface(type(IERC7401).interfaceId))
revert RMRKNestableTransferToNonRMRKNestableImplementer();
_checkDestination(to);
_checkForInheritanceLoop(tokenId, to, destinationId);

_beforeTokenTransfer(from, to, tokenId);
Expand Down Expand Up @@ -453,9 +445,7 @@ contract RMRKNestable is Context, IERC165, IERC721, IERC7401, RMRKCore {
bytes memory data
) internal virtual {
// It seems redundant, but otherwise it would revert with no error
if (!to.isContract()) revert RMRKIsNotContract();
if (!IERC165(to).supportsInterface(type(IERC7401).interfaceId))
revert RMRKMintToNonRMRKNestableImplementer();
_checkDestination(to);

_innerMint(to, tokenId, destinationId, data);
_sendToNFT(address(0), to, 0, destinationId, tokenId, data);
Expand Down Expand Up @@ -837,7 +827,7 @@ contract RMRKNestable is Context, IERC165, IERC721, IERC7401, RMRKCore {
uint256 tokenId,
bytes memory data
) private returns (bool) {
if (to.isContract()) {
if (to.code.length != 0) {
try
IERC721Receiver(to).onERC721Received(
_msgSender(),
Expand Down Expand Up @@ -877,7 +867,7 @@ contract RMRKNestable is Context, IERC165, IERC721, IERC7401, RMRKCore {
_requireMinted(parentId);

address childAddress = _msgSender();
if (!childAddress.isContract()) revert RMRKIsNotContract();
if (childAddress.code.length == 0) revert RMRKIsNotContract();

Child memory child = Child({
contractAddress: childAddress,
Expand Down Expand Up @@ -1179,6 +1169,18 @@ contract RMRKNestable is Context, IERC165, IERC721, IERC7401, RMRKCore {
return child;
}

/**
* @notice Checks the destination is valid for a Nest Transfer/Mint.
* @dev The destination must be a contract that implements the IERC7401 interface.
* @param to Address of the destination
*/
function _checkDestination(address to) internal view {
// It seems redundant, but otherwise it would revert with no error
if (to.code.length == 0) revert RMRKIsNotContract();
if (!IERC165(to).supportsInterface(type(IERC7401).interfaceId))
revert RMRKNestableTransferToNonRMRKNestableImplementer();
}

// HOOKS

/**
Expand Down
7 changes: 2 additions & 5 deletions contracts/RMRK/utils/RMRKCollectionUtils.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ pragma solidity ^0.8.21;
import "@openzeppelin/contracts/interfaces/IERC2981.sol";
import "@openzeppelin/contracts/token/ERC721/extensions/IERC721Metadata.sol";
import "@openzeppelin/contracts/token/ERC721/IERC721.sol";
import "@openzeppelin/contracts/utils/Address.sol";
import "../equippable/IERC6220.sol";
import "../nestable/IERC7401.sol";
import "../extension/soulbound/IERC6454.sol";
Expand All @@ -18,8 +17,6 @@ import "./IRMRKCollectionData.sol";
* @dev Extra utility functions for RMRK contracts.
*/
contract RMRKCollectionUtils {
using Address for address;

event CollectionTokensMetadataRefreshTriggered(address indexed collection);
event TokenMetadataRefreshTriggered(
address indexed collection,
Expand Down Expand Up @@ -198,7 +195,7 @@ contract RMRKCollectionUtils {
*/
function refreshCollectionTokensMetadata(address collectionAddress) public {
// To avoid some spam
if (!collectionAddress.isContract()) {
if (collectionAddress.code.length == 0) {
return;
}
emit CollectionTokensMetadataRefreshTriggered(collectionAddress);
Expand All @@ -215,7 +212,7 @@ contract RMRKCollectionUtils {
uint256 tokenId
) public {
// To avoid some spam
if (!collectionAddress.isContract()) {
if (collectionAddress.code.length == 0) {
return;
}
emit TokenMetadataRefreshTriggered(collectionAddress, tokenId);
Expand Down
2 changes: 1 addition & 1 deletion package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
"node": ">=16.0.0"
},
"dependencies": {
"@openzeppelin/contracts": "^4.6.0"
"@openzeppelin/contracts": "^5.0.0"
},
"devDependencies": {
"@defi-wonderland/smock": "^2.3.4",
Expand Down
4 changes: 2 additions & 2 deletions test/behavior/nestable.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ async function shouldBehaveLikeNestable(

await expect(nestMint(child, nonReceiver.address, parentId)).to.be.revertedWithCustomError(
child,
'RMRKMintToNonRMRKNestableImplementer',
'RMRKNestableTransferToNonRMRKNestableImplementer',
);
});

Expand Down Expand Up @@ -1037,7 +1037,7 @@ async function shouldBehaveLikeNestable(
it('cannot nest tranfer to address 0', async function () {
await expect(
nestTransfer(child, firstOwner, ADDRESS_ZERO, childId, parentId),
).to.be.revertedWithCustomError(child, 'ERC721TransferToTheZeroAddress');
).to.be.revertedWithCustomError(child, 'RMRKIsNotContract');
});

it('cannot nest tranfer to a non contract', async function () {
Expand Down
11 changes: 6 additions & 5 deletions test/implementations/lazyMintErc20Pay.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,8 +117,9 @@ async function shouldControlValidMintingErc20Pay(): Promise<void> {

await erc20.mint(addrs[0].address, ONE_ETH);
await erc20.approve(this.token.address, HALF_ETH);
await expect(this.token.mint(addrs[0].address, 1)).to.be.revertedWith(
'ERC20: insufficient allowance',
await expect(this.token.mint(addrs[0].address, 1)).to.be.revertedWithCustomError(
erc20,
'ERC20InsufficientAllowance',
);
});

Expand Down Expand Up @@ -179,9 +180,9 @@ async function shouldControlValidMintingErc20Pay(): Promise<void> {
expect(await this.token.totalSupply()).to.equal(10);
expect(await this.token.balanceOf(addrs[0].address)).to.equal(10);

await expect(this.token.connect(addrs[0]).mint(addrs[0].address, 1)).to.be.revertedWith(
'ERC20: insufficient allowance',
);
await expect(
this.token.connect(addrs[0]).mint(addrs[0].address, 1),
).to.be.revertedWithCustomError(erc20, 'ERC20InsufficientAllowance');
});

describe('Nest minting', async () => {
Expand Down
8 changes: 4 additions & 4 deletions yarn.lock
View file
Original file line number Diff line number Diff line change
Expand Up @@ -988,10 +988,10 @@
find-up "^4.1.0"
fs-extra "^8.1.0"

"@openzeppelin/contracts@^4.6.0":
version "4.8.0"
resolved "https://registry.yarnpkg.com/@openzeppelin/contracts/-/contracts-4.8.0.tgz#6854c37df205dd2c056bdfa1b853f5d732109109"
integrity sha512-AGuwhRRL+NaKx73WKRNzeCxOCOCxpaqF+kp8TJ89QzAipSwZy/NoflkWaL9bywXFRhIzXt8j38sfF7KBKCPWLw==
"@openzeppelin/contracts@^5.0.0":
version "5.0.0"
resolved "https://registry.yarnpkg.com/@openzeppelin/contracts/-/contracts-5.0.0.tgz#ee0e4b4564f101a5c4ee398cd4d73c0bd92b289c"
integrity sha512-bv2sdS6LKqVVMLI5+zqnNrNU/CA+6z6CmwFXm/MzmOPBRSO5reEJN7z0Gbzvs0/bv/MZZXNklubpwy3v2+azsw==

"@openzeppelin/test-helpers@^0.5.16":
version "0.5.16"
Expand Down

0 comments on commit d9015c0

Please sign in to comment.