search

bitshares / bsips

BitShares Improvement Proposals and Protocols. These technical documents describe the process of updating and improving the BitShares blockchain and technical ecosystem.
https://bitshares.github.io
63 stars 86 forks source link

BSIP67: Dynamic Market Fees #133

Open OpenLedgerApp opened 5 years ago

OpenLedgerApp commented 5 years ago

Dear BitShares Community,

We would like to introduce the Dynamic Market Fees BSIP. The purpose is to support high-volume trading and market makers. Thus making it more profitable for them. We believe it will bring more people to BitShares.

As per BitShares Core Team request we have spent some time drafting this BSIP. And here's the resulting BSIP: https://github.com/openledger/bsips/blob/bsip-dynamicmarketfees/bsip-00XX%20Dynamic%20market%20fees.md

The pull request is here:

133

Forum thread is here: https://bitsharestalk.org/index.php?topic=27589.0 Please share your opinion. If you think it might help BitShares, please voice your opinion and vote for it, when the worker is created.

Sincerely, Denis Sokolov OpenLedger

ryanRfox commented 5 years ago

I worked with @OpenLedgerApp to establish a drafting budget for this BSIP of 10 hours. It will be paid in weeks 50-51.

Revisions will continue within this budget.

pmconrad commented 5 years ago

Please squash this PR and use a sensible commit message.

pmconrad commented 5 years ago

This proposal introduces a significant cost in terms of CPU and RAM for all node operators. IMO this needs to be balanced against the potential benefits. I agree that this might incentivize market makers, but it is unclear if this can justify the significant cost.

OpenLedgerApp commented 5 years ago

That's a good point! We have thought about it. We are planning to check and monitor performance KPIs at the beginning of implementation. It will allow us to make sure CPU and RAM are tracked. If we face some issues and we don't have any turnaround, it will be indicated ASAP.

ryanRfox commented 5 years ago

This proposal introduces a significant cost in terms of CPU and RAM for all node operators. IMO this needs to be balanced against the potential benefits. I agree that this might incentivize market makers, but it is unclear if this can justify the significant cost.

@sschiessl-bcp has suggested elsewhere that we add a Risks section to the BSIP Template. @pmconrad may I request your craft your comment above into a paragraph highlighting the potential risk(s) you foresee? My desire is the identified risks will be added to the PR by @OpenLedgerApp

pmconrad commented 5 years ago

Suggestion:

Risks

CPU Overhead

The proposed fee calculations will have to be performed on every trade. This adds significant burden on top of the already quite heavyweight market engine. While this is probably not a big problem with today's transaction volume, it will further limit how far our chain scales in terms of tx/s.

RAM Overhead

The proposed calculations require logging the trade volume of all traders over a period of time, separately for each traded asset. With both the number of users and the number of assets growing over time, the required amount of RAM will grow squarely over time. This does not scale well.

ryanRfox commented 5 years ago

Thank you @pmconrad for adding the Risks section. May I request @OpenLedgerApp update the PR to include the Risks just prior to the Summary for Shareholders?

ryanRfox commented 5 years ago

My thoughts on calculation impacts/mitigation:

  1. I now understand why performing the calculation on every order would be problematic. Can we calculate a static table hourly, daily so the market can just lookup the fee?
  2. I now understand that storing complete order history for a month to use in calculation will consume substantial RAM. Can we mitigate this by calculating it into a volume average per hour, day?
  3. What other options can be explored to reduce the scaling impacts within this specification?
  4. I feel dynamic market fees are a valuable addition to the protocol and may provide an incentive for further adoption, so hope we continue to debate this topic.
OpenLedgerApp commented 5 years ago

@pmconrad Thanks for the input! we will add this section to the BSIP description. @ryanRfox we are analyzing your thoughts and will provide our feedback. In any case, we should think how we can have some metrics in advance (or at the beginning of implementation) to decrease RISKS of this BSIP.

pmconrad commented 5 years ago
  1. I now understand why performing the calculation on every order would be problematic. Can we calculate a static table hourly, daily so the market can just lookup the fee?

We must update the trade volume per user and asset for every trade.

  1. I now understand that storing complete order history for a month to use in calculation will consume substantial RAM. Can we mitigate this by calculating it into a volume average per hour, day?

This is already described in the section "30-day volume method". It requires two values per user and asset.

sschiessl-bcp commented 5 years ago

I would very much like this BSIP in two steps. First, introduce maker-taker, then maybe dynamic fees like indicated above.

Limit order would have fixed fee plus market fee, both for maker and taker. But, when filled

Thoughts?

xeroc commented 5 years ago

I would very much like this BSIP in two steps. First, introduce maker-taker, then maybe dynamic fees like indicated above.

This! "Keep it simple stupid" and do one thing at a time. Also, a BSIP that proposes only maker/taker fees will be much easier to get approved than something that introduces it together with something that adds significant additional burden to the chain

OpenLedgerApp commented 5 years ago

We have made a prototype in order to measure performance

As you can see in the picture attached, our worker has no significant influence on BitShares performance: image

For more details please refer the prototype. Here is the link - https://github.com/openledger/bitshares-core/commits/issue-dmf

abitmore commented 5 years ago

@OpenLedgerApp try test with 1M users? and around 1K trading pairs (although there can be more)?

OpenLedgerApp commented 5 years ago

@OpenLedgerApp try test with 1M users? and around 1K trading pairs (although there can be more)?

There is a result: dmf-perf-result

The sources are located - https://github.com/openledger/bitshares-core/commits/issue-dmf

ryanRfox commented 5 years ago

Sorry, I'm not understanding the graph. What is the X-axis representing? It seems like there are both red and blue plots for about 2/3 of the data, but only blue when the value jump. What accounts for the jump? Are there red values as well?

pmconrad commented 5 years ago

AFAICS your test creates 1 M users but only 1000 traders, and it trades only 1 asset pair.

OpenLedgerApp commented 5 years ago

AFAICS your test creates 1 M users but only 1000 traders, and it trades only 1 asset pair.

We have made test with 100 asset pair.

As you can see in the picture attached, our worker has no significant influence on BitShares performance:

image

sources are located - https://github.com/openledger/bitshares-core/commits/issue-dmf

pmconrad commented 5 years ago

Now you have 100 assets and 1000 traders, but each trader is using only 1 or 2 assets. Like abit said, please repeat your test with 1M traders and 1k trading pairs where each trader trades in a significant portion of these pairs.

Also, please explain your graphics, as Ryan has requested. I don't understand them either.

OpenLedgerApp commented 5 years ago

We will update our test in the near future.

OpenLedgerApp commented 5 years ago

Now you have 100 assets and 1000 traders, but each trader is using only 1 or 2 assets. Like abit said, please repeat your test with 1M traders and 1k trading pairs where each trader trades in a significant portion of these pairs.

We have modified the tests. There are 100 traders and 100 assets where each trader trades 100 assets. We have got the following results: Origin bitshares takes 2095465 milliseconds, and with DMF logic 2099182 milliseconds. You are welcome to change tests' parameters and rerun them: // How many traders to create const static auto traders_count = 100; // Count of cycles for trading const static auto iterations = 1; // How many assets to sell/buy const static auto uia_to_sell = 20; // How many assets to create const static auto ui_assets_count = 100;

New sources are located https://github.com/openledger/bitshares-core/commit/e4f9f44da756ef60a69cb6529a56c976a4362cc4

Also, please explain your graphics, as Ryan has requested. I don't understand them either.

Red line shows how much time origin bitshares take and an other line shows how much time the same test takes with DMF logic.

abitmore commented 5 years ago

@ryanRfox please assign BSIP number.

OpenLedgerApp commented 5 years ago

We would like to put our BSIP on voting. The prototype is ready for voting. We have finished all test and made Pull Request. There are some worries about computation resources. That why, we have made several performance tests, which showed good results.

What should we do to put this BSIP on voting?

@ryanRfox please let us know. Thanks in advance!

abitmore commented 5 years ago

@OpenLedgerApp please update the document and rename the file with the assigned BSIP number.

sschiessl-bcp commented 5 years ago

Please address all comments, and update your document.

Also I am still wondering if only maker/taker differentiation should be an option as well when voting as the simple version.