PaymentSplitter + ERC20

[cybertelx]

🧐 Motivation
PaymentSplitter (with Ether) already exists, so why not ERC20PaymentSplitter? Plus, the finance area probably needs more contracts.

📝 Details
PaymentSplitter (in contracts/finance), but with ERC20 support. Payees can withdraw any ERC20 token sent to the contract corresponding to their share in it, as long as they provide the address to the token, which should be checked with EIP 165.

[Amxx]

Hello @cybertelx, and thank you for this issue.

This is something we have on our roadmap, and for which I have some "work-in-progress" code:
https://github.com/Amxx/openzeppelin-contracts/tree/feature/paymentsplitter/contracts/finance

Keep in mind that this code has not gone through our testing and review process.

I'll share you interest, and we will possibly put that on the feature-set for an uncomming version.

[sabotagebeats]

@Amxx can you explain the usage for these work in progress code? Which contract needs to be deployed to become the recipient for WETH? unfortunately opensea forces the acceptance of weth which breaks paymentsplitter.

[Amxx]

THE PR linked above is pretty much WIP, I wouldn't recommend anyone using it right now. I'm sorry but we don't have an ERC20 payment splitter that I can just point you right now.

I can however confirm that this is on our roadmap, and will hopefully be addressed soon!

[sabotagebeats]

THE PR linked above is pretty much WIP, I wouldn't recommend anyone using it right now. I'm sorry but we don't have an ERC20 payment splitter that I can just point you right now.

I can however confirm that this is on our roadmap, and will hopefully be addressed soon!

yes the problem is that if weth tokens are sent by opensea they will be lost. there is no way to rescue them from the current payment splitter.

[Amxx]

if the payment splitter is not upgradeable, then NO, there are no way to retrieve it.

Not that is has always been clear in the PaymentSplitter that it is for base ETH ONLY.

Please do not use it in a context where ERC20 might be sent to it.

[sabotagebeats]

Please do not use it in a context where ERC20 might be sent to it.

yes however, i think if i can add this code to the end of paymentspliitter then it should be able to rescue the erc20 tokens.

    using SafeERC20 for IERC20;
    /**
     * additional code to rescue erc20 tokens 
     */
    function tokenRescue(IERC20 token) public {
        require(shares(msg.sender) > 0, "not a shareholder");
        uint256 amount = token.balanceOf(address(this));
        require(amount > 0, "no tokens to rescue");
        uint i;
        for (i = 0; i < _payees.length; i++) {
            tokenTransfer(token, payee(i),shares(payee(i)) * amount / totalShares());
        }
    }
    function tokenTransfer(IERC20 token, address recipient, uint256 amount) private {
        token.safeTransfer(recipient, amount);
    }

[splintred]

I understand that the pull request for this feature has not audited, but I want to be able to use this contract in a project that I'm working on now. What would be the safest way to do this? I'm using it to split royalty payments, so I'm currently thinking that I could deploy it separately and store a reference tot its address in my main contract. Then, if vulnerabilities are found, I could update the address in my main contract to the new version. Is this safe/advisable?

[frangio]

@splintred The mechanism that you describe sounds like a reasonable mitigation. I can't say whether this is safe as there are many other factors involved. If you want you can share more details over at our forum for further discussion.