Create withdrawal SharedRoyaltyToken interface

[barlock]

Overview

As a developer implementing a shared royalty token, I want to know I'm using best practices so that I don't break users (increasing gas costs) and ensure security (no reentrancy)

Reference

• Reentrancy topic
• Common Patterns

Questions

• Are there any known ERC standards for contract withdrawals?
• Can we calculate balance at withdrawal time?
• How do we handle rounding errors?

Acceptance

• Documented contract withdrawal pattern is written as a contract interface with NatSpec documentation (here)
• Documented questions are answered

Tasks

• [ ] Research existing ERCs and prior art
• [ ] Document results and proposed interface

[barlock]

Notes:

OpenZeppelin has an escrow contract that we can/should use to handle the escrow for the token. We can expose it's api via ours.

Rounding errors doesn't really seem to be an issue (??) as the math is handled in wei. Viewing OpenZeppelin's Payment Splitter, they don't make any special cases for handling it.

For understanding, if we can calculate balance at withdrawal time:

I wrote a tiny script to run a test scenario of splitting every payment by the number of franchisors. The gas cost per transaction follows this formula (except index 0, which spikes a bit) gasUsed = 7781 * franchisorIndex + 63135

There is a max gas limit around 8mil, that means our token can be sold a max number of ~1,000 times. The max cost in tx fees (assuming current USD market value and a gas price of 4Gwei) is ~$3.80.

☝️ That's not as bad as I was guessing. It's not amazing if we're assuming we could have a single digital asset (an album) that gets distributed via everyone who's owned it with the rights to sell an unlimited number of times (ie, a single token representing a single album).

I'm going to take another think about HMW make transfer's a fixed gas price while withdrawals increase with the number of transactions or amount that can be withdrawn.

Contract to test payment model

pragma solidity ^0.5.3; import 'openzeppelin-solidity/contracts/math/SafeMath.sol'; contract SharedRoyaltyToken { using SafeMath for uint256; event Deposited(address indexed payee, uint256 weiAmount); event Withdrawn(address indexed payee, uint256 weiAmount); mapping(address => uint256) private _deposits; address[] public franchisors; function makePayment() public payable { franchisors.push(msg.sender); uint256 payout = msg.value.div(franchisors.length); for (uint i=0; i

Contract test runner

const expect = require('jest-matchers'); const SharedRoyaltyToken = artifacts.require('SharedRoyaltyToken'); contract('SharedRoyaltyToken', accounts => { const accountOne = accounts[0]; const accountTwo = accounts[1]; let token; beforeEach(async () => { token = await SharedRoyaltyToken.deployed(); }); it('should put 10000 MetaCoin in the first account', async () => { let count = 0; await new Array(100).fill(1).reduce(async prev => { const result = await prev; if (result) { console.log(count + ', ' + result.receipt.gasUsed); count += 1; } return token.makePayment({ value: 1, from: accountOne }); }, Promise.resolve()); expect(true).toEqual(true); }); });

[barlock]

@breakpointer What do you think about my assumptions here?

[brian-lc]

Seem solid to me; rounding errors handled, escrow is a nice simple approach here, 7 million is plenty of headroom for transactions on one asset. 👍

[barlock]

Bah, I did the math wrong, max of 1020. Editing original post.

[barlock]

I'm not looking forward to implementing this. Here's hoping some other lucky person gets to!

The payment proposed above does payment calculation and distribution at transfer time, this means gas prices for transfer are linear. Ideally, it would be fixed.

Some operation that can be broken up needs to be linear so that it's possible to have (virtually) unlimited franchisors, and all franchisors can get all of the Eth they're owed. What I believe this to mean is that the only place to do the expensive calculations is at withdraw-time and withdraw needs to be able to limit the number of withdrawals by the gas expense.

I can see two different apis to do this, one more ux friendly, the other more dev friendly. I'll document below, bear in mind that for both, the following new things must be true.

• Withdrawals happen per 721, you cannot withdraw all earnings from all tokens at once, at least at the 721 level. (upstream contracts could iterate over all of them)
• Every token keeps track of a stream of payments, it's own transaction history uint[] payments
• Withdrawals will need to keep track of which payments have been withdrawn for each franchisor to ensure no double payments
• each "style" will need to implement a paymentBalanceOf(tokenId, franchisor, paymentStart, paymentEnd) function that, for any token franchisor, and range of payments, calculates what that franchisor is owed. This will become complicated with cycles of franchisors (think a franchisor selling the token to themselves)

The two different withdrawl patterns are:

• (Better for devs) withdrawals are paged by the user with a count that starts where they last withdrew from, and goes to the end. If they give too high of a count, it will revert and they'll need to try again
• (Better for users) withdraw transfers the most amount it can given the gas provided. returns true if done, or false if there is still more to withdraw. This will require the function to know how much gas calculating a balance is and can use gasLeft() to make sure it has enough to send the eth after. This seems tricky and I think the top one is better for a first pass at the very least as the balance calculation could vary a lot and might not be linear.

I'm assuming a "real" app would be able to streamline this whole withdrawal thing so the user never sees it anyway. Interface coming next.

[barlock]

The interface! A few notes. The "escrow" contract linked in the previous comment no longer applies. I still like the pattern and a custom one could be used in an implementation (a SharedRoyaltyEscrow anyone?) but that shouldn't affect the exposed abi.

Without further ado:

  /**
   * @notice Withdraws payment accumulated from transfer of a given token from
   *  the last withdrawn payment up to a _count

   * @param _tokenId The identifier for an NFT
   * @param _count The number of payments to traverse
   */
  function withdrawPayment(uint256 _tokenId, uint256 _count) external;

  /**
   * @notice Withdraws non-withdrawn payment accumulated from transfer of a
   *   given token
   * @dev Escrow should keep track of the payments that have been processed to
   *   avoid double withdrawals
   *
   * @param _tokenId The identifier for an NFT
   */
  function withdrawPayment(uint256 _tokenId) external;

  /**
   * @notice Gets balance of non-withdrawn payment accumulated from transfer of a
   *   given token up to a number of provided payments
   * @dev Used by `withdrawPayment` to calculate how much a franchisor is owed.
   * @dev Implement this function to
   *
   * @param _tokenId The identifier for an NFT
   * @param _start The payment index to start from
   * @param _count The number of payments to traverse
   */
  function paymentBalanceOf(uint256 _tokenId, uint256 _start, uint256 _count) public;

  /**
   * @notice Gets balance of non-withdrawn payment accumulated from transfer of a
   *   given token that has not been withdrawn
   *
   * @dev Defaults to overloaded `withdrawPayment` of a token's non-withdrawn balance
   *   to the last payment
   * @dev Used by `withdrawPayment` to calculate how much a franchisor is owed.
   *
   * @param _tokenId The identifier for an NFT
   */
  function paymentBalanceOf(uint256 _tokenId) public;

[brian-lc]

@barlock WRT "withdrawals are paged" Could you provide an example of the data structure you have in mind here? I thought we could have a simple balance (single float) to store what a franchisor would be owed at any given time. During the transfer transaction when eth is distributed we would be crediting those balances with a fixed rate * sale price.

[barlock]

I thought we could have a simple balance (single float) to store what a franchisor would be owed at any given time.

That was my original attempt and a clean solution. With that approach, you need to calculate the balance when a transfer happens. Given our "brain dead simple" model of equal royalties (the code in the original post), it limited the number of transfers to 1k. I don't believe that to be large enough for our purposes.

My second attempt, the one here, tries to perform that calculation at withdraw time rather than deposit time. The reason is that a withdraw can be broken a part into several transactions, where a transfer cannot. A rough attempt in code below, the data structure for the escrow will likely change but hopefully it will get the idea across.

struct Token {
  uint256[] payments;
  address[] franchisors;
  mapping(address => uint256) franchisorLastWithdrawIndex;
}

mapping (uint256 => Token) public tokens;

function transfer (uint256 _tokenId, address payable _newOwner) {
  Token token = tokens[_tokenId];

  token.franchisors.push(_newOwner);
  token.payments.push(msg.value);
}

function withdraw (uint256 _tokenId, uint256 _count) {
  // Payment balance of is implemented per style, so bookened would calculate this 
  // sender's balance (I realized now that maybe the api should specifiy which payee, need to check prior art) 
  // based on the `payments` array, start, and count
  uint256 withdrawBalance = this.paymentBalanceOf(
    _tokenId, 
    token.franchisorLastWithdrawIndex[msg.sender], 
    _count
  );

  msg.sender.transfer(withdrawBalance);
}

function withdraw (uint256 _tokenId) {
  Token token = tokens[_tokenId];

  this.withdraw(_tokenId, token.payments.length); 
}