Skip to content

Commit

Permalink
fix(check): update interface and move signature check
Browse files Browse the repository at this point in the history
  • Loading branch information
bitbeckers committed May 29, 2024
1 parent 6ada61b commit b4a47f1
Show file tree
Hide file tree
Showing 12 changed files with 92 additions and 40 deletions.
1 change: 0 additions & 1 deletion contracts/src/marketplace/ExecutionManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import {OrderStructs} from "./libraries/OrderStructs.sol";
// Interfaces
import {IExecutionManager} from "./interfaces/IExecutionManager.sol";
import {ICreatorFeeManager} from "./interfaces/ICreatorFeeManager.sol";
import {IHypercertToken} from "../protocol/interfaces/IHypercertToken.sol";

// Direct dependencies
import {InheritedStrategy} from "./InheritedStrategy.sol";
Expand Down
4 changes: 2 additions & 2 deletions contracts/src/marketplace/LooksRareProtocol.sol
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import {OrderStructs} from "./libraries/OrderStructs.sol";

// Interfaces
import {ILooksRareProtocol} from "./interfaces/ILooksRareProtocol.sol";
import {IHypercertToken} from "../protocol/interfaces/IHypercertToken.sol";
import {IHypercert1155Token} from "./interfaces/IHypercert1155Token.sol";

// Shared errors
import {
Expand Down Expand Up @@ -709,7 +709,7 @@ contract LooksRareProtocol is
returns (uint256 unitsHeldByItems)
{
for (uint256 i; i < itemIds.length;) {
unitsHeldByItems += IHypercertToken(collection).unitsOf(itemIds[i]);
unitsHeldByItems += IHypercert1155Token(collection).unitsOf(itemIds[i]);

unchecked {
++i;
Expand Down
19 changes: 15 additions & 4 deletions contracts/src/marketplace/TransferManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@ import {LowLevelHypercertCaller} from "./libraries/LowLevelHypercertCaller.sol";

// Interfaces and errors
import {ITransferManager} from "./interfaces/ITransferManager.sol";
import {AmountInvalid, LengthsInvalid} from "./errors/SharedErrors.sol";
import {IHypercert1155Token} from "./interfaces/IHypercert1155Token.sol";
import {AmountInvalid, LengthsInvalid, OrderInvalid} from "./errors/SharedErrors.sol";
import {UnitAmountInvalid} from "./errors/HypercertErrors.sol";
import {IHypercertToken} from "../protocol/interfaces/IHypercertToken.sol";

// Libraries
import {OrderStructs} from "./libraries/OrderStructs.sol";
Expand Down Expand Up @@ -205,13 +205,24 @@ contract TransferManager is

//The new amount is the difference between the total amount and the amount being split.
//This will underflow if the amount being split is greater than the total amount.
newAmounts[0] = IHypercertToken(collection).unitsOf(itemIds[0]) - amounts[0];
newAmounts[0] = IHypercert1155Token(collection).unitsOf(itemIds[0]) - amounts[0];
newAmounts[1] = amounts[0];

// If the new amount is 0, then the split is will revert but the whole fraction can be transferred.
// If the new amount is 0, the split is will revert but the whole fraction can be transferred.
if (newAmounts[0] == 0) {
_executeERC1155SafeTransferFrom(collection, from, to, itemIds[0], 1);
} else {
// Before splitting, the user must be the owner or approved for all.
// This needs to be checked because the marketplace doesn't require signer validation as other token
// standards handle this.
if (
from != IHypercert1155Token(collection).ownerOf(itemIds[0])
&& !IHypercert1155Token(collection).isApprovedForAll(
IHypercert1155Token(collection).ownerOf(itemIds[0]), from
)
) {
revert OrderInvalid();
}
_executeHypercertSplitFraction(collection, to, itemIds[0], newAmounts);
}
}
Expand Down
3 changes: 0 additions & 3 deletions contracts/src/marketplace/TransferSelectorNFT.sol
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,6 @@ import {CollectionTypeInvalid} from "./errors/SharedErrors.sol";
// Enums
import {CollectionType} from "./enums/CollectionType.sol";

// Interfaces
import {IHypercertToken} from "../protocol/interfaces/IHypercertToken.sol";

/**
* @title TransferSelectorNFT
* @notice This contract handles the logic for transferring non-fungible items.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
pragma solidity 0.8.17;

// Interface
import {IHypercertToken} from "@hypercerts/protocol/interfaces/IHypercertToken.sol";
import {IHypercert1155Token} from "../interfaces/IHypercert1155Token.sol";

// Libraries
import {OrderStructs} from "../libraries/OrderStructs.sol";
Expand Down Expand Up @@ -74,7 +74,7 @@ contract StrategyHypercertCollectionOffer is BaseStrategy {
// A collection order can only be executable for 1 fraction
if (
amounts.length != 1 || amounts[0] != 1 || itemUnitsTaker != itemUnitsMaker
|| IHypercertToken(makerBid.collection).unitsOf(offeredItemId) != itemUnitsMaker
|| IHypercert1155Token(makerBid.collection).unitsOf(offeredItemId) != itemUnitsMaker
) {
revert OrderInvalid();
}
Expand Down Expand Up @@ -117,7 +117,7 @@ contract StrategyHypercertCollectionOffer is BaseStrategy {
// A collection order can only be executable for 1 fraction
if (
amounts.length != 1 || amounts[0] != 1 || itemUnitsTaker != itemUnitsMaker
|| IHypercertToken(makerBid.collection).unitsOf(offeredItemId) != itemUnitsMaker
|| IHypercert1155Token(makerBid.collection).unitsOf(offeredItemId) != itemUnitsMaker
) {
revert OrderInvalid();
}
Expand Down Expand Up @@ -193,7 +193,7 @@ contract StrategyHypercertCollectionOffer is BaseStrategy {
// A collection order can only be executable for 1 fraction
if (
amounts.length != 1 || amounts[0] != 1 || itemUnitsTaker != itemUnitsMaker
|| IHypercertToken(collection).unitsOf(offeredItemId) != itemUnitsMaker
|| IHypercert1155Token(collection).unitsOf(offeredItemId) != itemUnitsMaker
) {
revert OrderInvalid();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
pragma solidity 0.8.17;

// Interface
import {IHypercertToken} from "@hypercerts/protocol/interfaces/IHypercertToken.sol";
import {IHypercert1155Token} from "../interfaces/IHypercert1155Token.sol";

// Libraries
import {OrderStructs} from "../libraries/OrderStructs.sol";
Expand Down Expand Up @@ -64,7 +64,7 @@ contract StrategyHypercertDutchAuction is BaseStrategy {

uint256 unitsPerItemLength = unitsPerItem.length;
for (uint256 i; i < unitsPerItemLength;) {
if (IHypercertToken(makerAsk.collection).unitsOf(makerAsk.itemIds[i]) != unitsPerItem[i]) {
if (IHypercert1155Token(makerAsk.collection).unitsOf(makerAsk.itemIds[i]) != unitsPerItem[i]) {
revert OrderInvalid();
}
unchecked {
Expand Down Expand Up @@ -124,7 +124,7 @@ contract StrategyHypercertDutchAuction is BaseStrategy {

for (uint256 i; i < itemIdsLength;) {
_validateAmountNoRevert(makerAsk.amounts[i], makerAsk.collectionType);
if ((IHypercertToken(makerAsk.collection).unitsOf(makerAsk.itemIds[i]) != unitsPerItem[i])) {
if ((IHypercert1155Token(makerAsk.collection).unitsOf(makerAsk.itemIds[i]) != unitsPerItem[i])) {
return (isValid, OrderInvalid.selector);
}
unchecked {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,7 @@ import {
// Base strategy contracts
import {BaseStrategy, IStrategy} from "./BaseStrategy.sol";

interface IHypercert1155Token {
function unitsOf(uint256 tokenId) external view returns (uint256);

function ownerOf(uint256 tokenId) external view returns (address);

function isApprovedForAll(address account, address operator) external view returns (bool);
}
import {IHypercert1155Token} from "../interfaces/IHypercert1155Token.sol";

/**
* @title StrategyHypercertFractionOffer
Expand Down Expand Up @@ -182,16 +176,6 @@ contract StrategyHypercertFractionOffer is BaseStrategy {
revert AmountInvalid();
}

//TODO Apply to other HC strats
if (
makerAsk.signer != IHypercert1155Token(makerAsk.collection).ownerOf(itemIds[0])
&& !IHypercert1155Token(makerAsk.collection).isApprovedForAll(
IHypercert1155Token(makerAsk.collection).ownerOf(itemIds[0]), makerAsk.signer
)
) {
revert OrderInvalid();
}

//units, pricePerUnit, proof[]
(uint256 unitAmount, uint256 pricePerUnit, bytes32[] memory proof) =
abi.decode(takerBid.additionalParameters, (uint256, uint256, bytes32[]));
Expand Down
1 change: 0 additions & 1 deletion contracts/src/marketplace/helpers/OrderValidatorV2A.sol
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import {MerkleProofCalldataWithNodes} from "../libraries/OpenZeppelin/MerkleProo
import {ICreatorFeeManager} from "../interfaces/ICreatorFeeManager.sol";
import {IStrategy} from "../interfaces/IStrategy.sol";
import {IRoyaltyFeeRegistry} from "../interfaces/IRoyaltyFeeRegistry.sol";
import {IHypercertToken} from "@hypercerts/protocol/interfaces/IHypercertToken.sol";

// Shared errors
import {OrderInvalid, CollectionTypeInvalid} from "../errors/SharedErrors.sol";
Expand Down
3 changes: 0 additions & 3 deletions contracts/src/marketplace/helpers/ProtocolHelpers.sol
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,6 @@ import {OrderStructs} from "../libraries/OrderStructs.sol";
// Other dependencies
import {LooksRareProtocol} from "../LooksRareProtocol.sol";

// Interfaces
import {IHypercertToken} from "@hypercerts/protocol/interfaces/IHypercertToken.sol";

/**
* @title ProtocolHelpers
* @notice This contract contains helper view functions for order creation.
Expand Down
12 changes: 12 additions & 0 deletions contracts/src/marketplace/interfaces/IHypercert1155Token.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// SPDX-License-Identifier: MIT
pragma solidity 0.8.17;

interface IHypercert1155Token {
function splitFraction(address to, uint256 tokenID, uint256[] memory _values) external;

function unitsOf(uint256 tokenId) external view returns (uint256);

function ownerOf(uint256 tokenId) external view returns (address);

function isApprovedForAll(address account, address operator) external view returns (bool);
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
pragma solidity 0.8.17;

// Interfaces
import {IHypercertToken} from "../../protocol/interfaces/IHypercertToken.sol";
import {IHypercert1155Token} from "../interfaces/IHypercert1155Token.sol";

/**
* @title LowLevelHypercertCaller
Expand All @@ -27,7 +27,7 @@ contract LowLevelHypercertCaller {
revert NotAContract();
}

(bool status,) = collection.call(abi.encodeCall(IHypercertToken.splitFraction, (to, tokenId, amounts)));
(bool status,) = collection.call(abi.encodeCall(IHypercert1155Token.splitFraction, (to, tokenId, amounts)));

if (!status) {
revert HypercertSplitFractionError();
Expand Down
53 changes: 53 additions & 0 deletions contracts/test/foundry/marketplace/TransferManager.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {IOwnableTwoSteps} from "@looksrare/contracts-libs/contracts/interfaces/I

// Libraries
import {OrderStructs} from "@hypercerts/marketplace/libraries/OrderStructs.sol";
import {OrderInvalid} from "@hypercerts/marketplace/errors/SharedErrors.sol";

// Core contracts
import {LooksRareProtocol} from "@hypercerts/marketplace/LooksRareProtocol.sol";
Expand Down Expand Up @@ -670,6 +671,58 @@ contract TransferManagerTest is ITransferManager, TestHelpers, TestParameters {
transferManager.removeOperator(address(0));
}

function testCannotExecuteHypercertSplitWhenSignerNotApprovedOrOwner() public {
_allowOperator(_transferrer);
_grantApprovals(_sender);

address anon = address(666);
_grantApprovals(anon);

uint256 itemId = fractionId1Hypercert;

vm.prank(_sender);
mockHypercertMinter.mintClaim(_sender, units1Hypercert, "https://example.com/1", FROM_CREATOR_ONLY);

uint256[] memory itemIds = new uint256[](1);
itemIds[0] = itemId;
uint256[] memory amounts = new uint256[](1);
amounts[0] = units1Hypercert - 3000;

vm.prank(_transferrer);
vm.expectRevert(OrderInvalid.selector);
transferManager.splitItemsHypercert(address(mockHypercertMinter), anon, _recipient, itemIds, amounts);

assertEq(mockHypercertMinter.unitsOf(_sender, itemId), units1Hypercert);
assertEq(mockHypercertMinter.unitsOf(_recipient, itemId + 1), 0);
}

function testCanExecuteHypercertSplitWhenSignerApprovedOrOwner() public {
_allowOperator(_transferrer);
_grantApprovals(_sender);

address anon = address(666);
_grantApprovals(anon);

vm.prank(_sender);
mockHypercertMinter.setApprovalForAll(anon, true);

uint256 itemId = fractionId1Hypercert;

vm.prank(_sender);
mockHypercertMinter.mintClaim(_sender, units1Hypercert, "https://example.com/1", FROM_CREATOR_ONLY);

uint256[] memory itemIds = new uint256[](1);
itemIds[0] = itemId;
uint256[] memory amounts = new uint256[](1);
amounts[0] = units1Hypercert - 3000;

vm.prank(_transferrer);
transferManager.splitItemsHypercert(address(mockHypercertMinter), anon, _recipient, itemIds, amounts);

assertEq(mockHypercertMinter.unitsOf(_sender, itemId), 3000);
assertEq(mockHypercertMinter.unitsOf(_recipient, itemId + 1), units1Hypercert - 3000);
}

function _generateValidBatchTransferItems() private returns (BatchTransferItem[] memory items) {
items = new ITransferManager.BatchTransferItem[](3);

Expand Down

0 comments on commit b4a47f1

Please sign in to comment.