From 54ff842db10f667f91e81fa17b1e66f44e6d8b6c Mon Sep 17 00:00:00 2001 From: Milo M Date: Mon, 6 Mar 2023 15:50:17 +0000 Subject: [PATCH] num_son no overwriting --- libraries/chain/account_evaluator.cpp | 118 ++++++++++++++++-- .../graphene/chain/protocol/account.hpp | 15 ++- tests/cli/son.cpp | 13 ++ 3 files changed, 132 insertions(+), 14 deletions(-) diff --git a/libraries/chain/account_evaluator.cpp b/libraries/chain/account_evaluator.cpp index 4ecb3307a..884bba2d0 100644 --- a/libraries/chain/account_evaluator.cpp +++ b/libraries/chain/account_evaluator.cpp @@ -53,7 +53,54 @@ void verify_authority_accounts( const database& db, const authority& a ) } } -void verify_account_votes( const database& db, const account_options& options ) +// Overwrites the num_son values from the origin to the destination for those sidechains which are found in the origin. +// Keeps the values of num_son for the sidechains which are found in the destination, but not in the origin. +// Returns false if an error is detected. +bool merge_num_sons( flat_map& destination, + const flat_map& origin, + fc::optional head_block_time = {}) +{ + const auto active_sidechains = head_block_time.valid() ? active_sidechain_types(*head_block_time) : all_sidechain_types; + bool success = true; + + for (const auto &ns : origin) + { + destination[ns.first] = ns.second; + if (active_sidechains.find(ns.first) == active_sidechains.end()) + { + success = false; + } + } + + return success; +} + +flat_map count_SON_votes_per_sidechain( const flat_set& votes ) +{ + flat_map SON_votes_per_sidechain = account_options::ext::empty_num_son(); + + for (const auto &vote : votes) + { + switch (vote.type()) + { + case vote_id_type::son_bitcoin: + SON_votes_per_sidechain[sidechain_type::bitcoin]++; + break; + case vote_id_type::son_hive: + SON_votes_per_sidechain[sidechain_type::hive]++; + break; + case vote_id_type::son_ethereum: + SON_votes_per_sidechain[sidechain_type::ethereum]++; + break; + default: + break; + } + } + + return SON_votes_per_sidechain; +} + +void verify_account_votes( const database& db, const account_options& options, fc::optional account = {} ) { // ensure account's votes satisfy requirements // NB only the part of vote checking that requires chain state is here, @@ -69,14 +116,40 @@ void verify_account_votes( const database& db, const account_options& options ) FC_ASSERT( options.num_committee <= chain_params.maximum_committee_count, "Voted for more committee members than currently allowed (${c})", ("c", chain_params.maximum_committee_count) ); FC_ASSERT( chain_params.extensions.value.maximum_son_count.valid() , "Invalid maximum son count" ); + + flat_map merged_num_sons = account_options::ext::empty_num_son(); + + // Merge with existing account if exists + if ( account.valid() && account->options.extensions.value.num_son.valid()) + { + merge_num_sons( merged_num_sons, *account->options.extensions.value.num_son, db.head_block_time() ); + } + + // Apply update operation on top if ( options.extensions.value.num_son.valid() ) { - for(const auto& num_sons : *options.extensions.value.num_son) - { - FC_ASSERT( num_sons.second <= *chain_params.extensions.value.maximum_son_count, - "Voted for more sons than currently allowed (${c})", ("c", *chain_params.extensions.value.maximum_son_count) ); - } + merge_num_sons( merged_num_sons, *options.extensions.value.num_son, db.head_block_time() ); + } + + for(const auto& num_sons : merged_num_sons) + { + FC_ASSERT( num_sons.second <= *chain_params.extensions.value.maximum_son_count, + "Voted for more sons than currently allowed (${c})", ("c", *chain_params.extensions.value.maximum_son_count) ); } + + // Count the votes for SONs and confirm that the account did not vote for less SONs than num_son + flat_map SON_votes_per_sidechain = count_SON_votes_per_sidechain(options.votes); + + for (const auto& number_of_votes : SON_votes_per_sidechain) + { + // Number of votes of account_options are also checked in account_options::do_evaluate, + // but there we are checking the value before merging num_sons, so the values should be checked again + const auto sidechain = number_of_votes.first; + FC_ASSERT( number_of_votes.second >= merged_num_sons[sidechain], + "Voted for less sons than specified in num_son (votes ${v} < num_son ${ns}) for sidechain ${s}", + ("v", number_of_votes.second) ("ns", merged_num_sons[sidechain]) ("s", sidechain) ); + } + FC_ASSERT( db.find_object(options.voting_account), "Invalid proxy account specified." ); uint32_t max_vote_id = gpo.next_available_vote_id; @@ -191,9 +264,10 @@ object_id_type account_create_evaluator::do_apply( const account_create_operatio obj.active = o.active; obj.options = o.options; - if (!obj.options.extensions.value.num_son.valid()) + obj.options.extensions.value.num_son = account_options::ext::empty_num_son(); + if ( o.options.extensions.value.num_son.valid() ) { - obj.options.extensions.value = account_options::ext(); + merge_num_sons( *obj.options.extensions.value.num_son, *o.options.extensions.value.num_son ); } obj.statistics = d.create([&obj](account_statistics_object& s){ @@ -295,7 +369,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, *acnt ); return void_result(); } FC_CAPTURE_AND_RETHROW( (o) ) } @@ -334,7 +408,31 @@ void_result account_update_evaluator::do_apply( const account_update_operation& a.active = *o.active; a.top_n_control_flags = 0; } - if( o.new_options ) a.options = *o.new_options; + + // New num_son structure initialized to 0 + flat_map new_num_son = account_options::ext::empty_num_son(); + + // If num_son of existing object is valid, we should merge the existing data + if ( a.options.extensions.value.num_son.valid() ) + { + merge_num_sons( new_num_son, *a.options.extensions.value.num_son ); + } + + // If num_son of the operation are valid, they should merge the existing data + if ( o.new_options ) + { + const auto new_options = *o.new_options; + + if ( new_options.extensions.value.num_son.valid() ) + { + merge_num_sons( new_num_son, *new_options.extensions.value.num_son ); + } + + a.options = *o.new_options; + } + + a.options.extensions.value.num_son = new_num_son; + if( o.extensions.value.owner_special_authority.valid() ) { a.owner_special_authority = *(o.extensions.value.owner_special_authority); diff --git a/libraries/chain/include/graphene/chain/protocol/account.hpp b/libraries/chain/include/graphene/chain/protocol/account.hpp index c80748854..c46caac5e 100644 --- a/libraries/chain/include/graphene/chain/protocol/account.hpp +++ b/libraries/chain/include/graphene/chain/protocol/account.hpp @@ -36,21 +36,28 @@ namespace graphene { namespace chain { bool is_cheap_name( const string& n ); /// These are the fields which can be updated by the active authority. - struct account_options + struct account_options { struct ext { /// The number of active son members this account votes the blockchain should appoint /// Must not exceed the actual number of son members voted for in @ref votes - optional< flat_map > num_son = []{ + optional< flat_map > num_son; + + /// Returns and empty num_son map with all sidechains + static flat_map empty_num_son() + { flat_map num_son; - for(const auto& active_sidechain_type : all_sidechain_types){ + for(const auto& active_sidechain_type : all_sidechain_types) + { num_son[active_sidechain_type] = 0; } + return num_son; - }(); + } }; + /// The memo key is the key this account will typically use to encrypt/sign transaction memos and other non- /// validated account activities. This field is here to prevent confusion if the active authority has zero or /// multiple keys in it. diff --git a/tests/cli/son.cpp b/tests/cli/son.cpp index 4662c57c3..e37585578 100644 --- a/tests/cli/son.cpp +++ b/tests/cli/son.cpp @@ -740,6 +740,19 @@ BOOST_AUTO_TEST_CASE( update_son_votes_test ) sidechain_type::ethereum, 0, true); BOOST_CHECK(generate_maintenance_block()); + // Vote for less SONs than num_son (2 votes, but num_son is 3) + accepted.clear(); + rejected.clear(); + accepted.push_back("son1account"); + accepted.push_back("son2account"); + BOOST_CHECK_THROW(update_votes_tx = con.wallet_api_ptr->update_son_votes("nathan", accepted, rejected, + sidechain_type::bitcoin, 3, true), fc::exception); + BOOST_CHECK_THROW(update_votes_tx = con.wallet_api_ptr->update_son_votes("nathan", accepted, rejected, + sidechain_type::hive, 3, true), fc::exception); + BOOST_CHECK_THROW(update_votes_tx = con.wallet_api_ptr->update_son_votes("nathan", accepted, rejected, + sidechain_type::ethereum, 3, true), fc::exception); + generate_block(); + // Verify the votes son1_obj = con.wallet_api_ptr->get_son("son1account"); son1_end_votes = son1_obj.total_votes;