Temp-account market fee sharing issue

[abitmore]

Bug Description
Many BitShares Mainnet accounts have committee-account as registrar and temp-account as referrer. The accounts were migrated from BitShares-0.x chain. When they trading assets that enabled market fee sharing, committee-account and temp-account will receive the shared market fees. However, temp-account is authorityless, which means everyone can transfer out the fund. This is not good UX.

I think we should redirect the shared fees to committee-account. And maybe update the accounts' referrer to committee-account.

@OpenLedgerApp

Impacts
Describe which portion(s) of BitShares Core may be impacted by this bug. Please tick at least one box.

• Protocol (the blockchain logic, consensus, validation, etc.)
• UX (the user experience, etc.)

CORE TEAM TASK LIST

• Evaluate / Prioritize Bug Report
• Refine User Stories / Requirements
• Define Test Cases
• Design / Develop Solution
• Perform QA/Testing
• Update Documentation

[OpenLedgerApp]

We have implemented this BSIP. So, we want to keep it on us.

[ryanRfox]

@OpenLedgerApp please advance this.

[OpenLedgerApp]

It seems a bit different as described above:
398 accounts have temp-account as registrar and committee-account as referrer.

As we can see in deposit_cashback(), neither registrar nor referrer gets cashback.

if( acct.get_id() == GRAPHENE_COMMITTEE_ACCOUNT || acct.get_id() == GRAPHENE_WITNESS_ACCOUNT ||
       acct.get_id() == GRAPHENE_RELAXED_COMMITTEE_ACCOUNT || acct.get_id() == GRAPHENE_NULL_ACCOUNT ||
       acct.get_id() == GRAPHENE_TEMP_ACCOUNT )
{
    // The blockchain's accounts do not get cashback; it simply goes to the reserve pool.
    modify( get_core_dynamic_data(), [amount](asset_dynamic_data_object& d) {
       d.current_supply -= amount;
    });
    return;
}

We propose to modify pay_market_fees() method and exclude blockchain's accounts (committee, witness, relaxed-committee, null and temp) from market fee sharing. So, all market fees accumulated in asset_dynamic_data_object and available to asset owner. Of course, this logic must be wrapped with hardfork

[abitmore]

The comment reads "The blockchain's accounts do not get cashback; it simply goes to the reserve pool". The beneficiary is still the network (or say the public / the community).

When talking about market fee sharing, IMHO the network should get the share. So my opinion is:

I think we should redirect the shared fees to committee-account. And maybe update the accounts' referrer to committee-account.

Of course, asset owners can setup white-lists to exclude committee-account.

[OpenLedgerApp]

@abitmore, thanks for your feedback.

Let’s see what think other Core member and community here.
@pmconrad, @oxarbitrage, @sschiessl-bcp

[sschiessl-bcp]

I agree. If market fee sharing is set, the network should benefit, i.e. everything goes to committee-account.

I would suggest to adjust

// The blockchain's accounts do not get cashback; it simply goes to the reserve pool.

to

// The blockchain's accounts get forwarded to committee-account if null of temp account

[pmconrad]

I'm not even sure if this qualifies as a bug. I agree it is not nice that the temp account could receive market fee payouts, but it's not like this would break anything. So IMO any change of the payout logic must either conform with the original intent of the feature, or be approved by community vote.

Since blockchain accounts are not explicitly excluded from market fee sharing in BSIP-43 we can't really modify the logic to exclude them.

Perhaps the most simple way to fix this would be to make committee_account the registrar of genesis accounts, like abit has suggested. AFAICS it's always been temp_account but I can't see a specific reason for this choice. I think using committee_account instead would be functionally equivalent in all respects except market fee sharing, and perhaps in referral statistics.

[sschiessl-bcp]

Please provide more background information what has happened.

Currently you have tracked 884fce4 and d5076ee with 170 lines of code changed, which are invoiced each 24 working hours. Could you add task and time tracking tables of the people involved for each invoice. Right now it's hard for me to do the usual spot checks on the work invoiced.

Thoughts?

[OpenLedgerApp]

Our team didn't face with temp-accounts previously. So, we needed to spend time on investigations to find a solution. As part of this issue, we used an elastic search mechanisms for various parts.
First of all, we needed to find how many temp-accounts are.
The second step we have analyzed how the fees are allocated for all accounts registered by temp-account.
Also, we needed to find out why temp-accounts don't receive deposit_cashback.
Finally, we have developed a solution based on our research.

[abitmore]

Fixed via #1870 and #2133.