Added improvements to auction_window. #898 #963
Added improvements to auction_window. #898 #963
Conversation
AKorpusenko
commented
Sep 26, 2018
- Added virtual operation for auction window reward returning to reward fund
- Added option to chose auction window reward destination: to_curators or to_reward_fund
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need changes in
discussion_helper::impl::set_pending_payout()
and
operation_history_plugin
|
||
comment_mode mode = first_payout; | ||
|
||
protocol::auction_window_reward_destination_type auction_window_reward_destination = protocol::destination_not_set; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use using
for such cases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Default to_author
. After HF default should be to_reward_fund
.
libraries/chain/database.cpp
Outdated
c.auction_window_reward_destination == protocol::to_curators | ||
) { | ||
claim += ((additional_claim * weight) / | ||
total_weight).to_uint64(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A very strange calculation....
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You lost claim for curators in auction window, because total_weight contains them.
You should deduct auction window weight from total_weight.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed the calculations.
@@ -59,10 +59,12 @@ namespace golos { namespace api { | |||
int32_t net_votes = 0; | |||
|
|||
comment_mode mode = not_set; | |||
protocol::auction_window_reward_destination_type auction_window_reward_destination = protocol::destination_not_set; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use using
....
libraries/chain/database.cpp
Outdated
@@ -2255,12 +2255,59 @@ namespace golos { namespace chain { | |||
share_type unclaimed_rewards = max_rewards; | |||
|
|||
if (c.total_vote_weight > 0 && c.allow_curation_rewards) { | |||
|
|||
uint32_t votes_after_auction_window_count = 0; | |||
uint32_t total_votes_count = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You don't use these variables.
What reason was to calculate them?
enum auction_window_reward_destination_type { | ||
to_reward_fund, | ||
to_curators, | ||
destination_not_set |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to_author
void comment_auction_window_reward_destination::validate() const { | ||
// TODO: Figure out how to write it correct | ||
// GOLOS_CHECK_PARAM(destination, { | ||
// GOLOS_CHECK_VALUE(destination == to_reward_fund || destination == to_curators, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good.
: destination(dest) { | ||
} | ||
|
||
auction_window_reward_destination_type destination = destination_not_set; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Default is not needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How do you test your changes?
@@ -9,6 +9,7 @@ namespace golos { namespace api { | |||
|
|||
using namespace golos::chain; | |||
using namespace golos::protocol; | |||
using auc_win_destination = protocol::auction_window_reward_destination_type; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use simple using protocol::auction_window_destination_type;
libraries/api/discussion_helper.cpp
Outdated
boost::make_tuple(0, d.created + d.auction_window_size) | ||
); | ||
|
||
while (itr != itr_after_auw && itr->comment == d.id) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You shouldn't do such iterations on runtime - it's too heavy and will generate forks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do it in the vote evaluator.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes it's really heavy and extra loop. Added one more field in comment_object
and added needed calculations to vote_evaluator
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This loop still exist.
libraries/api/discussion_helper.cpp
Outdated
@@ -193,7 +254,9 @@ namespace golos { namespace api { | |||
} | |||
} | |||
|
|||
if (db.has_hardfork(STEEMIT_HARDFORK_0_19__898)) { | |||
if (db.has_hardfork(STEEMIT_HARDFORK_0_19__898) && | |||
d.auction_window_reward_destination == protocol::to_reward_fund |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about case when all votes in auction window? You're lost such rewards.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, you're right. Fixed that case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copy logic from fixed pay_curators().
Your calculations are too complex.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here should only checking on c.auction_window_reward_destination != protocol::to_author
If exist any unclaimed_reward it should be returned to reward fund after HF19
libraries/chain/database.cpp
Outdated
boost::make_tuple(0, c.created + c.auction_window_size) | ||
); | ||
|
||
while (itr != itr_after_auw && itr->comment == c.id) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You shouldn't do such iterations on runtime - it's too heavy and will generate forks.
libraries/chain/database.cpp
Outdated
// to_curators case | ||
if (has_hardfork(STEEMIT_HARDFORK_0_19__898) && | ||
c.auction_window_reward_destination == protocol::to_curators) { | ||
if (itr->last_update >= c.created && itr->last_update < auw_time) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The check itr->last_update >= c.created
for what?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's extra check. Removed it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check is still exist
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Calculations are too complex, they should be more simple:
auto auction_window_reward = max_rewards.value * c.auction_window_weight / total_weight;
auto votes_after_auction_window_weight = total_weight - c.votes_in_auction_window_weight - c.auction_window_weight;
....
claim = ((max_rewards.value * weight) / total_weight).to_uint64();
if (c.auction_window_reward_destination == protocol::to_curators && itr->last_update >= auw_time) {
claim += ((auction_window_reward * weight) / votes_after_auction_window_weight).to_uint64();
}
libraries/chain/database.cpp
Outdated
votes_in_auction_window_weight).to_uint64(); | ||
} | ||
|
||
if (itr->last_update >= auw_time) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If there are no votes after auction window to where will be send such reward?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your calculations are too complex.
And you lost auction window in different places.
libraries/api/discussion_helper.cpp
Outdated
const auto &cvidx = db.get_index<comment_vote_index>().indices().get<by_comment_weight_voter>(); | ||
for (auto itr = cvidx.lower_bound(d.id); itr != cvidx.end() && itr->comment == d.id; ++itr) { | ||
if (db.has_hardfork(STEEMIT_HARDFORK_0_19__898) && | ||
d.auction_window_reward_destination == protocol::to_curators) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To one line
libraries/api/discussion_helper.cpp
Outdated
boost::make_tuple(0, d.created + d.auction_window_size) | ||
); | ||
|
||
while (itr != itr_after_auw && itr->comment == d.id) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This loop still exist.
libraries/chain/database.cpp
Outdated
|
||
|
||
votes_after_auction_window_weight = total_weight - | ||
c.votes_in_auction_window_weight - c.auction_window_weight; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To one line.
libraries/chain/database.cpp
Outdated
c.votes_in_auction_window_weight - c.auction_window_weight; | ||
|
||
votes_in_auction_window_reward = ((max_rewards.value * c.votes_in_auction_window_weight) / | ||
total_weight).to_uint64(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To one line
libraries/chain/database.cpp
Outdated
total_weight).to_uint64(); | ||
|
||
auction_window_reward = ((max_rewards.value * c.auction_window_weight) / | ||
total_weight).to_uint64(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To one line.
libraries/chain/database.cpp
Outdated
c.total_vote_weight > 0 && c.auction_window_reward_destination == protocol::to_reward_fund | ||
|| // Also in case when there are no votes after end of auction window, | ||
votes_after_auction_window_weight == 0 // tokens must go to reward fund | ||
) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here should be checking on c.auction_window_reward_destination != protocol::to_author
.
If any unclaimed rewards exist, they should be returned to reward fund after HF19
|
||
comment_mode mode = first_payout; | ||
|
||
auction_window_reward_destination_type auction_window_reward_destination = protocol::to_reward_fund; | ||
uint32_t auction_window_size; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This value should be initialized here and on post creation.
On voting the auction window size should be got from the post, not from properties.
libraries/chain/database.cpp
Outdated
// to_curators case | ||
if (has_hardfork(STEEMIT_HARDFORK_0_19__898) && | ||
c.auction_window_reward_destination == protocol::to_curators) { | ||
if (itr->last_update >= c.created && itr->last_update < auw_time) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Calculations are too complex, they should be more simple:
auto auction_window_reward = max_rewards.value * c.auction_window_weight / total_weight;
auto votes_after_auction_window_weight = total_weight - c.votes_in_auction_window_weight - c.auction_window_weight;
....
claim = ((max_rewards.value * weight) / total_weight).to_uint64();
if (c.auction_window_reward_destination == protocol::to_curators && itr->last_update >= auw_time) {
claim += ((auction_window_reward * weight) / votes_after_auction_window_weight).to_uint64();
}
libraries/api/discussion_helper.cpp
Outdated
@@ -193,7 +254,9 @@ namespace golos { namespace api { | |||
} | |||
} | |||
|
|||
if (db.has_hardfork(STEEMIT_HARDFORK_0_19__898)) { | |||
if (db.has_hardfork(STEEMIT_HARDFORK_0_19__898) && | |||
d.auction_window_reward_destination == protocol::to_reward_fund |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copy logic from fixed pay_curators().
Your calculations are too complex.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix logic.
libraries/api/discussion_helper.cpp
Outdated
const auto &cvidx = db.get_index<comment_vote_index>().indices().get<by_comment_weight_voter>(); | ||
for (auto itr = cvidx.lower_bound(d.id); itr != cvidx.end() && itr->comment == d.id; ++itr) { | ||
auto claim = ((max_rewards.value * uint128_t(itr->weight)) / total_weight).to_uint64(); | ||
uint64_t claim = ((max_rewards.value * weight) / total_weight).to_uint64(); | ||
if (db.has_hardfork(STEEMIT_HARDFORK_0_19__898) && d.auction_window_reward_destination == protocol::to_curators) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here should be checking on d.auction_window_reward_destination == protocol::to_curators && itr->last_update >= auw_time
without checking on HF.
And next checking is not needed
libraries/api/discussion_helper.cpp
Outdated
// to_curators case | ||
if (has_hardfork(STEEMIT_HARDFORK_0_19__898) && | ||
c.auction_window_reward_destination == protocol::to_curators && itr->last_update >= auw_time | ||
) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excessive checking, remove it
libraries/api/discussion_helper.cpp
Outdated
@@ -193,7 +254,9 @@ namespace golos { namespace api { | |||
} | |||
} | |||
|
|||
if (db.has_hardfork(STEEMIT_HARDFORK_0_19__898)) { | |||
if (db.has_hardfork(STEEMIT_HARDFORK_0_19__898) && | |||
d.auction_window_reward_destination == protocol::to_reward_fund |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here should only checking on c.auction_window_reward_destination != protocol::to_author
If exist any unclaimed_reward it should be returned to reward fund after HF19
libraries/chain/database.cpp
Outdated
uint64_t claim = ((max_rewards.value * weight) / total_weight).to_uint64(); | ||
// to_curators case | ||
if (has_hardfork(STEEMIT_HARDFORK_0_19__898) && | ||
c.auction_window_reward_destination == protocol::to_curators && itr->last_update >= auw_time |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here should be checking on d.auction_window_reward_destination == protocol::to_curators && itr->last_update >= auw_time
without checking on HF.
libraries/chain/database.cpp
Outdated
c.total_vote_weight > 0 && c.auction_window_reward_destination == protocol::to_reward_fund | ||
|| // Also in case when there are no votes after end of auction window, | ||
votes_after_auction_window_weight == 0 // tokens must go to reward fund | ||
) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here should be checking on c.auction_window_reward_destination != protocol::to_author
.
If any unclaimed rewards exist, they should be returned to reward fund after HF19
libraries/chain/steem_evaluator.cpp
Outdated
@@ -535,6 +535,26 @@ namespace golos { namespace chain { | |||
"Cannot allocate more than 100% of rewards to a comment"); | |||
}); | |||
} | |||
|
|||
void operator()( const comment_auction_window_reward_destination& cawrd ) const { | |||
if (_db.has_hardfork(STEEMIT_HARDFORK_0_19__898)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here should be: ASSERT_REQ_HF(STEEMIT_HARDFORK_0_19__898);
libraries/chain/steem_evaluator.cpp
Outdated
c.auction_window_reward_destination = cawrd.destination; | ||
}); | ||
} | ||
else { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Impossible case.
libraries/chain/steem_evaluator.cpp
Outdated
com.author = o.author; | ||
from_string(com.permlink, o.permlink); | ||
com.created = _db.head_block_time(); | ||
com.last_payout = fc::time_point_sec::min(); | ||
com.max_cashout_time = fc::time_point_sec::maximum(); | ||
com.reward_weight = reward_weight; | ||
|
||
const witness_schedule_object& wso = _db.get_witness_schedule_object(); | ||
com.auction_window_size = wso.median_props.auction_window_size; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move this up into if (_db.has_hardfork(STEEMIT_HARDFORK_0_19__898))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is one possible optimization for pending payouts.
But lets left it to the future updates.
@@ -62,7 +63,10 @@ namespace golos { namespace api { | |||
|
|||
comment_object::id_type root_comment; | |||
|
|||
uint128_t auction_window_weight = 0; | |||
auction_window_reward_destination_type auction_window_reward_destination; | |||
uint32_t auction_window_size; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Set default values here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
tests/tests/operation_tests.cpp
Outdated
double allowed_percent_delta = 0.01; // = 1e-2 | ||
double allowed_tokens_delta = 50; | ||
|
||
BOOST_CHECK_EQUAL(fabs(0.25 - voters_reward_percent) < allowed_percent_delta, true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
APPROX_CHECK_EQUAL, no? I invented it and you do not using it? :)
And same in another places.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
APPROX_CHECK_EQUAL
uses std::abs
while I need std::fabs
. Added APPROX_CHECK_DOUBLE_EQUAL
tests/tests/operation_tests.cpp
Outdated
|
||
GOLOS_CHECK_NO_THROW(push_tx_with_ops(tx, alice_private_key, cop)); | ||
generate_block(); | ||
auto & alice_post = db->get<comment_object, by_permlink>(boost::make_tuple(comment.author, comment.permlink)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- auto & variable_name - Where do you find this strange code style? :)
It is bad, same asauto &variable_name
Good isauto& variable_name
And same in another places. - std::make_tuple is better than boost::
And same in another places. - Why not db->get_comment?
And if fix - same in another places.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed all get<comment_object...
to get_comment
FC_LOG_AND_RETHROW() | ||
} | ||
|
||
BOOST_AUTO_TEST_CASE(auction_window_tokens_to_reward_fund_empty_case) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we really need a few test cases made by copy-paste with so small diffirences?
auction_window_tokens_to_reward_fund
auction_window_tokens_to_reward_fund_all_in_auw
etc
Isn't it better to join them to single case?
If no - at least simplify each of them, please.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunatly it's impossible to move all tests for destinations into one test. I've tried to make 3 post and write 3 cases in one test (one post has votes only in AUW, second one only after AUW, and third one has voter in and after AUW), but its impossible to separate total_comment_fund.reward_fund()
correctly. That's why there is no other way to check correctness of payments and modeled calculations of it. Tests are large and mostly copy-pasted, I agree but
¯\_(ツ)_/¯
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also for the same reasons as I wrote before it's impossible to move all testcases into one
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Simplify tests by simplyfing in details.
Do all these fixes in all tests.
|
||
share_type rwd = 0; | ||
for (int i = 0; i < voters_count; i++) { | ||
std::string acc_name = "voter" + std::to_string(i); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excessive variable acc_name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why? It's okay to write like that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@AKorpusenko It's
oka
y
to
wri
te
lik
e t
ha
t.
And
und
ers
tan
d su
ch w
wri
tin
gs.
Understood?
More lines = more complicated reading of code.
} | ||
auto& alice_acc = db->get_account(alice_post.author); | ||
|
||
double auw_tokens = (gpo.total_reward_fund_steem.amount.value); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use auto
everywhere if possible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I need exactly double
and i don't have double
value in right hand expression, thats why there is no auto in this place.
auto& alice_acc = db->get_account(alice_post.author); | ||
|
||
double auw_tokens = (gpo.total_reward_fund_steem.amount.value); | ||
double total_payout = (alice_acc.posting_rewards.value) * 100.0 / 75.0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use 75*STEEMIT_1_PERCENT and etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What for? It makes expression much more complicated!
#define STEEMIT_100_PERCENT 10000
#define STEEMIT_1_PERCENT (STEEMIT_100_PERCENT/100)
As for me it's not obvious at all why should I use STEEMIT_1_PERCENT, while the x * 75 / 100
is clear and more understandable, that I need to get 75% of x
.
|
||
double auw_tokens = (gpo.total_reward_fund_steem.amount.value); | ||
double total_payout = (alice_acc.posting_rewards.value) * 100.0 / 75.0; | ||
double voters_reward = (rwd.value); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do not define useless variables.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
WTF? I do use them in calculations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@AKorpusenko You using values. Use values. But don't define variables...
double voters_reward = (rwd.value); // ...or if such value already defined
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please, write code as simple as possible. No excessive variables or another things. If could be omitted without negative effect- omit.
(I haven't yet checked all)
BOOST_CHECK_EQUAL(abs(auw_tokens - total_comment_fund.reward_fund().amount.value) < allowed_tokens_delta, true); | ||
BOOST_CHECK_EQUAL(fabs(0.25 - modeled_voters_reward_percent) < allowed_percent_delta, true); | ||
APPROX_CHECK_DOUBLE_EQUAL(0.25, voters_reward_percent, allowed_percent_delta); | ||
APPROX_CHECK_EQUAL(auw_tokens, total_comment_fund.reward_fund().amount.value, allowed_tokens_delta); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-
APPROX_CHECK_EQUAL uses <=. Your APPROX_CHECK_DOUBLE_EQUAL uses <. Inconsistency.
And if it is allowed delta - you should use <=. -
Isn't it better to name it APPROX_CHECK_EQUAL_DOUBLE?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Fix the last comment.
- Rebase to one commit.
It is ready to merge.
libraries/chain/database.cpp
Outdated
// to_curators case | ||
if (c.auction_window_reward_destination == protocol::to_curators && itr->last_update >= auw_time) { | ||
claim += ((auction_window_reward * weight) / votes_after_auction_window_weight).to_uint64(); | ||
if (c.auction_window_reward_destination == protocol::to_curators && !heaviest_vote_after_auw_founded) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Duplicate checking on c.auction_window_reward_destination == protocol::to_curators
Remove it.
a07219b
to
c4edde9
Compare
c4edde9
to
0dcec22
Compare
cccb4e6
to
4f3429d
Compare
4f3429d
to
fdc0aac
Compare