diff --git a/src/IPToken.sol b/src/IPToken.sol index ecab5dd9..c4c6aed4 100644 --- a/src/IPToken.sol +++ b/src/IPToken.sol @@ -5,7 +5,7 @@ import { ERC20BurnableUpgradeable } from "@openzeppelin/contracts-upgradeable/to import { OwnableUpgradeable } from "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol"; import { Strings } from "@openzeppelin/contracts/utils/Strings.sol"; import { Base64 } from "@openzeppelin/contracts/utils/Base64.sol"; -import { Tokenizer, MustOwnIpnft } from "./Tokenizer.sol"; +import { Tokenizer, MustControlIpnft } from "./Tokenizer.sol"; struct Metadata { uint256 ipnftId; @@ -47,9 +47,9 @@ contract IPToken is ERC20BurnableUpgradeable, OwnableUpgradeable { _disableInitializers(); } - modifier onlyTokenizerOrIPNFTHolder() { - if (_msgSender() != owner() && _msgSender() != Tokenizer(owner()).ownerOf(_metadata.ipnftId)) { - revert MustOwnIpnft(); + modifier onlyTokenizerOrIPNFTController() { + if (_msgSender() != owner() && _msgSender() != Tokenizer(owner()).controllerOf(_metadata.ipnftId)) { + revert MustControlIpnft(); } _; } @@ -63,7 +63,7 @@ contract IPToken is ERC20BurnableUpgradeable, OwnableUpgradeable { * @param receiver address * @param amount uint256 */ - function issue(address receiver, uint256 amount) external onlyTokenizerOrIPNFTHolder { + function issue(address receiver, uint256 amount) external onlyTokenizerOrIPNFTController { if (capped) { revert TokenCapped(); } @@ -74,7 +74,7 @@ contract IPToken is ERC20BurnableUpgradeable, OwnableUpgradeable { /** * @notice mark this token as capped. After calling this, no new tokens can be `issue`d */ - function cap() external onlyTokenizerOrIPNFTHolder { + function cap() external onlyTokenizerOrIPNFTController { capped = true; emit Capped(totalIssued); } diff --git a/src/SalesShareDistributor.sol b/src/SalesShareDistributor.sol index 4377d8bf..16208780 100644 --- a/src/SalesShareDistributor.sol +++ b/src/SalesShareDistributor.sol @@ -7,7 +7,9 @@ import { UUPSUpgradeable } from "@openzeppelin/contracts-upgradeable/proxy/utils import { OwnableUpgradeable } from "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol"; import { ReentrancyGuardUpgradeable } from "@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.sol"; import { SafeERC20 } from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; +import { IPNFT } from "./IPNFT.sol"; import { IPToken, Metadata } from "./IPToken.sol"; +import { Tokenizer, MustControlIpnft } from "./Tokenizer.sol"; import { SchmackoSwap, ListingState } from "./SchmackoSwap.sol"; import { IPermissioner, TermsAcceptedPermissioner } from "./Permissioner.sol"; @@ -21,9 +23,9 @@ struct Sales { error ListingNotFulfilled(); error ListingMismatch(); error InsufficientBalance(); +error NotSalesBeneficiary(); error UncappedToken(); -error OnlyIssuer(); - +error OnlySeller(); error NotClaimingYet(); /** @@ -90,57 +92,61 @@ contract SalesShareDistributor is UUPSUpgradeable, OwnableUpgradeable, Reentranc } /** - * @notice anyone should be able to call this function after having observed the sale - * rn we restrict it to the original owner since they must provide a permissioner that controls the claiming rules - * this is a deep dependency on our own sales contract + * @notice release sales shares for a Schmackoswap transaction + * @dev todo: *anyone* should be able to call this function after having observed the sale; right now we restrict it to the creator of the trade since they were in control of the IPNFT before + * @dev this has a deep dependency on our own swap contract * - * @param tokenContract IPToken the tokenContract of the IPToken + * @param ipt IPToken the tokenContract of the IPToken * @param listingId uint256 the listing id on Schmackoswap * @param permissioner IPermissioner the permissioner that permits claims */ - function afterSale(IPToken tokenContract, uint256 listingId, IPermissioner permissioner) external { - Metadata memory metadata = tokenContract.metadata(); - //todo: this should be the *former* holder of the IPNFT, not the 1st owner :) - if (_msgSender() != metadata.originalOwner) { - revert OnlyIssuer(); - } - - (, uint256 ipnftId,, IERC20 _paymentToken, uint256 askPrice, address beneficiary, ListingState listingState) = + function afterSale(IPToken ipt, uint256 listingId, IPermissioner permissioner) external { + (, uint256 ipnftId, address seller, IERC20 _paymentToken, uint256 askPrice, address beneficiary, ListingState listingState) = schmackoSwap.listings(listingId); + if (_msgSender() != seller) { + revert OnlySeller(); + } + if (listingState != ListingState.FULFILLED) { revert ListingNotFulfilled(); } + Metadata memory metadata = ipt.metadata(); if (ipnftId != metadata.ipnftId) { revert ListingMismatch(); } if (beneficiary != address(this)) { - revert InsufficientBalance(); + revert NotSalesBeneficiary(); } - _startClaimingPhase(tokenContract, listingId, _paymentToken, askPrice, permissioner); + _startClaimingPhase(ipt, listingId, _paymentToken, askPrice, permissioner); } //audit: ensure that no one can withdraw arbitrary amounts here //by simply creating a new IPToken instance and claim an arbitrary value - + //todo: try breaking this by providing a fake IPT with a fake Tokenizer owner + //todo: this must be called by the beneficiary of a sale we don't control. /** * @notice When the sales beneficiary has not been set to the underlying erc20 token address but to the original owner's wallet instead, - * they can invoke this method to start the claiming phase manually. This e.g. allows sales off the record. + * they can invoke this method to start the claiming phase manually. This e.g. allows sales off the record ("OpenSea"). * - * Requires the originalOwner to behave honestly / in favor of the molecules holders - * Requires the caller to have approved `price` of `paymentToken` to this contract + * Requires the originalOwner to behave honestly / in favor of the IPT holders + * Requires the caller to have approved `paidPrice` of `paymentToken` to this contract * * @param tokenContract IPToken the IPToken token contract * @param paymentToken IERC20 the payment token contract address - * @param paidPrice uint256 the price the NFT has been sold for - * @param permissioner IPermissioner the permissioner that permits claims + * @param paidPrice uint256 the price the NFT has been sold for + * @param permissioner IPermissioner the permissioner that permits claims */ - function afterSale(IPToken tokenContract, IERC20 paymentToken, uint256 paidPrice, IPermissioner permissioner) external nonReentrant { + function UNSAFE_afterSale(IPToken tokenContract, IERC20 paymentToken, uint256 paidPrice, IPermissioner permissioner) external nonReentrant { Metadata memory metadata = tokenContract.metadata(); - //todo: this should be the *former* holder of the IPNFT, not the 1st owner :) + + Tokenizer tokenizer = Tokenizer(tokenContract.owner()); + + //todo: this should be a selected beneficiary of the IPNFT's sales proceeds, and not the original owner :) + //idea is to allow *several* sales proceeds to be notified here, create unique sales ids for each and let users claim the all of them at once if (_msgSender() != metadata.originalOwner) { - revert OnlyIssuer(); + revert MustControlIpnft(); } //create a fake (but valid) schmackoswap listing id @@ -148,7 +154,7 @@ contract SalesShareDistributor is UUPSUpgradeable, OwnableUpgradeable, Reentranc keccak256( abi.encode( SchmackoSwap.Listing( - IERC721(address(0)), //this should be the IPNFT address + IERC721(address(tokenizer.getIPNFTContract())), metadata.ipnftId, _msgSender(), paymentToken, @@ -164,15 +170,13 @@ contract SalesShareDistributor is UUPSUpgradeable, OwnableUpgradeable, Reentranc paymentToken.safeTransferFrom(_msgSender(), address(this), paidPrice); } - function _startClaimingPhase(IPToken tokenContract, uint256 fulfilledListingId, IERC20 _paymentToken, uint256 price, IPermissioner permissioner) - internal - { - //todo: this actually must be enforced before a sale starts + function _startClaimingPhase(IPToken ipt, uint256 fulfilledListingId, IERC20 _paymentToken, uint256 price, IPermissioner permissioner) internal { + //todo: this *should* be enforced before a sale starts // if (!tokenContract.capped()) { // revert UncappedToken(); // } - sales[address(tokenContract)] = Sales(fulfilledListingId, _paymentToken, price, permissioner); - emit SalesActivated(address(tokenContract), address(_paymentToken), price); + sales[address(ipt)] = Sales(fulfilledListingId, _paymentToken, price, permissioner); + emit SalesActivated(address(ipt), address(_paymentToken), price); } function _authorizeUpgrade(address newImplementation) internal override onlyOwner { } diff --git a/src/Tokenizer.sol b/src/Tokenizer.sol index 2c16c7e9..b8ab3934 100644 --- a/src/Tokenizer.sol +++ b/src/Tokenizer.sol @@ -9,7 +9,7 @@ import { IPToken, Metadata as TokenMetadata } from "./IPToken.sol"; import { IPermissioner } from "./Permissioner.sol"; import { IPNFT } from "./IPNFT.sol"; -error MustOwnIpnft(); +error MustControlIpnft(); error AlreadyTokenized(); error ZeroAddress(); error IPTNotControlledByTokenizer(); @@ -64,6 +64,10 @@ contract Tokenizer is UUPSUpgradeable, OwnableUpgradeable { _disableInitializers(); } + function getIPNFTContract() public view returns (IPNFT) { + return ipnft; + } + //todo: try breaking this with a faked IPToken modifier onlyControlledIPTs(IPToken ipToken) { TokenMetadata memory metadata = ipToken.metadata(); @@ -72,8 +76,8 @@ contract Tokenizer is UUPSUpgradeable, OwnableUpgradeable { revert IPTNotControlledByTokenizer(); } - if (_msgSender() != ipnft.ownerOf(metadata.ipnftId)) { - revert MustOwnIpnft(); + if (_msgSender() != controllerOf(metadata.ipnftId)) { + revert MustControlIpnft(); } _; } @@ -121,8 +125,8 @@ contract Tokenizer is UUPSUpgradeable, OwnableUpgradeable { string memory agreementCid, bytes calldata signedAgreement ) external returns (IPToken token) { - if (ipnft.ownerOf(ipnftId) != _msgSender()) { - revert MustOwnIpnft(); + if (_msgSender() != controllerOf(ipnftId)) { + revert MustControlIpnft(); } if (address(synthesized[ipnftId]) != address(0)) { revert AlreadyTokenized(); @@ -170,8 +174,8 @@ contract Tokenizer is UUPSUpgradeable, OwnableUpgradeable { ipToken.cap(); } - /// @dev this will be called by IPTs to avoid handing over yet another IPNFT address (they already know this Tokenizer contract as their owner) - function ownerOf(uint256 ipnftId) external view returns (address) { + /// @dev this will be called by IPTs. Right now the controller is the IPNFT's current owner, it can be a Governor in the future. + function controllerOf(uint256 ipnftId) public view returns (address) { return ipnft.ownerOf(ipnftId); } diff --git a/test/CrowdSalePermissioned.t.sol b/test/CrowdSalePermissioned.t.sol index 05c96484..86fb14b0 100644 --- a/test/CrowdSalePermissioned.t.sol +++ b/test/CrowdSalePermissioned.t.sol @@ -13,7 +13,7 @@ import { IPToken, Metadata } from "../src/IPToken.sol"; import { CrowdSale, Sale, SaleInfo, SaleState, BadDecimals } from "../src/crowdsale/CrowdSale.sol"; import { StakedLockingCrowdSale, BadPrice } from "../src/crowdsale/StakedLockingCrowdSale.sol"; import { IPermissioner, TermsAcceptedPermissioner, InvalidSignature, BlindPermissioner } from "../src/Permissioner.sol"; -import { MustOwnIpnft, AlreadyTokenized, Tokenizer, ZeroAddress } from "../src/Tokenizer.sol"; +import { MustControlIpnft, AlreadyTokenized, Tokenizer, ZeroAddress } from "../src/Tokenizer.sol"; import { IPNFT } from "../src/IPNFT.sol"; import { TokenVesting } from "@moleculeprotocol/token-vesting/TokenVesting.sol"; import { TimelockedToken } from "../src/TimelockedToken.sol"; diff --git a/test/Forking/Tokenizer13UpgradeForkTest.t.sol b/test/Forking/Tokenizer13UpgradeForkTest.t.sol index e257910a..15fccfa4 100644 --- a/test/Forking/Tokenizer13UpgradeForkTest.t.sol +++ b/test/Forking/Tokenizer13UpgradeForkTest.t.sol @@ -8,7 +8,7 @@ import { SafeERC20Upgradeable } from "@openzeppelin/contracts-upgradeable/token/ import { IPNFT } from "../../src/IPNFT.sol"; -import { MustOwnIpnft, AlreadyTokenized, Tokenizer } from "../../src/Tokenizer.sol"; +import { MustControlIpnft, AlreadyTokenized, Tokenizer } from "../../src/Tokenizer.sol"; import { Tokenizer12 } from "../../src/helpers/test-upgrades/Tokenizer12.sol"; import { IPToken12, OnlyIssuerOrOwner } from "../../src/helpers/test-upgrades/IPToken12.sol"; import { IPToken, TokenCapped, Metadata } from "../../src/IPToken.sol"; @@ -96,7 +96,7 @@ contract Tokenizer13UpgradeForkTest is Test { tokenizer13.tokenizeIpnft(2, 1_000_000 ether, "VITA-FAST", "bafkreig274nfj7srmtnb5wd5wlwm3ig2s63wovlz7i3noodjlfz2tm3n5q", bytes("")); vm.startPrank(alice); - vm.expectRevert(MustOwnIpnft.selector); + vm.expectRevert(MustControlIpnft.selector); tokenizer13.tokenizeIpnft(2, 1_000_000 ether, "VITA-FAST", "bafkreig274nfj7srmtnb5wd5wlwm3ig2s63wovlz7i3noodjlfz2tm3n5q", bytes("")); } @@ -223,9 +223,9 @@ contract Tokenizer13UpgradeForkTest is Test { assertEq(vitaFAST13.balanceOf(bob), 1_000_000 ether); // but they cannot do that using the tokenizer: - vm.expectRevert(MustOwnIpnft.selector); + vm.expectRevert(MustControlIpnft.selector); tokenizer13.issue(vitaFAST13, 1_000_000 ether, alice); - vm.expectRevert(MustOwnIpnft.selector); + vm.expectRevert(MustControlIpnft.selector); tokenizer13.cap(vitaFAST13); //but they unfortunately also can cap the token: diff --git a/test/Permissioner.t.sol b/test/Permissioner.t.sol index ab26782b..b5400a0e 100644 --- a/test/Permissioner.t.sol +++ b/test/Permissioner.t.sol @@ -12,7 +12,7 @@ import { IPNFT } from "../src/IPNFT.sol"; import { Safe } from "safe-global/safe-contracts/Safe.sol"; import { SafeProxyFactory } from "safe-global/safe-contracts/proxies/SafeProxyFactory.sol"; import { Enum } from "safe-global/safe-contracts/common/Enum.sol"; -import { MustOwnIpnft, AlreadyTokenized, Tokenizer, ZeroAddress } from "../src/Tokenizer.sol"; +import { MustControlIpnft, AlreadyTokenized, Tokenizer, ZeroAddress } from "../src/Tokenizer.sol"; import "./helpers/MakeGnosisWallet.sol"; diff --git a/test/SalesShareDistributor.t.sol b/test/SalesShareDistributor.t.sol index c645b007..1085acbf 100644 --- a/test/SalesShareDistributor.t.sol +++ b/test/SalesShareDistributor.t.sol @@ -24,8 +24,10 @@ import { ListingMismatch, NotClaimingYet, UncappedToken, - OnlyIssuer, - InsufficientBalance + OnlySeller, + MustControlIpnft, + InsufficientBalance, + NotSalesBeneficiary } from "../src/SalesShareDistributor.sol"; import { IPToken } from "../src/IPToken.sol"; @@ -160,17 +162,17 @@ contract SalesShareDistributorTest is Test { vm.startPrank(ipnftBuyer); erc20.transfer(originalOwner, 1_000_000 ether); - // only the owner can manually start the claiming phase. + // only the origina owner can atm manually start the claiming phase. vm.startPrank(bob); - vm.expectRevert(OnlyIssuer.selector); - distributor.afterSale(tokenContract, erc20, 1_000_000 ether, blindPermissioner); + vm.expectRevert(MustControlIpnft.selector); + distributor.UNSAFE_afterSale(tokenContract, erc20, 1_000_000 ether, blindPermissioner); vm.startPrank(originalOwner); vm.expectRevert(); // not approved - distributor.afterSale(tokenContract, erc20, 1_000_000 ether, blindPermissioner); + distributor.UNSAFE_afterSale(tokenContract, erc20, 1_000_000 ether, blindPermissioner); erc20.approve(address(distributor), 1_000_000 ether); - distributor.afterSale(tokenContract, erc20, 1_000_000 ether, blindPermissioner); + distributor.UNSAFE_afterSale(tokenContract, erc20, 1_000_000 ether, blindPermissioner); assertEq(erc20.balanceOf(address(originalOwner)), 0); (uint256 fulfilledListingId,,,) = distributor.sales(address(tokenContract)); @@ -204,7 +206,7 @@ contract SalesShareDistributorTest is Test { vm.startPrank(originalOwner); erc20.approve(address(distributor), 1_000_000 ether); - distributor.afterSale(tokenContract, erc20, 1_000_000 ether, permissioner); + distributor.UNSAFE_afterSale(tokenContract, erc20, 1_000_000 ether, permissioner); vm.startPrank(alice); (, uint256 amount) = distributor.claimableTokens(tokenContract, alice); @@ -303,11 +305,14 @@ contract SalesShareDistributorTest is Test { vm.startPrank(originalOwner); erc20.approve(address(distributor), salesPrice); - distributor.afterSale(tokenContract, erc20, salesPrice, blindPermissioner); + distributor.UNSAFE_afterSale(tokenContract, erc20, salesPrice, blindPermissioner); vm.stopPrank(); } function testClaimingFraud() public { + address anotherOwner = makeAddr("anotherOwner"); + address unknown = makeAddr("unknown"); + vm.startPrank(originalOwner); IPToken tokenContract1 = tokenizer.tokenizeIpnft(1, 100_000, "MOLE", agreementCid, ""); tokenContract1.cap(); @@ -316,14 +321,14 @@ contract SalesShareDistributorTest is Test { uint256 listingId1 = schmackoSwap.list(IERC721(address(ipnft)), 1, erc20, 1000 ether, address(distributor)); schmackoSwap.changeBuyerAllowance(listingId1, ipnftBuyer, true); - vm.startPrank(bob); - vm.deal(bob, MINTING_FEE); + vm.startPrank(anotherOwner); + vm.deal(anotherOwner, MINTING_FEE); uint256 reservationId = ipnft.reserve(); - ipnft.mintReservation{ value: MINTING_FEE }(bob, reservationId, ipfsUri, DEFAULT_SYMBOL, ""); + ipnft.mintReservation{ value: MINTING_FEE }(anotherOwner, reservationId, ipfsUri, DEFAULT_SYMBOL, ""); ipnft.setApprovalForAll(address(schmackoSwap), true); IPToken tokenContract2 = tokenizer.tokenizeIpnft(2, 70_000, "MOLE", agreementCid, ""); tokenContract2.cap(); - uint256 listingId2 = schmackoSwap.list(IERC721(address(ipnft)), 2, erc20, 700 ether, address(originalOwner)); + uint256 listingId2 = schmackoSwap.list(IERC721(address(ipnft)), 2, erc20, 700 ether, unknown); schmackoSwap.changeBuyerAllowance(listingId2, ipnftBuyer, true); vm.stopPrank(); @@ -332,16 +337,16 @@ contract SalesShareDistributorTest is Test { schmackoSwap.fulfill(listingId1); schmackoSwap.fulfill(listingId2); - vm.startPrank(bob); - vm.expectRevert(InsufficientBalance.selector); + vm.startPrank(anotherOwner); + vm.expectRevert(NotSalesBeneficiary.selector); distributor.afterSale(tokenContract2, listingId2, blindPermissioner); - vm.startPrank(originalOwner); vm.expectRevert(ListingMismatch.selector); distributor.afterSale(tokenContract1, listingId2, blindPermissioner); - vm.expectRevert(OnlyIssuer.selector); - distributor.afterSale(tokenContract2, listingId2, blindPermissioner); + vm.startPrank(originalOwner); + vm.expectRevert(OnlySeller.selector); + distributor.afterSale(tokenContract1, listingId2, blindPermissioner); distributor.afterSale(tokenContract1, listingId1, blindPermissioner); diff --git a/test/Tokenizer.t.sol b/test/Tokenizer.t.sol index faec0a39..b33dc276 100644 --- a/test/Tokenizer.t.sol +++ b/test/Tokenizer.t.sol @@ -17,7 +17,7 @@ import { IPNFT } from "../src/IPNFT.sol"; import { AcceptAllAuthorizer } from "./helpers/AcceptAllAuthorizer.sol"; import { FakeERC20 } from "../src/helpers/FakeERC20.sol"; -import { MustOwnIpnft, AlreadyTokenized, Tokenizer, ZeroAddress } from "../src/Tokenizer.sol"; +import { MustControlIpnft, AlreadyTokenized, Tokenizer, ZeroAddress } from "../src/Tokenizer.sol"; import { IPToken, TokenCapped } from "../src/IPToken.sol"; import { Molecules } from "../src/helpers/test-upgrades/Molecules.sol"; @@ -132,14 +132,14 @@ contract TokenizerTest is Test { tokenizer.issue(tokenContract, 50_000, originalOwner); vm.startPrank(bob); - vm.expectRevert(MustOwnIpnft.selector); + vm.expectRevert(MustControlIpnft.selector); tokenContract.issue(bob, 12345); - vm.expectRevert(MustOwnIpnft.selector); + vm.expectRevert(MustControlIpnft.selector); tokenizer.issue(tokenContract, 12345, bob); - vm.expectRevert(MustOwnIpnft.selector); + vm.expectRevert(MustControlIpnft.selector); tokenContract.cap(); - vm.expectRevert(MustOwnIpnft.selector); + vm.expectRevert(MustControlIpnft.selector); tokenizer.cap(tokenContract); assertEq(tokenContract.balanceOf(alice), 25_000); @@ -170,10 +170,10 @@ contract TokenizerTest is Test { //the original owner *cannot* issue tokens anymore //this actually worked before 1.3 since IPTs were bound to their original owner vm.startPrank(originalOwner); - vm.expectRevert(MustOwnIpnft.selector); + vm.expectRevert(MustControlIpnft.selector); tokenContract.issue(alice, 50_000); - vm.expectRevert(MustOwnIpnft.selector); + vm.expectRevert(MustControlIpnft.selector); tokenizer.issue(tokenContract, 50_000, bob); } @@ -189,7 +189,7 @@ contract TokenizerTest is Test { function testCannotTokenizeIfNotOwner() public { vm.startPrank(alice); - vm.expectRevert(MustOwnIpnft.selector); + vm.expectRevert(MustControlIpnft.selector); tokenizer.tokenizeIpnft(1, 100_000, "IPT", agreementCid, ""); vm.stopPrank(); }