Fee distribution module

[ethanfrey]

With #232 we will not only have microscopic voluntary fees, but hopefully real turnover. Currently the fees collected in the middleware go to a hardcoded address. This address could be set to a real value (not /dev/null) in the genesis file, but the question becomes, "what address"?

Clearly the answer is my personal public key address :)

But seriously, we could conceive of some multisig that then is responsible for handling payouts according to a schedule, but that seems quite faulty. Better would be the address of a "smart contract" defined in a "distribution" module. This would collect all the fees and then have custom logic as to the distribution of the pool.

The current mvp version is a "distribution contract" that:

• Can be defined in the genesis file
• Has a unique, predicable address that can receive tokens
  • Note: this can be like multisig contract, along with the tool/README to predict the genesis addresses
• There is a special "governance" key that can control the release of these tokens
  • This will likely be a multisig contract, but any address works here given how permissioning works in weave
• There is a list of addresses that can receive tokens (referred to as recipients)
  • Note, after reflection, this is not the validator addresses, as the keys used to sign blocks should never be in a wallet... this can be the wallets of the admins of the validators
• The contract has an optional MinProportion and MaxProportion that can limit acceptable range of distribution message if present
• The contract has a UpdateThreshold (rename this), that is the maximum amount that can be held in the contract when we modify the recipient list.
• There is one message (DistributeProportionallyMsg) that accepts a list of addresses with an integer value for each. It must be approved by the "governance key". This will release all tokens held by this address proportionally to the values given the Msg.
  • We should enforce that all recipients have an entry on this Tx, even if it is 0
  • If there are defined limits (MinProportion and MaxProportion), all values must be in that range
  • Otherwise, what reasonable numbers are is left up to the "governance key" for the time being
• There is another message (UpdateRecipients) that will change the recipient list to whatever the "governance key" decides.
  • Note that the contract must have less than "UpdateThreshold" tokens undistributed, to ensure recipients are paid their outstanding share before changing them out.

If you have any more questions on this, please add them in a comment.
You can also add sample protobuf defs for the contract, and the two messages to make sure we are on the same page before starting.

[husio]

@ethanfrey Can you clarify how this functionality is expected to be working?

This is how I imagine the flow:

1. A multisig contract is created. This contract is ownerd by a group of
   "admins" that will maintain the revenue stream.
2. A Revenue instance is created. Creation of a revenue results in an address
   where collected fees are stored. Admin key must be used to sign a revenue
   distribution request.
3. ...system is running, fees are collected...
4. A DistributeMsg request is created and signed by the admin key. It
   contains a list of addresses with proportions for how to split the collected
   funds. All funds are transferred from the revenue account to requested
   destinations.
5. Go to step 3

I do not understand why do we need the MinProportion, MaxProportion and
list of recipients declared on the revenue if we expect the list of the same
addresses with corresponding weights declared within the DistributeMsg as
well.
I could understand this as a sanity control layer if not that all is controlled
by the same admin key. This mean, that the same person(s) creating
DistributeMsg can change the Revenue configuration as well.

It would make sense for me to change to one of the following:

1. Recipients with their weight are defined on the Revenue. When distributing
   funds, configuration of the revenue is used. Recipients can be updated with an
   UpdateRecipientsMsg. DistributeMsg does not contain anything except the
   revenue ID.
2. Recipients are not declared on the revenue. When DistributeMsg is
   submitted it must contain the list of recipient addresses together with the
   proportion weights.

MinWeight and MaxWeight is not needed.

Below is the protobuf declaration that I have created based on the initial description:

// Revenue represents an account with funds collected from the fees. This is a
// temporary account used for storing fees that are later distributed between
// the owners.
message Revenue {
  // Amount represents the total value stored within this revenue instance.
  x.Coin amount = 1;

  // Admin key belongs to the governance entities. It can be used to transfer
  // stored amount to an another account.
  // While not enforced it is best to use a multisig contract here.
  bytes admin = 2 [(gogoproto.customtype) = "weave.Address"];

  // Recipients holds any number of addresses that the collected revenue is
  // distributed to. This does not contain the weights required for the
  // transfer operation, but declares addresses that must be involved in each
  // funds distribution.
  repeated byte recipients = 3 [(gogoproto.customtype) = "weave.Address"];

  // MinWeight declares minimal weight value set for a single recipient allowed
  // during the funds distribution.
  // This value must not be less than zero nor greater than MaxWeight.
  int32 MinWeight = 4;

  // MaxWeight declares maximum weight value set for a single recipient allowed
  // during the funds distribution.
  // This value must not be less than MinWeight.
  int32 MaxWeight = 5;
}

// DistributeMsg is a request to distribute all funds collected within a single
// revenue instance. Revenue is distributed between recipients. Request must be
// signed using admin key.
message DistributeMsg {
  // Revenue ID reference an ID of a revenue instance that the collected fees
  // should be distributed between recipients.
  bytes revenue_id = 1 [(gogoproto.customname) = "RevenueID"];

  // Recipient contains list of recipient addresses together with their
  // weights. All recipient addresses declared in corresponding revenue entity
  // must be covered, even if the weight is zero.
  repeated Recipient recipients = 2;
}

message Recipient {
  // An address that the funds should be transferred to.
  // This should not be the validator addresses, as the keys used to sign
  // blocks should never be in a wallet. This can be the wallets of the admins
  // of the validators.
  bytes address = 1 [(gogoproto.customtype) = "weave.Address"];

  // Weight defines what part of the total revenue goes to this recipient.
  // Each recipient receives part of the total revenue amount proportional to
  // the weight. For example, if there are two recipients with weights 1 and 2
  // accordingly, distribution will be 1/3 to the first address and 2/3 to the
  // second one.
  // Weight must respect MinWight and MaxWeight declared for the revenue.
  int32 weight = 2;
}

[ethanfrey]

Hi, lots of questions there... I will answer in a few different comments, on the different points.

The desired flow is very much like you describe, but these parts must be done in genesis, as the fee collector must be set in genesis.

1. A multisig contract is created. This contract is ownerd by a group of
   "admins" that will maintain the revenue stream.
2. A Revenue instance is created. Creation of a revenue results in an address
   where collected fees are stored. Admin key must be used to sign a revenue
   distribution request.
3. The address of the Revenue instance is registered with "gconf": cash:fee_collector (or similar)

The design should allow use of "Revenue" for other cases as well, any time multiple people want to split an income stream, but main use for us is setup in genesis to serve as distribution for fee collector.

[ethanfrey]

I do not understand why do we need the MinProportion, MaxProportion and list of recipients declared on the revenue if we expect the list of the same addresses with corresponding weights declared within the DistributeMsg as well.

Yes, this is just a sanity check.

Imagine I am a validator and pledge to run for a month. I want some guarantee I will be paid for work done. This is the real requirement, that governance cannot redirect funds for past work, and if they redirect them for the future, I can stop working, as it is clear I will not be paid.

That said, I think your solution here is even better than that whole set of sanity checks I had:

Recipients with their weight are defined on the `Revenue`. When distributing
  funds, configuration of the revenue is used. Recipients can be updated with an
  `UpdateRecipientsMsg`. `DistributeMsg` does not contain anything except the
  revenue ID.

Thus, we actually commit to payout ahead of time (like a salary), and can adjust future payouts based on good/poor work.

One key point here is that I cannot execute UpdateRecipientsMsg unless I have "recently" executed DistributeMsg

[ethanfrey]

Below is the protobuf declaration that I have created based on the initial description:

Yes, this looks fine, except for

message Revenue {
  // Amount represents the total value stored within this revenue instance.
  x.Coin amount = 1;

Since Revenue only receives tokens from a Send transaction, and we don't have onSend hooks (which are powerful, but a major security hole in ethereum), the Revenue contract does not know it's balance, but should have access to the cash.Controller to query it's balance and Send the contents.

[ethanfrey]

I like your idea of Recipients with their weight are defined on the Revenue, can you make a protobuf proposal for these object, using Recipients class as now.

We can have a simple DistributeMsg, with IssueMsg and UpdateMsg having a longer list of Recipients.

Or you can combine Distribute and Update, so we can only do them at the same time.
If there is no list of recipients on DistributeMsg, then just distribute. If there is a list, then distribute then update, atomically.

I think this is simpler than my original ideas and will also add more sanity checks and controls.

Thank you for the deep thinking and feedback

[husio]

The address of the Revenue instance is registered with "gconf": cash:fee_collector (or similar)

If there will be a single place where all the revenue address is stored (ie. gconf under "cash:fee_collector"), then it does not make sense to create more than one instance of Revenue. Otherwise, multiple instances will fight for whom the funds are send to (which configuration is used).

If this package/extension (revenue distribution) is intended to be used with middleware and only a single revenue stream exists, then the revenue bucket feels a bit odd. We create a namespace for a singleton.
If we were using an SQL database then this situation is comparable to creating a table for storing only one row that always must exist.
I am wondering if this can be done better, in a way that feels more right and familiar in design.

The design should allow the use of "Revenue" for other cases as well, any time multiple people want to split an income stream, but main use for us is set up in the genesis to serve as distribution for fee collector.

This might be cool, but to achieve that each revenue instance must have a unique address. That means that gconf cannot be used to define the address.

What if a revenue was declaring an address where the funds are stored? For the middleware use case, a gconf could hold the address. For other use cases any other instance of revenue could be used. An example genesis could look like this:

{
  "feedist": [ 
     {
      "address": "addr123",
      "admin": "key123",
      "recipients": [
         {"address": "r1", "weight": 1},
         {"address": "r2", "weight": 2},
      ]
    }
   ],
  "gconf": {
     "cash:fee_collector": "addr123",
  }
}

What do you think?

[ethanfrey]

I understand your concerns.
I often look for reusable solutions, so one piece of code doesn't just solve one problem, but several. But the downside is my tendency to over-abstract.

However, let me give you a very concrete example:
In any PoS chain (and likely ours when we add more sophisticated economics), fees go to the validators running the system, but they then must distribute that among the people who bond to them. Often this is just done on off-chain trust and there are many cases (such as in Tezos) with "bakers" (validators) not paying out and keeping it.

If we use a revenue to distribute fees among validators, why can't the validator register a Revenue as the recipient as it's share, and split it among all people staking to it. If we do payouts eg. hourly, this would be simple yet good enough to build a sufficiently complex, yet trustable economy.

Of course this method really encourages your proposal of pre-registering who receives the funds.

Reflecting on my proposal above to merge Distribute and Update, I think they should be separate. With Distribute only taking an address of a Revenue and Update taking new recipient list. We could consider allowing anyone (or any recipient) to call Distribute (with a high gas cost maybe), while Update is restricted to the admin, and automatically runs Distribute before updating the list.

What do you think?

[ethanfrey]

Further on, I don't see Revenue/feedist as a middleware, but a handler that takes some events. It doesn't apply to all transactions (FeeMiddleware does), but only has 3 messages (including issue), which must be called explicitly.

As a genesis configuration, this seems roughly correct, with feedist as a list, like multisig. My only change would be to remove address and use the same technique as with multisig. The reason for this is that while we can provide a name to the feedist to use as the primary key if desired, in the permission system, addresses would look like Sha256("/dist/$name") for consistency. If we want to change how permissions (Conditions, Addresses, Auth) works that is open for discussion, but a much larger change out of scope of this PR and I would not require such a change here.

{
  "feedist": [ 
     {
      "admin": "key123",  // really 40 character hex, valid address
      "recipients": [
         {"address": "r1", "weight": 1},  // same, address is always 40 character hex
         {"address": "r2", "weight": 2},
      ]
    }
   ],
  "gconf": {
     "cash:fee_collector": "feedist1hash",  // this is a predictable but not selectable address for the above contract 
  }
}

[husio]

As a genesis configuration, this seems roughly correct, with feedist as a list, like multisig. My only change would be to remove address and use the same technique as with multisig.

That will do it. I will also provide the too for listing the first N revenue ids.

Reflecting on my proposal above to merge Distribute and Update, I think they should be separate. With Distribute only taking an address of a Revenue and Update taking new recipient list. We could consider allowing anyone (or any recipient) to call Distribute (with a high gas cost maybe), while Update is restricted to the admin, and automatically runs Distribute before updating the list.

That sounds good.

I will implement what I have in mind right now. I think this is close to your expectation for this functionality.