Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

add last_vote_time to account stats object #1449

Merged
merged 6 commits into from
Dec 16, 2018
Merged

add last_vote_time to account stats object #1449

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
5d1c15c
add last_vote_time
oxarbitrage Nov 26, 2018
5e24f65
change last_vote_time to do_apply
oxarbitrage Nov 30, 2018
5688e7b
minor spacing changes
oxarbitrage Nov 30, 2018
956f1c9
bump database
oxarbitrage Dec 14, 2018
115ed51
clarify proxy vote test times
oxarbitrage Dec 14, 2018
0171861
Merge branch 'develop' into issue1393
oxarbitrage Dec 15, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions libraries/chain/account_evaluator.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ void verify_authority_accounts( const database& db, const authority& a )
}
}

void verify_account_votes( const database& db, const account_options& options )
void verify_account_votes( database& db, const account_options& options, const account_id_type account = account_id_type(0))
{
// ensure account's votes satisfy requirements
// NB only the part of vote checking that requires chain state is here,
Expand Down Expand Up @@ -117,9 +117,15 @@ void verify_account_votes( const database& db, const account_options& options )
}
}
}
if(account != account_id_type(0) &&
(options.votes != account(db).options.votes || options.voting_account != account(db).options.voting_account)) {
auto &stats_obj = db.get_account_stats_by_owner(account);
db.modify(stats_obj, [&](account_statistics_object &obj) {
obj.last_vote_time = db.head_block_time();
});
}
Copy link
Member

@abitmore abitmore Nov 26, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This means every time when do_evaluate is called (E.G. by validate_transaction API ?), the data will be updated. But do_evaluate can fail.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Usually as an unwritten principle we don't modify database in do_evaluate, but probably it's practically fine to do so. @pmconrad your opinion?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that do_evaluate must not modify the database.
IMO it's a good design decision to separate the various stages of operation evaluation, and we should stick with it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

got it, thanks. ill refactor to do the update in do_apply or something different.

}


void_result account_create_evaluator::do_evaluate( const account_create_operation& op )
{ try {
database& d = db();
Expand Down Expand Up @@ -306,7 +312,7 @@ void_result account_update_evaluator::do_evaluate( const account_update_operatio
acnt = &o.account(d);

if( o.new_options.valid() )
verify_account_votes( d, *o.new_options );
verify_account_votes(d, *o.new_options, o.account);

return void_result();
} FC_CAPTURE_AND_RETHROW( (o) ) }
Expand Down
3 changes: 3 additions & 0 deletions libraries/chain/include/graphene/chain/account_object.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,8 @@ namespace graphene { namespace chain {

bool is_voting = false; ///< redundately store whether this account is voting for better maintenance performance

time_point_sec last_vote_time; // add last time voted

/// Whether this account owns some CORE asset and is voting
inline bool has_some_core_voting() const
{
Expand Down Expand Up @@ -453,6 +455,7 @@ FC_REFLECT_DERIVED( graphene::chain::account_statistics_object,
(core_in_balance)
(has_cashback_vb)
(is_voting)
(last_vote_time)
(lifetime_fees_paid)
(pending_fees)(pending_vested_fees)
)
Expand Down
125 changes: 125 additions & 0 deletions tests/tests/voting_tests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -414,5 +414,130 @@ BOOST_AUTO_TEST_CASE(invalid_voting_account)

} FC_LOG_AND_RETHROW()
}
BOOST_AUTO_TEST_CASE(last_voting_date)
{
try
{
ACTORS((alice));

transfer(committee_account, alice_id, asset(100));

// we are going to vote for this witness
auto witness1 = witness_id_type(1)(db);

auto stats_obj = db.get_account_stats_by_owner(alice_id);
BOOST_CHECK_EQUAL(stats_obj.last_vote_time.sec_since_epoch(), 0);

// alice votes
graphene::chain::account_update_operation op;
op.account = alice_id;
op.new_options = alice.options;
op.new_options->votes.insert(witness1.vote_id);
trx.operations.push_back(op);
sign(trx, alice_private_key);
PUSH_TX( db, trx, ~0 );

auto now = db.head_block_time().sec_since_epoch();

// last_vote_time is updated for alice
stats_obj = db.get_account_stats_by_owner(alice_id);
BOOST_CHECK_EQUAL(stats_obj.last_vote_time.sec_since_epoch(), now);

} FC_LOG_AND_RETHROW()
}
BOOST_AUTO_TEST_CASE(last_voting_date_proxy)
{
try
{
ACTORS((alice)(proxy)(bob));

transfer(committee_account, alice_id, asset(100));
transfer(committee_account, bob_id, asset(200));
transfer(committee_account, proxy_id, asset(300));

generate_block();

// witness to vote for
auto witness1 = witness_id_type(1)(db);

// alice changes proxy, this is voting activity
{
graphene::chain::account_update_operation op;
op.account = alice_id;
op.new_options = alice_id(db).options;
op.new_options->voting_account = proxy_id;
trx.operations.push_back(op);
sign(trx, alice_private_key);
PUSH_TX( db, trx, ~0 );
}
// alice last_vote_time is updated
auto alice_stats_obj = db.get_account_stats_by_owner(alice_id);
auto now = db.head_block_time().sec_since_epoch();
BOOST_CHECK_EQUAL(alice_stats_obj.last_vote_time.sec_since_epoch(), now);

generate_block();

// alice update account but no proxy or voting changes are done
{
graphene::chain::account_update_operation op;
op.account = alice_id;
op.new_options = alice_id(db).options;
trx.operations.push_back(op);
sign(trx, alice_private_key);
set_expiration( db, trx );
PUSH_TX( db, trx, ~0 );
}
// last_vote_time is not updated
now = db.head_block_time().sec_since_epoch();
alice_stats_obj = db.get_account_stats_by_owner(alice_id);
BOOST_CHECK(alice_stats_obj.last_vote_time.sec_since_epoch() != now);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

better check that last_vote_time equals the previous now


generate_block();

// bob votes
{
graphene::chain::account_update_operation op;
op.account = bob_id;
op.new_options = bob_id(db).options;
op.new_options->votes.insert(witness1.vote_id);
trx.operations.push_back(op);
sign(trx, bob_private_key);
set_expiration( db, trx );
PUSH_TX(db, trx, ~0);
}

// last_vote_time for bob is updated as he voted
now = db.head_block_time().sec_since_epoch();
auto bob_stats_obj = db.get_account_stats_by_owner(bob_id);
BOOST_CHECK_EQUAL(bob_stats_obj.last_vote_time.sec_since_epoch(), now);

generate_block();

// proxy votes
{
graphene::chain::account_update_operation op;
op.account = proxy_id;
op.new_options = proxy_id(db).options;
op.new_options->votes.insert(witness1.vote_id);
trx.operations.push_back(op);
sign(trx, proxy_private_key);
PUSH_TX(db, trx, ~0);
}

// proxy just voted so the last_vote_time is updated
now = db.head_block_time().sec_since_epoch();
auto proxy_stats_obj = db.get_account_stats_by_owner(proxy_id);
BOOST_CHECK_EQUAL(proxy_stats_obj.last_vote_time.sec_since_epoch(), now);

// alice haves proxy, proxy votes but last_vote_time is not updated for alice
alice_stats_obj = db.get_account_stats_by_owner(alice_id);
BOOST_CHECK(alice_stats_obj.last_vote_time.sec_since_epoch() != now);

// bob haves nothing to do with proxy so last_vote_time is not updated
bob_stats_obj = db.get_account_stats_by_owner(bob_id);
BOOST_CHECK(bob_stats_obj.last_vote_time.sec_since_epoch() != now);

} FC_LOG_AND_RETHROW()
}

BOOST_AUTO_TEST_SUITE_END()