Skip to content

Commit

Permalink
Merge pull request #129 from G7DAO/fix/get-all-valid-listings
Browse files Browse the repository at this point in the history
Fix: get all valid listings function
  • Loading branch information
karacurt authored May 16, 2024
2 parents db9aa1d + cfd1e3d commit 9c97b9c
Show file tree
Hide file tree
Showing 7 changed files with 175 additions and 158 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -102,48 +102,20 @@ contract DirectListingsLogic is IDirectListings, ReentrancyGuard, ERC2771Context
function createListing(
ListingParameters calldata _params
) external onlyListerRole onlyAssetRole(_params.assetContract) returns (uint256 listingId) {
listingId = _getNextListingId();
address listingCreator = _msgSender();
TokenType tokenType = _getTokenType(_params.assetContract);

uint128 startTime = _params.startTimestamp;
uint128 endTime = _params.endTimestamp;
require(startTime < endTime, "Marketplace: endTimestamp not greater than startTimestamp.");
if (startTime < block.timestamp) {
require(startTime + 60 minutes >= block.timestamp, "Marketplace: invalid startTimestamp.");

startTime = uint128(block.timestamp);
endTime = endTime == type(uint128).max
? endTime
: startTime + (_params.endTimestamp - _params.startTimestamp);
}

_validateNewListing(listingCreator, _params, tokenType);

Listing memory listing = Listing({
listingId: listingId,
listingCreator: listingCreator,
assetContract: _params.assetContract,
tokenId: _params.tokenId,
quantity: _params.quantity,
currency: _params.currency,
pricePerToken: _params.pricePerToken,
startTimestamp: startTime,
endTimestamp: endTime,
reserved: _params.reserved,
tokenType: tokenType,
status: IDirectListings.Status.CREATED
});

_directListingsStorage().listings[listingId] = listing;

emit NewListing(listingCreator, listingId, _params.assetContract, listing);
listingId = _createListing(_params, _msgSender());
}

function adminCreateListing(
ListingParameters calldata _params,
address listingCreator
) external onlyManagerRole onlyAssetRole(_params.assetContract) returns (uint256 listingId) {
listingId = _createListing(_params, listingCreator);
}

function _createListing(
ListingParameters calldata _params,
address listingCreator
) internal returns (uint256 listingId) {
listingId = _getNextListingId();
TokenType tokenType = _getTokenType(_params.assetContract);

Expand All @@ -159,7 +131,7 @@ contract DirectListingsLogic is IDirectListings, ReentrancyGuard, ERC2771Context
: startTime + (_params.endTimestamp - _params.startTimestamp);
}

_validateNewListing(listingCreator, _params, tokenType);
_validateNewListing(_params, tokenType);

Listing memory listing = Listing({
listingId: listingId,
Expand Down Expand Up @@ -188,65 +160,7 @@ contract DirectListingsLogic is IDirectListings, ReentrancyGuard, ERC2771Context
uint256 _listingId,
ListingParameters memory _params
) external onlyExistingListing(_listingId) onlyAssetRole(_params.assetContract) onlyListingCreator(_msgSender(), _listingId) {
address listingCreator = _msgSender();
Listing memory listing = _directListingsStorage().listings[_listingId];
TokenType tokenType = _getTokenType(_params.assetContract);

require(listing.endTimestamp > block.timestamp, "Marketplace: listing expired.");

require(
listing.assetContract == _params.assetContract && listing.tokenId == _params.tokenId,
"Marketplace: cannot update what token is listed."
);

uint128 startTime = _params.startTimestamp;
uint128 endTime = _params.endTimestamp;
require(startTime < endTime, "Marketplace: endTimestamp not greater than startTimestamp.");
require(
listing.startTimestamp > block.timestamp ||
(startTime == listing.startTimestamp && endTime > block.timestamp),
"Marketplace: listing already active."
);
if (startTime != listing.startTimestamp && startTime < block.timestamp) {
require(startTime + 60 minutes >= block.timestamp, "Marketplace: invalid startTimestamp.");

startTime = uint128(block.timestamp);

endTime = endTime == listing.endTimestamp || endTime == type(uint128).max
? endTime
: startTime + (_params.endTimestamp - _params.startTimestamp);
}

{
uint256 _approvedCurrencyPrice = _directListingsStorage().currencyPriceForListing[_listingId][
_params.currency
];
require(
_approvedCurrencyPrice == 0 || _params.pricePerToken == _approvedCurrencyPrice,
"Marketplace: price different from approved price"
);
}

_validateNewListing(listingCreator, _params, tokenType);

listing = Listing({
listingId: _listingId,
listingCreator: listingCreator,
assetContract: _params.assetContract,
tokenId: _params.tokenId,
quantity: _params.quantity,
currency: _params.currency,
pricePerToken: _params.pricePerToken,
startTimestamp: startTime,
endTimestamp: endTime,
reserved: _params.reserved,
tokenType: tokenType,
status: IDirectListings.Status.CREATED
});

_directListingsStorage().listings[_listingId] = listing;

emit UpdatedListing(listingCreator, _listingId, _params.assetContract, listing);
_updateListing(_listingId, _params, _msgSender());
}

/// @notice Update parameters of a listing of NFTs.
Expand All @@ -255,6 +169,10 @@ contract DirectListingsLogic is IDirectListings, ReentrancyGuard, ERC2771Context
ListingParameters memory _params,
address listingCreator
) external onlyManagerRole onlyExistingListing(_listingId) onlyAssetRole(_params.assetContract) onlyListingCreator(listingCreator, _listingId) {
_updateListing(_listingId, _params, listingCreator);
}

function _updateListing(uint256 _listingId, ListingParameters memory _params, address listingCreator) internal {
Listing memory listing = _directListingsStorage().listings[_listingId];
TokenType tokenType = _getTokenType(_params.assetContract);

Expand All @@ -270,7 +188,7 @@ contract DirectListingsLogic is IDirectListings, ReentrancyGuard, ERC2771Context
require(startTime < endTime, "Marketplace: endTimestamp not greater than startTimestamp.");
require(
listing.startTimestamp > block.timestamp ||
(startTime == listing.startTimestamp && endTime > block.timestamp),
(startTime == listing.startTimestamp && endTime > block.timestamp),
"Marketplace: listing already active."
);
if (startTime != listing.startTimestamp && startTime < block.timestamp) {
Expand All @@ -285,15 +203,15 @@ contract DirectListingsLogic is IDirectListings, ReentrancyGuard, ERC2771Context

{
uint256 _approvedCurrencyPrice = _directListingsStorage().currencyPriceForListing[_listingId][
_params.currency
];
_params.currency
];
require(
_approvedCurrencyPrice == 0 || _params.pricePerToken == _approvedCurrencyPrice,
"Marketplace: price different from approved price"
);
}

_validateNewListing(listingCreator, _params, tokenType);
_validateNewListing(_params, tokenType);

listing = Listing({
listingId: _listingId,
Expand Down Expand Up @@ -356,7 +274,6 @@ contract DirectListingsLogic is IDirectListings, ReentrancyGuard, ERC2771Context
emit CurrencyApprovedForListing(_listingId, _currency, _pricePerTokenInCurrency);
}


/// @notice Approve a currency as a form of payment for the listing for Admins.
function adminApproveCurrencyForListing(
uint256 _listingId,
Expand Down Expand Up @@ -397,14 +314,13 @@ contract DirectListingsLogic is IDirectListings, ReentrancyGuard, ERC2771Context
);

require(
_validateOwnershipAndApproval(
listing.listingCreator,
listing.assetContract,
listing.tokenId,
_quantity,
listing.tokenType
),
"Marketplace: not owner or approved tokens."
_validateOwnership(
address(this),
listing.assetContract,
listing.tokenId,
_quantity,
listing.tokenType),
"Marketplace: token is not owned by marketplace."
);

uint256 targetTotalPrice;
Expand Down Expand Up @@ -537,14 +453,14 @@ contract DirectListingsLogic is IDirectListings, ReentrancyGuard, ERC2771Context
);

require(
_validateOwnershipAndApproval(
listing.listingCreator,
listing.assetContract,
listing.tokenId,
_quantity,
_validateOwnership(
address(this),
listing.assetContract,
listing.tokenId,
_quantity,
listing.tokenType
),
"Marketplace: not owner or approved tokens."
),
"Marketplace: asset not owned by marketplace."
);

uint256 targetTotalPrice;
Expand Down Expand Up @@ -612,9 +528,9 @@ contract DirectListingsLogic is IDirectListings, ReentrancyGuard, ERC2771Context

function _decodeData(bytes calldata _data) private view returns (address, uint256, uint256) {
(address contractAddress, uint256 chainId, uint256 listingId) = abi.decode(
_data,
_data,
(address, uint256, uint256)
);
);
return (contractAddress, chainId, listingId);
}

Expand Down Expand Up @@ -653,7 +569,7 @@ contract DirectListingsLogic is IDirectListings, ReentrancyGuard, ERC2771Context
/// @notice Returns all non-cancelled listings.
function getAllListings(uint256 _startId, uint256 _endId) external view returns (Listing[] memory _allListings) {
if (_directListingsStorage().totalListings == 0) return _allListings;
require(_startId <= _endId && _endId < _directListingsStorage().totalListings, "invalid range");
require(_startId <= _endId && _endId <= _directListingsStorage().totalListings, "invalid range");

_allListings = new Listing[](_endId - _startId + 1);

Expand All @@ -672,7 +588,7 @@ contract DirectListingsLogic is IDirectListings, ReentrancyGuard, ERC2771Context
uint256 _endId
) external view returns (Listing[] memory _validListings) {
if (_directListingsStorage().totalListings == 0) return _validListings;
require(_startId <= _endId && _endId < _directListingsStorage().totalListings, "invalid range");
require(_startId <= _endId && _endId <= _directListingsStorage().totalListings, "invalid range");

Listing[] memory _listings = new Listing[](_endId - _startId + 1);
uint256 _listingCount;
Expand Down Expand Up @@ -720,20 +636,9 @@ contract DirectListingsLogic is IDirectListings, ReentrancyGuard, ERC2771Context
}

/// @dev Checks whether the listing creator owns and has approved marketplace to transfer listed tokens.
function _validateNewListing(address creator, ListingParameters memory _params, TokenType _tokenType) internal view {
function _validateNewListing(ListingParameters memory _params, TokenType _tokenType) internal view {
require(_params.quantity > 0, "Marketplace: listing zero quantity.");
require(_params.quantity == 1 || _tokenType == TokenType.ERC1155, "Marketplace: listing invalid quantity.");

require(
_validateOwnershipAndApproval(
creator,
_params.assetContract,
_params.tokenId,
_params.quantity,
_tokenType
),
"Marketplace: not owner or approved tokens."
);
}

/// @dev Checks whether the listing exists, is active, and if the lister has sufficient balance.
Expand All @@ -742,8 +647,8 @@ contract DirectListingsLogic is IDirectListings, ReentrancyGuard, ERC2771Context
_targetListing.startTimestamp <= block.timestamp &&
_targetListing.endTimestamp > block.timestamp &&
_targetListing.status == IDirectListings.Status.CREATED &&
_validateOwnershipAndApproval(
_targetListing.listingCreator,
_validateOwnership(
address(this),
_targetListing.assetContract,
_targetListing.tokenId,
_targetListing.quantity,
Expand All @@ -752,40 +657,28 @@ contract DirectListingsLogic is IDirectListings, ReentrancyGuard, ERC2771Context
}

/// @dev Validates that `_tokenOwner` owns and has approved Marketplace to transfer NFTs.
function _validateOwnershipAndApproval(
function _validateOwnership(
address _tokenOwner,
address _assetContract,
uint256 _tokenId,
uint256 _quantity,
TokenType _tokenType
) internal view returns (bool isValid) {
address market = address(this);

if (_tokenType == TokenType.ERC1155) {
isValid =
IERC1155(_assetContract).balanceOf(_tokenOwner, _tokenId) >= _quantity &&
IERC1155(_assetContract).isApprovedForAll(_tokenOwner, market);
isValid = IERC1155(_assetContract).balanceOf(_tokenOwner, _tokenId) >= _quantity;
} else if (_tokenType == TokenType.ERC721) {
address owner;
address operator;

// failsafe for reverts in case of non-existent tokens
try IERC721(_assetContract).ownerOf(_tokenId) returns (address _owner) {
owner = _owner;

// Nesting the approval check inside this try block, to run only if owner check doesn't revert.
// If the previous check for owner fails, then the return value will always evaluate to false.
try IERC721(_assetContract).getApproved(_tokenId) returns (address _operator) {
operator = _operator;
} catch {}
} catch {}

isValid =
owner == _tokenOwner &&
(operator == market || IERC721(_assetContract).isApprovedForAll(_tokenOwner, market));
isValid = owner == _tokenOwner;
}
}


/// @dev Validates that `_tokenOwner` owns and has approved Markeplace to transfer the appropriate amount of currency
function _validateERC20BalAndAllowance(address _tokenOwner, address _currency, uint256 _amount) internal view {
require(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ contract EnglishAuctionsLogic is IEnglishAuctions, ReentrancyGuard, ERC2771Conte

function getAllAuctions(uint256 _startId, uint256 _endId) external view returns (Auction[] memory _allAuctions) {
if(_englishAuctionsStorage().totalAuctions == 0) return _allAuctions;
require(_startId <= _endId && _endId < _englishAuctionsStorage().totalAuctions, "invalid range");
require(_startId <= _endId && _endId <= _englishAuctionsStorage().totalAuctions, "invalid range");

_allAuctions = new Auction[](_endId - _startId + 1);

Expand All @@ -241,7 +241,7 @@ contract EnglishAuctionsLogic is IEnglishAuctions, ReentrancyGuard, ERC2771Conte
uint256 _endId
) external view returns (Auction[] memory _validAuctions) {
if(_englishAuctionsStorage().totalAuctions == 0) return _validAuctions;
require(_startId <= _endId && _endId < _englishAuctionsStorage().totalAuctions, "invalid range");
require(_startId <= _endId && _endId <= _englishAuctionsStorage().totalAuctions, "invalid range");

Auction[] memory _auctions = new Auction[](_endId - _startId + 1);
uint256 _auctionCount;
Expand Down
4 changes: 2 additions & 2 deletions contracts/upgradeables/marketplace/offers/OffersLogic.sol
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ contract OffersLogic is IOffers, ReentrancyGuard, ERC2771ContextConsumer {
/// @dev Returns all existing offers within the specified range.
function getAllOffers(uint256 _startId, uint256 _endId) external view returns (Offer[] memory _allOffers) {
if(_offersStorage().totalOffers == 0) return _allOffers;
require(_startId <= _endId && _endId < _offersStorage().totalOffers, "invalid range");
require(_startId <= _endId && _endId <= _offersStorage().totalOffers, "invalid range");

_allOffers = new Offer[](_endId - _startId + 1);

Expand All @@ -162,7 +162,7 @@ contract OffersLogic is IOffers, ReentrancyGuard, ERC2771ContextConsumer {
/// @dev Returns offers within the specified range, where offeror has sufficient balance.
function getAllValidOffers(uint256 _startId, uint256 _endId) external view returns (Offer[] memory _validOffers) {
if(_offersStorage().totalOffers == 0) return _validOffers;
require(_startId <= _endId && _endId < _offersStorage().totalOffers, "invalid range");
require(_startId <= _endId && _endId <= _offersStorage().totalOffers, "invalid range");

Offer[] memory _offers = new Offer[](_endId - _startId + 1);
uint256 _offerCount;
Expand Down
4 changes: 2 additions & 2 deletions test/hardhatTests/englishAuctions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ describe('EnglishAuction', function () {
});
describe('getAllAuctions', function () {
it('Should get all Auctions', async function () {
expect(await englishAuction.getAllAuctions(0, 0)).to.be.deep.equal([Object.values(auction)]);
expect(await englishAuction.getAllAuctions(1, 1)).to.be.deep.equal([Object.values(auction)]);
});
it('Should NOT get all Auctions if start index is greater than end index', async function () {
await expect(englishAuction.getAllAuctions(1, 0)).to.be.revertedWith('invalid range');
Expand All @@ -220,7 +220,7 @@ describe('EnglishAuction', function () {
});
describe('getAllValidAuctions', function () {
it('Should get all valid Auctions', async function () {
expect(await englishAuction.getAllValidAuctions(0, 0)).to.be.deep.equal([
expect(await englishAuction.getAllValidAuctions(1, 1)).to.be.deep.equal([
Object.values(auction),
]);
});
Expand Down
Loading

0 comments on commit 9c97b9c

Please sign in to comment.