New limits algorithm for comments and votes #533 #825 #951

maslenitsa93 · 2018-09-10T22:10:33Z

Checklists:

Use testnet.

Run daemon.
Wait for hardfork 19.
Run following script for comments:

./cli_wallet --server-rpc-endpoint=ws://127.0.0.1:8091
set_password qwer
unlock qwer
import_key 5JVFFWRLwz6JoP9kguuRFfytToGU6cLgBVTL9t6NB3D3BQLbUBS
create_account cyberfounder test "{}" "30.000 GOLOS" true
transfer_to_vesting cyberfounder test "30.000 GOLOS" true
post_comment  test test "" test hello world "{}" true
post_comment  test test20 test test hello world "{}" true
post_comment  test test21 test test hello world "{}" true
post_comment  test test22 test test hello world "{}" true
post_comment  test test23 test test hello world "{}" true
post_comment  test test24 test test hello world "{}" true
post_comment  test test25 test test hello world "{}" true
post_comment  test test26 test test hello world "{}" true
post_comment  test test27 test test hello world "{}" true
post_comment  test test28 test test hello world "{}" true
post_comment  test test29 test test hello world "{}" true
post_comment  test test30 test test hello world "{}" true

Or this one for votes:

./cli_wallet --server-rpc-endpoint=ws://127.0.0.1:8091
set_password qwer
unlock qwer
import_key 5JVFFWRLwz6JoP9kguuRFfytToGU6cLgBVTL9t6NB3D3BQLbUBS
create_account cyberfounder test "{}" "30.000 GOLOS" true
transfer_to_vesting cyberfounder test "30.000 GOLOS" true
create_account cyberfounder test20 "{}" "30.000 GOLOS" true
transfer_to_vesting cyberfounder test20 "30.000 GOLOS" true
create_account cyberfounder test21 "{}" "30.000 GOLOS" true
transfer_to_vesting cyberfounder test21 "30.000 GOLOS" true
create_account cyberfounder test22 "{}" "30.000 GOLOS" true
transfer_to_vesting cyberfounder test22 "30.000 GOLOS" true
create_account cyberfounder test23 "{}" "30.000 GOLOS" true
transfer_to_vesting cyberfounder test23 "30.000 GOLOS" true
create_account cyberfounder test24 "{}" "30.000 GOLOS" true
transfer_to_vesting cyberfounder test24 "30.000 GOLOS" true
create_account cyberfounder test25 "{}" "30.000 GOLOS" true
transfer_to_vesting cyberfounder test25 "30.000 GOLOS" true
create_account cyberfounder test26 "{}" "30.000 GOLOS" true
transfer_to_vesting cyberfounder test26 "30.000 GOLOS" true
create_account cyberfounder test27 "{}" "30.000 GOLOS" true
transfer_to_vesting cyberfounder test27 "30.000 GOLOS" true
create_account cyberfounder test28 "{}" "30.000 GOLOS" true
transfer_to_vesting cyberfounder test28 "30.000 GOLOS" true
create_account cyberfounder test29 "{}" "30.000 GOLOS" true
transfer_to_vesting cyberfounder test29 "30.000 GOLOS" true
create_account cyberfounder test30 "{}" "30.000 GOLOS" true
transfer_to_vesting cyberfounder test30 "30.000 GOLOS" true
post_comment  test20 test20 "" test hello world "{}" true
post_comment  test21 test21 "" test hello world "{}" true
post_comment  test22 test22 "" test hello world "{}" true
post_comment  test23 test23 "" test hello world "{}" true
post_comment  test24 test24 "" test hello world "{}" true
post_comment  test25 test25 "" test hello world "{}" true
post_comment  test26 test26 "" test hello world "{}" true
post_comment  test27 test27 "" test hello world "{}" true
post_comment  test28 test28 "" test hello world "{}" true
post_comment  test29 test29 "" test hello world "{}" true
post_comment  test30 test30 "" test hello world "{}" true
vote test test20 test20 10 true
vote test test21 test21 10 true
vote test test22 test22 10 true
vote test test23 test23 10 true
vote test test24 test24 10 true
vote test test25 test25 10 true
vote test test26 test26 10 true
vote test test27 test27 10 true
vote test test28 test28 10 true
vote test test29 test29 10 true
vote test test30 test30 10 true

It will fail with bandwidth_exception. Wait for few seconds and run again - it will end successfully.

libraries/chain/steem_evaluator.cpp

libraries/protocol/include/golos/protocol/config.hpp

libraries/chain/database.cpp

libraries/protocol/include/golos/protocol/steem_operations.hpp

libraries/chain/steem_evaluator.cpp

afalaleev · 2018-09-13T06:54:54Z

libraries/chain/database.cpp

+                std::nth_element(
+                    active.begin(), active.begin() + active.size() / 2, active.end(),
+                    [&](const auto* a, const auto* b) {
+                        return a->props.*sort_by < b->props.*sort_by;


You should use std::tie() in this place.

libraries/chain/database.cpp

libraries/chain/steem_evaluator.cpp

afalaleev · 2018-09-14T06:58:06Z

libraries/chain/database.cpp

+                    active.begin(), active.begin() + active.size() / 2, active.end(),
+                    [&](const auto* a, const auto* b) {
+                        return std::tie(a->props.*window, a->props.*items) <
+                            std::tie(b->props.*window, b->props.*items);


Fix sorting of items.

Example:

10 10 25 5 50 1 <---- 75 75 100 100

window 50 and items 1 is a very bad case in comparing with others.

@afalaleev Which case is good for this example?
What if sort by consumption (window / items) and use median?

afalaleev · 2018-09-14T07:00:34Z

libraries/chain/steem_evaluator.cpp

+                    GOLOS_CHECK_BANDWIDTH(current_capacity, consumption,
+                        bandwidth_exception::vote_bandwidth,
+                        "Can only vote ${vpw} times in ${vw} seconds.",
+                        ("cpw", mprops.votes_per_window)("cw", mprops.votes_window));


Give items normal names, they are used by clients to detect errors.

You should figure out how the errors subsystem works.

afalaleev · 2018-09-14T07:18:06Z

libraries/wallet/wallet.cpp

@@ -2194,6 +2198,10 @@ fc::ecc::private_key wallet_api::derive_private_key(const std::string& prefix_st
            SET_PROP(max_referral_interest_rate);
            SET_PROP(max_referral_term_sec);
            SET_PROP(max_referral_break_fee);
+            SET_PROP(comments_window);


Here should be different types of chain_properties.
You will merge changes in other commit?

@afalaleev Yes, will fix conflicts after all work.

afalaleev · 2018-09-15T06:01:35Z

libraries/chain/database.cpp

+                std::nth_element(
+                    active.begin(), active.begin() + active.size() / 2, active.end(),
+                    [&](const auto* a, const auto* b) {
+                        return a->props.*window / a->props.*items <


Fix sorting function, you should use std::tie().

Example from issue:

window | items ------------------- 20 | 1 200 | 10

These two settings are not the same.

@afalaleev Which of these 2 settings should be selected? What behaviour do you want in result?

#533 (comment)

@afalaleev Here is window / items (let's write "/" only with spaces - otherwise it could be misleaded with OR), not std::tie.

How could I use tie if it gives bad result here: https://pastebin.com/VwB470nz

afalaleev · 2018-09-15T06:03:16Z

libraries/chain/database.cpp

+                );
+                auto* median = active[active.size() / 2];
+                std::tie(median_props.*window, median_props.*items) = 
+                    std::tie(median->props.*window, median->props.*items);


Coping of values is more simple for understand.

afalaleev · 2018-09-15T06:04:44Z

libraries/chain/database.cpp

+            calc_median_battery(&chain_properties_19::comments_window,
+                &chain_properties_19::comments_per_window);
+            calc_median_battery(&chain_properties_19::votes_window,
+                &chain_properties_19::votes_per_window);


Change typing to one line.

afalaleev · 2018-09-15T07:17:31Z

libraries/chain/steem_evaluator.cpp

+                if (_db.has_hardfork(STEEMIT_HARDFORK_0_19__533)) {
+                    auto consumption = mprops.comments_window / mprops.comments_per_window;
+
+                    auto regenerated_capacity = std::min(mprops.votes_window, uint16_t(elapsed_seconds));


After 4,5 hours there will be an overflow.

afalaleev · 2018-09-15T07:18:23Z

libraries/chain/steem_evaluator.cpp

+                    if (_db.has_hardfork(STEEMIT_HARDFORK_0_19__533)) {
+                        auto consumption = mprops.comments_window / mprops.comments_per_window;
+
+                        auto regenerated_capacity = std::min(mprops.comments_window, uint16_t(elapsed_seconds));


After 4,5 hours there will be an overflow.

afalaleev · 2018-09-17T07:16:52Z

libraries/chain/database.cpp

+                    [&](const auto* a, const auto* b) {
+                        auto a_consumption = a->props.*window / a->props.*items;
+                        auto b_consumption = b->props.*window / b->props.*items;
+                        return std::tie(a_consumption, a->props.*items) <


To one line.

afalaleev · 2018-09-17T09:50:39Z

libraries/chain/steem_evaluator.cpp

+                auto elapsed_seconds = (_db.head_block_time() - voter.last_vote_time).to_seconds();
+
+                if (_db.has_hardfork(STEEMIT_HARDFORK_0_19__533)) {
+                    auto consumption = uint16_t(mprops.comments_window / mprops.comments_per_window);


afalaleev · 2018-09-17T09:50:51Z

libraries/chain/steem_evaluator.cpp

+                    auto consumption = uint16_t(mprops.comments_window / mprops.comments_per_window);
+
+                    auto regenerated_capacity = std::min(uint32_t(mprops.votes_window), uint32_t(elapsed_seconds));
+                    auto current_capacity = std::min(uint32_t(voter.voting_capacity + regenerated_capacity), uint32_t(mprops.comments_window));


The same as for comments.

afalaleev · 2018-09-17T09:55:32Z

libraries/chain/steem_evaluator.cpp

+                        auto consumption = uint16_t(mprops.comments_window / mprops.comments_per_window);
+
+                        auto regenerated_capacity = std::min(uint32_t(mprops.comments_window), uint32_t(elapsed_seconds));
+                        auto current_capacity = std::min(uint32_t(auth.comments_capacity + regenerated_capacity), uint32_t(mprops.comments_window));


auto consumption = mprops.comments_window / mprops.comments_per_window; auto regenerated_capacity = std::min(uint32_t(mprops.comments_window), elapsed_seconds); auto current_capacity = std::min(uint16_t(auth.comments_capacity + regenerated_capacity), mprops.comments_window);

afalaleev · 2018-09-17T09:56:11Z

libraries/chain/steem_evaluator.cpp

+                        }
+
+                        db().modify(auth, [&](account_object &a) {
+                            a.comments_capacity = uint16_t(current_capacity - consumption);


current_capacity is already uint16_t, see comment above

afalaleev · 2018-09-17T09:56:52Z

libraries/chain/steem_evaluator.cpp


-                GOLOS_CHECK_BANDWIDTH(_db.head_block_time(), voter.last_vote_time + STEEMIT_MIN_VOTE_INTERVAL_SEC-1,
+                    _db.modify(voter, [&](account_object &a) {
+                        a.voting_capacity = uint16_t(current_capacity - consumption);


The same as for comments.

afalaleev · 2018-09-17T09:57:31Z

libraries/protocol/steem_operations.cpp

@@ -231,6 +231,10 @@ namespace golos { namespace protocol {
            GOLOS_CHECK_VALUE_LE(max_referral_interest_rate, GOLOS_MAX_REFERRAL_INTEREST_RATE);
            GOLOS_CHECK_VALUE_LE(max_referral_term_sec, GOLOS_MAX_REFERRAL_TERM_SEC);
            GOLOS_CHECK_VALUE_LEGE(max_referral_break_fee.amount, 0, GOLOS_MAX_REFERRAL_BREAK_FEE.amount);
+            GOLOS_CHECK_VALUE_GE(comments_window, 1);
+            GOLOS_CHECK_VALUE_LEGE(comments_per_window, 1, comments_window);
+            GOLOS_CHECK_VALUE_GE(votes_window, 1);


Add upper limit on std::numeric_limits<uint16_t>::max() / 2

afalaleev · 2018-09-17T09:57:42Z

libraries/protocol/steem_operations.cpp

@@ -231,6 +231,10 @@ namespace golos { namespace protocol {
            GOLOS_CHECK_VALUE_LE(max_referral_interest_rate, GOLOS_MAX_REFERRAL_INTEREST_RATE);
            GOLOS_CHECK_VALUE_LE(max_referral_term_sec, GOLOS_MAX_REFERRAL_TERM_SEC);
            GOLOS_CHECK_VALUE_LEGE(max_referral_break_fee.amount, 0, GOLOS_MAX_REFERRAL_BREAK_FEE.amount);
+            GOLOS_CHECK_VALUE_GE(comments_window, 1);


Add upper limit on std::numeric_limits<uint16_t>::max() / 2

afalaleev · 2018-09-17T09:59:42Z

libraries/chain/database.cpp

@@ -1888,6 +1888,20 @@ namespace golos { namespace chain {
                median_props.*param = active[active.size() / 2]->props.*param;
            };

+            auto calc_median_battery = [&](auto&& window, auto&& items) {
+                std::nth_element(
+                    active.begin(), active.begin() + active.size() / 2, active.end(),


May be add median = active.size() / 2 ? It is used 3 times in this function.

afalaleev

Looks good. Rebase to one commit.

maslenitsa93 · 2018-09-18T05:21:38Z

Rebased, conflicts fixed.

afalaleev · 2018-09-18T08:17:14Z

libraries/wallet/wallet.cpp

@@ -2228,7 +2232,7 @@ fc::ecc::private_key wallet_api::derive_private_key(const std::string& prefix_st
            signed_transaction tx;
            chain_properties_update_operation op;
            chain_api_properties ap;
-            chain_properties_18 p;
+            chain_properties p;


You break compatibility with old nodes with daemon version less HF19.
Return back the previous implementation.

afalaleev · 2018-09-18T08:19:32Z

libraries/wallet/wallet.cpp

-                SET_PROP(p19, max_referral_break_fee);
-                op.props = p19;
-            }
+#define SET_PROP(X) {if (!!props.X) p.X = *(props.X); else if (!!ap.X) p.X = *(ap.X);}


Return back the previous implementation:

if (hf >= hardfork_version(0, STEEMIT_HARDFORK_0_19) || !!props.max_referral_interest_rate || !!props.max_referral_term_sec || !!props.max_referral_break_fee) {

@afalaleev Okay. But I forgot - should I add also checks on 533's properties into this if or not?
If should - why we not implemented it in this branch and you approved without it...

maslenitsa93 added hardfork medium labels Sep 10, 2018

maslenitsa93 added this to the 0.19.0 milestone Sep 10, 2018

maslenitsa93 self-assigned this Sep 10, 2018

maslenitsa93 requested a review from afalaleev September 10, 2018 22:11

maslenitsa93 force-pushed the 533-battery-algorithm-for-comments-votes branch from 573a29a to 6c04cdc Compare September 11, 2018 07:58

maslenitsa93 requested review from afalaleev and removed request for afalaleev September 11, 2018 11:47

afalaleev suggested changes Sep 11, 2018

View reviewed changes

maslenitsa93 requested a review from afalaleev September 13, 2018 06:08

maslenitsa93 changed the title ~~New limits algorithm for comments and votes #533~~ New limits algorithm for comments and votes #533 #825 Sep 13, 2018

maslenitsa93 force-pushed the 533-battery-algorithm-for-comments-votes branch from 594a779 to aec2ec2 Compare September 13, 2018 07:01

afalaleev suggested changes Sep 13, 2018

View reviewed changes

maslenitsa93 force-pushed the 533-battery-algorithm-for-comments-votes branch from aec2ec2 to 208c949 Compare September 13, 2018 09:16

maslenitsa93 requested a review from afalaleev September 13, 2018 10:57

afalaleev suggested changes Sep 14, 2018

View reviewed changes

maslenitsa93 requested a review from afalaleev September 14, 2018 12:58

afalaleev suggested changes Sep 16, 2018

View reviewed changes

maslenitsa93 requested a review from afalaleev September 17, 2018 07:16

afalaleev suggested changes Sep 17, 2018

View reviewed changes

maslenitsa93 requested a review from afalaleev September 17, 2018 10:59

afalaleev approved these changes Sep 18, 2018

View reviewed changes

maslenitsa93 force-pushed the 533-battery-algorithm-for-comments-votes branch 2 times, most recently from c092dcb to b7b83f0 Compare September 18, 2018 05:21

maslenitsa93 force-pushed the 533-battery-algorithm-for-comments-votes branch from b7b83f0 to db4e008 Compare September 18, 2018 06:47

afalaleev suggested changes Sep 18, 2018

View reviewed changes

maslenitsa93 force-pushed the 533-battery-algorithm-for-comments-votes branch from db4e008 to c0b818d Compare September 18, 2018 09:04

maslenitsa93 requested a review from afalaleev September 18, 2018 09:06

afalaleev approved these changes Sep 18, 2018

View reviewed changes

New limits algorithm for comments and votes #533 #825

e649b1d

maslenitsa93 force-pushed the 533-battery-algorithm-for-comments-votes branch from c0b818d to e649b1d Compare September 18, 2018 10:48

afalaleev merged commit ccc1204 into golos-v0.19.0 Sep 18, 2018

afalaleev deleted the 533-battery-algorithm-for-comments-votes branch September 18, 2018 13:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New limits algorithm for comments and votes #533 #825 #951

New limits algorithm for comments and votes #533 #825 #951

maslenitsa93 commented Sep 10, 2018 •

edited

Loading

afalaleev Sep 13, 2018

afalaleev Sep 14, 2018

maslenitsa93 Sep 14, 2018

afalaleev Sep 14, 2018

afalaleev Sep 14, 2018

maslenitsa93 Sep 14, 2018

afalaleev Sep 15, 2018

maslenitsa93 Sep 16, 2018

afalaleev Sep 16, 2018

maslenitsa93 Sep 17, 2018 •

edited

Loading

afalaleev Sep 15, 2018

afalaleev Sep 15, 2018

afalaleev Sep 15, 2018

afalaleev Sep 15, 2018

afalaleev Sep 17, 2018

afalaleev Sep 17, 2018

afalaleev Sep 17, 2018

afalaleev Sep 17, 2018

afalaleev Sep 17, 2018

afalaleev Sep 17, 2018

afalaleev Sep 17, 2018

afalaleev Sep 17, 2018

afalaleev Sep 17, 2018

afalaleev Sep 17, 2018

afalaleev left a comment

maslenitsa93 commented Sep 18, 2018

afalaleev Sep 18, 2018

afalaleev Sep 18, 2018

maslenitsa93 Sep 18, 2018

New limits algorithm for comments and votes #533 #825 #951

New limits algorithm for comments and votes #533 #825 #951

Conversation

maslenitsa93 commented Sep 10, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maslenitsa93 Sep 17, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

afalaleev left a comment

Choose a reason for hiding this comment

maslenitsa93 commented Sep 18, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maslenitsa93 commented Sep 10, 2018 •

edited

Loading

maslenitsa93 Sep 17, 2018 •

edited

Loading