ERC721PaymentSplitter Extension Contract Proposal

[hanselb]

🧐 Motivation

We are trying to find an official way to discretely/transparently distribute rewards among token holders. I am certain we can accomplish this by combining the ERC721, ERC721Enumerable, PaymentSplitter contracts. This would allow creators to share ETH or ERC-20 tokens proportionally among holders.

📝 Details I have extended ERC721Enumerable and used most of the functions of PaymentSplitter but removed payees[] and made _totalShares a function that returns the ERC721Enumerable's totalSupply (or override to maxSupply if we want to save shares for future token holders).

This allows me to add two public methods to the ERC721, release(tokenId) and release(account) to distribute payment using ownerOf(tokenId) and tokenOfOwnerByIndex(account) internally.

I also added _pendingPayment(account) and _pendingPayment(tokenId) and use it to check on before token transfer for release to owner (or leaving it for future claim by changing _releaseOnTranser to false which can be set in the constructor)

I am adding this to start the dialog while I work on cleaning my code and tests before submitting my PR. I have a working prototype of this concept here 0x68ddddc10323f8b8e3fe3514dcdb70381af12e85

[mw2000]

@hanselb have you been able to clean the code and add tests yet? If not I can go ahead and submit a PR for this.

[hanselb]

@mw2000 I have not been able to work on it too much. Please let me know if you do create a PR that way I can take a look at it 🙏

[hanselb]

@mw2000 would you benefit from me posting what I have in this thread to use as reference? I would love to collaborate and get this done as soon as we can 🙌🏽

[mw2000]

Hey @hanselb yup that could be really helpful! I haven't begun work on this yet, but this would be a good starting point.

[mw2000]

I also would want to tag @Amxx snd @frangio to this thread to hear their thoughts about it.

[hanselb]

Just got this to compile on remix but I have not tested this structure a lot. My original design didn't include the release functions for ERC20 tokens I have added those back to keep it matched in features to the current PaymentSplitter.

@mw2000 please feel free to optimize/refactor this as needed. As long as we have the same logic I don't mind making big changes to the structure of this. Or if you find any issues with my design, of course, please let me know so I can help.

I was also thinking about a model where we could create an ERC721PaymentSplitter for an existing ERC721 collection. Maybe one where we would pass the address of the collection to the constructor and we ask the questions to the contract. Let me know if you guys would benefit from that pattern too 🙌🏽

Adding @Amxx and @frangio as requested . Thank you so much guys.

// SPDX-License-Identifier: MIT
// OpenZeppelin Contracts (token/ERC721/extensions/ERC721PaymentSplitter.sol)

pragma solidity ^0.8.0;

import "@openzeppelin/contracts/token/ERC721/IERC721.sol";
import "@openzeppelin/contracts/token/ERC721/extensions/ERC721Enumerable.sol";
import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
import "@openzeppelin/contracts/utils/Address.sol";

/**
 * @dev Extension of ERC721 with the PaymentSplitter, a standardized way to split Ether and ERC20 payments
 * among a group of accounts, split can be  in equal parts and based on {totalSupply} and {ownerOf}.
 *
 * In this Extension PaymentSplitter {_totalShares} is a funciton which returns ERC721Enumerable
 * {totalSupply}, each token owner receives `1` share. {tokenOfOwnerByIndex} is used at the time of
 * release to get the current owner to release the payments.
 *
 * IMPORTANT: On transfer, any pending payment will be released to the previous owner or kept in the token.
 * This can be modified by setting property {_releaseOnTransfer} to false.
 *
 */
abstract contract ERC721PaymentSplitter is ERC721, ERC721Enumerable {
    event PaymentReleased(address to, uint256 amount);
    event ERC20PaymentReleased(IERC20 indexed token, address to, uint256 amount);
    event PaymentReceived(address from, uint256 amount);

    uint256 private _totalReleased;
    bool private _releaseOnTransfer = true;
    mapping(uint256 => uint256) private _released;

    mapping(IERC20 => uint256) private _erc20TotalReleased;
    mapping(IERC20 => mapping(uint256 => uint256)) private _erc20Released;

    function _beforeTokenTransfer(
        address from,
        address to,
        uint256 tokenId
    ) internal virtual override(ERC721, ERC721Enumerable) {
        super._beforeTokenTransfer(from, to, tokenId);
        if (from != address(0) && _pendingPayment(tokenId) > 0 && _releaseOnTransfer) {
            release(tokenId);
        }
    }

    /**
     * @dev The Ether received will be logged with {PaymentReceived} events. Note that these events are not fully
     * reliable: it's possible for a contract to receive Ether without triggering this function. This only affects the
     * reliability of the events, and not the actual splitting of Ether.
     *
     * To learn more about this see the Solidity documentation for
     * https://solidity.readthedocs.io/en/latest/contracts.html#fallback-function[fallback
     * functions].
     */
    receive() external payable virtual {
        emit PaymentReceived(_msgSender(), msg.value);
    }

    function supportsInterface(bytes4 interfaceId) public view override(ERC721, ERC721Enumerable) returns (bool) {
        return super.supportsInterface(interfaceId);
    }

    function release(IERC20 token, address payable account) public virtual {
        uint256 _ownedTokens = balanceOf(account);
        require(_ownedTokens > 0, "ERC721PaymentSplitter: account has no tokens");
        uint256 totalPayment = _pendingPayment(token, account);
        require(totalPayment > 0, "ERC721PaymentSplitter: account is not due payment");

        Address.sendValue(account, totalPayment);

        _totalReleased += totalPayment;
        for (uint256 i = 0; i < (_ownedTokens); i++) {
            uint256 currentToken = tokenOfOwnerByIndex(account, i);
            uint256 paymentForToken = _pendingPayment(token,currentToken);
            _released[currentToken] += paymentForToken;
        }
        emit PaymentReleased(account, totalPayment);
    }

    function release(address payable account) public virtual {
        uint256 _ownedTokens = balanceOf(account);
        require(_ownedTokens > 0, "ERC721PaymentSplitter: account has no tokens");
        uint256 totalPayment = _pendingPayment(account);
        require(totalPayment > 0, "ERC721PaymentSplitter: account is not due payment");

        Address.sendValue(account, totalPayment);

        _totalReleased += totalPayment;
        for (uint256 i = 0; i < (_ownedTokens); i++) {
            uint256 currentToken = tokenOfOwnerByIndex(account, i);
            uint256 paymentForToken = _pendingPayment(currentToken);
            _released[currentToken] += paymentForToken;
        }
        emit PaymentReleased(account, totalPayment);
    }

    function release(uint256 tokenId) public virtual {
        address tokenOwner = ownerOf(tokenId);
        require(tokenOwner != address(0), "ERC721PaymentSplitter: tokenId owner is the zero address");

        uint256 _tokenPendingPayment = _pendingPayment(tokenId);
        require(_tokenPendingPayment > 0, "ERC721PaymentSplitter: tokenId is not due any payments");
        Address.sendValue(payable(tokenOwner), _tokenPendingPayment);
        _totalReleased += _tokenPendingPayment;
        _released[tokenId] += _tokenPendingPayment;
        emit PaymentReleased(tokenOwner, _tokenPendingPayment);
    }

    /**
     * @dev Triggers a transfer to `account` of the amount of `token` tokens they are owed, according to their
     * percentage of the total shares and their previous withdrawals. `token` must be the address of an IERC20
     * contract.
     */
    function release(IERC20 token, uint256 tokenId) public virtual {
        address tokenOwner = ownerOf(tokenId);
        require(tokenOwner != address(0), "ERC721PaymentSplitter: tokenId owner is the zero address");

        uint256 payment = _pendingPayment(token, tokenId);

        uint256 _tokenPendingPayment = _pendingPayment(token, tokenId);
        require(_tokenPendingPayment > 0, "ERC721PaymentSplitter: tokenId is not due any payments");

        _erc20Released[token][tokenId] += payment;
        _erc20TotalReleased[token] += payment;

        SafeERC20.safeTransfer(token, tokenOwner, payment);
        emit ERC20PaymentReleased(token, tokenOwner, payment);
    }

    function _pendingPayment(IERC20 token, uint256 tokenId) private view returns (uint256) {
        return (token.balanceOf(address(this)) + totalReleased(token)) / totalSupply() - released(token, tokenId);
    }

    function _pendingPayment(uint256 tokenId) private view returns (uint256) {
        return (address(this).balance + totalReleased()) / totalSupply() - _released[tokenId];
    }

    function _pendingPayment(address account) private view returns (uint256) {
        uint256 totalPayment = 0;
        uint256 _ownedTokens = balanceOf(account);

        for (uint256 i = 0; i < (_ownedTokens); i++) {
            uint256 currentToken = tokenOfOwnerByIndex(account, i);
            uint256 paymentForToken = _pendingPayment(currentToken);
            totalPayment += paymentForToken;
        }
        return totalPayment;
    }

    function _pendingPayment(IERC20 token, address account) private view returns (uint256) {
        uint256 totalPayment = 0;
        uint256 _ownedTokens = balanceOf(account);

        for (uint256 i = 0; i < (_ownedTokens); i++) {
            uint256 currentToken = tokenOfOwnerByIndex(account, i);
            uint256 paymentForToken = _pendingPayment(token, currentToken);
            totalPayment += paymentForToken;
        }
        return totalPayment;
    }

    function totalReleased() public view returns (uint256) {
        return _totalReleased;
    }

    /**
     * @dev Getter for the amount of `token` tokens already released to a payee. `token` should be the address of an
     * IERC20 contract.
     */
    function released(IERC20 token, uint256 tokenId) public view returns (uint256) {
        return _erc20Released[token][tokenId];
    }

    /**
     * @dev Getter for the total amount of `token` already released. `token` should be the address of an IERC20
     * contract.
     */
    function totalReleased(IERC20 token) public view returns (uint256) {
        return _erc20TotalReleased[token];
    }
}

[hanselb]

using it

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.4;

import "./ERC721PaymentSplitter.sol";

contract MyToken is ERC721PaymentSplitter {
    constructor() ERC721("MyToken", "MTK") {}   
}

[Amxx]

Hello,

We usually try to separate concerns and avoid code duplication. What you are doing doesn't really fit into this philosophy. Also, this code relies on unbounded loops (which is always a big no to us) and on ERC721Enumerable (which we discourage people from using).

If you want to look into a "cleaner" approach, I worked on an AbstractPaymentSplitter a few month ago. It would be very easy to take the code in TokenizedSplitter and to adapt it to use with an ERC721 core.

... we don't plan on publishing that soon, but if we ever want to provide that code, we will go with the more modular/light solution.

Be aware that this code is not audited. We don't recommend using it in production without a deep review first.

[hanselb]

Hi @Amxx

I’m trying to solve a problem that most creators/artist have out here which is giving back to their community. At a high level how would you design this and make it available for mass adoption?

I would love to work with you guys to come up with a clean design for this.

I would also love to know why you guys don’t find this needed to be made public to help people. There is so much dirtier code out there to accomplish this that I felt compelled to submit this. I believe it is a crucial thing to solve.

The more transparent and elegant solution, the better, but I would love to not push this on the back burner if possible. I believe some kind of standard pattern would allow people to deliver splits on their tokens for their community without using platforms like 0xSplits etc which is another point of trust and it’s picking up momentum.

[hanselb]

I’m afraid my understanding is limited to be able surgically pull from the abstract payment splitter @Amxx provided. I just started solidity programming this year so I am still learning.

@mw2000 are you be able to combine these ideas for a clean design? I would love to get your input.

[hanselb]

The problem I had using the original PaymentSplitter is that you cannot rely on mapping(address => uint256) private _released; because tokenId needs to be the main tracker (since owner can change)

I had to change to mapping(uint256 => uint256) private _released; for tokenId => amount.

[Amxx]

The problem I had using the original PaymentSplitter is that you cannot rely on mapping(address => uint256) private _released; because tokenId needs to be the main tracker (since owner can change)

I had to change to mapping(uint256 => uint256) private _released; for tokenId => amount.

This has the downside that, if a user owns 1000 tokens, he must do 1000 claims (one per token). This would be super expensive. You can keep a mapping(address => uint256) private _released; if you rebalance token transfers with virtual releases. This is what I do in the AbstractPaymentSplitter linked above.

[hanselb]

Hi @Amxx thank you for replying. I look some more through the AbstractPaymentSplitter and I see that you use Distribution. AddressToIntWithTotal to handle the releases and adjustments. From what I can understand, this is still based on address being the main axis for the PaymentSplitter. Is the default behavior to transfer anything owed to previous owner on transfer? How can we ensure that multiple owners for the same token don't drain the contract? how do we keep track of what has been released per token? Does the contract ensures that every token is empty at the point of transfer?

Using the unbounded loops inside release(account) is to cover a single owner that wants to cash out from multiple tokens, making them pay for this made sense to me.

[Amxx]

From what I can understand, this is still based on address being the main axis for the PaymentSplitter.

Yes, this allows a user to release the funds corresponding to alls its token in a single operation. This was designed to work with ERC20 balances, but it would also work with ERC721 balances.

how do we keep track of what has been released per token?

When a transfer (of token) happens, a hook must be triggered that creates "virtual" release to compensate in case the payment asset has not been released.

[hanselb]

I understand now. Thank you so much @Amxx for the explanation, I can see now why you made those tradeoffs. This has given me much to think about while I come up with better designs.