From 16ccb2323e3fd6b9239418e2e6445a8d67c9cde3 Mon Sep 17 00:00:00 2001 From: Rohan Kulkarni Date: Tue, 25 Jan 2022 17:28:03 -0500 Subject: [PATCH 01/16] [feat] Offers v1.0 --- contracts/modules/Offers/V1/OffersV1.sol | 259 +++++++++++++++++ .../Offers/V1/Offers.integration.t.sol | 166 +++++++++++ contracts/test/modules/Offers/V1/Offers.t.sol | 267 ++++++++++++++++++ 3 files changed, 692 insertions(+) create mode 100644 contracts/modules/Offers/V1/OffersV1.sol create mode 100644 contracts/test/modules/Offers/V1/Offers.integration.t.sol create mode 100644 contracts/test/modules/Offers/V1/Offers.t.sol diff --git a/contracts/modules/Offers/V1/OffersV1.sol b/contracts/modules/Offers/V1/OffersV1.sol new file mode 100644 index 00000000..acae5161 --- /dev/null +++ b/contracts/modules/Offers/V1/OffersV1.sol @@ -0,0 +1,259 @@ +// SPDX-License-Identifier: GPL-3.0 +pragma solidity 0.8.10; + +/// ------------ IMPORTS ------------ + +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 {ERC721TransferHelper} from "../../../transferHelpers/ERC721TransferHelper.sol"; +import {UniversalExchangeEventV1} from "../../../common/UniversalExchangeEvent/V1/UniversalExchangeEventV1.sol"; +import {IncomingTransferSupportV1} from "../../../common/IncomingTransferSupport/V1/IncomingTransferSupportV1.sol"; +import {FeePayoutSupportV1} from "../../../common/FeePayoutSupport/FeePayoutSupportV1.sol"; +import {ModuleNamingSupportV1} from "../../../common/ModuleNamingSupport/ModuleNamingSupportV1.sol"; + +/// @title Offers V1 +/// @author kulkarohan +/// @notice This module allows users to place ETH/ERC-20 offers for any ERC-721 token +contract OffersV1 is ReentrancyGuard, UniversalExchangeEventV1, IncomingTransferSupportV1, FeePayoutSupportV1, ModuleNamingSupportV1 { + /// @dev The indicator to pass all remaining gas when paying out royalties + uint256 private constant USE_ALL_GAS_FLAG = 0; + + /// @notice The number of offers placed + uint256 public offerCount; + + /// @notice The ZORA ERC-721 Transfer Helper + ERC721TransferHelper public immutable erc721TransferHelper; + + /// @notice The metadata of an offer + /// @param seller The address of the seller placing the offer + /// @param currency The address of the ERC-20 selling, or address(0) for ETH + /// @param findersFeeBps The fee to the referrer of the offer + /// @param amount The amount selling + struct Offer { + address seller; + address currency; + uint16 findersFeeBps; + uint256 amount; + } + + /// ------------ STORAGE ------------ + + /// @notice The metadata for a given offer + /// @dev ERC-721 token address => ERC-721 token ID => Offer ID => Offer + mapping(address => mapping(uint256 => mapping(uint256 => Offer))) public offers; + + /// @notice The offers for a given NFT + /// @dev ERC-721 token address => ERC-721 token ID => offer IDs + mapping(address => mapping(uint256 => uint256[])) public offersForNFT; + + /// ------------ EVENTS ------------ + + /// @notice Emitted when an offer is created + /// @param tokenContract The ERC-721 token address of the created offer + /// @param tokenId The ERC-721 token ID of the created offer + /// @param id The ID of the created offer + /// @param offer The metadata of the created offer + event NFTOfferCreated(address indexed tokenContract, uint256 indexed tokenId, uint256 indexed id, Offer offer); + + /// @notice Emitted when an offer is updated + /// @param tokenContract The ERC-721 token address of the updated offer + /// @param tokenId The ERC-721 token ID of the updated offer + /// @param id The ID of the updated offer + /// @param offer The metadata of the updated offer + event NFTOfferAmountUpdated(address indexed tokenContract, uint256 indexed tokenId, uint256 indexed id, Offer offer); + + /// @notice Emitted when an offer is canceled + /// @param tokenContract The ERC-721 token address of the canceled offer + /// @param tokenId The ERC-721 token ID of the canceled offer + /// @param id The ID of the canceled offer + /// @param offer The metadata of the canceled offer + event NFTOfferCanceled(address indexed tokenContract, uint256 indexed tokenId, uint256 indexed id, Offer offer); + + /// @notice Emitted when an offer is filled + /// @param tokenContract The ERC-721 token address of the filled offer + /// @param tokenId The ERC-721 token ID of the filled offer + /// @param id The ID of the filled offer + /// @param buyer The address of the buyer who filled the offer + /// @param finder The address of the finder who referred the offer + /// @param offer The metadata of the filled offer + event NFTOfferFilled(address indexed tokenContract, uint256 indexed tokenId, uint256 indexed id, address buyer, address finder, Offer offer); + + /// ------------ CONSTRUCTOR ------------ + + /// @param _erc20TransferHelper The ZORA ERC-20 Transfer Helper address + /// @param _erc721TransferHelper The ZORA ERC-721 Transfer Helper address + /// @param _royaltyEngine The Manifold Royalty Engine address + /// @param _protocolFeeSettings The ZoraProtocolFeeSettingsV1 address + /// @param _wethAddress The WETH token address + constructor( + address _erc20TransferHelper, + address _erc721TransferHelper, + address _royaltyEngine, + address _protocolFeeSettings, + address _wethAddress + ) + IncomingTransferSupportV1(_erc20TransferHelper) + FeePayoutSupportV1(_royaltyEngine, _protocolFeeSettings, _wethAddress, ERC721TransferHelper(_erc721TransferHelper).ZMM().registrar()) + ModuleNamingSupportV1("Offers: v1.0") + { + erc721TransferHelper = ERC721TransferHelper(_erc721TransferHelper); + } + + /// ------------ SELLER FUNCTIONS ------------ + + /// @notice Creates an offer for an NFT + /// @param _tokenContract The address of the desired ERC-721 token + /// @param _tokenId The ID of the desired ERC-721 token + /// @param _currency The address of the offering ERC-20 token, or address(0) for ETH + /// @param _amount The amount offering + /// @param _findersFeeBps The bps of the amount (post-royalties) to send to a referrer of the sale + /// @return The ID of the created offer + function createNFTOffer( + address _tokenContract, + uint256 _tokenId, + address _currency, + uint256 _amount, + uint16 _findersFeeBps + ) external payable nonReentrant returns (uint256) { + require(IERC721(_tokenContract).ownerOf(_tokenId) != msg.sender, "createNFTOffer cannot place offer on own NFT"); + require(_findersFeeBps <= 10000, "createNFTOffer finders fee bps must be less than or equal to 10000"); + + // Ensure valid payment and take custody of offer + _handleIncomingTransfer(_amount, _currency); + + // Get offer ID + offerCount++; + + // Store offer metadata + offers[_tokenContract][_tokenId][offerCount] = Offer({ + seller: msg.sender, + currency: _currency, + findersFeeBps: _findersFeeBps, + amount: _amount + }); + + // Add ID to offers placed for NFT + offersForNFT[_tokenContract][_tokenId].push(offerCount); + + emit NFTOfferCreated(_tokenContract, _tokenId, offerCount, offers[_tokenContract][_tokenId][offerCount]); + + return offerCount; + } + + /// @notice Updates the amount of an offer + /// @param _tokenContract The address of the offer ERC-721 token + /// @param _tokenId The ID of the offer ERC-721 token + /// @param _offerId The ID of the offer + /// @param _amount The new offer amount + function setNFTOfferAmount( + address _tokenContract, + uint256 _tokenId, + uint256 _offerId, + uint256 _amount + ) external payable nonReentrant { + Offer storage offer = offers[_tokenContract][_tokenId][offerCount]; + + require(offer.seller == msg.sender, "setNFTOfferAmount must be seller"); + require(_amount != 0 && _amount != offer.amount, "setNFTOfferAmount _amount cannot be 0 or previous amount"); + + // Get initial offer + uint256 prevAmount = offer.amount; + + // If update is increase -- + if (_amount > prevAmount) { + // Ensure valid payment and take custody + uint256 increaseAmount = _amount - prevAmount; + _handleIncomingTransfer(increaseAmount, offer.currency); + + // Increase offer + offer.amount += increaseAmount; + + // If update is decrease -- + } else if (_amount < prevAmount) { + // Refund difference + uint256 decreaseAmount = prevAmount - _amount; + _handleOutgoingTransfer(offer.seller, decreaseAmount, offer.currency, USE_ALL_GAS_FLAG); + + // Decrease offer + offer.amount -= decreaseAmount; + } + + emit NFTOfferAmountUpdated(_tokenContract, _tokenId, _offerId, offer); + } + + /// @notice Cancels and refunds the offer for an NFT + /// @param _tokenContract The ERC-721 token address of the offer + /// @param _tokenId The ERC-721 token ID of the offer + /// @param _offerId The ID of the offer + function cancelNFTOffer( + address _tokenContract, + uint256 _tokenId, + uint256 _offerId + ) external nonReentrant { + Offer storage offer = offers[_tokenContract][_tokenId][offerCount]; + + require(offer.seller == msg.sender, "cancelNFTOffer must be seller"); + + // Refund offered amount + _handleOutgoingTransfer(offer.seller, offer.amount, offer.currency, USE_ALL_GAS_FLAG); + + emit NFTOfferCanceled(_tokenContract, _tokenId, _offerId, offer); + + delete offers[_tokenContract][_tokenId][offerCount]; + } + + /// ------------ BUYER FUNCTIONS ------------ + + /// @notice Fills the offer for an owned NFT, in exchange for ETH/ERC-20 tokens + /// @param _tokenContract The address of the ERC-721 token to sell + /// @param _tokenId The ID of the ERC-721 token to sell + /// @param _offerId The ID of the offer to fill + /// @param _currency The address of ERC-20 token to accept, or address(0) for ETH + /// @param _amount The offered amount to accept + /// @param _finder The address of the offer referrer + function fillNFTOffer( + address _tokenContract, + uint256 _tokenId, + uint256 _offerId, + address _currency, + uint256 _amount, + address _finder + ) external nonReentrant { + Offer storage offer = offers[_tokenContract][_tokenId][offerCount]; + + require(offer.seller != address(0), "fillNFTOffer must be active offer"); + require(IERC721(_tokenContract).ownerOf(_tokenId) == msg.sender, "fillNFTOffer must be token owner"); + require(offer.currency == _currency, "fillNFTOffer _currency must match offer currency"); + require(offer.amount == _amount, "fillNFTOffer _amount must match offer amount"); + + // Payout respective parties, ensuring royalties are honored + (uint256 remainingProfit, ) = _handleRoyaltyPayout(_tokenContract, _tokenId, offer.amount, offer.currency, USE_ALL_GAS_FLAG); + + // Payout optional protocol fee + remainingProfit = _handleProtocolFeePayout(remainingProfit, offer.currency); + + // Payout optional finders fee + if (_finder != address(0)) { + uint256 findersFee = (remainingProfit * offer.findersFeeBps) / 10000; + _handleOutgoingTransfer(_finder, findersFee, offer.currency, USE_ALL_GAS_FLAG); + + remainingProfit -= findersFee; + } + + // Transfer remaining ETH/ERC-20 to seller + _handleOutgoingTransfer(msg.sender, remainingProfit, offer.currency, USE_ALL_GAS_FLAG); + + // Transfer NFT to buyer + erc721TransferHelper.transferFrom(_tokenContract, msg.sender, offer.seller, _tokenId); + + ExchangeDetails memory userAExchangeDetails = ExchangeDetails({tokenContract: offer.currency, tokenId: 0, amount: offer.amount}); + + ExchangeDetails memory userBExchangeDetails = ExchangeDetails({tokenContract: _tokenContract, tokenId: _tokenId, amount: 1}); + + emit ExchangeExecuted(offer.seller, msg.sender, userAExchangeDetails, userBExchangeDetails); + emit NFTOfferFilled(_tokenContract, _tokenId, _offerId, msg.sender, _finder, offer); + + delete offers[_tokenContract][_tokenId][offerCount]; + } +} diff --git a/contracts/test/modules/Offers/V1/Offers.integration.t.sol b/contracts/test/modules/Offers/V1/Offers.integration.t.sol new file mode 100644 index 00000000..b1ca6400 --- /dev/null +++ b/contracts/test/modules/Offers/V1/Offers.integration.t.sol @@ -0,0 +1,166 @@ +// SPDX-License-Identifier: GPL-3.0 +pragma solidity 0.8.10; + +import {DSTest} from "ds-test/test.sol"; + +import {OffersV1} from "../../../../modules/Offers/V1/OffersV1.sol"; +import {Zorb} from "../../../utils/users/Zorb.sol"; +import {ZoraRegistrar} from "../../../utils/users/ZoraRegistrar.sol"; +import {ZoraModuleManager} from "../../../../ZoraModuleManager.sol"; +import {ZoraProtocolFeeSettings} from "../../../../auxiliary/ZoraProtocolFeeSettings/ZoraProtocolFeeSettings.sol"; +import {ERC20TransferHelper} from "../../../../transferHelpers/ERC20TransferHelper.sol"; +import {ERC721TransferHelper} from "../../../../transferHelpers/ERC721TransferHelper.sol"; +import {RoyaltyEngine} from "../../../utils/modules/RoyaltyEngine.sol"; + +import {TestERC721} from "../../../utils/tokens/TestERC721.sol"; +import {WETH} from "../../../utils/tokens/WETH.sol"; +import {VM} from "../../../utils/VM.sol"; + +/// @title OffersV1IntegrationTest +/// @notice Integration Tests for Offers v1.0 +contract OffersV1IntegrationTest is DSTest { + VM internal vm; + + ZoraRegistrar internal registrar; + ZoraProtocolFeeSettings internal ZPFS; + ZoraModuleManager internal ZMM; + ERC20TransferHelper internal erc20TransferHelper; + ERC721TransferHelper internal erc721TransferHelper; + RoyaltyEngine internal royaltyEngine; + + OffersV1 internal offers; + TestERC721 internal token; + WETH internal weth; + + Zorb internal seller; + Zorb internal buyer; + Zorb internal finder; + Zorb internal royaltyRecipient; + + function setUp() public { + // Cheatcodes + vm = VM(0x7109709ECfa91a80626fF3989D68f67F5b1DD12D); + + // Deploy V3 + registrar = new ZoraRegistrar(); + ZPFS = new ZoraProtocolFeeSettings(); + ZMM = new ZoraModuleManager(address(registrar), address(ZPFS)); + erc20TransferHelper = new ERC20TransferHelper(address(ZMM)); + erc721TransferHelper = new ERC721TransferHelper(address(ZMM)); + + // Init V3 + registrar.init(ZMM); + ZPFS.init(address(ZMM), address(0)); + + // Create users + seller = new Zorb(address(ZMM)); + buyer = new Zorb(address(ZMM)); + finder = new Zorb(address(ZMM)); + royaltyRecipient = new Zorb(address(ZMM)); + + // Deploy mocks + royaltyEngine = new RoyaltyEngine(address(royaltyRecipient)); + token = new TestERC721(); + weth = new WETH(); + + // Deploy Offers v1.0 + offers = new OffersV1(address(erc20TransferHelper), address(erc721TransferHelper), address(royaltyEngine), address(ZPFS), address(weth)); + registrar.registerModule(address(offers)); + + // Set seller balance + vm.deal(address(seller), 100 ether); + + // Mint buyer token + token.mint(address(buyer), 0); + + // Seller swap 50 ETH <> 50 WETH + vm.prank(address(seller)); + weth.deposit{value: 50 ether}(); + + // Users approve Offers module + seller.setApprovalForModule(address(offers), true); + buyer.setApprovalForModule(address(offers), true); + + // Seller approve ERC20TransferHelper + vm.prank(address(seller)); + weth.approve(address(erc20TransferHelper), 50 ether); + + // Buyer approve ERC721TransferHelper + vm.prank(address(buyer)); + token.setApprovalForAll(address(erc721TransferHelper), true); + } + + /// ------------ ETH Offer ------------ /// + + function runETH() public { + vm.prank(address(seller)); + uint256 id = offers.createNFTOffer{value: 1 ether}(address(token), 0, address(0), 1 ether, 1000); + + vm.prank(address(buyer)); + offers.fillNFTOffer(address(token), 0, id, address(0), 1 ether, address(finder)); + } + + function test_ETHIntegration() public { + uint256 beforeSellerBalance = address(seller).balance; + uint256 beforeBuyerBalance = address(buyer).balance; + uint256 beforeRoyaltyRecipientBalance = address(royaltyRecipient).balance; + uint256 beforeFinderBalance = address(finder).balance; + address beforeTokenOwner = token.ownerOf(0); + + runETH(); + + uint256 afterSellerBalance = address(seller).balance; + uint256 afterBuyerBalance = address(buyer).balance; + uint256 afterRoyaltyRecipientBalance = address(royaltyRecipient).balance; + uint256 afterFinderBalance = address(finder).balance; + address afterTokenOwner = token.ownerOf(0); + + // 1 ETH withdrawn from seller + require((beforeSellerBalance - afterSellerBalance) == 1 ether); + // 0.05 ETH creator royalty + require((afterRoyaltyRecipientBalance - beforeRoyaltyRecipientBalance) == 0.05 ether); + // 1000 bps finders fee (Remaining 0.95 ETH * 10% finders fee = 0.095 ETH) + require((afterFinderBalance - beforeFinderBalance) == 0.095 ether); + // Remaining 0.855 ETH paid to buyer + require((afterBuyerBalance - beforeBuyerBalance) == 0.855 ether); + // NFT transferred to seller + require((beforeTokenOwner == address(buyer)) && afterTokenOwner == address(seller)); + } + + /// ------------ ERC-20 Offer ------------ /// + + function runERC20() public { + vm.prank(address(seller)); + uint256 id = offers.createNFTOffer(address(token), 0, address(weth), 1 ether, 1000); + + vm.prank(address(buyer)); + offers.fillNFTOffer(address(token), 0, id, address(weth), 1 ether, address(finder)); + } + + function test_ERC20Integration() public { + uint256 beforeSellerBalance = weth.balanceOf(address(seller)); + uint256 beforeBuyerBalance = weth.balanceOf(address(buyer)); + uint256 beforeRoyaltyRecipientBalance = weth.balanceOf(address(royaltyRecipient)); + uint256 beforeFinderBalance = weth.balanceOf(address(finder)); + address beforeTokenOwner = token.ownerOf(0); + + runERC20(); + + uint256 afterSellerBalance = weth.balanceOf(address(seller)); + uint256 afterBuyerBalance = weth.balanceOf(address(buyer)); + uint256 afterRoyaltyRecipientBalance = weth.balanceOf(address(royaltyRecipient)); + uint256 afterFinderBalance = weth.balanceOf(address(finder)); + address afterTokenOwner = token.ownerOf(0); + + // 1 WETH withdrawn from seller + require((beforeSellerBalance - afterSellerBalance) == 1 ether); + // 0.05 WETH creator royalty + require((afterRoyaltyRecipientBalance - beforeRoyaltyRecipientBalance) == 0.05 ether); + // 0.095 WETH finders fee (0.95 WETH * 10% finders fee) + require((afterFinderBalance - beforeFinderBalance) == 0.095 ether); + // Remaining 0.855 WETH paid to buyer + require((afterBuyerBalance - beforeBuyerBalance) == 0.855 ether); + // NFT transferred to seller + require((beforeTokenOwner == address(buyer)) && afterTokenOwner == address(seller)); + } +} diff --git a/contracts/test/modules/Offers/V1/Offers.t.sol b/contracts/test/modules/Offers/V1/Offers.t.sol new file mode 100644 index 00000000..6ccc8ca0 --- /dev/null +++ b/contracts/test/modules/Offers/V1/Offers.t.sol @@ -0,0 +1,267 @@ +// SPDX-License-Identifier: GPL-3.0 +pragma solidity 0.8.10; + +import {DSTest} from "ds-test/test.sol"; + +import {OffersV1} from "../../../../modules/Offers/V1/OffersV1.sol"; +import {Zorb} from "../../../utils/users/Zorb.sol"; +import {ZoraRegistrar} from "../../../utils/users/ZoraRegistrar.sol"; +import {ZoraModuleManager} from "../../../../ZoraModuleManager.sol"; +import {ZoraProtocolFeeSettings} from "../../../../auxiliary/ZoraProtocolFeeSettings/ZoraProtocolFeeSettings.sol"; +import {ERC20TransferHelper} from "../../../../transferHelpers/ERC20TransferHelper.sol"; +import {ERC721TransferHelper} from "../../../../transferHelpers/ERC721TransferHelper.sol"; +import {RoyaltyEngine} from "../../../utils/modules/RoyaltyEngine.sol"; + +import {TestERC721} from "../../../utils/tokens/TestERC721.sol"; +import {WETH} from "../../../utils/tokens/WETH.sol"; +import {VM} from "../../../utils/VM.sol"; + +/// @title OffersV1Test +/// @notice Unit Tests for Offers v1.0 +contract OffersV1Test is DSTest { + VM internal vm; + + ZoraRegistrar internal registrar; + ZoraProtocolFeeSettings internal ZPFS; + ZoraModuleManager internal ZMM; + ERC20TransferHelper internal erc20TransferHelper; + ERC721TransferHelper internal erc721TransferHelper; + RoyaltyEngine internal royaltyEngine; + + OffersV1 internal offers; + TestERC721 internal token; + WETH internal weth; + + Zorb internal seller; + Zorb internal buyer; + Zorb internal finder; + Zorb internal royaltyRecipient; + + function setUp() public { + // Cheatcodes + vm = VM(0x7109709ECfa91a80626fF3989D68f67F5b1DD12D); + + // Deploy V3 + registrar = new ZoraRegistrar(); + ZPFS = new ZoraProtocolFeeSettings(); + ZMM = new ZoraModuleManager(address(registrar), address(ZPFS)); + erc20TransferHelper = new ERC20TransferHelper(address(ZMM)); + erc721TransferHelper = new ERC721TransferHelper(address(ZMM)); + + // Init V3 + registrar.init(ZMM); + ZPFS.init(address(ZMM), address(0)); + + // Create users + seller = new Zorb(address(ZMM)); + buyer = new Zorb(address(ZMM)); + finder = new Zorb(address(ZMM)); + royaltyRecipient = new Zorb(address(ZMM)); + + // Deploy mocks + royaltyEngine = new RoyaltyEngine(address(royaltyRecipient)); + token = new TestERC721(); + weth = new WETH(); + + // Deploy Offers v1.0 + offers = new OffersV1(address(erc20TransferHelper), address(erc721TransferHelper), address(royaltyEngine), address(ZPFS), address(weth)); + registrar.registerModule(address(offers)); + + // Set seller balance + vm.deal(address(seller), 100 ether); + + // Mint buyer token + token.mint(address(buyer), 0); + + // Seller swap 50 ETH <> 50 WETH + vm.prank(address(seller)); + weth.deposit{value: 50 ether}(); + + // Users approve Offers module + seller.setApprovalForModule(address(offers), true); + buyer.setApprovalForModule(address(offers), true); + + // Seller approve ERC20TransferHelper + vm.prank(address(seller)); + weth.approve(address(erc20TransferHelper), 50 ether); + + // Buyer approve ERC721TransferHelper + vm.prank(address(buyer)); + token.setApprovalForAll(address(erc721TransferHelper), true); + } + + /// ------------ CREATE NFT OFFER ------------ /// + + function testGas_CreateOffer() public { + offers.createNFTOffer{value: 1 ether}(address(token), 0, address(0), 1 ether, 1000); + } + + function test_CreateOffer() public { + vm.prank(address(seller)); + offers.createNFTOffer{value: 1 ether}(address(token), 0, address(0), 1 ether, 1000); + + (address offeror, , , ) = offers.offers(address(token), 0, 1); + + require(offeror == address(seller)); + } + + function testFail_CannotCreateOfferOnOwnNFT() public { + vm.prank(address(buyer)); + offers.createNFTOffer{value: 1 ether}(address(token), 0, address(0), 1 ether, 1000); + } + + function testFail_CannotCreateOfferWithoutAttachingFunds() public { + vm.prank(address(seller)); + offers.createNFTOffer(address(token), 0, address(0), 1 ether, 1000); + } + + /// ------------ UPDATE NFT OFFER ------------ /// + + function test_IncreaseOffer() public { + vm.startPrank(address(seller)); + + offers.createNFTOffer{value: 1 ether}(address(token), 0, address(0), 1 ether, 1000); + offers.setNFTOfferAmount{value: 1 ether}(address(token), 0, 1, 2 ether); + + vm.stopPrank(); + + (, , , uint256 amount) = offers.offers(address(token), 0, 1); + + require(amount == 2 ether); + } + + function test_DecreaseOffer() public { + vm.startPrank(address(seller)); + + offers.createNFTOffer{value: 1 ether}(address(token), 0, address(0), 1 ether, 1000); + offers.setNFTOfferAmount(address(token), 0, 1, 0.5 ether); + + vm.stopPrank(); + + (, , , uint256 amount) = offers.offers(address(token), 0, 1); + + require(amount == 0.5 ether); + } + + function testRevert_OnlySellerCanUpdateOffer() public { + vm.prank(address(seller)); + offers.createNFTOffer{value: 1 ether}(address(token), 0, address(0), 1 ether, 1000); + + vm.expectRevert("setNFTOfferAmount must be seller"); + offers.setNFTOfferAmount(address(token), 0, 1, 0.5 ether); + } + + function testRevert_CannotIncreaseOfferWithoutAttachingFunds() public { + vm.startPrank(address(seller)); + + offers.createNFTOffer{value: 1 ether}(address(token), 0, address(0), 1 ether, 1000); + vm.expectRevert("_handleIncomingTransfer msg value less than expected amount"); + offers.setNFTOfferAmount(address(token), 0, 1, 2 ether); + + vm.stopPrank(); + } + + function testRevert_CannotUpdateInactiveOffer() public { + vm.prank(address(seller)); + offers.createNFTOffer{value: 1 ether}(address(token), 0, address(0), 1 ether, 1000); + + vm.prank(address(buyer)); + offers.fillNFTOffer(address(token), 0, 1, address(0), 1 ether, address(finder)); + + vm.prank(address(seller)); + vm.expectRevert("setNFTOfferAmount must be seller"); + offers.setNFTOfferAmount(address(token), 0, 1, 0.5 ether); + } + + /// ------------ CANCEL NFT OFFER ------------ /// + + function test_CancelNFTOffer() public { + vm.startPrank(address(seller)); + + offers.createNFTOffer{value: 1 ether}(address(token), 0, address(0), 1 ether, 1000); + + (, , , uint256 beforeAmount) = offers.offers(address(token), 0, 1); + require(beforeAmount == 1 ether); + + offers.cancelNFTOffer(address(token), 0, 1); + + (, , , uint256 afterAmount) = offers.offers(address(token), 0, 1); + require(afterAmount == 0); + + vm.stopPrank(); + } + + function testRevert_CannotCancelInactiveOffer() public { + vm.prank(address(seller)); + offers.createNFTOffer{value: 1 ether}(address(token), 0, address(0), 1 ether, 1000); + + vm.prank(address(buyer)); + offers.fillNFTOffer(address(token), 0, 1, address(0), 1 ether, address(finder)); + + vm.prank(address(seller)); + vm.expectRevert("cancelNFTOffer must be seller"); + offers.cancelNFTOffer(address(token), 0, 1); + } + + function testRevert_OnlySellerCanCancelOffer() public { + vm.prank(address(seller)); + offers.createNFTOffer{value: 1 ether}(address(token), 0, address(0), 1 ether, 1000); + + vm.expectRevert("cancelNFTOffer must be seller"); + offers.cancelNFTOffer(address(token), 0, 1); + } + + /// ------------ FILL NFT OFFER ------------ /// + + function test_FillNFTOffer() public { + vm.prank(address(seller)); + offers.createNFTOffer{value: 1 ether}(address(token), 0, address(0), 1 ether, 1000); + + address beforeTokenOwner = token.ownerOf(0); + + vm.prank(address(buyer)); + offers.fillNFTOffer(address(token), 0, 1, address(0), 1 ether, address(finder)); + + address afterTokenOwner = token.ownerOf(0); + + require(beforeTokenOwner == address(buyer) && afterTokenOwner == address(seller)); + } + + function testRevert_OnlyTokenHolderCanFillOffer() public { + vm.prank(address(seller)); + uint256 id = offers.createNFTOffer{value: 1 ether}(address(token), 0, address(0), 1 ether, 1000); + + vm.expectRevert("fillNFTOffer must be token owner"); + offers.fillNFTOffer(address(token), 0, id, address(0), 1 ether, address(finder)); + } + + function testRevert_CannotFillInactiveOffer() public { + vm.prank(address(seller)); + uint256 id = offers.createNFTOffer{value: 1 ether}(address(token), 0, address(0), 1 ether, 1000); + + vm.prank(address(buyer)); + offers.fillNFTOffer(address(token), 0, id, address(0), 1 ether, address(finder)); + + vm.prank(address(buyer)); + vm.expectRevert("fillNFTOffer must be active offer"); + offers.fillNFTOffer(address(token), 0, id, address(0), 1 ether, address(finder)); + } + + function testRevert_AcceptCurrencyMustMatchOffer() public { + vm.prank(address(seller)); + uint256 id = offers.createNFTOffer{value: 1 ether}(address(token), 0, address(0), 1 ether, 1000); + + vm.prank(address(buyer)); + vm.expectRevert("fillNFTOffer _currency must match offer currency"); + offers.fillNFTOffer(address(token), 0, id, address(weth), 1 ether, address(finder)); + } + + function testRevert_AcceptAmountMustMatchOffer() public { + vm.prank(address(seller)); + uint256 id = offers.createNFTOffer{value: 1 ether}(address(token), 0, address(0), 1 ether, 1000); + + vm.prank(address(buyer)); + vm.expectRevert("fillNFTOffer _amount must match offer amount"); + offers.fillNFTOffer(address(token), 0, id, address(0), 0.5 ether, address(finder)); + } +} From 553433f474e9cfc9978e6ba83819cf57479b0f53 Mon Sep 17 00:00:00 2001 From: Rohan Kulkarni Date: Thu, 27 Jan 2022 02:12:17 -0500 Subject: [PATCH 02/16] [fix] offer storage retrieval --- contracts/modules/Offers/V1/OffersV1.sol | 38 ++++++++++-------------- 1 file changed, 15 insertions(+), 23 deletions(-) diff --git a/contracts/modules/Offers/V1/OffersV1.sol b/contracts/modules/Offers/V1/OffersV1.sol index acae5161..4a888513 100644 --- a/contracts/modules/Offers/V1/OffersV1.sol +++ b/contracts/modules/Offers/V1/OffersV1.sol @@ -119,13 +119,11 @@ contract OffersV1 is ReentrancyGuard, UniversalExchangeEventV1, IncomingTransfer require(IERC721(_tokenContract).ownerOf(_tokenId) != msg.sender, "createNFTOffer cannot place offer on own NFT"); require(_findersFeeBps <= 10000, "createNFTOffer finders fee bps must be less than or equal to 10000"); - // Ensure valid payment and take custody of offer + // Validate offer and take custody _handleIncomingTransfer(_amount, _currency); - // Get offer ID offerCount++; - // Store offer metadata offers[_tokenContract][_tokenId][offerCount] = Offer({ seller: msg.sender, currency: _currency, @@ -133,7 +131,6 @@ contract OffersV1 is ReentrancyGuard, UniversalExchangeEventV1, IncomingTransfer amount: _amount }); - // Add ID to offers placed for NFT offersForNFT[_tokenContract][_tokenId].push(offerCount); emit NFTOfferCreated(_tokenContract, _tokenId, offerCount, offers[_tokenContract][_tokenId][offerCount]); @@ -152,30 +149,26 @@ contract OffersV1 is ReentrancyGuard, UniversalExchangeEventV1, IncomingTransfer uint256 _offerId, uint256 _amount ) external payable nonReentrant { - Offer storage offer = offers[_tokenContract][_tokenId][offerCount]; + Offer storage offer = offers[_tokenContract][_tokenId][_offerId]; require(offer.seller == msg.sender, "setNFTOfferAmount must be seller"); require(_amount != 0 && _amount != offer.amount, "setNFTOfferAmount _amount cannot be 0 or previous amount"); - // Get initial offer uint256 prevAmount = offer.amount; - // If update is increase -- if (_amount > prevAmount) { - // Ensure valid payment and take custody uint256 increaseAmount = _amount - prevAmount; + + // Validate offer increase and take custody _handleIncomingTransfer(increaseAmount, offer.currency); - // Increase offer offer.amount += increaseAmount; - - // If update is decrease -- } else if (_amount < prevAmount) { - // Refund difference uint256 decreaseAmount = prevAmount - _amount; + + // Refund offer difference _handleOutgoingTransfer(offer.seller, decreaseAmount, offer.currency, USE_ALL_GAS_FLAG); - // Decrease offer offer.amount -= decreaseAmount; } @@ -191,26 +184,26 @@ contract OffersV1 is ReentrancyGuard, UniversalExchangeEventV1, IncomingTransfer uint256 _tokenId, uint256 _offerId ) external nonReentrant { - Offer storage offer = offers[_tokenContract][_tokenId][offerCount]; + Offer storage offer = offers[_tokenContract][_tokenId][_offerId]; require(offer.seller == msg.sender, "cancelNFTOffer must be seller"); - // Refund offered amount + // Refund offer _handleOutgoingTransfer(offer.seller, offer.amount, offer.currency, USE_ALL_GAS_FLAG); emit NFTOfferCanceled(_tokenContract, _tokenId, _offerId, offer); - delete offers[_tokenContract][_tokenId][offerCount]; + delete offers[_tokenContract][_tokenId][_offerId]; } /// ------------ BUYER FUNCTIONS ------------ /// @notice Fills the offer for an owned NFT, in exchange for ETH/ERC-20 tokens - /// @param _tokenContract The address of the ERC-721 token to sell - /// @param _tokenId The ID of the ERC-721 token to sell + /// @param _tokenContract The address of the ERC-721 token to transfer + /// @param _tokenId The ID of the ERC-721 token to transfer /// @param _offerId The ID of the offer to fill - /// @param _currency The address of ERC-20 token to accept, or address(0) for ETH - /// @param _amount The offered amount to accept + /// @param _currency The address of ERC-20 token to purchase, or address(0) for ETH + /// @param _amount The amount to purchase /// @param _finder The address of the offer referrer function fillNFTOffer( address _tokenContract, @@ -220,7 +213,7 @@ contract OffersV1 is ReentrancyGuard, UniversalExchangeEventV1, IncomingTransfer uint256 _amount, address _finder ) external nonReentrant { - Offer storage offer = offers[_tokenContract][_tokenId][offerCount]; + Offer storage offer = offers[_tokenContract][_tokenId][_offerId]; require(offer.seller != address(0), "fillNFTOffer must be active offer"); require(IERC721(_tokenContract).ownerOf(_tokenId) == msg.sender, "fillNFTOffer must be token owner"); @@ -248,12 +241,11 @@ contract OffersV1 is ReentrancyGuard, UniversalExchangeEventV1, IncomingTransfer erc721TransferHelper.transferFrom(_tokenContract, msg.sender, offer.seller, _tokenId); ExchangeDetails memory userAExchangeDetails = ExchangeDetails({tokenContract: offer.currency, tokenId: 0, amount: offer.amount}); - ExchangeDetails memory userBExchangeDetails = ExchangeDetails({tokenContract: _tokenContract, tokenId: _tokenId, amount: 1}); emit ExchangeExecuted(offer.seller, msg.sender, userAExchangeDetails, userBExchangeDetails); emit NFTOfferFilled(_tokenContract, _tokenId, _offerId, msg.sender, _finder, offer); - delete offers[_tokenContract][_tokenId][offerCount]; + delete offers[_tokenContract][_tokenId][_offerId]; } } From 213968a46e1d85367b6484fd77c50856f4e594a1 Mon Sep 17 00:00:00 2001 From: Rohan Kulkarni Date: Thu, 27 Jan 2022 21:53:55 -0500 Subject: [PATCH 03/16] [feat] support for updating offers with different currencies --- contracts/modules/Offers/V1/OffersV1.sol | 114 +++++++------ contracts/test/ZoraModuleManager.t.sol | 4 +- contracts/test/modules/Offers/V1/Offers.t.sol | 161 +++++++++++------- 3 files changed, 168 insertions(+), 111 deletions(-) diff --git a/contracts/modules/Offers/V1/OffersV1.sol b/contracts/modules/Offers/V1/OffersV1.sol index 4a888513..d30f051b 100644 --- a/contracts/modules/Offers/V1/OffersV1.sol +++ b/contracts/modules/Offers/V1/OffersV1.sol @@ -14,24 +14,24 @@ import {ModuleNamingSupportV1} from "../../../common/ModuleNamingSupport/ModuleN /// @title Offers V1 /// @author kulkarohan -/// @notice This module allows users to place ETH/ERC-20 offers for any ERC-721 token +/// @notice This module allows users to make ETH/ERC-20 offers for any ERC-721 token contract OffersV1 is ReentrancyGuard, UniversalExchangeEventV1, IncomingTransferSupportV1, FeePayoutSupportV1, ModuleNamingSupportV1 { /// @dev The indicator to pass all remaining gas when paying out royalties uint256 private constant USE_ALL_GAS_FLAG = 0; - /// @notice The number of offers placed + /// @notice The total number of offers made uint256 public offerCount; /// @notice The ZORA ERC-721 Transfer Helper ERC721TransferHelper public immutable erc721TransferHelper; /// @notice The metadata of an offer - /// @param seller The address of the seller placing the offer - /// @param currency The address of the ERC-20 selling, or address(0) for ETH + /// @param maker The address of the user that made the offer + /// @param currency The address of the ERC-20 offered, or address(0) for ETH /// @param findersFeeBps The fee to the referrer of the offer - /// @param amount The amount selling + /// @param amount The amount of ETH/ERC-20 tokens offered struct Offer { - address seller; + address maker; address currency; uint16 findersFeeBps; uint256 amount; @@ -61,7 +61,7 @@ contract OffersV1 is ReentrancyGuard, UniversalExchangeEventV1, IncomingTransfer /// @param tokenId The ERC-721 token ID of the updated offer /// @param id The ID of the updated offer /// @param offer The metadata of the updated offer - event NFTOfferAmountUpdated(address indexed tokenContract, uint256 indexed tokenId, uint256 indexed id, Offer offer); + event NFTOfferUpdated(address indexed tokenContract, uint256 indexed tokenId, uint256 indexed id, Offer offer); /// @notice Emitted when an offer is canceled /// @param tokenContract The ERC-721 token address of the canceled offer @@ -100,12 +100,12 @@ contract OffersV1 is ReentrancyGuard, UniversalExchangeEventV1, IncomingTransfer erc721TransferHelper = ERC721TransferHelper(_erc721TransferHelper); } - /// ------------ SELLER FUNCTIONS ------------ + /// ------------ MAKER FUNCTIONS ------------ /// @notice Creates an offer for an NFT /// @param _tokenContract The address of the desired ERC-721 token /// @param _tokenId The ID of the desired ERC-721 token - /// @param _currency The address of the offering ERC-20 token, or address(0) for ETH + /// @param _currency The address of the ERC-20 token offering, or address(0) for ETH /// @param _amount The amount offering /// @param _findersFeeBps The bps of the amount (post-royalties) to send to a referrer of the sale /// @return The ID of the created offer @@ -116,7 +116,6 @@ contract OffersV1 is ReentrancyGuard, UniversalExchangeEventV1, IncomingTransfer uint256 _amount, uint16 _findersFeeBps ) external payable nonReentrant returns (uint256) { - require(IERC721(_tokenContract).ownerOf(_tokenId) != msg.sender, "createNFTOffer cannot place offer on own NFT"); require(_findersFeeBps <= 10000, "createNFTOffer finders fee bps must be less than or equal to 10000"); // Validate offer and take custody @@ -125,7 +124,7 @@ contract OffersV1 is ReentrancyGuard, UniversalExchangeEventV1, IncomingTransfer offerCount++; offers[_tokenContract][_tokenId][offerCount] = Offer({ - seller: msg.sender, + maker: msg.sender, currency: _currency, findersFeeBps: _findersFeeBps, amount: _amount @@ -138,44 +137,64 @@ contract OffersV1 is ReentrancyGuard, UniversalExchangeEventV1, IncomingTransfer return offerCount; } - /// @notice Updates the amount of an offer + /// @notice Updates the given offer for an NFT /// @param _tokenContract The address of the offer ERC-721 token /// @param _tokenId The ID of the offer ERC-721 token /// @param _offerId The ID of the offer - /// @param _amount The new offer amount - function setNFTOfferAmount( + /// @param _currency The address of the ERC-20 token offering, or address(0) for ETH + /// @param _amount The new amount offering + function setNFTOffer( address _tokenContract, uint256 _tokenId, uint256 _offerId, + address _currency, uint256 _amount ) external payable nonReentrant { Offer storage offer = offers[_tokenContract][_tokenId][_offerId]; - require(offer.seller == msg.sender, "setNFTOfferAmount must be seller"); - require(_amount != 0 && _amount != offer.amount, "setNFTOfferAmount _amount cannot be 0 or previous amount"); - - uint256 prevAmount = offer.amount; - - if (_amount > prevAmount) { - uint256 increaseAmount = _amount - prevAmount; - - // Validate offer increase and take custody - _handleIncomingTransfer(increaseAmount, offer.currency); - - offer.amount += increaseAmount; - } else if (_amount < prevAmount) { - uint256 decreaseAmount = prevAmount - _amount; - - // Refund offer difference - _handleOutgoingTransfer(offer.seller, decreaseAmount, offer.currency, USE_ALL_GAS_FLAG); - - offer.amount -= decreaseAmount; + require(offer.maker == msg.sender, "setNFTOffer must be maker"); + + // If same currency -- + if (_currency == offer.currency) { + // Get initial amount + uint256 prevAmount = offer.amount; + // Ensure valid update + require(_amount > 0 && _amount != prevAmount, "setNFTOffer invalid _amount"); + + // If offer increase -- + if (_amount > prevAmount) { + // Get delta + uint256 increaseAmount = _amount - prevAmount; + // Custody increase + _handleIncomingTransfer(increaseAmount, offer.currency); + // Update storage + offer.amount += increaseAmount; + + // Else offer decrease -- + } else { + // Get delta + uint256 decreaseAmount = prevAmount - _amount; + // Refund difference + _handleOutgoingTransfer(offer.maker, decreaseAmount, offer.currency, USE_ALL_GAS_FLAG); + // Update storage + offer.amount -= decreaseAmount; + } + // Else other currency -- + } else { + // Refund previous + _handleOutgoingTransfer(offer.maker, offer.amount, offer.currency, USE_ALL_GAS_FLAG); + // Custody new + _handleIncomingTransfer(_amount, _currency); + + // Update storage + offer.currency = _currency; + offer.amount = _amount; } - emit NFTOfferAmountUpdated(_tokenContract, _tokenId, _offerId, offer); + emit NFTOfferUpdated(_tokenContract, _tokenId, _offerId, offer); } - /// @notice Cancels and refunds the offer for an NFT + /// @notice Cancels and refunds the given offer for an NFT /// @param _tokenContract The ERC-721 token address of the offer /// @param _tokenId The ERC-721 token ID of the offer /// @param _offerId The ID of the offer @@ -186,24 +205,24 @@ contract OffersV1 is ReentrancyGuard, UniversalExchangeEventV1, IncomingTransfer ) external nonReentrant { Offer storage offer = offers[_tokenContract][_tokenId][_offerId]; - require(offer.seller == msg.sender, "cancelNFTOffer must be seller"); + require(offer.maker == msg.sender, "cancelNFTOffer must be maker"); // Refund offer - _handleOutgoingTransfer(offer.seller, offer.amount, offer.currency, USE_ALL_GAS_FLAG); + _handleOutgoingTransfer(offer.maker, offer.amount, offer.currency, USE_ALL_GAS_FLAG); emit NFTOfferCanceled(_tokenContract, _tokenId, _offerId, offer); delete offers[_tokenContract][_tokenId][_offerId]; } - /// ------------ BUYER FUNCTIONS ------------ + /// ------------ TAKER FUNCTIONS ------------ - /// @notice Fills the offer for an owned NFT, in exchange for ETH/ERC-20 tokens + /// @notice Fills a given offer for an owned NFT, in exchange for ETH/ERC-20 tokens /// @param _tokenContract The address of the ERC-721 token to transfer /// @param _tokenId The ID of the ERC-721 token to transfer /// @param _offerId The ID of the offer to fill - /// @param _currency The address of ERC-20 token to purchase, or address(0) for ETH - /// @param _amount The amount to purchase + /// @param _currency The address of the ERC-20 to take, or address(0) for ETH + /// @param _amount The amount to take /// @param _finder The address of the offer referrer function fillNFTOffer( address _tokenContract, @@ -215,10 +234,9 @@ contract OffersV1 is ReentrancyGuard, UniversalExchangeEventV1, IncomingTransfer ) external nonReentrant { Offer storage offer = offers[_tokenContract][_tokenId][_offerId]; - require(offer.seller != address(0), "fillNFTOffer must be active offer"); + require(offer.maker != address(0), "fillNFTOffer must be active offer"); require(IERC721(_tokenContract).ownerOf(_tokenId) == msg.sender, "fillNFTOffer must be token owner"); - require(offer.currency == _currency, "fillNFTOffer _currency must match offer currency"); - require(offer.amount == _amount, "fillNFTOffer _amount must match offer amount"); + require(offer.currency == _currency && offer.amount == _amount, "fillNFTOffer _currency & _amount must match offer"); // Payout respective parties, ensuring royalties are honored (uint256 remainingProfit, ) = _handleRoyaltyPayout(_tokenContract, _tokenId, offer.amount, offer.currency, USE_ALL_GAS_FLAG); @@ -234,16 +252,16 @@ contract OffersV1 is ReentrancyGuard, UniversalExchangeEventV1, IncomingTransfer remainingProfit -= findersFee; } - // Transfer remaining ETH/ERC-20 to seller + // Transfer remaining ETH/ERC-20 tokens to offer taker _handleOutgoingTransfer(msg.sender, remainingProfit, offer.currency, USE_ALL_GAS_FLAG); - // Transfer NFT to buyer - erc721TransferHelper.transferFrom(_tokenContract, msg.sender, offer.seller, _tokenId); + // Transfer NFT to offer maker + erc721TransferHelper.transferFrom(_tokenContract, msg.sender, offer.maker, _tokenId); ExchangeDetails memory userAExchangeDetails = ExchangeDetails({tokenContract: offer.currency, tokenId: 0, amount: offer.amount}); ExchangeDetails memory userBExchangeDetails = ExchangeDetails({tokenContract: _tokenContract, tokenId: _tokenId, amount: 1}); - emit ExchangeExecuted(offer.seller, msg.sender, userAExchangeDetails, userBExchangeDetails); + emit ExchangeExecuted(offer.maker, msg.sender, userAExchangeDetails, userBExchangeDetails); emit NFTOfferFilled(_tokenContract, _tokenId, _offerId, msg.sender, _finder, offer); delete offers[_tokenContract][_tokenId][_offerId]; diff --git a/contracts/test/ZoraModuleManager.t.sol b/contracts/test/ZoraModuleManager.t.sol index bdff4ceb..2a27d07b 100644 --- a/contracts/test/ZoraModuleManager.t.sol +++ b/contracts/test/ZoraModuleManager.t.sol @@ -48,7 +48,7 @@ contract ZoraModuleManagerTest is DSTest { module = batchModules[0]; } - /// ------------ APPROVE MODULE ------------ + /// ------------ APPROVE MODULE ------------ /// function test_SetApproval() public { registrar.registerModule(module); @@ -62,7 +62,7 @@ contract ZoraModuleManagerTest is DSTest { bob.setApprovalForModule(module, true); } - /// ------------ APPROVE MODULE BATCH ------------ + /// ------------ APPROVE MODULE BATCH ------------ /// function test_SetBatchApproval() public { for (uint256 i = 0; i < 3; i++) { diff --git a/contracts/test/modules/Offers/V1/Offers.t.sol b/contracts/test/modules/Offers/V1/Offers.t.sol index 6ccc8ca0..fcd2fa18 100644 --- a/contracts/test/modules/Offers/V1/Offers.t.sol +++ b/contracts/test/modules/Offers/V1/Offers.t.sol @@ -32,8 +32,8 @@ contract OffersV1Test is DSTest { TestERC721 internal token; WETH internal weth; - Zorb internal seller; - Zorb internal buyer; + Zorb internal maker; + Zorb internal taker; Zorb internal finder; Zorb internal royaltyRecipient; @@ -53,8 +53,8 @@ contract OffersV1Test is DSTest { ZPFS.init(address(ZMM), address(0)); // Create users - seller = new Zorb(address(ZMM)); - buyer = new Zorb(address(ZMM)); + maker = new Zorb(address(ZMM)); + taker = new Zorb(address(ZMM)); finder = new Zorb(address(ZMM)); royaltyRecipient = new Zorb(address(ZMM)); @@ -67,26 +67,26 @@ contract OffersV1Test is DSTest { offers = new OffersV1(address(erc20TransferHelper), address(erc721TransferHelper), address(royaltyEngine), address(ZPFS), address(weth)); registrar.registerModule(address(offers)); - // Set seller balance - vm.deal(address(seller), 100 ether); + // Set maker balance + vm.deal(address(maker), 100 ether); - // Mint buyer token - token.mint(address(buyer), 0); + // Mint taker token + token.mint(address(taker), 0); - // Seller swap 50 ETH <> 50 WETH - vm.prank(address(seller)); + // Maker swap 50 ETH <> 50 WETH + vm.prank(address(maker)); weth.deposit{value: 50 ether}(); // Users approve Offers module - seller.setApprovalForModule(address(offers), true); - buyer.setApprovalForModule(address(offers), true); + maker.setApprovalForModule(address(offers), true); + taker.setApprovalForModule(address(offers), true); - // Seller approve ERC20TransferHelper - vm.prank(address(seller)); + // Maker approve ERC20TransferHelper + vm.prank(address(maker)); weth.approve(address(erc20TransferHelper), 50 ether); - // Buyer approve ERC721TransferHelper - vm.prank(address(buyer)); + // Taker approve ERC721TransferHelper + vm.prank(address(taker)); token.setApprovalForAll(address(erc721TransferHelper), true); } @@ -97,86 +97,125 @@ contract OffersV1Test is DSTest { } function test_CreateOffer() public { - vm.prank(address(seller)); + vm.prank(address(maker)); offers.createNFTOffer{value: 1 ether}(address(token), 0, address(0), 1 ether, 1000); (address offeror, , , ) = offers.offers(address(token), 0, 1); - require(offeror == address(seller)); - } - - function testFail_CannotCreateOfferOnOwnNFT() public { - vm.prank(address(buyer)); - offers.createNFTOffer{value: 1 ether}(address(token), 0, address(0), 1 ether, 1000); + require(offeror == address(maker)); } function testFail_CannotCreateOfferWithoutAttachingFunds() public { - vm.prank(address(seller)); + vm.prank(address(maker)); offers.createNFTOffer(address(token), 0, address(0), 1 ether, 1000); } - /// ------------ UPDATE NFT OFFER ------------ /// + /// ------------ SET NFT OFFER ------------ /// - function test_IncreaseOffer() public { - vm.startPrank(address(seller)); + function test_IncreaseETHOffer() public { + vm.startPrank(address(maker)); offers.createNFTOffer{value: 1 ether}(address(token), 0, address(0), 1 ether, 1000); - offers.setNFTOfferAmount{value: 1 ether}(address(token), 0, 1, 2 ether); + + vm.warp(1 hours); + + offers.setNFTOffer{value: 1 ether}(address(token), 0, 1, address(0), 2 ether); vm.stopPrank(); (, , , uint256 amount) = offers.offers(address(token), 0, 1); require(amount == 2 ether); + require(address(offers).balance == 2 ether); } - function test_DecreaseOffer() public { - vm.startPrank(address(seller)); + function test_DecreaseETHOffer() public { + vm.startPrank(address(maker)); offers.createNFTOffer{value: 1 ether}(address(token), 0, address(0), 1 ether, 1000); - offers.setNFTOfferAmount(address(token), 0, 1, 0.5 ether); + + vm.warp(1 hours); + + offers.setNFTOffer(address(token), 0, 1, address(0), 0.5 ether); + + vm.stopPrank(); + + (, , , uint256 amount) = offers.offers(address(token), 0, 1); + + require(amount == 0.5 ether); + require(address(offers).balance == 0.5 ether); + } + + function test_IncreaseETHOfferWithERC20() public { + vm.startPrank(address(maker)); + + offers.createNFTOffer{value: 1 ether}(address(token), 0, address(0), 1 ether, 1000); + + vm.warp(1 hours); + + offers.setNFTOffer(address(token), 0, 1, address(weth), 2 ether); + + vm.stopPrank(); + + (, , , uint256 amount) = offers.offers(address(token), 0, 1); + + require(amount == 2 ether); + require(weth.balanceOf(address(offers)) == 2 ether); + require(address(offers).balance == 0 ether); + } + + function test_DecreaseETHOfferWithERC20() public { + vm.startPrank(address(maker)); + + offers.createNFTOffer{value: 1 ether}(address(token), 0, address(0), 1 ether, 1000); + + vm.warp(1 hours); + + offers.setNFTOffer(address(token), 0, 1, address(weth), 0.5 ether); vm.stopPrank(); (, , , uint256 amount) = offers.offers(address(token), 0, 1); require(amount == 0.5 ether); + require(weth.balanceOf(address(offers)) == 0.5 ether); + require(address(offers).balance == 0 ether); } function testRevert_OnlySellerCanUpdateOffer() public { - vm.prank(address(seller)); + vm.prank(address(maker)); offers.createNFTOffer{value: 1 ether}(address(token), 0, address(0), 1 ether, 1000); - vm.expectRevert("setNFTOfferAmount must be seller"); - offers.setNFTOfferAmount(address(token), 0, 1, 0.5 ether); + vm.expectRevert("setNFTOffer must be maker"); + offers.setNFTOffer(address(token), 0, 1, address(0), 0.5 ether); } function testRevert_CannotIncreaseOfferWithoutAttachingFunds() public { - vm.startPrank(address(seller)); + vm.startPrank(address(maker)); offers.createNFTOffer{value: 1 ether}(address(token), 0, address(0), 1 ether, 1000); vm.expectRevert("_handleIncomingTransfer msg value less than expected amount"); - offers.setNFTOfferAmount(address(token), 0, 1, 2 ether); + offers.setNFTOffer(address(token), 0, 1, address(0), 2 ether); vm.stopPrank(); } function testRevert_CannotUpdateInactiveOffer() public { - vm.prank(address(seller)); + vm.prank(address(maker)); offers.createNFTOffer{value: 1 ether}(address(token), 0, address(0), 1 ether, 1000); - vm.prank(address(buyer)); + vm.prank(address(taker)); offers.fillNFTOffer(address(token), 0, 1, address(0), 1 ether, address(finder)); - vm.prank(address(seller)); - vm.expectRevert("setNFTOfferAmount must be seller"); - offers.setNFTOfferAmount(address(token), 0, 1, 0.5 ether); + vm.prank(address(maker)); + vm.expectRevert("setNFTOffer must be maker"); + offers.setNFTOffer(address(token), 0, 1, address(0), 0.5 ether); } /// ------------ CANCEL NFT OFFER ------------ /// function test_CancelNFTOffer() public { - vm.startPrank(address(seller)); + vm.startPrank(address(maker)); offers.createNFTOffer{value: 1 ether}(address(token), 0, address(0), 1 ether, 1000); @@ -192,43 +231,43 @@ contract OffersV1Test is DSTest { } function testRevert_CannotCancelInactiveOffer() public { - vm.prank(address(seller)); + vm.prank(address(maker)); offers.createNFTOffer{value: 1 ether}(address(token), 0, address(0), 1 ether, 1000); - vm.prank(address(buyer)); + vm.prank(address(taker)); offers.fillNFTOffer(address(token), 0, 1, address(0), 1 ether, address(finder)); - vm.prank(address(seller)); - vm.expectRevert("cancelNFTOffer must be seller"); + vm.prank(address(maker)); + vm.expectRevert("cancelNFTOffer must be maker"); offers.cancelNFTOffer(address(token), 0, 1); } function testRevert_OnlySellerCanCancelOffer() public { - vm.prank(address(seller)); + vm.prank(address(maker)); offers.createNFTOffer{value: 1 ether}(address(token), 0, address(0), 1 ether, 1000); - vm.expectRevert("cancelNFTOffer must be seller"); + vm.expectRevert("cancelNFTOffer must be maker"); offers.cancelNFTOffer(address(token), 0, 1); } /// ------------ FILL NFT OFFER ------------ /// function test_FillNFTOffer() public { - vm.prank(address(seller)); + vm.prank(address(maker)); offers.createNFTOffer{value: 1 ether}(address(token), 0, address(0), 1 ether, 1000); address beforeTokenOwner = token.ownerOf(0); - vm.prank(address(buyer)); + vm.prank(address(taker)); offers.fillNFTOffer(address(token), 0, 1, address(0), 1 ether, address(finder)); address afterTokenOwner = token.ownerOf(0); - require(beforeTokenOwner == address(buyer) && afterTokenOwner == address(seller)); + require(beforeTokenOwner == address(taker) && afterTokenOwner == address(maker)); } function testRevert_OnlyTokenHolderCanFillOffer() public { - vm.prank(address(seller)); + vm.prank(address(maker)); uint256 id = offers.createNFTOffer{value: 1 ether}(address(token), 0, address(0), 1 ether, 1000); vm.expectRevert("fillNFTOffer must be token owner"); @@ -236,32 +275,32 @@ contract OffersV1Test is DSTest { } function testRevert_CannotFillInactiveOffer() public { - vm.prank(address(seller)); + vm.prank(address(maker)); uint256 id = offers.createNFTOffer{value: 1 ether}(address(token), 0, address(0), 1 ether, 1000); - vm.prank(address(buyer)); + vm.prank(address(taker)); offers.fillNFTOffer(address(token), 0, id, address(0), 1 ether, address(finder)); - vm.prank(address(buyer)); + vm.prank(address(taker)); vm.expectRevert("fillNFTOffer must be active offer"); offers.fillNFTOffer(address(token), 0, id, address(0), 1 ether, address(finder)); } function testRevert_AcceptCurrencyMustMatchOffer() public { - vm.prank(address(seller)); + vm.prank(address(maker)); uint256 id = offers.createNFTOffer{value: 1 ether}(address(token), 0, address(0), 1 ether, 1000); - vm.prank(address(buyer)); - vm.expectRevert("fillNFTOffer _currency must match offer currency"); + vm.prank(address(taker)); + vm.expectRevert("fillNFTOffer _currency & _amount must match offer"); offers.fillNFTOffer(address(token), 0, id, address(weth), 1 ether, address(finder)); } function testRevert_AcceptAmountMustMatchOffer() public { - vm.prank(address(seller)); + vm.prank(address(maker)); uint256 id = offers.createNFTOffer{value: 1 ether}(address(token), 0, address(0), 1 ether, 1000); - vm.prank(address(buyer)); - vm.expectRevert("fillNFTOffer _amount must match offer amount"); + vm.prank(address(taker)); + vm.expectRevert("fillNFTOffer _currency & _amount must match offer"); offers.fillNFTOffer(address(token), 0, id, address(0), 0.5 ether, address(finder)); } } From af68c40d5f60b13c9f5687cf9e793d28d3252c12 Mon Sep 17 00:00:00 2001 From: Rohan Kulkarni Date: Thu, 27 Jan 2022 22:14:30 -0500 Subject: [PATCH 04/16] [feat] nft offers --> offers --- contracts/modules/Offers/V1/OffersV1.sol | 38 ++++---- .../Offers/V1/Offers.integration.t.sol | 8 +- contracts/test/modules/Offers/V1/Offers.t.sol | 88 +++++++++---------- 3 files changed, 67 insertions(+), 67 deletions(-) diff --git a/contracts/modules/Offers/V1/OffersV1.sol b/contracts/modules/Offers/V1/OffersV1.sol index d30f051b..ebb41185 100644 --- a/contracts/modules/Offers/V1/OffersV1.sol +++ b/contracts/modules/Offers/V1/OffersV1.sol @@ -54,21 +54,21 @@ contract OffersV1 is ReentrancyGuard, UniversalExchangeEventV1, IncomingTransfer /// @param tokenId The ERC-721 token ID of the created offer /// @param id The ID of the created offer /// @param offer The metadata of the created offer - event NFTOfferCreated(address indexed tokenContract, uint256 indexed tokenId, uint256 indexed id, Offer offer); + event OfferCreated(address indexed tokenContract, uint256 indexed tokenId, uint256 indexed id, Offer offer); /// @notice Emitted when an offer is updated /// @param tokenContract The ERC-721 token address of the updated offer /// @param tokenId The ERC-721 token ID of the updated offer /// @param id The ID of the updated offer /// @param offer The metadata of the updated offer - event NFTOfferUpdated(address indexed tokenContract, uint256 indexed tokenId, uint256 indexed id, Offer offer); + event OfferUpdated(address indexed tokenContract, uint256 indexed tokenId, uint256 indexed id, Offer offer); /// @notice Emitted when an offer is canceled /// @param tokenContract The ERC-721 token address of the canceled offer /// @param tokenId The ERC-721 token ID of the canceled offer /// @param id The ID of the canceled offer /// @param offer The metadata of the canceled offer - event NFTOfferCanceled(address indexed tokenContract, uint256 indexed tokenId, uint256 indexed id, Offer offer); + event OfferCanceled(address indexed tokenContract, uint256 indexed tokenId, uint256 indexed id, Offer offer); /// @notice Emitted when an offer is filled /// @param tokenContract The ERC-721 token address of the filled offer @@ -77,7 +77,7 @@ contract OffersV1 is ReentrancyGuard, UniversalExchangeEventV1, IncomingTransfer /// @param buyer The address of the buyer who filled the offer /// @param finder The address of the finder who referred the offer /// @param offer The metadata of the filled offer - event NFTOfferFilled(address indexed tokenContract, uint256 indexed tokenId, uint256 indexed id, address buyer, address finder, Offer offer); + event OfferFilled(address indexed tokenContract, uint256 indexed tokenId, uint256 indexed id, address buyer, address finder, Offer offer); /// ------------ CONSTRUCTOR ------------ @@ -109,14 +109,14 @@ contract OffersV1 is ReentrancyGuard, UniversalExchangeEventV1, IncomingTransfer /// @param _amount The amount offering /// @param _findersFeeBps The bps of the amount (post-royalties) to send to a referrer of the sale /// @return The ID of the created offer - function createNFTOffer( + function createOffer( address _tokenContract, uint256 _tokenId, address _currency, uint256 _amount, uint16 _findersFeeBps ) external payable nonReentrant returns (uint256) { - require(_findersFeeBps <= 10000, "createNFTOffer finders fee bps must be less than or equal to 10000"); + require(_findersFeeBps <= 10000, "createOffer finders fee bps must be less than or equal to 10000"); // Validate offer and take custody _handleIncomingTransfer(_amount, _currency); @@ -132,7 +132,7 @@ contract OffersV1 is ReentrancyGuard, UniversalExchangeEventV1, IncomingTransfer offersForNFT[_tokenContract][_tokenId].push(offerCount); - emit NFTOfferCreated(_tokenContract, _tokenId, offerCount, offers[_tokenContract][_tokenId][offerCount]); + emit OfferCreated(_tokenContract, _tokenId, offerCount, offers[_tokenContract][_tokenId][offerCount]); return offerCount; } @@ -143,7 +143,7 @@ contract OffersV1 is ReentrancyGuard, UniversalExchangeEventV1, IncomingTransfer /// @param _offerId The ID of the offer /// @param _currency The address of the ERC-20 token offering, or address(0) for ETH /// @param _amount The new amount offering - function setNFTOffer( + function setOffer( address _tokenContract, uint256 _tokenId, uint256 _offerId, @@ -152,14 +152,14 @@ contract OffersV1 is ReentrancyGuard, UniversalExchangeEventV1, IncomingTransfer ) external payable nonReentrant { Offer storage offer = offers[_tokenContract][_tokenId][_offerId]; - require(offer.maker == msg.sender, "setNFTOffer must be maker"); + require(offer.maker == msg.sender, "setOffer must be maker"); // If same currency -- if (_currency == offer.currency) { // Get initial amount uint256 prevAmount = offer.amount; // Ensure valid update - require(_amount > 0 && _amount != prevAmount, "setNFTOffer invalid _amount"); + require(_amount > 0 && _amount != prevAmount, "setOffer invalid _amount"); // If offer increase -- if (_amount > prevAmount) { @@ -191,26 +191,26 @@ contract OffersV1 is ReentrancyGuard, UniversalExchangeEventV1, IncomingTransfer offer.amount = _amount; } - emit NFTOfferUpdated(_tokenContract, _tokenId, _offerId, offer); + emit OfferUpdated(_tokenContract, _tokenId, _offerId, offer); } /// @notice Cancels and refunds the given offer for an NFT /// @param _tokenContract The ERC-721 token address of the offer /// @param _tokenId The ERC-721 token ID of the offer /// @param _offerId The ID of the offer - function cancelNFTOffer( + function cancelOffer( address _tokenContract, uint256 _tokenId, uint256 _offerId ) external nonReentrant { Offer storage offer = offers[_tokenContract][_tokenId][_offerId]; - require(offer.maker == msg.sender, "cancelNFTOffer must be maker"); + require(offer.maker == msg.sender, "cancelOffer must be maker"); // Refund offer _handleOutgoingTransfer(offer.maker, offer.amount, offer.currency, USE_ALL_GAS_FLAG); - emit NFTOfferCanceled(_tokenContract, _tokenId, _offerId, offer); + emit OfferCanceled(_tokenContract, _tokenId, _offerId, offer); delete offers[_tokenContract][_tokenId][_offerId]; } @@ -224,7 +224,7 @@ contract OffersV1 is ReentrancyGuard, UniversalExchangeEventV1, IncomingTransfer /// @param _currency The address of the ERC-20 to take, or address(0) for ETH /// @param _amount The amount to take /// @param _finder The address of the offer referrer - function fillNFTOffer( + function fillOffer( address _tokenContract, uint256 _tokenId, uint256 _offerId, @@ -234,9 +234,9 @@ contract OffersV1 is ReentrancyGuard, UniversalExchangeEventV1, IncomingTransfer ) external nonReentrant { Offer storage offer = offers[_tokenContract][_tokenId][_offerId]; - require(offer.maker != address(0), "fillNFTOffer must be active offer"); - require(IERC721(_tokenContract).ownerOf(_tokenId) == msg.sender, "fillNFTOffer must be token owner"); - require(offer.currency == _currency && offer.amount == _amount, "fillNFTOffer _currency & _amount must match offer"); + require(offer.maker != address(0), "fillOffer must be active offer"); + require(IERC721(_tokenContract).ownerOf(_tokenId) == msg.sender, "fillOffer must be token owner"); + require(offer.currency == _currency && offer.amount == _amount, "fillOffer _currency & _amount must match offer"); // Payout respective parties, ensuring royalties are honored (uint256 remainingProfit, ) = _handleRoyaltyPayout(_tokenContract, _tokenId, offer.amount, offer.currency, USE_ALL_GAS_FLAG); @@ -262,7 +262,7 @@ contract OffersV1 is ReentrancyGuard, UniversalExchangeEventV1, IncomingTransfer ExchangeDetails memory userBExchangeDetails = ExchangeDetails({tokenContract: _tokenContract, tokenId: _tokenId, amount: 1}); emit ExchangeExecuted(offer.maker, msg.sender, userAExchangeDetails, userBExchangeDetails); - emit NFTOfferFilled(_tokenContract, _tokenId, _offerId, msg.sender, _finder, offer); + emit OfferFilled(_tokenContract, _tokenId, _offerId, msg.sender, _finder, offer); delete offers[_tokenContract][_tokenId][_offerId]; } diff --git a/contracts/test/modules/Offers/V1/Offers.integration.t.sol b/contracts/test/modules/Offers/V1/Offers.integration.t.sol index b1ca6400..bc19cb79 100644 --- a/contracts/test/modules/Offers/V1/Offers.integration.t.sol +++ b/contracts/test/modules/Offers/V1/Offers.integration.t.sol @@ -94,10 +94,10 @@ contract OffersV1IntegrationTest is DSTest { function runETH() public { vm.prank(address(seller)); - uint256 id = offers.createNFTOffer{value: 1 ether}(address(token), 0, address(0), 1 ether, 1000); + uint256 id = offers.createOffer{value: 1 ether}(address(token), 0, address(0), 1 ether, 1000); vm.prank(address(buyer)); - offers.fillNFTOffer(address(token), 0, id, address(0), 1 ether, address(finder)); + offers.fillOffer(address(token), 0, id, address(0), 1 ether, address(finder)); } function test_ETHIntegration() public { @@ -131,10 +131,10 @@ contract OffersV1IntegrationTest is DSTest { function runERC20() public { vm.prank(address(seller)); - uint256 id = offers.createNFTOffer(address(token), 0, address(weth), 1 ether, 1000); + uint256 id = offers.createOffer(address(token), 0, address(weth), 1 ether, 1000); vm.prank(address(buyer)); - offers.fillNFTOffer(address(token), 0, id, address(weth), 1 ether, address(finder)); + offers.fillOffer(address(token), 0, id, address(weth), 1 ether, address(finder)); } function test_ERC20Integration() public { diff --git a/contracts/test/modules/Offers/V1/Offers.t.sol b/contracts/test/modules/Offers/V1/Offers.t.sol index fcd2fa18..e680d414 100644 --- a/contracts/test/modules/Offers/V1/Offers.t.sol +++ b/contracts/test/modules/Offers/V1/Offers.t.sol @@ -93,12 +93,12 @@ contract OffersV1Test is DSTest { /// ------------ CREATE NFT OFFER ------------ /// function testGas_CreateOffer() public { - offers.createNFTOffer{value: 1 ether}(address(token), 0, address(0), 1 ether, 1000); + offers.createOffer{value: 1 ether}(address(token), 0, address(0), 1 ether, 1000); } function test_CreateOffer() public { vm.prank(address(maker)); - offers.createNFTOffer{value: 1 ether}(address(token), 0, address(0), 1 ether, 1000); + offers.createOffer{value: 1 ether}(address(token), 0, address(0), 1 ether, 1000); (address offeror, , , ) = offers.offers(address(token), 0, 1); @@ -107,7 +107,7 @@ contract OffersV1Test is DSTest { function testFail_CannotCreateOfferWithoutAttachingFunds() public { vm.prank(address(maker)); - offers.createNFTOffer(address(token), 0, address(0), 1 ether, 1000); + offers.createOffer(address(token), 0, address(0), 1 ether, 1000); } /// ------------ SET NFT OFFER ------------ /// @@ -115,11 +115,11 @@ contract OffersV1Test is DSTest { function test_IncreaseETHOffer() public { vm.startPrank(address(maker)); - offers.createNFTOffer{value: 1 ether}(address(token), 0, address(0), 1 ether, 1000); + offers.createOffer{value: 1 ether}(address(token), 0, address(0), 1 ether, 1000); vm.warp(1 hours); - offers.setNFTOffer{value: 1 ether}(address(token), 0, 1, address(0), 2 ether); + offers.setOffer{value: 1 ether}(address(token), 0, 1, address(0), 2 ether); vm.stopPrank(); @@ -132,11 +132,11 @@ contract OffersV1Test is DSTest { function test_DecreaseETHOffer() public { vm.startPrank(address(maker)); - offers.createNFTOffer{value: 1 ether}(address(token), 0, address(0), 1 ether, 1000); + offers.createOffer{value: 1 ether}(address(token), 0, address(0), 1 ether, 1000); vm.warp(1 hours); - offers.setNFTOffer(address(token), 0, 1, address(0), 0.5 ether); + offers.setOffer(address(token), 0, 1, address(0), 0.5 ether); vm.stopPrank(); @@ -149,11 +149,11 @@ contract OffersV1Test is DSTest { function test_IncreaseETHOfferWithERC20() public { vm.startPrank(address(maker)); - offers.createNFTOffer{value: 1 ether}(address(token), 0, address(0), 1 ether, 1000); + offers.createOffer{value: 1 ether}(address(token), 0, address(0), 1 ether, 1000); vm.warp(1 hours); - offers.setNFTOffer(address(token), 0, 1, address(weth), 2 ether); + offers.setOffer(address(token), 0, 1, address(weth), 2 ether); vm.stopPrank(); @@ -167,11 +167,11 @@ contract OffersV1Test is DSTest { function test_DecreaseETHOfferWithERC20() public { vm.startPrank(address(maker)); - offers.createNFTOffer{value: 1 ether}(address(token), 0, address(0), 1 ether, 1000); + offers.createOffer{value: 1 ether}(address(token), 0, address(0), 1 ether, 1000); vm.warp(1 hours); - offers.setNFTOffer(address(token), 0, 1, address(weth), 0.5 ether); + offers.setOffer(address(token), 0, 1, address(weth), 0.5 ether); vm.stopPrank(); @@ -184,32 +184,32 @@ contract OffersV1Test is DSTest { function testRevert_OnlySellerCanUpdateOffer() public { vm.prank(address(maker)); - offers.createNFTOffer{value: 1 ether}(address(token), 0, address(0), 1 ether, 1000); + offers.createOffer{value: 1 ether}(address(token), 0, address(0), 1 ether, 1000); - vm.expectRevert("setNFTOffer must be maker"); - offers.setNFTOffer(address(token), 0, 1, address(0), 0.5 ether); + vm.expectRevert("setOffer must be maker"); + offers.setOffer(address(token), 0, 1, address(0), 0.5 ether); } function testRevert_CannotIncreaseOfferWithoutAttachingFunds() public { vm.startPrank(address(maker)); - offers.createNFTOffer{value: 1 ether}(address(token), 0, address(0), 1 ether, 1000); + offers.createOffer{value: 1 ether}(address(token), 0, address(0), 1 ether, 1000); vm.expectRevert("_handleIncomingTransfer msg value less than expected amount"); - offers.setNFTOffer(address(token), 0, 1, address(0), 2 ether); + offers.setOffer(address(token), 0, 1, address(0), 2 ether); vm.stopPrank(); } function testRevert_CannotUpdateInactiveOffer() public { vm.prank(address(maker)); - offers.createNFTOffer{value: 1 ether}(address(token), 0, address(0), 1 ether, 1000); + offers.createOffer{value: 1 ether}(address(token), 0, address(0), 1 ether, 1000); vm.prank(address(taker)); - offers.fillNFTOffer(address(token), 0, 1, address(0), 1 ether, address(finder)); + offers.fillOffer(address(token), 0, 1, address(0), 1 ether, address(finder)); vm.prank(address(maker)); - vm.expectRevert("setNFTOffer must be maker"); - offers.setNFTOffer(address(token), 0, 1, address(0), 0.5 ether); + vm.expectRevert("setOffer must be maker"); + offers.setOffer(address(token), 0, 1, address(0), 0.5 ether); } /// ------------ CANCEL NFT OFFER ------------ /// @@ -217,12 +217,12 @@ contract OffersV1Test is DSTest { function test_CancelNFTOffer() public { vm.startPrank(address(maker)); - offers.createNFTOffer{value: 1 ether}(address(token), 0, address(0), 1 ether, 1000); + offers.createOffer{value: 1 ether}(address(token), 0, address(0), 1 ether, 1000); (, , , uint256 beforeAmount) = offers.offers(address(token), 0, 1); require(beforeAmount == 1 ether); - offers.cancelNFTOffer(address(token), 0, 1); + offers.cancelOffer(address(token), 0, 1); (, , , uint256 afterAmount) = offers.offers(address(token), 0, 1); require(afterAmount == 0); @@ -232,34 +232,34 @@ contract OffersV1Test is DSTest { function testRevert_CannotCancelInactiveOffer() public { vm.prank(address(maker)); - offers.createNFTOffer{value: 1 ether}(address(token), 0, address(0), 1 ether, 1000); + offers.createOffer{value: 1 ether}(address(token), 0, address(0), 1 ether, 1000); vm.prank(address(taker)); - offers.fillNFTOffer(address(token), 0, 1, address(0), 1 ether, address(finder)); + offers.fillOffer(address(token), 0, 1, address(0), 1 ether, address(finder)); vm.prank(address(maker)); - vm.expectRevert("cancelNFTOffer must be maker"); - offers.cancelNFTOffer(address(token), 0, 1); + vm.expectRevert("cancelOffer must be maker"); + offers.cancelOffer(address(token), 0, 1); } function testRevert_OnlySellerCanCancelOffer() public { vm.prank(address(maker)); - offers.createNFTOffer{value: 1 ether}(address(token), 0, address(0), 1 ether, 1000); + offers.createOffer{value: 1 ether}(address(token), 0, address(0), 1 ether, 1000); - vm.expectRevert("cancelNFTOffer must be maker"); - offers.cancelNFTOffer(address(token), 0, 1); + vm.expectRevert("cancelOffer must be maker"); + offers.cancelOffer(address(token), 0, 1); } /// ------------ FILL NFT OFFER ------------ /// function test_FillNFTOffer() public { vm.prank(address(maker)); - offers.createNFTOffer{value: 1 ether}(address(token), 0, address(0), 1 ether, 1000); + offers.createOffer{value: 1 ether}(address(token), 0, address(0), 1 ether, 1000); address beforeTokenOwner = token.ownerOf(0); vm.prank(address(taker)); - offers.fillNFTOffer(address(token), 0, 1, address(0), 1 ether, address(finder)); + offers.fillOffer(address(token), 0, 1, address(0), 1 ether, address(finder)); address afterTokenOwner = token.ownerOf(0); @@ -268,39 +268,39 @@ contract OffersV1Test is DSTest { function testRevert_OnlyTokenHolderCanFillOffer() public { vm.prank(address(maker)); - uint256 id = offers.createNFTOffer{value: 1 ether}(address(token), 0, address(0), 1 ether, 1000); + uint256 id = offers.createOffer{value: 1 ether}(address(token), 0, address(0), 1 ether, 1000); - vm.expectRevert("fillNFTOffer must be token owner"); - offers.fillNFTOffer(address(token), 0, id, address(0), 1 ether, address(finder)); + vm.expectRevert("fillOffer must be token owner"); + offers.fillOffer(address(token), 0, id, address(0), 1 ether, address(finder)); } function testRevert_CannotFillInactiveOffer() public { vm.prank(address(maker)); - uint256 id = offers.createNFTOffer{value: 1 ether}(address(token), 0, address(0), 1 ether, 1000); + uint256 id = offers.createOffer{value: 1 ether}(address(token), 0, address(0), 1 ether, 1000); vm.prank(address(taker)); - offers.fillNFTOffer(address(token), 0, id, address(0), 1 ether, address(finder)); + offers.fillOffer(address(token), 0, id, address(0), 1 ether, address(finder)); vm.prank(address(taker)); - vm.expectRevert("fillNFTOffer must be active offer"); - offers.fillNFTOffer(address(token), 0, id, address(0), 1 ether, address(finder)); + vm.expectRevert("fillOffer must be active offer"); + offers.fillOffer(address(token), 0, id, address(0), 1 ether, address(finder)); } function testRevert_AcceptCurrencyMustMatchOffer() public { vm.prank(address(maker)); - uint256 id = offers.createNFTOffer{value: 1 ether}(address(token), 0, address(0), 1 ether, 1000); + uint256 id = offers.createOffer{value: 1 ether}(address(token), 0, address(0), 1 ether, 1000); vm.prank(address(taker)); - vm.expectRevert("fillNFTOffer _currency & _amount must match offer"); - offers.fillNFTOffer(address(token), 0, id, address(weth), 1 ether, address(finder)); + vm.expectRevert("fillOffer _currency & _amount must match offer"); + offers.fillOffer(address(token), 0, id, address(weth), 1 ether, address(finder)); } function testRevert_AcceptAmountMustMatchOffer() public { vm.prank(address(maker)); - uint256 id = offers.createNFTOffer{value: 1 ether}(address(token), 0, address(0), 1 ether, 1000); + uint256 id = offers.createOffer{value: 1 ether}(address(token), 0, address(0), 1 ether, 1000); vm.prank(address(taker)); - vm.expectRevert("fillNFTOffer _currency & _amount must match offer"); - offers.fillNFTOffer(address(token), 0, id, address(0), 0.5 ether, address(finder)); + vm.expectRevert("fillOffer _currency & _amount must match offer"); + offers.fillOffer(address(token), 0, id, address(0), 0.5 ether, address(finder)); } } From 8606e5290f28f44f907cde7ca53886a1a41ab4a3 Mon Sep 17 00:00:00 2001 From: Rohan Kulkarni Date: Fri, 28 Jan 2022 00:47:53 -0500 Subject: [PATCH 05/16] [feat] setOffer() --> setOfferAmount() --- contracts/modules/Offers/V1/OffersV1.sol | 6 +++--- contracts/test/modules/Offers/V1/Offers.t.sol | 18 +++++++++--------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/contracts/modules/Offers/V1/OffersV1.sol b/contracts/modules/Offers/V1/OffersV1.sol index ebb41185..675459ec 100644 --- a/contracts/modules/Offers/V1/OffersV1.sol +++ b/contracts/modules/Offers/V1/OffersV1.sol @@ -143,7 +143,7 @@ contract OffersV1 is ReentrancyGuard, UniversalExchangeEventV1, IncomingTransfer /// @param _offerId The ID of the offer /// @param _currency The address of the ERC-20 token offering, or address(0) for ETH /// @param _amount The new amount offering - function setOffer( + function setOfferAmount( address _tokenContract, uint256 _tokenId, uint256 _offerId, @@ -152,14 +152,14 @@ contract OffersV1 is ReentrancyGuard, UniversalExchangeEventV1, IncomingTransfer ) external payable nonReentrant { Offer storage offer = offers[_tokenContract][_tokenId][_offerId]; - require(offer.maker == msg.sender, "setOffer must be maker"); + require(offer.maker == msg.sender, "setOfferAmount must be maker"); // If same currency -- if (_currency == offer.currency) { // Get initial amount uint256 prevAmount = offer.amount; // Ensure valid update - require(_amount > 0 && _amount != prevAmount, "setOffer invalid _amount"); + require(_amount > 0 && _amount != prevAmount, "setOfferAmount invalid _amount"); // If offer increase -- if (_amount > prevAmount) { diff --git a/contracts/test/modules/Offers/V1/Offers.t.sol b/contracts/test/modules/Offers/V1/Offers.t.sol index e680d414..a0ea2f50 100644 --- a/contracts/test/modules/Offers/V1/Offers.t.sol +++ b/contracts/test/modules/Offers/V1/Offers.t.sol @@ -119,7 +119,7 @@ contract OffersV1Test is DSTest { vm.warp(1 hours); - offers.setOffer{value: 1 ether}(address(token), 0, 1, address(0), 2 ether); + offers.setOfferAmount{value: 1 ether}(address(token), 0, 1, address(0), 2 ether); vm.stopPrank(); @@ -136,7 +136,7 @@ contract OffersV1Test is DSTest { vm.warp(1 hours); - offers.setOffer(address(token), 0, 1, address(0), 0.5 ether); + offers.setOfferAmount(address(token), 0, 1, address(0), 0.5 ether); vm.stopPrank(); @@ -153,7 +153,7 @@ contract OffersV1Test is DSTest { vm.warp(1 hours); - offers.setOffer(address(token), 0, 1, address(weth), 2 ether); + offers.setOfferAmount(address(token), 0, 1, address(weth), 2 ether); vm.stopPrank(); @@ -171,7 +171,7 @@ contract OffersV1Test is DSTest { vm.warp(1 hours); - offers.setOffer(address(token), 0, 1, address(weth), 0.5 ether); + offers.setOfferAmount(address(token), 0, 1, address(weth), 0.5 ether); vm.stopPrank(); @@ -186,8 +186,8 @@ contract OffersV1Test is DSTest { vm.prank(address(maker)); offers.createOffer{value: 1 ether}(address(token), 0, address(0), 1 ether, 1000); - vm.expectRevert("setOffer must be maker"); - offers.setOffer(address(token), 0, 1, address(0), 0.5 ether); + vm.expectRevert("setOfferAmount must be maker"); + offers.setOfferAmount(address(token), 0, 1, address(0), 0.5 ether); } function testRevert_CannotIncreaseOfferWithoutAttachingFunds() public { @@ -195,7 +195,7 @@ contract OffersV1Test is DSTest { offers.createOffer{value: 1 ether}(address(token), 0, address(0), 1 ether, 1000); vm.expectRevert("_handleIncomingTransfer msg value less than expected amount"); - offers.setOffer(address(token), 0, 1, address(0), 2 ether); + offers.setOfferAmount(address(token), 0, 1, address(0), 2 ether); vm.stopPrank(); } @@ -208,8 +208,8 @@ contract OffersV1Test is DSTest { offers.fillOffer(address(token), 0, 1, address(0), 1 ether, address(finder)); vm.prank(address(maker)); - vm.expectRevert("setOffer must be maker"); - offers.setOffer(address(token), 0, 1, address(0), 0.5 ether); + vm.expectRevert("setOfferAmount must be maker"); + offers.setOfferAmount(address(token), 0, 1, address(0), 0.5 ether); } /// ------------ CANCEL NFT OFFER ------------ /// From 995936aa85b0f4ae87d833bf828900c07358c650 Mon Sep 17 00:00:00 2001 From: Rohan Kulkarni Date: Fri, 28 Jan 2022 11:40:52 -0500 Subject: [PATCH 06/16] [chore] comment cleaning --- contracts/modules/Offers/V1/OffersV1.sol | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/contracts/modules/Offers/V1/OffersV1.sol b/contracts/modules/Offers/V1/OffersV1.sol index 675459ec..0e00941e 100644 --- a/contracts/modules/Offers/V1/OffersV1.sol +++ b/contracts/modules/Offers/V1/OffersV1.sol @@ -26,7 +26,7 @@ contract OffersV1 is ReentrancyGuard, UniversalExchangeEventV1, IncomingTransfer ERC721TransferHelper public immutable erc721TransferHelper; /// @notice The metadata of an offer - /// @param maker The address of the user that made the offer + /// @param maker The address of the user who made the offer /// @param currency The address of the ERC-20 offered, or address(0) for ETH /// @param findersFeeBps The fee to the referrer of the offer /// @param amount The amount of ETH/ERC-20 tokens offered @@ -44,7 +44,7 @@ contract OffersV1 is ReentrancyGuard, UniversalExchangeEventV1, IncomingTransfer mapping(address => mapping(uint256 => mapping(uint256 => Offer))) public offers; /// @notice The offers for a given NFT - /// @dev ERC-721 token address => ERC-721 token ID => offer IDs + /// @dev ERC-721 token address => ERC-721 token ID => Offer IDs mapping(address => mapping(uint256 => uint256[])) public offersForNFT; /// ------------ EVENTS ------------ @@ -74,10 +74,10 @@ contract OffersV1 is ReentrancyGuard, UniversalExchangeEventV1, IncomingTransfer /// @param tokenContract The ERC-721 token address of the filled offer /// @param tokenId The ERC-721 token ID of the filled offer /// @param id The ID of the filled offer - /// @param buyer The address of the buyer who filled the offer + /// @param taker The address of the taker who filled the offer /// @param finder The address of the finder who referred the offer /// @param offer The metadata of the filled offer - event OfferFilled(address indexed tokenContract, uint256 indexed tokenId, uint256 indexed id, address buyer, address finder, Offer offer); + event OfferFilled(address indexed tokenContract, uint256 indexed tokenId, uint256 indexed id, address taker, address finder, Offer offer); /// ------------ CONSTRUCTOR ------------ @@ -181,9 +181,9 @@ contract OffersV1 is ReentrancyGuard, UniversalExchangeEventV1, IncomingTransfer } // Else other currency -- } else { - // Refund previous + // Refund previous offer _handleOutgoingTransfer(offer.maker, offer.amount, offer.currency, USE_ALL_GAS_FLAG); - // Custody new + // Custody new offer _handleIncomingTransfer(_amount, _currency); // Update storage From 598c8512746d6508697a6e1ec18abea80a4f737b Mon Sep 17 00:00:00 2001 From: Rohan Kulkarni Date: Fri, 28 Jan 2022 12:51:57 -0500 Subject: [PATCH 07/16] [feat] UML for Offers v1.0 --- uml/OffersV1/cancelOffer.txt | 13 +++++++++++++ uml/OffersV1/createOffer.txt | 16 ++++++++++++++++ uml/OffersV1/fillOffer.txt | 27 +++++++++++++++++++++++++++ uml/OffersV1/setOfferAmount.txt | 21 +++++++++++++++++++++ 4 files changed, 77 insertions(+) create mode 100644 uml/OffersV1/cancelOffer.txt create mode 100644 uml/OffersV1/createOffer.txt create mode 100644 uml/OffersV1/fillOffer.txt create mode 100644 uml/OffersV1/setOfferAmount.txt diff --git a/uml/OffersV1/cancelOffer.txt b/uml/OffersV1/cancelOffer.txt new file mode 100644 index 00000000..4c6cb5f1 --- /dev/null +++ b/uml/OffersV1/cancelOffer.txt @@ -0,0 +1,13 @@ +@startuml +actor Caller +participant OffersV1 +participant ERC20TransferHelper + +Caller -> OffersV1 : cancelOffer() +OffersV1 -> ERC20TransferHelper : transferFrom() +ERC20TransferHelper -> ERC20TransferHelper : refund tokens from escrow +OffersV1 -> OffersV1 : emit OfferCanceled() +OffersV1 -> OffersV1 : delete offer + +@enduml + diff --git a/uml/OffersV1/createOffer.txt b/uml/OffersV1/createOffer.txt new file mode 100644 index 00000000..67c2ffb8 --- /dev/null +++ b/uml/OffersV1/createOffer.txt @@ -0,0 +1,16 @@ +@startuml +actor Caller +participant OffersV1 +participant ERC20TransferHelper + +Caller -> OffersV1 : createOffer() +OffersV1 -> ERC20TransferHelper : transferFrom() +ERC20TransferHelper -> ERC20TransferHelper : transfer tokens into escrow +OffersV1 -> OffersV1 : offer count ++ +OffersV1 -> OffersV1 : create offer +OffersV1 -> OffersV1 : offersFor[NFT].append(id) +OffersV1 -> OffersV1 : emit OfferCreated() +OffersV1 -> Caller :id + +@enduml + diff --git a/uml/OffersV1/fillOffer.txt b/uml/OffersV1/fillOffer.txt new file mode 100644 index 00000000..6a9642c5 --- /dev/null +++ b/uml/OffersV1/fillOffer.txt @@ -0,0 +1,27 @@ +@startuml +actor Caller +participant OffersV1 +participant ERC721TransferHelper + +Caller -> OffersV1 : fillOffer() + +OffersV1 -> OffersV1 : validate token owner + +OffersV1 -> OffersV1 : handle royalty payouts + +alt finders fee configured for this offer? + + OffersV1 -> OffersV1 : handle finders fee payout + +else noop + +end + +OffersV1 -> ERC721TransferHelper : transferFrom() +ERC721TransferHelper -> ERC721TransferHelper : transfer NFT from taker to maker + +OffersV1 -> OffersV1 : emit ExchangeExecuted() +OffersV1 -> OffersV1 : emit OfferFilled() +OffersV1 -> OffersV1 : delete offer from contract + +@enduml \ No newline at end of file diff --git a/uml/OffersV1/setOfferAmount.txt b/uml/OffersV1/setOfferAmount.txt new file mode 100644 index 00000000..d14310d3 --- /dev/null +++ b/uml/OffersV1/setOfferAmount.txt @@ -0,0 +1,21 @@ +@startuml +actor Caller +participant OffersV1 +participant ERC20TransferHelper + +Caller -> OffersV1 : setOfferAmount() + +alt same token? + + OffersV1 -> ERC20TransferHelper : retrieve increase / refund decrease + +else different token + + OffersV1 -> ERC20TransferHelper : refund previous offer + + OffersV1 -> ERC20TransferHelper : retrieve new offer +end + +OffersV1 -> OffersV1 : emit OfferUpdated() + +@enduml \ No newline at end of file From 3425fc7649790cf964a1443b7b364b332caeae28 Mon Sep 17 00:00:00 2001 From: tysonbattistella Date: Fri, 28 Jan 2022 12:54:08 -0500 Subject: [PATCH 08/16] [feat] UML --- uml/OffersV1/cancelOffer.atxt | 29 ++++++++++++++++++ uml/OffersV1/createOffer.atxt | 40 +++++++++++++++++++++++++ uml/OffersV1/fillOffer.atxt | 51 ++++++++++++++++++++++++++++++++ uml/OffersV1/setOfferAmount.atxt | 33 +++++++++++++++++++++ 4 files changed, 153 insertions(+) create mode 100644 uml/OffersV1/cancelOffer.atxt create mode 100644 uml/OffersV1/createOffer.atxt create mode 100644 uml/OffersV1/fillOffer.atxt create mode 100644 uml/OffersV1/setOfferAmount.atxt diff --git a/uml/OffersV1/cancelOffer.atxt b/uml/OffersV1/cancelOffer.atxt new file mode 100644 index 00000000..a32b9fe9 --- /dev/null +++ b/uml/OffersV1/cancelOffer.atxt @@ -0,0 +1,29 @@ + ,-. + `-' + /|\ + | ,--------. ,-------------------. + / \ |OffersV1| |ERC20TransferHelper| + Caller `---+----' `---------+---------' + | cancelOffer() | | + | -----------------> | + | | | + | | transferFrom() | + | | ------------------------> + | | | + | | |----. + | | | | refund tokens from escrow + | | |<---' + | | | + | |----. + | | | emit OfferCanceled() + | |<---' + | | | + | |----. | + | | | delete offer | + | |<---' | + Caller ,---+----. ,---------+---------. + ,-. |OffersV1| |ERC20TransferHelper| + `-' `--------' `-------------------' + /|\ + | + / \ diff --git a/uml/OffersV1/createOffer.atxt b/uml/OffersV1/createOffer.atxt new file mode 100644 index 00000000..2933234d --- /dev/null +++ b/uml/OffersV1/createOffer.atxt @@ -0,0 +1,40 @@ + ,-. + `-' + /|\ + | ,--------. ,-------------------. + / \ |OffersV1| |ERC20TransferHelper| + Caller `---+----' `---------+---------' + | createOffer() | | + | -----------------> | + | | | + | | transferFrom() | + | | -----------------------------> + | | | + | | |----. + | | | | transfer tokens into escrow + | | |<---' + | | | + | |----. | + | | | offer count ++ | + | |<---' | + | | | + | |----. | + | | | create offer | + | |<---' | + | | | + | |----. + | | | offersFor[NFT].append(id) + | |<---' + | | | + | |----. | + | | | emit OfferCreated() | + | |<---' | + | | | + | id | | + | <----------------- | + Caller ,---+----. ,---------+---------. + ,-. |OffersV1| |ERC20TransferHelper| + `-' `--------' `-------------------' + /|\ + | + / \ diff --git a/uml/OffersV1/fillOffer.atxt b/uml/OffersV1/fillOffer.atxt new file mode 100644 index 00000000..6c941010 --- /dev/null +++ b/uml/OffersV1/fillOffer.atxt @@ -0,0 +1,51 @@ + ,-. + `-' + /|\ + | ,--------. ,--------------------. + / \ |OffersV1| |ERC721TransferHelper| + Caller `---+----' `---------+----------' + | fillOffer() | | + | -----------------> | + | | | + | |----. | + | | | validate token owner | + | |<---' | + | | | + | |----. | + | | | handle royalty payouts | + | |<---' | + | | | + | | | + | __________________________________________________ + | ! ALT / finders fee configured for this offer? ! + | !_____/ | | ! + | ! |----. | ! + | ! | | handle finders fee payout| ! + | ! |<---' | ! + | !~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~! + | !~[noop]~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~! + | | | + | | transferFrom() | + | | ------------------------------> + | | | + | | |----. + | | | | transfer NFT from taker to maker + | | |<---' + | | | + | |----. | + | | | emit ExchangeExecuted() | + | |<---' | + | | | + | |----. | + | | | emit OfferFilled() | + | |<---' | + | | | + | |----. + | | | delete offer from contract + | |<---' + Caller ,---+----. ,---------+----------. + ,-. |OffersV1| |ERC721TransferHelper| + `-' `--------' `--------------------' + /|\ + | + / \ diff --git a/uml/OffersV1/setOfferAmount.atxt b/uml/OffersV1/setOfferAmount.atxt new file mode 100644 index 00000000..737f7a6d --- /dev/null +++ b/uml/OffersV1/setOfferAmount.atxt @@ -0,0 +1,33 @@ + ,-. + `-' + /|\ + | ,--------. ,-------------------. + / \ |OffersV1| |ERC20TransferHelper| + Caller `---+----' `---------+---------' + | setOfferAmount() | | + | -----------------> | + | | | + | | | + | _______________________________________________________________________ + | ! ALT / same token? | ! + | !_____/ | | ! + | ! | retrieve increase / refund decrease| ! + | ! | -----------------------------------> ! + | !~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~! + | ! [different token] | ! + | ! | refund previous offer | ! + | ! | -----------------------------------> ! + | ! | | ! + | ! | retrieve new offer | ! + | ! | -----------------------------------> ! + | !~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~! + | | | + | |----. | + | | | emit OfferUpdated() | + | |<---' | + Caller ,---+----. ,---------+---------. + ,-. |OffersV1| |ERC20TransferHelper| + `-' `--------' `-------------------' + /|\ + | + / \ From 60b2875a80c6cdfb13bab838b7da00c283b5b346 Mon Sep 17 00:00:00 2001 From: Rohan Kulkarni Date: Fri, 28 Jan 2022 12:59:01 -0500 Subject: [PATCH 09/16] [feat] UML integrated --- contracts/modules/Offers/V1/OffersV1.sol | 153 +++++++++++++++++++++++ 1 file changed, 153 insertions(+) diff --git a/contracts/modules/Offers/V1/OffersV1.sol b/contracts/modules/Offers/V1/OffersV1.sol index 0e00941e..67f4304b 100644 --- a/contracts/modules/Offers/V1/OffersV1.sol +++ b/contracts/modules/Offers/V1/OffersV1.sol @@ -102,6 +102,46 @@ contract OffersV1 is ReentrancyGuard, UniversalExchangeEventV1, IncomingTransfer /// ------------ MAKER FUNCTIONS ------------ + // ,-. + // `-' + // /|\ + // | ,--------. ,-------------------. + // / \ |OffersV1| |ERC20TransferHelper| + // Caller `---+----' `---------+---------' + // | createOffer() | | + // | -----------------> | + // | | | + // | | transferFrom() | + // | | -----------------------------> + // | | | + // | | |----. + // | | | | transfer tokens into escrow + // | | |<---' + // | | | + // | |----. | + // | | | offer count ++ | + // | |<---' | + // | | | + // | |----. | + // | | | create offer | + // | |<---' | + // | | | + // | |----. + // | | | offersFor[NFT].append(id) + // | |<---' + // | | | + // | |----. | + // | | | emit OfferCreated() | + // | |<---' | + // | | | + // | id | | + // | <----------------- | + // Caller ,---+----. ,---------+---------. + // ,-. |OffersV1| |ERC20TransferHelper| + // `-' `--------' `-------------------' + // /|\ + // | + // / \ /// @notice Creates an offer for an NFT /// @param _tokenContract The address of the desired ERC-721 token /// @param _tokenId The ID of the desired ERC-721 token @@ -137,6 +177,39 @@ contract OffersV1 is ReentrancyGuard, UniversalExchangeEventV1, IncomingTransfer return offerCount; } + // ,-. + // `-' + // /|\ + // | ,--------. ,-------------------. + // / \ |OffersV1| |ERC20TransferHelper| + // Caller `---+----' `---------+---------' + // | setOfferAmount() | | + // | -----------------> | + // | | | + // | | | + // | _______________________________________________________________________ + // | ! ALT / same token? | ! + // | !_____/ | | ! + // | ! | retrieve increase / refund decrease| ! + // | ! | -----------------------------------> ! + // | !~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~! + // | ! [different token] | ! + // | ! | refund previous offer | ! + // | ! | -----------------------------------> ! + // | ! | | ! + // | ! | retrieve new offer | ! + // | ! | -----------------------------------> ! + // | !~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~! + // | | | + // | |----. | + // | | | emit OfferUpdated() | + // | |<---' | + // Caller ,---+----. ,---------+---------. + // ,-. |OffersV1| |ERC20TransferHelper| + // `-' `--------' `-------------------' + // /|\ + // | + // / \ /// @notice Updates the given offer for an NFT /// @param _tokenContract The address of the offer ERC-721 token /// @param _tokenId The ID of the offer ERC-721 token @@ -194,6 +267,35 @@ contract OffersV1 is ReentrancyGuard, UniversalExchangeEventV1, IncomingTransfer emit OfferUpdated(_tokenContract, _tokenId, _offerId, offer); } + // ,-. + // `-' + // /|\ + // | ,--------. ,-------------------. + // / \ |OffersV1| |ERC20TransferHelper| + // Caller `---+----' `---------+---------' + // | cancelOffer() | | + // | -----------------> | + // | | | + // | | transferFrom() | + // | | ------------------------> + // | | | + // | | |----. + // | | | | refund tokens from escrow + // | | |<---' + // | | | + // | |----. + // | | | emit OfferCanceled() + // | |<---' + // | | | + // | |----. | + // | | | delete offer | + // | |<---' | + // Caller ,---+----. ,---------+---------. + // ,-. |OffersV1| |ERC20TransferHelper| + // `-' `--------' `-------------------' + // /|\ + // | + // / \ /// @notice Cancels and refunds the given offer for an NFT /// @param _tokenContract The ERC-721 token address of the offer /// @param _tokenId The ERC-721 token ID of the offer @@ -217,6 +319,57 @@ contract OffersV1 is ReentrancyGuard, UniversalExchangeEventV1, IncomingTransfer /// ------------ TAKER FUNCTIONS ------------ + // ,-. + // `-' + // /|\ + // | ,--------. ,--------------------. + // / \ |OffersV1| |ERC721TransferHelper| + // Caller `---+----' `---------+----------' + // | fillOffer() | | + // | -----------------> | + // | | | + // | |----. | + // | | | validate token owner | + // | |<---' | + // | | | + // | |----. | + // | | | handle royalty payouts | + // | |<---' | + // | | | + // | | | + // | __________________________________________________ + // | ! ALT / finders fee configured for this offer? ! + // | !_____/ | | ! + // | ! |----. | ! + // | ! | | handle finders fee payout| ! + // | ! |<---' | ! + // | !~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~! + // | !~[noop]~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~! + // | | | + // | | transferFrom() | + // | | ------------------------------> + // | | | + // | | |----. + // | | | | transfer NFT from taker to maker + // | | |<---' + // | | | + // | |----. | + // | | | emit ExchangeExecuted() | + // | |<---' | + // | | | + // | |----. | + // | | | emit OfferFilled() | + // | |<---' | + // | | | + // | |----. + // | | | delete offer from contract + // | |<---' + // Caller ,---+----. ,---------+----------. + // ,-. |OffersV1| |ERC721TransferHelper| + // `-' `--------' `--------------------' + // /|\ + // | + // / \ /// @notice Fills a given offer for an owned NFT, in exchange for ETH/ERC-20 tokens /// @param _tokenContract The address of the ERC-721 token to transfer /// @param _tokenId The ID of the ERC-721 token to transfer From 8275791145676f09697b520dee482fd08bc6217f Mon Sep 17 00:00:00 2001 From: "almndbtr.eth" <78528185+almndbtr@users.noreply.github.com> Date: Fri, 28 Jan 2022 23:36:54 +0000 Subject: [PATCH 10/16] test: assert failure when findersFeeBps exceeds limit --- contracts/test/modules/Offers/V1/Offers.t.sol | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/contracts/test/modules/Offers/V1/Offers.t.sol b/contracts/test/modules/Offers/V1/Offers.t.sol index a0ea2f50..bae23d7b 100644 --- a/contracts/test/modules/Offers/V1/Offers.t.sol +++ b/contracts/test/modules/Offers/V1/Offers.t.sol @@ -110,6 +110,11 @@ contract OffersV1Test is DSTest { offers.createOffer(address(token), 0, address(0), 1 ether, 1000); } + function testFail_CannotCreateOfferWithInvalidFindersFeeBps() public { + vm.prank(address(maker)); + offers.createOffer(address(token), 0, address(0), 1 ether, 10001); + } + /// ------------ SET NFT OFFER ------------ /// function test_IncreaseETHOffer() public { From 93e8ef7e570d908c7388113b756d718be6860913 Mon Sep 17 00:00:00 2001 From: "almndbtr.eth" <78528185+almndbtr@users.noreply.github.com> Date: Fri, 28 Jan 2022 23:55:37 +0000 Subject: [PATCH 11/16] test: expect revert when setting new offer is same as previous --- contracts/test/modules/Offers/V1/Offers.t.sol | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/contracts/test/modules/Offers/V1/Offers.t.sol b/contracts/test/modules/Offers/V1/Offers.t.sol index bae23d7b..27cde30a 100644 --- a/contracts/test/modules/Offers/V1/Offers.t.sol +++ b/contracts/test/modules/Offers/V1/Offers.t.sol @@ -205,6 +205,20 @@ contract OffersV1Test is DSTest { vm.stopPrank(); } + function testRevert_CannotUpdateOfferWithPreviousAmount() public { + vm.startPrank(address(maker)); + + offers.createOffer{value: 1 ether}(address(token), 0, address(0), 1 ether, 1000); + + vm.warp(1 hours); + + vm.expectRevert("setOfferAmount invalid _amount"); + + offers.setOfferAmount{value: 1 ether}(address(token), 0, 1, address(0), 1 ether); + + vm.stopPrank(); + } + function testRevert_CannotUpdateInactiveOffer() public { vm.prank(address(maker)); offers.createOffer{value: 1 ether}(address(token), 0, address(0), 1 ether, 1000); From 0249071101495705c081978050575956780dd11a Mon Sep 17 00:00:00 2001 From: Rohan Kulkarni Date: Sat, 29 Jan 2022 23:27:04 -0500 Subject: [PATCH 12/16] [fix] read offer from memory on fill/cancel --- contracts/modules/Offers/V1/OffersV1.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/contracts/modules/Offers/V1/OffersV1.sol b/contracts/modules/Offers/V1/OffersV1.sol index 67f4304b..6097c245 100644 --- a/contracts/modules/Offers/V1/OffersV1.sol +++ b/contracts/modules/Offers/V1/OffersV1.sol @@ -305,7 +305,7 @@ contract OffersV1 is ReentrancyGuard, UniversalExchangeEventV1, IncomingTransfer uint256 _tokenId, uint256 _offerId ) external nonReentrant { - Offer storage offer = offers[_tokenContract][_tokenId][_offerId]; + Offer memory offer = offers[_tokenContract][_tokenId][_offerId]; require(offer.maker == msg.sender, "cancelOffer must be maker"); @@ -385,7 +385,7 @@ contract OffersV1 is ReentrancyGuard, UniversalExchangeEventV1, IncomingTransfer uint256 _amount, address _finder ) external nonReentrant { - Offer storage offer = offers[_tokenContract][_tokenId][_offerId]; + Offer memory offer = offers[_tokenContract][_tokenId][_offerId]; require(offer.maker != address(0), "fillOffer must be active offer"); require(IERC721(_tokenContract).ownerOf(_tokenId) == msg.sender, "fillOffer must be token owner"); From e059928e0a8b7174c9bbec9a1001c6cc601fbf2a Mon Sep 17 00:00:00 2001 From: Rohan Kulkarni Date: Mon, 31 Jan 2022 15:18:51 -0500 Subject: [PATCH 13/16] [feat] gas golf from @transmissions11 --- contracts/modules/Offers/V1/OffersV1.sol | 34 ++++++++++++++---------- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/contracts/modules/Offers/V1/OffersV1.sol b/contracts/modules/Offers/V1/OffersV1.sol index 6097c245..c3323dd5 100644 --- a/contracts/modules/Offers/V1/OffersV1.sol +++ b/contracts/modules/Offers/V1/OffersV1.sol @@ -161,7 +161,10 @@ contract OffersV1 is ReentrancyGuard, UniversalExchangeEventV1, IncomingTransfer // Validate offer and take custody _handleIncomingTransfer(_amount, _currency); - offerCount++; + // "the sun will devour the earth before it could ever overflow" - @transmissions11 + unchecked { + offerCount++; + } offers[_tokenContract][_tokenId][offerCount] = Offer({ maker: msg.sender, @@ -236,21 +239,24 @@ contract OffersV1 is ReentrancyGuard, UniversalExchangeEventV1, IncomingTransfer // If offer increase -- if (_amount > prevAmount) { - // Get delta - uint256 increaseAmount = _amount - prevAmount; - // Custody increase - _handleIncomingTransfer(increaseAmount, offer.currency); - // Update storage - offer.amount += increaseAmount; - + unchecked { + // Get delta + uint256 increaseAmount = _amount - prevAmount; + // Custody increase + _handleIncomingTransfer(increaseAmount, offer.currency); + // Update storage + offer.amount += increaseAmount; + } // Else offer decrease -- } else { - // Get delta - uint256 decreaseAmount = prevAmount - _amount; - // Refund difference - _handleOutgoingTransfer(offer.maker, decreaseAmount, offer.currency, USE_ALL_GAS_FLAG); - // Update storage - offer.amount -= decreaseAmount; + unchecked { + // Get delta + uint256 decreaseAmount = prevAmount - _amount; + // Refund difference + _handleOutgoingTransfer(offer.maker, decreaseAmount, offer.currency, USE_ALL_GAS_FLAG); + // Update storage + offer.amount -= decreaseAmount; + } } // Else other currency -- } else { From e9bf2e6f6172347d28723da6255aca6f316bb8f0 Mon Sep 17 00:00:00 2001 From: Rohan Kulkarni Date: Fri, 4 Feb 2022 19:02:24 -0500 Subject: [PATCH 14/16] [feat] golf recap --- contracts/modules/Offers/V1/OffersV1.sol | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/contracts/modules/Offers/V1/OffersV1.sol b/contracts/modules/Offers/V1/OffersV1.sol index c3323dd5..b2ea77e3 100644 --- a/contracts/modules/Offers/V1/OffersV1.sol +++ b/contracts/modules/Offers/V1/OffersV1.sol @@ -119,7 +119,7 @@ contract OffersV1 is ReentrancyGuard, UniversalExchangeEventV1, IncomingTransfer // | | |<---' // | | | // | |----. | - // | | | offer count ++ | + // | | | ++offerCount | // | |<---' | // | | | // | |----. | @@ -162,9 +162,20 @@ contract OffersV1 is ReentrancyGuard, UniversalExchangeEventV1, IncomingTransfer _handleIncomingTransfer(_amount, _currency); // "the sun will devour the earth before it could ever overflow" - @transmissions11 - unchecked { - offerCount++; - } + // offerCount++ --> unchecked { offerCount++ } + + // "Although the increment part is cheaper with unchecked, the opcodes after become more expensive for some reason" - @joshieDo + // unchecked { offerCount++ } --> offerCount++ + + // TURNS OUT: + // UNCHECKED CHECKED + // NON-OPTIMIZED 130,037 gas < 130,149 gas + // OPTIMIZED 127,932 gas > *127,298 gas* + + // "Earlier today while reviewing c4rena findings I learned that doing ++offerCount would save 5 gas per increment here" - @devtooligan + // offerCount++ --> ++offerCount + + ++offerCount; offers[_tokenContract][_tokenId][offerCount] = Offer({ maker: msg.sender, From 5d2150fd469c73be2bfc870bed78759e3bf8f2b9 Mon Sep 17 00:00:00 2001 From: Rohan Kulkarni Date: Sun, 6 Feb 2022 02:50:16 -0500 Subject: [PATCH 15/16] [feat] update for deploy --- contracts/modules/Offers/V1/OffersV1.sol | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/contracts/modules/Offers/V1/OffersV1.sol b/contracts/modules/Offers/V1/OffersV1.sol index b2ea77e3..bc35ede2 100644 --- a/contracts/modules/Offers/V1/OffersV1.sol +++ b/contracts/modules/Offers/V1/OffersV1.sol @@ -167,14 +167,13 @@ contract OffersV1 is ReentrancyGuard, UniversalExchangeEventV1, IncomingTransfer // "Although the increment part is cheaper with unchecked, the opcodes after become more expensive for some reason" - @joshieDo // unchecked { offerCount++ } --> offerCount++ - // TURNS OUT: - // UNCHECKED CHECKED - // NON-OPTIMIZED 130,037 gas < 130,149 gas - // OPTIMIZED 127,932 gas > *127,298 gas* - // "Earlier today while reviewing c4rena findings I learned that doing ++offerCount would save 5 gas per increment here" - @devtooligan // offerCount++ --> ++offerCount + // TLDR; unchecked checked + // non-optimized 130,037 gas < 130,149 gas + // optimized 127,932 gas > *127,298 gas* + ++offerCount; offers[_tokenContract][_tokenId][offerCount] = Offer({ From 615343486fab6a4c1d3cf07b9cef5d1317c7feda Mon Sep 17 00:00:00 2001 From: Rohan Kulkarni Date: Tue, 8 Feb 2022 15:50:57 -0500 Subject: [PATCH 16/16] [feat] deploy --- addresses/1.json | 3 ++- addresses/4.json | 5 +++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/addresses/1.json b/addresses/1.json index cb7cd27f..0acc6c52 100644 --- a/addresses/1.json +++ b/addresses/1.json @@ -6,5 +6,6 @@ "ERC20TransferHelper": "0xCCA379FDF4Beda63c4bB0e2A3179Ae62c8716794", "ERC721TransferHelper": "0x909e9efE4D87d1a6018C2065aE642b6D0447bc91", "AsksV1": "0xCe6cEf2A9028e1C3B21647ae3B4251038109f42a", - "AsksV1_1": "0x6170B3C3A54C3d8c854934cBC314eD479b2B29A3" + "AsksV1_1": "0x6170B3C3A54C3d8c854934cBC314eD479b2B29A3", + "OffersV1": "0x76744367ae5a056381868f716bdf0b13ae1aeaa3" } diff --git a/addresses/4.json b/addresses/4.json index 9e9add05..db6556aa 100644 --- a/addresses/4.json +++ b/addresses/4.json @@ -6,5 +6,6 @@ "ERC20TransferHelper": "0x408AbC192a5e9696085EBaFC7C5A88e19e66241b", "ERC721TransferHelper": "0x029AA5a949C9C90916729D50537062cb73b5Ac92", "AsksV1": "0x850356153abBdFA1B473e2D86F2DF11a85B408B8", - "AsksV1_1": "0xA98D3729265C88c5b3f861a0c501622750fF4806" -} \ No newline at end of file + "AsksV1_1": "0xA98D3729265C88c5b3f861a0c501622750fF4806", + "OffersV1": "0x1240ef9f9c56ee981d10cffc6ba5807b6c7fecaa" +}