Skip to content

Commit

Permalink
IPT/Tokenizer: "controller" terminology leaves room for interpretation
Browse files Browse the repository at this point in the history 
updates sales distributor to acknowledge IPT controllers

Signed-off-by: stadolf <[email protected]>
  • Loading branch information
@elmariachi111
elmariachi111 committed Jul 9, 2024
1 parent 8566a19 commit daeedba
Show file tree
Hide file tree
Showing 8 changed files with 90 additions and 77 deletions.
12 changes: 6 additions & 6 deletions src/IPToken.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
}
_;
}
Expand All @@ -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();
}
Expand All @@ -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);
}
Expand Down
68 changes: 36 additions & 32 deletions src/SalesShareDistributor.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand All @@ -21,9 +23,9 @@ struct Sales {
error ListingNotFulfilled();
error ListingMismatch();
error InsufficientBalance();
error NotSalesBeneficiary();
error UncappedToken();
error OnlyIssuer();

error OnlySeller();
error NotClaimingYet();

/**
Expand Down Expand Up @@ -90,65 +92,69 @@ 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
uint256 fulfilledListingId = uint256(
keccak256(
abi.encode(
SchmackoSwap.Listing(
IERC721(address(0)), //this should be the IPNFT address
IERC721(address(tokenizer.getIPNFTContract())),
metadata.ipnftId,
_msgSender(),
paymentToken,
Expand All @@ -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 { }
Expand Down
18 changes: 11 additions & 7 deletions src/Tokenizer.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand All @@ -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();
}
_;
}
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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);
}

Expand Down
2 changes: 1 addition & 1 deletion test/CrowdSalePermissioned.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down
8 changes: 4 additions & 4 deletions test/Forking/Tokenizer13UpgradeForkTest.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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(""));
}

Expand Down Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion test/Permissioner.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand Down
41 changes: 23 additions & 18 deletions test/SalesShareDistributor.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,10 @@ import {
ListingMismatch,
NotClaimingYet,
UncappedToken,
OnlyIssuer,
InsufficientBalance
OnlySeller,
MustControlIpnft,
InsufficientBalance,
NotSalesBeneficiary
} from "../src/SalesShareDistributor.sol";

import { IPToken } from "../src/IPToken.sol";
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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();
Expand All @@ -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();

Expand All @@ -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);

Expand Down
Loading

0 comments on commit daeedba

Please sign in to comment.