From 60da4f4ea3eefcc3473c984fac2f925fa212e5e1 Mon Sep 17 00:00:00 2001 From: James Geary <36774175+jgeary@users.noreply.github.com> Date: Mon, 31 Oct 2022 15:16:17 -0400 Subject: [PATCH 1/2] standardize ierc165 usage --- contracts/modules/Asks/Omnibus/AsksOmnibus.sol | 4 ++-- contracts/modules/Offers/Omnibus/OffersOmnibus.sol | 1 - .../modules/ReserveAuction/Omnibus/ReserveAuctionOmnibus.sol | 4 ++-- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/contracts/modules/Asks/Omnibus/AsksOmnibus.sol b/contracts/modules/Asks/Omnibus/AsksOmnibus.sol index a824b7ae..44e49c5d 100644 --- a/contracts/modules/Asks/Omnibus/AsksOmnibus.sol +++ b/contracts/modules/Asks/Omnibus/AsksOmnibus.sol @@ -4,6 +4,7 @@ pragma solidity 0.8.10; import {ReentrancyGuard} from "@rari-capital/solmate/src/utils/ReentrancyGuard.sol"; import {IERC721} from "@openzeppelin/contracts/token/ERC721/IERC721.sol"; import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; +import {IERC165} from "@openzeppelin/contracts/utils/introspection/IERC165.sol"; import {ERC721TransferHelper} from "../../../transferHelpers/ERC721TransferHelper.sol"; import {IncomingTransferSupportV1} from "../../../common/IncomingTransferSupport/V1/IncomingTransferSupportV1.sol"; @@ -66,11 +67,10 @@ contract AsksOmnibus is IAsksOmnibus, ReentrancyGuard, IncomingTransferSupportV1 } /// @notice Implements EIP-165 for standard interface detection - /// @dev `0x01ffc9a7` is the IERC165 interface id /// @param _interfaceId The identifier of a given interface /// @return If the given interface is supported function supportsInterface(bytes4 _interfaceId) external pure returns (bool) { - return _interfaceId == type(IAsksOmnibus).interfaceId || _interfaceId == 0x01ffc9a7; + return _interfaceId == type(IAsksOmnibus).interfaceId || _interfaceId == type(IERC165).interfaceId; } /// @notice Creates a simple ETH ask for a given NFT diff --git a/contracts/modules/Offers/Omnibus/OffersOmnibus.sol b/contracts/modules/Offers/Omnibus/OffersOmnibus.sol index a75940e3..b2253fd4 100644 --- a/contracts/modules/Offers/Omnibus/OffersOmnibus.sol +++ b/contracts/modules/Offers/Omnibus/OffersOmnibus.sol @@ -74,7 +74,6 @@ contract OffersOmnibus is IOffersOmnibus, ReentrancyGuard, IncomingTransferSuppo } /// @notice Implements EIP-165 for standard interface detection - /// @dev `0x01ffc9a7` is the IERC165 interface id /// @param _interfaceId The identifier of a given interface /// @return If the given interface is supported function supportsInterface(bytes4 _interfaceId) external pure returns (bool) { diff --git a/contracts/modules/ReserveAuction/Omnibus/ReserveAuctionOmnibus.sol b/contracts/modules/ReserveAuction/Omnibus/ReserveAuctionOmnibus.sol index 5557f18c..463ce4eb 100644 --- a/contracts/modules/ReserveAuction/Omnibus/ReserveAuctionOmnibus.sol +++ b/contracts/modules/ReserveAuction/Omnibus/ReserveAuctionOmnibus.sol @@ -4,6 +4,7 @@ pragma solidity 0.8.10; import {ReentrancyGuard} from "@rari-capital/solmate/src/utils/ReentrancyGuard.sol"; import {IERC721} from "@openzeppelin/contracts/token/ERC721/IERC721.sol"; import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; +import {IERC165} from "@openzeppelin/contracts/utils/introspection/IERC165.sol"; import {ERC721TransferHelper} from "../../../transferHelpers/ERC721TransferHelper.sol"; import {IncomingTransferSupportV1} from "../../../common/IncomingTransferSupport/V1/IncomingTransferSupportV1.sol"; @@ -88,11 +89,10 @@ contract ReserveAuctionOmnibus is } /// @notice Implements EIP-165 for standard interface detection - /// @dev `0x01ffc9a7` is the IERC165 interface id /// @param _interfaceId The identifier of a given interface /// @return If the given interface is supported function supportsInterface(bytes4 _interfaceId) external pure returns (bool) { - return _interfaceId == type(IReserveAuctionOmnibus).interfaceId || _interfaceId == 0x01ffc9a7; + return _interfaceId == type(IReserveAuctionOmnibus).interfaceId || _interfaceId == type(IERC165).interfaceId; } /// @notice Creates a simple ETH auction for a given NFT From 2b389806e74b6155f2b8e81ccc1cd2e18a9e859b Mon Sep 17 00:00:00 2001 From: James Geary <36774175+jgeary@users.noreply.github.com> Date: Mon, 31 Oct 2022 16:45:58 -0400 Subject: [PATCH 2/2] standardize tests --- .../Asks/Omnibus/AsksDataStorage.t.sol | 2 - .../Offers/Omnibus/OffersDataStorage.t.sol | 136 ++++++++++++++++++ .../Omnibus/ReserveAuctionDataStorage.t.sol | 2 - 3 files changed, 136 insertions(+), 4 deletions(-) create mode 100644 contracts/test/modules/Offers/Omnibus/OffersDataStorage.t.sol diff --git a/contracts/test/modules/Asks/Omnibus/AsksDataStorage.t.sol b/contracts/test/modules/Asks/Omnibus/AsksDataStorage.t.sol index f59356a6..8a69cb6f 100644 --- a/contracts/test/modules/Asks/Omnibus/AsksDataStorage.t.sol +++ b/contracts/test/modules/Asks/Omnibus/AsksDataStorage.t.sol @@ -80,8 +80,6 @@ contract StorageTestBaseMinimal is AsksDataStorage { } } -/// @title -/// @notice contract AsksDataStorageTest is DSTest { VM internal vm; diff --git a/contracts/test/modules/Offers/Omnibus/OffersDataStorage.t.sol b/contracts/test/modules/Offers/Omnibus/OffersDataStorage.t.sol new file mode 100644 index 00000000..a446cf92 --- /dev/null +++ b/contracts/test/modules/Offers/Omnibus/OffersDataStorage.t.sol @@ -0,0 +1,136 @@ +// SPDX-License-Identifier: GPL-3.0 +pragma solidity 0.8.10; + +import {DSTest} from "ds-test/test.sol"; + +import {OffersDataStorage} from "../../../../modules/Offers/Omnibus/OffersDataStorage.sol"; +import {VM} from "../../../utils/VM.sol"; + +contract StorageTestBaseFull is OffersDataStorage { + uint256 offerCounter; + + function newOffer(address tokenContract, uint256 tokenId) public { + StoredOffer storage offer = offers[tokenContract][tokenId][++offerCounter]; + offer.maker = address(0x111); + offer.amount = 0.4 ether; + _setETHorERC20Currency(offer, address(0x113)); + _setListingFee(offer, 1, address(0x115)); + _setFindersFee(offer, 2); + _setExpiry(offer, uint96(block.timestamp + 1_000)); + } + + function getExpectedActiveFeatures() public pure returns (uint32) { + return FEATURE_MASK_LISTING_FEE | FEATURE_MASK_FINDERS_FEE | FEATURE_MASK_EXPIRY | FEATURE_MASK_ERC20_CURRENCY; + } + + function hasFeature( + address tokenContract, + uint256 tokenId, + uint32 feature, + uint256 offerId + ) public view returns (bool) { + StoredOffer storage offer = offers[tokenContract][tokenId][offerId]; + return _hasFeature(offer.features, feature); + } + + function getFullOffer( + address tokenContract, + uint256 tokenId, + uint256 offerId + ) public view returns (FullOffer memory) { + return _getFullOffer(offers[tokenContract][tokenId][offerId]); + } + + function updatePrice( + address tokenContract, + uint256 tokenId, + address currency, + uint256 amount, + uint256 offerId + ) public { + StoredOffer storage offer = offers[tokenContract][tokenId][offerId]; + _setETHorERC20Currency(offer, currency); + offer.amount = amount; + } +} + +contract StorageTestBaseMinimal is OffersDataStorage { + uint256 offerCounter; + + function newOffer(address tokenContract, uint256 tokenId) public { + StoredOffer storage offer = offers[tokenContract][tokenId][++offerCounter]; + offer.maker = address(0x111); + offer.amount = 0.4 ether; + offer.features = 0; + } + + function getExpectedActiveFeatures() public pure returns (uint32) { + return 0; + } + + function hasFeature( + address tokenContract, + uint256 tokenId, + uint32 feature, + uint256 offerId + ) public view returns (bool) { + StoredOffer storage offer = offers[tokenContract][tokenId][offerId]; + return _hasFeature(offer.features, feature); + } + + function getFullOffer( + address tokenContract, + uint256 tokenId, + uint256 offerId + ) public view returns (FullOffer memory) { + return _getFullOffer(offers[tokenContract][tokenId][offerId]); + } +} + +contract OffersDataStorageTest is DSTest { + VM internal vm; + + uint32 constant FEATURE_MASK_LISTING_FEE = 1 << 3; + uint32 constant FEATURE_MASK_FINDERS_FEE = 1 << 4; + uint32 constant FEATURE_MASK_EXPIRY = 1 << 5; + uint32 constant FEATURE_MASK_ERC20_CURRENCY = 1 << 6; + + function test_OfferStorageMinimalInit() public { + StorageTestBaseMinimal dataStorage = new StorageTestBaseMinimal(); + dataStorage.newOffer(address(0x112), 21); + OffersDataStorage.FullOffer memory offer = dataStorage.getFullOffer(address(0x112), 21, 1); + assertEq(offer.amount, 0.4 ether, "price wrong"); + assertEq(offer.maker, address(0x111), "seller wrong"); + assertEq(offer.expiry, 0, "incorrect expiry"); + assertEq(offer.currency, address(0), "incorrect currency"); + assertEq(offer.findersFeeBps, 0, "incorrect finders fee"); + assertEq(offer.listingFeeBps, 0, "incorrect listing fee"); + assertEq(offer.listingFeeRecipient, address(0), "incorrect listing fee"); + } + + function test_OfferStorageInit() public { + StorageTestBaseFull dataStorage = new StorageTestBaseFull(); + dataStorage.newOffer(address(0x121), 21); + OffersDataStorage.FullOffer memory offer = dataStorage.getFullOffer(address(0x121), 21, 1); + assertEq(offer.amount, 0.4 ether, "price wrong"); + assertEq(offer.maker, address(0x111), "seller wrong"); + assertEq(offer.expiry, block.timestamp + 1_000, "incorrect expiry"); + assertEq(offer.currency, address(0x113), "incorrect currency"); + assertEq(offer.findersFeeBps, 2, "incorrect finders fee"); + assertEq(offer.listingFeeBps, 1, "incorrect listing fee"); + assertEq(offer.listingFeeRecipient, address(0x115), "incorrect listing fee"); + } + + function test_OfferStorageUpdatePrice() public { + StorageTestBaseFull dataStorage = new StorageTestBaseFull(); + dataStorage.newOffer(address(0x121), 21); + OffersDataStorage.FullOffer memory offer = dataStorage.getFullOffer(address(0x121), 21, 1); + assertEq(offer.amount, 0.4 ether, "price wrong"); + assertEq(offer.currency, address(0x113), "incorrect currency"); + dataStorage.updatePrice(address(0x121), 21, address(0), 1 ether, 1); + offer = dataStorage.getFullOffer(address(0x121), 21, 1); + assertEq(offer.amount, 1 ether, "price wrong"); + assertEq(offer.currency, address(0), "incorrect currency"); + assertTrue(!dataStorage.hasFeature(address(0x121), 21, FEATURE_MASK_ERC20_CURRENCY, 1)); + } +} diff --git a/contracts/test/modules/ReserveAuction/Omnibus/ReserveAuctionDataStorage.t.sol b/contracts/test/modules/ReserveAuction/Omnibus/ReserveAuctionDataStorage.t.sol index c554f23e..32b2ef58 100644 --- a/contracts/test/modules/ReserveAuction/Omnibus/ReserveAuctionDataStorage.t.sol +++ b/contracts/test/modules/ReserveAuction/Omnibus/ReserveAuctionDataStorage.t.sol @@ -56,8 +56,6 @@ contract StorageTestBaseMinimal is ReserveAuctionDataStorage { } } -/// @title -/// @notice contract AuctionDataStorageTest is DSTest { uint32 constant FEATURE_MASK_LISTING_FEE = 1 << 3; uint32 constant FEATURE_MASK_FINDERS_FEE = 1 << 4;