Author Topic: Percentage based transfer fee [BSIP10] implemented  (Read 26854 times)

0 Members and 1 Guest are viewing this topic.

Offline svk

I don't foresee this needing a lot of GUI work, a day at most (a day being 8 hours). As long as there's a flag on the asset that says what type of fee to use, and only transfers are affected, it should be trivial to add a switch to whichever transfer operation type that asset uses.

Fees need a little bit of work but not a whole lot, assuming the percent fee structure is easily available either in the asset object or in global parameters. We estimate fees using the global parameters and the operation type, so that would need to be extended to handle percent fees but that's not hard. And final operation fees are fetched from the witness node so that should not require any work on the GUI.
@svk Imo here is a list of work to be done in GUI:
* add an selection box on the "create asset" page and "update asset" page, so the issuer can set fee mode easily
* show transfer fee mode on asset details page, and maybe somewhere related
* use new operation to do all transfers (although the original operation can still be used on assets with flat mode)
* extend fee estimation to handle percent fees.
* the fee schedule page

Did I miss something?

I think Jakub will provide a document about data structure changes and API changes. Or you can dig into my code (after CNX reviewed)

Thanks for support!

OK.

How is the fee mode switched? Does it use the existing flags and permissions structure?
Worker: dev.bitsharesblocks

Offline abit

  • Committee member
  • Hero Member
  • *
  • Posts: 4664
    • View Profile
    • Abit's Hive Blog
  • BitShares: abit
  • GitHub: abitmore
To @xeroc, @cube and others who are testing or going to test my code:

I'm about to add an extension to the fee structure for easier future extension. After this change, something in the test chain will become incompatible. If replay doesn't work, need to reset the chain to a state before the first propose_fee_change after applied my previous change.

I will post here when the work is done.

I won't add hard fork logic for this change since it's too much work and not very useful imo.

Sorry for the inconvenience, and thanks for support!
BitShares committee member: abit
BitShares witness: in.abit

Offline abit

  • Committee member
  • Hero Member
  • *
  • Posts: 4664
    • View Profile
    • Abit's Hive Blog
  • BitShares: abit
  • GitHub: abitmore
I don't foresee this needing a lot of GUI work, a day at most (a day being 8 hours). As long as there's a flag on the asset that says what type of fee to use, and only transfers are affected, it should be trivial to add a switch to whichever transfer operation type that asset uses.

Fees need a little bit of work but not a whole lot, assuming the percent fee structure is easily available either in the asset object or in global parameters. We estimate fees using the global parameters and the operation type, so that would need to be extended to handle percent fees but that's not hard. And final operation fees are fetched from the witness node so that should not require any work on the GUI.
@svk Imo here is a list of work to be done in GUI:
* add an selection box on the "create asset" page and "update asset" page, so the issuer can set fee mode easily
* show transfer fee mode on asset details page, and maybe somewhere related
* use new operation to do all transfers (although the original operation can still be used on assets with flat mode)
* extend fee estimation to handle percent fees.
* the fee schedule page

Did I miss something?

I think Jakub will provide a document about data structure changes and API changes. Or you can dig into my code (after CNX reviewed)

Thanks for support!
BitShares committee member: abit
BitShares witness: in.abit

jakub

  • Guest
It is not key. The key is that we need to keep the rules stable. So people can have time to understand it.
We should not change key rules so frequency. We need more hard work and time to prove the system and the community is reliable. And that is why ETH and NEM price grow 3 times without any rule or feature change.

This proposal does not change the existing rules,  it just an opt-in feature decided by issuers.
Those who don't like it can choose to do nothing and the current flat transfer fees will still apply.

Offline xiahui135

  • Sr. Member
  • ****
  • Posts: 496
    • View Profile
It is not key. The key is that we need to keep the rules stable. So people can have time to understand it.
We should not change key rules so frequency. We need more hard work and time to prove the system and the community is reliable. And that is why ETH and NEM price grow 3 times without any rule or feature change.

Offline abit

  • Committee member
  • Hero Member
  • *
  • Posts: 4664
    • View Profile
    • Abit's Hive Blog
  • BitShares: abit
  • GitHub: abitmore
@abit ,
just to make sure: does your implementation cover all assets (including UIAs and FBAs) or just smart-coins (public & private)?
All assets except BTS. I have no idea about FBAs so far.
BitShares committee member: abit
BitShares witness: in.abit

jakub

  • Guest
@abit ,
just to make sure: does your implementation cover all assets (including UIAs and FBAs) or just smart-coins (public & private)?

jakub

  • Guest
According to the code, CER is a member variable of asset_update_operation, so it's required when creating such operation. For whatever reason, it has to be valid. And the asset will be updated according to the operation when the operation executes. It's current behavior/feature.

Maybe we can change the logic here:
* don't force CER to be valid when updating an asset
* if it's valid, change CER of the asset as the operation requested
* if it's invalid, don't change CER of the asset

I'll check the code to figure out whether it's possible to make it this way. Or maybe need input from CNX about why it's designed like this and whether it's proper to change the behavior.

If it's OK to change the behavior, since it's not included in BSIP10, we may need to add it. Wish it's not too complex so that I needn't to charge more for developing this feature.

I understand the above but still I don't see the reason why you need to store the CER value in the committee proposal instead of retrieving its current value directly from the asset whenever it's needed to calculate a transfer fee.

In other words, why the algorithm cannot work like this?
(1) A new transfer for asset X is about to be sent,
(2) Retrieve from the blockchain the current value of CER for asset X,
(3) Use this value to calculate the transfer fee,
(4) Charge the sender with this fee.

Is there some ambiguity about what the value of CER for asset X is at any given point in time?
Well, the committee proposal is to "enable percentage based fee" for a committee-owned asset for example bitUSD. The real operation in the committee proposal is "asset_update_operation". CER is REQUIRED to be included in "asset_update_operation", which is current feature/behavior/limitation. In short, you have to manually set/change CER to enable percentage based fee. I proposed to change this behavior in last post, but I won't make this change unless it's included in BSIP.

Once percentage based fee is enabled for bitUSD, when a transfer is requested by a user, the fee would be calculated according to the algorithm you write above.

Anyway, after manually set CER with asset_update_operation, CER would be corrected maybe after one price feed, so perhaps it's not a big deal to leave the feature unchanged.

OK, I agree that it's not a big deal at this stage.
If it becomes problematic, we can address it in the future.

Offline abit

  • Committee member
  • Hero Member
  • *
  • Posts: 4664
    • View Profile
    • Abit's Hive Blog
  • BitShares: abit
  • GitHub: abitmore
According to the code, CER is a member variable of asset_update_operation, so it's required when creating such operation. For whatever reason, it has to be valid. And the asset will be updated according to the operation when the operation executes. It's current behavior/feature.

Maybe we can change the logic here:
* don't force CER to be valid when updating an asset
* if it's valid, change CER of the asset as the operation requested
* if it's invalid, don't change CER of the asset

I'll check the code to figure out whether it's possible to make it this way. Or maybe need input from CNX about why it's designed like this and whether it's proper to change the behavior.

If it's OK to change the behavior, since it's not included in BSIP10, we may need to add it. Wish it's not too complex so that I needn't to charge more for developing this feature.

I understand the above but still I don't see the reason why you need to store the CER value in the committee proposal instead of retrieving its current value directly from the asset whenever it's needed to calculate a transfer fee.

In other words, why the algorithm cannot work like this?
(1) A new transfer for asset X is about to be sent,
(2) Retrieve from the blockchain the current value of CER for asset X,
(3) Use this value to calculate the transfer fee,
(4) Charge the sender with this fee.

Is there some ambiguity about what the value of CER for asset X is at any given point in time?
Well, the committee proposal is to "enable percentage based fee" for a committee-owned asset for example bitUSD. The real operation in the committee proposal is "asset_update_operation". CER is REQUIRED to be included in "asset_update_operation", which is current feature/behavior/limitation. In short, you have to manually set/change CER to enable percentage based fee. I proposed to change this behavior in last post, but I won't make this change unless it's included in BSIP.

Once percentage based fee is enabled for bitUSD, when a transfer is requested by a user, the fee would be calculated according to the algorithm you write above.

Anyway, after manually set CER with asset_update_operation, CER would be corrected maybe after one price feed, so perhaps it's not a big deal to leave the feature unchanged.
BitShares committee member: abit
BitShares witness: in.abit

Offline cube

  • Hero Member
  • *****
  • Posts: 1404
  • Bit by bit, we will get there!
    • View Profile
  • BitShares: bitcube
Could you please include 'documentation for developer' into your proposal?  This is much needed if we need another developer to take over and maintain or expand the developed codes.

Actually, from the very beginning, when abit offered to implement this BSIP,  it was my intention (and part of the "deal" with abit) to make documentation an important part of the worker proposal.
As abit has already declared, he is not very keen on writing detailed descriptions so I offered to team up with him and take on this task.
My offer is based on the assumption that abit will be willing to share with me his know-how and help me with understanding all the nuances and decisions he's made when doing the coding.

So there are going to be two tiers of documentation:
(1) in-code comments by abit
(2) a detailed documentation/tutorial, written by me, explaining the whole context, i.e. some kind of guide for developers willing to follow in abit's footsteps


I am really glad we are moving towards better and richer set of documentation.  I appreciate your effort and drive towards this goal.
ID: bitcube
bitcube is a dedicated witness and committe member. Please vote for bitcube.

jakub

  • Guest
According to the code, CER is a member variable of asset_update_operation, so it's required when creating such operation. For whatever reason, it has to be valid. And the asset will be updated according to the operation when the operation executes. It's current behavior/feature.

Maybe we can change the logic here:
* don't force CER to be valid when updating an asset
* if it's valid, change CER of the asset as the operation requested
* if it's invalid, don't change CER of the asset

I'll check the code to figure out whether it's possible to make it this way. Or maybe need input from CNX about why it's designed like this and whether it's proper to change the behavior.

If it's OK to change the behavior, since it's not included in BSIP10, we may need to add it. Wish it's not too complex so that I needn't to charge more for developing this feature.

I understand the above but still I don't see the reason why you need to store the CER value in the committee proposal instead of retrieving its current value directly from the asset whenever it's needed to calculate a transfer fee.

In other words, why the algorithm cannot work like this?
(1) A new transfer for asset X is about to be sent,
(2) Retrieve from the blockchain the current value of CER for asset X,
(3) Use this value to calculate the transfer fee,
(4) Charge the sender with this fee.

Is there some ambiguity about what the value of CER for asset X is at any given point in time?

Offline abit

  • Committee member
  • Hero Member
  • *
  • Posts: 4664
    • View Profile
    • Abit's Hive Blog
  • BitShares: abit
  • GitHub: abitmore
To change the transfer_fee_mode of a committee-member owned asset(BitUSD and etc), we need to propose a committee proposal, and committee members vote on it. A valid core_exchange_rate is required in the proposal. That's it.

Let me know if understand it correctly:
(1) If the committee wants to switch a given smart-coin to percentage-based fee model, it has to create a committee proposal and this proposal has to contain a certain fixed value for CER.
(2) After the committee proposal is accepted, it might happen that the committee decides to modify the actual CER and thus create a discrepancy between the value of CER embedded in the committee proposal and the current value of CER.

Is this correct?
Correct.

Quote
If so, what is the reason that we need to embed the value of CER in the committee proposal and cannot use its current value when calculating a transfer fee?
According to the code, CER is a member variable of asset_update_operation, so it's required when creating such operation. For whatever reason, it has to be valid. And the asset will be updated according to the operation when the operation executes. It's current behavior/feature.

Maybe we can change the logic here:
* don't force CER to be valid when updating an asset
* if it's valid, change CER of the asset as the operation requested
* if it's invalid, don't change CER of the asset

I'll check the code to figure out whether it's possible to make it this way. Or maybe need input from CNX about why it's designed like this and whether it's proper to change the behavior.

If it's OK to change the behavior, since it's not included in BSIP10, we may need to add it. Wish it's not too complex so that I needn't to charge more for developing this feature.
BitShares committee member: abit
BitShares witness: in.abit

jakub

  • Guest
To change the transfer_fee_mode of a committee-member owned asset(BitUSD and etc), we need to propose a committee proposal, and committee members vote on it. A valid core_exchange_rate is required in the proposal. That's it.

Let me know if understand it correctly:
(1) If the committee wants to switch a given smart-coin to percentage-based fee model, it has to create a committee proposal and this proposal has to contain a certain fixed value for CER.
(2) After the committee proposal is accepted, it might happen that the committee decides to modify the actual CER and thus create a discrepancy between the value of CER embedded in the committee proposal and the current value of CER.

Is this correct?
If so, what is the reason that we need to embed the value of CER in the committee proposal and cannot use its current value when calculating a transfer fee?

jakub

  • Guest
Could you please include 'documentation for developer' into your proposal?  This is much needed if we need another developer to take over and maintain or expand the developed codes.

@cube
Actually, from the very beginning, when @abit offered to implement this BSIP,  it was my intention (and part of the "deal" with abit) to make documentation an important part of the worker proposal.
As abit has already declared, he is not very keen on writing detailed descriptions so I offered to team up with him and take on this task.
My offer is based on the assumption that abit will be willing to share with me his know-how and help me with understanding all the nuances and decisions he's made when doing the coding.

So there are going to be two tiers of documentation:
(1) in-code comments by abit
(2) a detailed documentation/tutorial, written by me, explaining the whole context, i.e. some kind of guide for developers willing to follow in abit's footsteps

This is a fragment of my PM sent to abit earlier on today:

I wrote to abit:
Quote
Ideally, we could "sell" the worker proposal as a double benefit for the ecosystem:
- for the blockchain: a new functionality that opens up BitShares to tips and micro-payments
- for the whole ecosystem: a detailed description how to implement stuff without relying on CNX so that other coders & entrepreneurs can follow our example

To which he replied:
Quote
Regarding to docs, if you're going to write it, of course I will help you.
« Last Edit: January 22, 2016, 04:48:44 pm by jakub »

Offline abit

  • Committee member
  • Hero Member
  • *
  • Posts: 4664
    • View Profile
    • Abit's Hive Blog
  • BitShares: abit
  • GitHub: abitmore

The 3M BTS I proposed is for "my work", which includes coding of "witness_node" and "cli_wallet", as well as "my" testing in one or more private networks and/or public networks.
...

Could you please include 'documentation for developer' into your proposal?  This is much needed if we need another developer to take over and maintain or expand the developed codes.
The word "documentation" is unbounded. My in-code document level would be no less than what we already have right now. To be honest I'm not good at writing documents, sorry. If the one who wants to "take over and maintain or expand" the codes is willing to write a document, I'm more than glad to help.
« Last Edit: January 22, 2016, 02:18:59 pm by abit »
BitShares committee member: abit
BitShares witness: in.abit