From d892bac4ba79faf835c60e66d029daf7dfc72a65 Mon Sep 17 00:00:00 2001 From: Alexey Frolov Date: Mon, 22 Apr 2019 18:00:21 +0300 Subject: [PATCH 01/14] Prevent to create cycled accounts + unit tests --- libraries/chain/account_evaluator.cpp | 97 +++++- .../chain/hardfork.d/CYCLED_ACCOUNTS_HF.hf | 4 + tests/tests/operation_tests.cpp | 287 ++++++++++++++++++ 3 files changed, 385 insertions(+), 3 deletions(-) create mode 100644 libraries/chain/hardfork.d/CYCLED_ACCOUNTS_HF.hf diff --git a/libraries/chain/account_evaluator.cpp b/libraries/chain/account_evaluator.cpp index 674de46c43..914381dea2 100644 --- a/libraries/chain/account_evaluator.cpp +++ b/libraries/chain/account_evaluator.cpp @@ -36,8 +36,88 @@ #include #include +#include -namespace graphene { namespace chain { +namespace graphene { namespace chain { namespace detail { + + template + struct authority_cycle_detector + { + using authority_t = flat_set; + + const database& db; + /** Getter that returns an authority to check. */ + Lambda get_authority; + + /** + * Detects if autority contains a cycle. + * Throw exeption if it finds a cycle. + * + * @param d a database + * @param auth_to_check an authority to check + * @param account_id for that account need to check authority + * @param getter Lambda that returns an authority to check. + */ + explicit authority_cycle_detector(const database& d, const authority& auth_to_check, const account_id_type& account_id, Lambda getter) + : db{d}, get_authority{getter} + { + /** Set of accounts ids used to detect a cycle.*/ + authority_t account_ids{}; + const auto& chain_params = db.get_global_properties().parameters; + detect_cycle(account_id, auth_to_check, account_ids, chain_params.maximum_authority_membership); + } + + /** + * Recursively detects if autority contains a cycle. + * Get authorities for each account from auth_to_check and check for cycle + * taking in account path + * Throw exeption if it finds a cycle. + * + * @param account_id find a cycle from this account + * @param auth_to_check an authority to check + * @param path a chain off accounts that led to account_id + */ + void detect_cycle(const account_id_type& account_id, const authority& auth_to_check, authority_t& path, uint16_t max_depth) + { + GRAPHENE_ASSERT(max_depth > 0, internal_verify_auth_max_auth_exceeded, "Maximum authority membership exceeded" ); + + auto inserted = false; + std::tie(std::ignore, inserted) = path.insert(account_id); + FC_ASSERT(inserted, "Account ${a} already is in authority.", ("a", account_id)); + + for(const auto& el : auth_to_check.account_auths) + { + const auto& account = el.first(db); + const auto& auth = get_authority(account); + detect_cycle(el.first, auth, path, max_depth - 1); + } + path.erase(account_id); + } + }; + + void verify_account_authorities( const database& db, + const account_id_type& account_id, + const fc::optional& active, + const fc::optional& owner) + { + if( db.head_block_time() < HARDFORK_CYCLED_ACCOUNTS_TIME ) + { + return; + } + + if( active ) + { + auto active_auth_getter = [](const account_object& account) { return account.active; }; + authority_cycle_detector validator{ db, *active, account_id, active_auth_getter }; + } + + if( owner ) + { + auto owner_auth_getter = [](const account_object& account) { return account.owner; }; + authority_cycle_detector validator{ db, *owner, account_id, owner_auth_getter }; + } + } +}//detail void verify_authority_accounts( const database& db, const authority& a ) { @@ -133,6 +213,9 @@ void_result account_create_evaluator::do_evaluate( const account_create_operatio { verify_authority_accounts( d, op.owner ); verify_authority_accounts( d, op.active ); + + // avoid cycles in authorities + detail::verify_account_authorities(d, account_id_type{}, op.active, op.owner); } GRAPHENE_RECODE_EXC( internal_verify_auth_max_auth_exceeded, account_create_max_auth_exceeded ) GRAPHENE_RECODE_EXC( internal_verify_auth_account_not_found, account_create_auth_account_not_found ) @@ -276,8 +359,16 @@ void_result account_update_evaluator::do_evaluate( const account_update_operatio try { - if( o.owner ) verify_authority_accounts( d, *o.owner ); - if( o.active ) verify_authority_accounts( d, *o.active ); + if( o.owner ) + { + verify_authority_accounts( d, *o.owner ); + } + if( o.active ) + { + verify_authority_accounts( d, *o.active ); + } + // avoid cycles in authorities started from o.account + detail::verify_account_authorities(d, o.account, o.active, o.owner); } GRAPHENE_RECODE_EXC( internal_verify_auth_max_auth_exceeded, account_update_max_auth_exceeded ) GRAPHENE_RECODE_EXC( internal_verify_auth_account_not_found, account_update_auth_account_not_found ) diff --git a/libraries/chain/hardfork.d/CYCLED_ACCOUNTS_HF.hf b/libraries/chain/hardfork.d/CYCLED_ACCOUNTS_HF.hf new file mode 100644 index 0000000000..bd4ef0a85f --- /dev/null +++ b/libraries/chain/hardfork.d/CYCLED_ACCOUNTS_HF.hf @@ -0,0 +1,4 @@ +// #HARDFORK_CYCLED_ACCOUNTS_TIME Prevent to create/update account authorities that locks account. +#ifndef HARDFORK_CYCLED_ACCOUNTS_TIME +#define HARDFORK_CYCLED_ACCOUNTS_TIME (fc::time_point_sec( 1550102400 )) // Thursday, February 14, 2019 12:00:00 AM +#endif diff --git a/tests/tests/operation_tests.cpp b/tests/tests/operation_tests.cpp index ce5255f4e8..8a8e79a9d9 100644 --- a/tests/tests/operation_tests.cpp +++ b/tests/tests/operation_tests.cpp @@ -962,6 +962,293 @@ BOOST_AUTO_TEST_CASE( update_account ) } } +BOOST_AUTO_TEST_CASE( create_account_with_cycled_authority_before_HARDFORK_CYCLED_ACCOUNTS_TIME_test ) +{ + try { + ACTORS( (bob) (alice) ); + + auto update_auth = [&](const account_update_operation& op){ + trx.operations.clear(); + trx.operations.push_back(op); + PUSH_TX( db, trx, ~0 ); + }; + + account_update_operation op; + + op.account = bob_id; + op.active = authority(1, alice_id, 1); + op.owner = op.active; + update_auth(op); + + op.account = alice_id; + op.active = authority(1, bob_id, 1); + op.owner = op.active; + update_auth(op); + + trx.operations.clear(); + account_create_operation create_op = make_account("allowed"); + create_op.owner = authority(1, bob_id, 1); + create_op.active = authority(1, alice_id, 1); + trx.operations.push_back(create_op); + PUSH_TX( db, trx, ~0 ); + } + FC_LOG_AND_RETHROW() +} + +BOOST_AUTO_TEST_CASE( create_account_with_cycled_authority_after_HARDFORK_CYCLED_ACCOUNTS_TIME_test ) +{ + try { + INVOKE(create_account_with_cycled_authority_before_HARDFORK_CYCLED_ACCOUNTS_TIME_test); + GET_ACTOR(bob); + GET_ACTOR(alice); + + generate_blocks(HARDFORK_CYCLED_ACCOUNTS_TIME); + set_expiration( db, trx ); + + trx.operations.clear(); + account_create_operation create_op = make_account("disabled"); + create_op.owner = authority(1, bob_id, 1); + create_op.active = authority(1, alice_id, 1); + trx.operations.push_back(create_op); + GRAPHENE_CHECK_THROW(PUSH_TX( db, trx, ~0 ), fc::exception); + } + FC_LOG_AND_RETHROW() +} + +BOOST_AUTO_TEST_CASE( update_account_self_cycled_authority_before_HARDFORK_CYCLED_ACCOUNTS_TIME_test ) +{ + try { + ACTOR(nathan); + + trx.operations.clear(); + + account_update_operation op; + op.account = nathan_id; + op.owner = authority(1, nathan_id, 1); + op.active = authority(1, nathan_id, 1); + trx.operations.push_back(op); + PUSH_TX( db, trx, ~0 ); + } + FC_LOG_AND_RETHROW() +} + +BOOST_AUTO_TEST_CASE( update_account_self_cycled_authority_after_HARDFORK_CYCLED_ACCOUNTS_TIME_test ) +{ + try { + ACTOR(nathan); + + generate_blocks(HARDFORK_CYCLED_ACCOUNTS_TIME); + set_expiration( db, trx ); + + auto update_auth = [&](const account_update_operation& op){ + trx.operations.clear(); + trx.operations.push_back(op); + PUSH_TX( db, trx, ~0 ); + }; + + account_update_operation op; + op.account = nathan_id; + // make cycle to self in active auth + { + op.active = authority(1, nathan_id, 1); + GRAPHENE_CHECK_THROW(update_auth(op), fc::exception); + } + // make cycle to self in owner auth + { + op.owner = {}; + op.active = authority(1, nathan_id, 1); + GRAPHENE_CHECK_THROW(update_auth(op), fc::exception); + } + } + FC_LOG_AND_RETHROW() +} + +BOOST_AUTO_TEST_CASE( update_account_authority_test ) +{ + try { + ACTORS( (nathan) (bob) (alice) (jill) (izzy) ); + + auto update_auth = [&](const account_update_operation& op){ + trx.operations.clear(); + trx.operations.push_back(op); + PUSH_TX( db, trx, ~0 ); + }; + + // make cycle with other accounts in owner auth + // | | | + // | account | authority | + // |___________|_________________| + // | bob | alice, jill | + // | alice | jill, nathan | + // | jill | izzy | + // | izzy | nathan | + // |___________|_________________| + + // bob---->alice-----------------| + // | | | + // | |->jill |->nathan<-| + // | |->izzy + + account_update_operation op; + { + op.account = bob_id; + op.active = authority(2, alice_id, 1, jill_id, 1); + update_auth(op); + { + GET_ACTOR(bob); + BOOST_CHECK(bob.active == op.active); + } + + op.account = alice_id; + op.active = authority(2, jill_id, 1, nathan_id, 1); + update_auth(op); + { + GET_ACTOR(alice); + BOOST_CHECK(alice.active == op.active); + } + + op.account = jill_id; + op.active = authority(1, izzy_id, 1); + update_auth(op); + { + GET_ACTOR(jill); + BOOST_CHECK(jill.active == op.active); + } + + op.account = izzy_id; + op.active = authority(1, nathan_id, 1); + update_auth(op); + { + GET_ACTOR(izzy); + BOOST_CHECK(izzy.active == op.active); + } + } + } + FC_LOG_AND_RETHROW() +} + +BOOST_AUTO_TEST_CASE( update_account_make_cycled_authority_before_HARDFORK_CYCLED_ACCOUNTS_TIME_test) +{ + // make cycle with other accounts in owner auth + // | | | + // | account | authority | + // |___________|_________________| + // | bob | alice, jill | + // | alice | jill, nathan | + // | jill | izzy | + // | izzy | nathan | + // | nathan | bob | + // nathan wonts to add bob to auth + // bob---->alice-----------------| + // | | | + // | |->jill |->nathan<-| + // | |->izzy | + // | | + // |<---------------------| lead to cycle!!! + + try { + INVOKE(update_account_authority_test); + GET_ACTOR(nathan); + GET_ACTOR(bob); + + account_update_operation op; + op.account = nathan_id; + op.active = authority(1, bob_id, 1); + + trx.operations.clear(); + trx.operations.push_back(op); + PUSH_TX( db, trx, ~0 ); + } + FC_LOG_AND_RETHROW() +} + +BOOST_AUTO_TEST_CASE( update_account_make_cycled_authority_after_HARDFORK_CYCLED_ACCOUNTS_TIME_test) +{ + // make cycle with other accounts in owner auth + // | | | + // | account | authority | + // |___________|_________________| + // | bob | alice, jill | + // | alice | jill, nathan | + // | jill | izzy | + // | izzy | nathan | + // | nathan | bob | + // nathan wonts to add bob to auth + // bob---->alice-----------------| + // | | | + // | |->jill |->nathan<-| + // | |->izzy | + // | | + // |<---------------------| lead to cycle!!! + + try { + INVOKE(update_account_authority_test); + generate_blocks(HARDFORK_CYCLED_ACCOUNTS_TIME); + set_expiration( db, trx ); + + GET_ACTOR(nathan); + GET_ACTOR(bob); + + account_update_operation op; + op.account = nathan_id; + op.active = authority(1, bob_id, 1); + + trx.operations.clear(); + trx.operations.push_back(op); + GRAPHENE_CHECK_THROW(PUSH_TX( db, trx, ~0 ), fc::exception); + } + FC_LOG_AND_RETHROW() +} + +BOOST_AUTO_TEST_CASE( update_account_max_authority_depth_before_HARDFORK_CYCLED_ACCOUNTS_TIME_test) +{ + try { + INVOKE(update_account_authority_test); + + GET_ACTOR(nathan); + GET_ACTOR(bob); + + db.modify( db.get_global_properties(), [&]( global_property_object& gpo ) { + gpo.parameters.maximum_authority_membership = 1; + }); + + account_update_operation op; + op.account = nathan_id; + op.active = authority(1, bob_id, 1); + + trx.operations.clear(); + trx.operations.push_back(op); + PUSH_TX( db, trx, ~0 ); + } + FC_LOG_AND_RETHROW() +} + +BOOST_AUTO_TEST_CASE( update_account_max_authority_depth_after_HARDFORK_CYCLED_ACCOUNTS_TIME_test) +{ + try { + INVOKE(update_account_authority_test); + + GET_ACTOR(nathan); + GET_ACTOR(bob); + + generate_blocks(HARDFORK_CYCLED_ACCOUNTS_TIME); + set_expiration( db, trx ); + + db.modify( db.get_global_properties(), [&]( global_property_object& gpo ) { + gpo.parameters.maximum_authority_membership = 1; + }); + + account_update_operation op; + op.account = nathan_id; + op.active = authority(1, bob_id, 1); + + trx.operations.clear(); + trx.operations.push_back(op); + GRAPHENE_CHECK_THROW(PUSH_TX( db, trx, ~0 ), fc::exception); + } + FC_LOG_AND_RETHROW() +} + BOOST_AUTO_TEST_CASE( transfer_core_asset ) { try { From 089622448de03541bbb8de05ddc4c96f11af47cb Mon Sep 17 00:00:00 2001 From: Alexey Frolov Date: Mon, 22 Apr 2019 18:03:12 +0300 Subject: [PATCH 02/14] Add unit tests: verify_cycled_authority --- tests/tests/operation_tests.cpp | 58 +++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/tests/tests/operation_tests.cpp b/tests/tests/operation_tests.cpp index 8a8e79a9d9..6eedb829f8 100644 --- a/tests/tests/operation_tests.cpp +++ b/tests/tests/operation_tests.cpp @@ -962,6 +962,64 @@ BOOST_AUTO_TEST_CASE( update_account ) } } +BOOST_AUTO_TEST_CASE( update_account_lead_to_cycled_authority_before_HARDFORK_CYCLED_ACCOUNTS_TIME_test ) +{ + /* + see https://github.com/bitshares/bsips/pull/149#discussion_r270407694 + Precondition: + There are three accounts Alice, Bob and Charlie + Steps: + 1. Alice sets Bob to her active and owner authority + 2. Charlie sets Alice to his active and owner authority + 3. Alice sets Charlie to her active and owner authority + 4. Bob sets Charlie to his active and owner authority + Expected: + Before HARDFORK_CYCLED_ACCOUNTS_TIME it should work + */ + try { + ACTORS( (bob) (alice) (charlie) ); + + auto update_auth = [&](const account_update_operation& op){ + trx.operations.clear(); + trx.operations.push_back(op); + PUSH_TX( db, trx, ~0 ); + }; + + { + account_update_operation op; + op.account = alice_id; + op.active = authority(1, bob_id, 1); + op.owner = op.active; + update_auth(op); + } + + { + account_update_operation op; + op.account = charlie_id; + op.active = authority(1, alice_id, 1); + op.owner = op.active; + update_auth(op); + } + + { + account_update_operation op; + op.account = alice_id; + op.active = authority(1, charlie_id, 1); + op.owner = op.active; + update_auth(op); + } + + { + account_update_operation op; + op.account = bob_id; + op.active = authority(1, charlie_id, 1); + op.owner = op.active; + update_auth(op); + } + } + FC_LOG_AND_RETHROW() +} + BOOST_AUTO_TEST_CASE( create_account_with_cycled_authority_before_HARDFORK_CYCLED_ACCOUNTS_TIME_test ) { try { From e93b2032664291c1104f8f2f58f1471d453008e6 Mon Sep 17 00:00:00 2001 From: Alexey Frolov Date: Mon, 22 Apr 2019 18:05:15 +0300 Subject: [PATCH 03/14] Add verify_cycled_authority Find locked accounts. Prevent update + create locked account. UT. Add unit tests: max auth depth limit --- libraries/chain/account_evaluator.cpp | 115 ++++------- .../include/graphene/protocol/transaction.hpp | 4 + libraries/protocol/transaction.cpp | 47 ++++- tests/elasticsearch/main.cpp | 143 +++++++++++++ tests/tests/operation_tests.cpp | 188 ++++++++++++++++-- 5 files changed, 397 insertions(+), 100 deletions(-) diff --git a/libraries/chain/account_evaluator.cpp b/libraries/chain/account_evaluator.cpp index 914381dea2..cfc0a5f349 100644 --- a/libraries/chain/account_evaluator.cpp +++ b/libraries/chain/account_evaluator.cpp @@ -38,86 +38,58 @@ #include #include -namespace graphene { namespace chain { namespace detail { +namespace graphene { namespace chain { - template - struct authority_cycle_detector - { - using authority_t = flat_set; - - const database& db; - /** Getter that returns an authority to check. */ - Lambda get_authority; - - /** - * Detects if autority contains a cycle. - * Throw exeption if it finds a cycle. - * - * @param d a database - * @param auth_to_check an authority to check - * @param account_id for that account need to check authority - * @param getter Lambda that returns an authority to check. - */ - explicit authority_cycle_detector(const database& d, const authority& auth_to_check, const account_id_type& account_id, Lambda getter) - : db{d}, get_authority{getter} - { - /** Set of accounts ids used to detect a cycle.*/ - authority_t account_ids{}; - const auto& chain_params = db.get_global_properties().parameters; - detect_cycle(account_id, auth_to_check, account_ids, chain_params.maximum_authority_membership); - } + void verify_cycled_authority( const account_id_type& id, + const std::function& get_active, + const std::function& get_owner, + uint32_t max_recursion_depth ); - /** - * Recursively detects if autority contains a cycle. - * Get authorities for each account from auth_to_check and check for cycle - * taking in account path - * Throw exeption if it finds a cycle. - * - * @param account_id find a cycle from this account - * @param auth_to_check an authority to check - * @param path a chain off accounts that led to account_id - */ - void detect_cycle(const account_id_type& account_id, const authority& auth_to_check, authority_t& path, uint16_t max_depth) - { - GRAPHENE_ASSERT(max_depth > 0, internal_verify_auth_max_auth_exceeded, "Maximum authority membership exceeded" ); - - auto inserted = false; - std::tie(std::ignore, inserted) = path.insert(account_id); - FC_ASSERT(inserted, "Account ${a} already is in authority.", ("a", account_id)); +namespace detail { - for(const auto& el : auth_to_check.account_auths) - { - const auto& account = el.first(db); - const auto& auth = get_authority(account); - detect_cycle(el.first, auth, path, max_depth - 1); - } - path.erase(account_id); - } - }; - - void verify_account_authorities( const database& db, - const account_id_type& account_id, - const fc::optional& active, - const fc::optional& owner) + template + void check_account_authorities(const account_id_type account_id, + const DB& db, + const optional& active, + const optional& owner) { if( db.head_block_time() < HARDFORK_CYCLED_ACCOUNTS_TIME ) { return; } - if( active ) + const auto null_auth = authority::null_authority(); + const auto no_account = account_id_type{}; + + auto get_active = [&account_id, &db, &active, &owner, &null_auth, &no_account]( account_id_type id ) { - auto active_auth_getter = [](const account_object& account) { return account.active; }; - authority_cycle_detector validator{ db, *active, account_id, active_auth_getter }; - } + if ( (no_account == id) || (account_id == id) ) + { + if (active) + { + return &(*active); + } + return &null_auth; + } + return &id(db).active; + }; - if( owner ) + auto get_owner = [&account_id, &db, &active, &owner, &null_auth, &no_account]( account_id_type id ) { - auto owner_auth_getter = [](const account_object& account) { return account.owner; }; - authority_cycle_detector validator{ db, *owner, account_id, owner_auth_getter }; - } - } -}//detail + if ( (no_account == id) || (account_id == id) ) + { + if (owner) + { + return &(*owner); + } + return &null_auth; + } + return &id(db).owner; + }; + + verify_cycled_authority(account_id, get_active, get_owner, db.get_global_properties().parameters.max_authority_depth ); + }; +} //detail void verify_authority_accounts( const database& db, const authority& a ) { @@ -214,8 +186,7 @@ void_result account_create_evaluator::do_evaluate( const account_create_operatio verify_authority_accounts( d, op.owner ); verify_authority_accounts( d, op.active ); - // avoid cycles in authorities - detail::verify_account_authorities(d, account_id_type{}, op.active, op.owner); + detail::check_account_authorities({}, d, op.active, op.owner); } GRAPHENE_RECODE_EXC( internal_verify_auth_max_auth_exceeded, account_create_max_auth_exceeded ) GRAPHENE_RECODE_EXC( internal_verify_auth_account_not_found, account_create_auth_account_not_found ) @@ -367,8 +338,8 @@ void_result account_update_evaluator::do_evaluate( const account_update_operatio { verify_authority_accounts( d, *o.active ); } - // avoid cycles in authorities started from o.account - detail::verify_account_authorities(d, o.account, o.active, o.owner); + + detail::check_account_authorities(o.account, d, o.active, o.owner); } GRAPHENE_RECODE_EXC( internal_verify_auth_max_auth_exceeded, account_update_max_auth_exceeded ) GRAPHENE_RECODE_EXC( internal_verify_auth_account_not_found, account_update_auth_account_not_found ) diff --git a/libraries/protocol/include/graphene/protocol/transaction.hpp b/libraries/protocol/include/graphene/protocol/transaction.hpp index 17b7874d41..513383a8f9 100644 --- a/libraries/protocol/include/graphene/protocol/transaction.hpp +++ b/libraries/protocol/include/graphene/protocol/transaction.hpp @@ -258,6 +258,10 @@ namespace graphene { namespace protocol { const flat_set& active_aprovals = flat_set(), const flat_set& owner_approvals = flat_set()); +// void verify_cycled_authority( const account_id_type& id, +// const std::function& get_active, +// const std::function& get_owner, +// uint32_t max_recursion_depth ); /** * @brief captures the result of evaluating the operations contained in the transaction * diff --git a/libraries/protocol/transaction.cpp b/libraries/protocol/transaction.cpp index 42b1d8ee0f..4ab92931bb 100644 --- a/libraries/protocol/transaction.cpp +++ b/libraries/protocol/transaction.cpp @@ -122,9 +122,11 @@ struct sign_state auto itr = provided_signatures.find(k); if( itr == provided_signatures.end() ) { - auto pk = available_keys.find(k); - if( pk != available_keys.end() ) + if( is_key_available(k) ) + { return provided_signatures[k] = true; + } + return false; } return itr->second = true; @@ -156,10 +158,13 @@ struct sign_state if( itr == provided_address_sigs->end() ) { auto aitr = available_address_sigs->find(a); - if( aitr != available_address_sigs->end() ) { - auto pk = available_keys.find(aitr->second); - if( pk != available_keys.end() ) + if( aitr != available_address_sigs->end() ) + { + if( is_key_available(aitr->second) ) + { return provided_signatures[aitr->second] = true; + } + return false; } } @@ -172,6 +177,17 @@ struct sign_state return check_authority( get_active(id) ) || ( allow_non_immediate_owner && check_authority( get_owner(id) ) ); } + bool is_key_available(const public_key_type &key) + { + if (verify_keyset) + { + return true; + } + + auto pk = available_keys.find(key); + return (pk != available_keys.end()); + } + /** * Checks to see if we have signatures of the active authorites of * the accounts specified in authority or the keys specified. @@ -240,12 +256,14 @@ struct sign_state const std::function& owner, bool allow_owner, uint32_t max_recursion_depth = GRAPHENE_MAX_SIG_CHECK_DEPTH, - const flat_set& keys = empty_keyset ) + const flat_set& keys = empty_keyset, + const bool verify = false ) : get_active(active), get_owner(owner), allow_non_immediate_owner(allow_owner), max_recursion(max_recursion_depth), - available_keys(keys) + available_keys(keys), + verify_keyset(verify) { for( const auto& key : sigs ) provided_signatures[ key ] = false; @@ -258,12 +276,27 @@ struct sign_state const bool allow_non_immediate_owner; const uint32_t max_recursion; const flat_set& available_keys; + const bool verify_keyset; flat_map provided_signatures; flat_set approved_by; }; +void verify_cycled_authority( const account_id_type& id, + const std::function& get_active, + const std::function& get_owner, + uint32_t max_recursion_depth ) +{ try { + sign_state s( empty_keyset, get_active, get_owner, true, max_recursion_depth, empty_keyset, true ); + + GRAPHENE_ASSERT( s.check_authority(id), + tx_missing_active_auth, + "Missing Authority ${id}", + ("id",id)("auth",*get_active(id))("owner",*get_owner(id)) + ); +} FC_CAPTURE_AND_RETHROW( (id)(max_recursion_depth) ) } + void verify_authority( const vector& ops, const flat_set& sigs, const std::function& get_active, const std::function& get_owner, diff --git a/tests/elasticsearch/main.cpp b/tests/elasticsearch/main.cpp index 33f8b11db9..acabe68e85 100644 --- a/tests/elasticsearch/main.cpp +++ b/tests/elasticsearch/main.cpp @@ -29,6 +29,7 @@ #include #include "../common/database_fixture.hpp" +#include "../../libraries/chain/account_evaluator.cpp" #define BOOST_TEST_MODULE Elastic Search Database Tests #include @@ -37,6 +38,14 @@ using namespace graphene::chain; using namespace graphene::chain::test; using namespace graphene::app; +#include +#include +#include +#include +#include + +namespace fs = boost::filesystem; + BOOST_FIXTURE_TEST_SUITE( elasticsearch_tests, database_fixture ) BOOST_AUTO_TEST_CASE(elasticsearch_account_history) { @@ -210,4 +219,138 @@ BOOST_AUTO_TEST_CASE(elasticsearch_suite) { } } +struct locked_account_finder +{ + const database& db; + std::unique_ptr curl; + const std::string potentially_locked_file = "/potentially_locked.json"; + fs::path current_path; + + mutable std::map accounts; + + locked_account_finder(const database& d) + : db(d) + , curl(curl_easy_init(), curl_easy_cleanup) + { + current_path = fs::current_path(); + } + + time_point_sec head_block_time() const + { + return HARDFORK_CYCLED_ACCOUNTS_TIME; + } + + const global_property_object& get_global_properties() const + { + return db.get_global_properties(); + } + + const account_object& get( account_id_type id ) const + { + auto it = accounts.find(id); + if (accounts.end() != it) + { + return it->second; + } + + auto account = get_from_bts(id); + auto res = accounts.insert(std::make_pair(account.get_id(), account)); + return res.first->second; + } + + account_object get_from_bts(account_id_type account_id) const + { + variant vo; + to_variant(account_id, vo); + auto id_str = vo.as_string(); + + graphene::utilities::CurlRequest curl_request; + curl_request.handler = curl.get(); + curl_request.url = "http://127.0.0.1:8092/rpc"; + curl_request.type = "POST"; + curl_request.query = R"({"jsonrpc":"2.0","method":"get_account","params":[")" + id_str + R"("],"id":1})"; + auto response = doCurl(curl_request); + + variant variant_response = fc::json::from_string(response); + optional account; + fc::from_variant(variant_response["result"], account, FC_PACK_MAX_DEPTH); + return *account; + } + + void store_potentially_locked_accounts(const fs::path& file_path) const + { + constexpr auto ES_URL = "http://bselastic.dev.aetsoft.by/"; + graphene::utilities::ES es; + es.curl = curl.get(); + es.elasticsearch_url = ES_URL; + es.index_prefix = "objects-account"; + es.endpoint = es.index_prefix + "/_search?size=0&pretty=true"; + es.query = R"({"_source": ["object_id"])"; + es.query.append(R"(,"query":{"bool":{"should":[{"bool":{"must_not":{"term":{"active_account_auths.keyword":"[]"}}}})"); + es.query.append(R"(,{"bool":{"must_not":{"term":{"owner_account_auths.keyword":"[]"}}}}]}}})"); + auto res = graphene::utilities::simpleQuery(es); + variant json_result = fc::json::from_string(res); + auto count = json_result["hits"]["total"].as_string(); + std::cout << "total:" << count << std::endl; + + es.endpoint = es.index_prefix + "/_search?size=" + count +"&pretty=true"; + res = graphene::utilities::simpleQuery(es); + + // save accounts to file + std::ofstream file(file_path.string(), std::ofstream::out); + std::copy(res.begin(),res.end(), std::ostreambuf_iterator(file)); + } + + variant get_potentially_locked(const fs::path& file_path) const + { + std::ifstream file(file_path.string()); + std::string data((std::istreambuf_iterator(file)), std::istreambuf_iterator()); + return fc::json::from_string(data); + } + + void run() const + { + auto potentially_locked_path = current_path; + potentially_locked_path += potentially_locked_file; + if ( !fs::exists( fs::status(potentially_locked_path) ) ) + { + store_potentially_locked_accounts(potentially_locked_path); + } + auto potentially_locked_accounts = get_potentially_locked(potentially_locked_path); + + const auto total_count = potentially_locked_accounts["hits"]["total"].as_int64(); + + auto locked_path = current_path; + locked_path += "/locked.txt"; + std::ofstream locked_file(locked_path.string(), std::ofstream::out); + + for(auto i = 0; i < total_count; ++i) + { + account_id_type account_id; + const auto v_account_id = potentially_locked_accounts["hits"]["hits"][size_t(i)]["_source"]["object_id"]; + from_variant(v_account_id, account_id); + + auto account = get(account_id); + std::cout << "id: " << v_account_id.as_string() << " checked: " << i << " from: " << total_count << std::endl; + try + { + graphene::chain::detail::check_account_authorities(account.get_id(), *this, account.active, account.owner); + } + catch(const tx_missing_active_auth &) + { + locked_file << v_account_id.as_string() << std::endl; + } + } + } +}; + +BOOST_AUTO_TEST_CASE(find_locked_accounts) { + try { + + locked_account_finder finder(db); + finder.run(); + } + FC_LOG_AND_RETHROW() +} + BOOST_AUTO_TEST_SUITE_END() diff --git a/tests/tests/operation_tests.cpp b/tests/tests/operation_tests.cpp index 6eedb829f8..05b2ac0966 100644 --- a/tests/tests/operation_tests.cpp +++ b/tests/tests/operation_tests.cpp @@ -1093,30 +1093,68 @@ BOOST_AUTO_TEST_CASE( update_account_self_cycled_authority_before_HARDFORK_CYCLE BOOST_AUTO_TEST_CASE( update_account_self_cycled_authority_after_HARDFORK_CYCLED_ACCOUNTS_TIME_test ) { try { - ACTOR(nathan); + ACTOR(alice); + ACTOR(bob); + + fund(bob); + fund(alice); generate_blocks(HARDFORK_CYCLED_ACCOUNTS_TIME); + trx.clear(); set_expiration( db, trx ); - auto update_auth = [&](const account_update_operation& op){ - trx.operations.clear(); - trx.operations.push_back(op); - PUSH_TX( db, trx, ~0 ); + auto fill_transfer = [&](const account_object& from, + const account_object& to, + const asset& amount, + const asset& fee = asset()) + { + transfer_operation trans; + trans.from = from.id; + trans.to = to.id; + trans.amount = amount; + trx.operations.push_back(trans); + + if( fee == asset() ) + { + for( auto& op : trx.operations ) db.current_fee_schedule().set_fee(op); + } + trx.validate(); }; - account_update_operation op; - op.account = nathan_id; - // make cycle to self in active auth - { - op.active = authority(1, nathan_id, 1); - GRAPHENE_CHECK_THROW(update_auth(op), fc::exception); - } - // make cycle to self in owner auth - { - op.owner = {}; - op.active = authority(1, nathan_id, 1); - GRAPHENE_CHECK_THROW(update_auth(op), fc::exception); - } + // make cycle to self in active and owner authority for Alice + account_update_operation auo; + auo.account = alice_id; + auo.owner = authority(1, alice_id, 1); + auo.active = authority(1, alice_id, 1); + trx.operations.push_back( auo ); + sign( trx, alice_private_key ); + GRAPHENE_CHECK_THROW(PUSH_TX( db, trx ), fc::exception); + trx.clear(); + + // Add Alice to Bob's authority + auo.account = bob_id; + auo.active = authority( 1, alice_id, 1 ); + auo.owner = authority( 1, alice_id, 1 ); + trx.operations.push_back( auo ); + sign( trx, bob_private_key ); + PUSH_TX( db, trx ); + trx.clear(); + + // make cycle to self in active and owner authority via Bob for Alice + auo.account = alice_id; + auo.owner = authority(1, bob_id, 1); + auo.active = authority(1, bob_id, 1); + trx.operations.push_back( auo ); + sign( trx, alice_private_key ); + //PUSH_TX( db, trx ); + GRAPHENE_CHECK_THROW(PUSH_TX( db, trx ), fc::exception); + trx.clear(); + + fill_transfer(alice, bob, asset(10)); + sign( trx, bob_private_key ); + sign( trx, alice_private_key ); + GRAPHENE_CHECK_THROW(PUSH_TX( db, trx ), fc::exception); + trx.clear(); } FC_LOG_AND_RETHROW() } @@ -1151,6 +1189,7 @@ BOOST_AUTO_TEST_CASE( update_account_authority_test ) { op.account = bob_id; op.active = authority(2, alice_id, 1, jill_id, 1); + op.owner = op.active; update_auth(op); { GET_ACTOR(bob); @@ -1159,6 +1198,7 @@ BOOST_AUTO_TEST_CASE( update_account_authority_test ) op.account = alice_id; op.active = authority(2, jill_id, 1, nathan_id, 1); + op.owner = op.active; update_auth(op); { GET_ACTOR(alice); @@ -1167,6 +1207,7 @@ BOOST_AUTO_TEST_CASE( update_account_authority_test ) op.account = jill_id; op.active = authority(1, izzy_id, 1); + op.owner = op.active; update_auth(op); { GET_ACTOR(jill); @@ -1175,6 +1216,7 @@ BOOST_AUTO_TEST_CASE( update_account_authority_test ) op.account = izzy_id; op.active = authority(1, nathan_id, 1); + op.owner = op.active; update_auth(op); { GET_ACTOR(izzy); @@ -1215,7 +1257,7 @@ BOOST_AUTO_TEST_CASE( update_account_make_cycled_authority_before_HARDFORK_CYCLE trx.operations.clear(); trx.operations.push_back(op); - PUSH_TX( db, trx, ~0 ); + GRAPHENE_CHECK_THROW(PUSH_TX( db, trx, ~0 ), fc::exception); } FC_LOG_AND_RETHROW() } @@ -1250,10 +1292,11 @@ BOOST_AUTO_TEST_CASE( update_account_make_cycled_authority_after_HARDFORK_CYCLED account_update_operation op; op.account = nathan_id; op.active = authority(1, bob_id, 1); + op.owner = authority(1, bob_id, 1); trx.operations.clear(); trx.operations.push_back(op); - GRAPHENE_CHECK_THROW(PUSH_TX( db, trx, ~0 ), fc::exception); + PUSH_TX( db, trx, ~0 ); } FC_LOG_AND_RETHROW() } @@ -1293,7 +1336,7 @@ BOOST_AUTO_TEST_CASE( update_account_max_authority_depth_after_HARDFORK_CYCLED_A set_expiration( db, trx ); db.modify( db.get_global_properties(), [&]( global_property_object& gpo ) { - gpo.parameters.maximum_authority_membership = 1; + gpo.parameters.max_authority_depth = 1; }); account_update_operation op; @@ -1307,6 +1350,109 @@ BOOST_AUTO_TEST_CASE( update_account_max_authority_depth_after_HARDFORK_CYCLED_A FC_LOG_AND_RETHROW() } +BOOST_AUTO_TEST_CASE( update_account_exceed_max_authority_depth_after_HARDFORK_CYCLED_ACCOUNTS_TIME_test ) +{ + // make account authority with exceeded depth limit + // this special case possible despite prevention logic defined after HARDFORK_CYCLED_ACCOUNTS_TIME + // + // initial accounts: alice, bob, charlie, dan + // step by step delegate authorities + // 1) alice--->bob + // 2) bob--->charlie + // 3) charlie--->dan + // account alice locked due to exceeded authority depth limit!!! + + try { + ACTORS((alice)(bob)(charlie)(dan)) + + fund(alice); + fund(bob); + fund(charlie); + + generate_blocks(HARDFORK_CYCLED_ACCOUNTS_TIME); + trx.clear(); + set_expiration( db, trx ); + + auto delegate_authorities = [&](const account_id_type& from, + const private_key_type& from_key, + const account_id_type& to) + { + account_update_operation auo; + auo.account = from; + auo.active = authority( 1, to, 1 ); + auo.owner = authority( 1, to, 1 ); + trx.operations.push_back( auo ); + sign( trx, from_key ); + PUSH_TX( db, trx ); + trx.clear(); + }; + + delegate_authorities(alice_id, alice_private_key, bob_id); + delegate_authorities(bob_id, bob_private_key, charlie_id); + delegate_authorities(charlie_id, charlie_private_key, dan_id); + + { + transfer_operation tr_op; + tr_op.from = alice_id; + tr_op.to = dan_id; + tr_op.amount = asset(10); + + trx.operations = { tr_op }; + sign( trx, dan_private_key ); + trx.validate(); + + GRAPHENE_CHECK_THROW(PUSH_TX( db, trx ), graphene::chain::tx_missing_active_auth); + trx.clear(); + } + + } FC_LOG_AND_RETHROW() +} + +BOOST_AUTO_TEST_CASE( cant_create_auth_chain_longer_than_max_authority_depth_limit_after_HARDFORK_CYCLED_ACCOUNTS_TIME_test ) +{ + // + // initial accounts: alice, bob, charlie, dan + // step by step delegate authorities + // 1) charlie--->dan + // 2) bob--->charlie + // 3) alice--->bob + // alice can't delegete auth to bob due to exceeded authority depth limit!!! + + try { + ACTORS((alice)(bob)(charlie)(dan)) + + fund(alice); + fund(bob); + fund(charlie); + + generate_blocks(HARDFORK_CYCLED_ACCOUNTS_TIME); + trx.clear(); + set_expiration( db, trx ); + + auto delegate_authorities = [&](const account_id_type& from, + const private_key_type& from_key, + const account_id_type& to) + { + account_update_operation auo; + auo.account = from; + auo.active = authority( 1, to, 1 ); + auo.owner = authority( 1, to, 1 ); + trx.operations.push_back( auo ); + sign( trx, from_key ); + PUSH_TX( db, trx ); + trx.clear(); + }; + + delegate_authorities(charlie_id, charlie_private_key, dan_id); + delegate_authorities(bob_id, bob_private_key, charlie_id); + + GRAPHENE_CHECK_THROW( + delegate_authorities(alice_id, alice_private_key, bob_id), + graphene::chain::tx_missing_active_auth); + } + FC_LOG_AND_RETHROW() +} + BOOST_AUTO_TEST_CASE( transfer_core_asset ) { try { From d5fdd5a79d7c4ff5381c10d643b12ffb1caf4a25 Mon Sep 17 00:00:00 2001 From: Alexey Frolov Date: Mon, 22 Apr 2019 18:11:51 +0300 Subject: [PATCH 04/14] Fix unit tests after merge Remove unnecessary code --- .../include/graphene/protocol/transaction.hpp | 4 - tests/elasticsearch/main.cpp | 143 ------------------ tests/tests/operation_tests.cpp | 8 +- 3 files changed, 5 insertions(+), 150 deletions(-) diff --git a/libraries/protocol/include/graphene/protocol/transaction.hpp b/libraries/protocol/include/graphene/protocol/transaction.hpp index 513383a8f9..17b7874d41 100644 --- a/libraries/protocol/include/graphene/protocol/transaction.hpp +++ b/libraries/protocol/include/graphene/protocol/transaction.hpp @@ -258,10 +258,6 @@ namespace graphene { namespace protocol { const flat_set& active_aprovals = flat_set(), const flat_set& owner_approvals = flat_set()); -// void verify_cycled_authority( const account_id_type& id, -// const std::function& get_active, -// const std::function& get_owner, -// uint32_t max_recursion_depth ); /** * @brief captures the result of evaluating the operations contained in the transaction * diff --git a/tests/elasticsearch/main.cpp b/tests/elasticsearch/main.cpp index acabe68e85..33f8b11db9 100644 --- a/tests/elasticsearch/main.cpp +++ b/tests/elasticsearch/main.cpp @@ -29,7 +29,6 @@ #include #include "../common/database_fixture.hpp" -#include "../../libraries/chain/account_evaluator.cpp" #define BOOST_TEST_MODULE Elastic Search Database Tests #include @@ -38,14 +37,6 @@ using namespace graphene::chain; using namespace graphene::chain::test; using namespace graphene::app; -#include -#include -#include -#include -#include - -namespace fs = boost::filesystem; - BOOST_FIXTURE_TEST_SUITE( elasticsearch_tests, database_fixture ) BOOST_AUTO_TEST_CASE(elasticsearch_account_history) { @@ -219,138 +210,4 @@ BOOST_AUTO_TEST_CASE(elasticsearch_suite) { } } -struct locked_account_finder -{ - const database& db; - std::unique_ptr curl; - const std::string potentially_locked_file = "/potentially_locked.json"; - fs::path current_path; - - mutable std::map accounts; - - locked_account_finder(const database& d) - : db(d) - , curl(curl_easy_init(), curl_easy_cleanup) - { - current_path = fs::current_path(); - } - - time_point_sec head_block_time() const - { - return HARDFORK_CYCLED_ACCOUNTS_TIME; - } - - const global_property_object& get_global_properties() const - { - return db.get_global_properties(); - } - - const account_object& get( account_id_type id ) const - { - auto it = accounts.find(id); - if (accounts.end() != it) - { - return it->second; - } - - auto account = get_from_bts(id); - auto res = accounts.insert(std::make_pair(account.get_id(), account)); - return res.first->second; - } - - account_object get_from_bts(account_id_type account_id) const - { - variant vo; - to_variant(account_id, vo); - auto id_str = vo.as_string(); - - graphene::utilities::CurlRequest curl_request; - curl_request.handler = curl.get(); - curl_request.url = "http://127.0.0.1:8092/rpc"; - curl_request.type = "POST"; - curl_request.query = R"({"jsonrpc":"2.0","method":"get_account","params":[")" + id_str + R"("],"id":1})"; - auto response = doCurl(curl_request); - - variant variant_response = fc::json::from_string(response); - optional account; - fc::from_variant(variant_response["result"], account, FC_PACK_MAX_DEPTH); - return *account; - } - - void store_potentially_locked_accounts(const fs::path& file_path) const - { - constexpr auto ES_URL = "http://bselastic.dev.aetsoft.by/"; - graphene::utilities::ES es; - es.curl = curl.get(); - es.elasticsearch_url = ES_URL; - es.index_prefix = "objects-account"; - es.endpoint = es.index_prefix + "/_search?size=0&pretty=true"; - es.query = R"({"_source": ["object_id"])"; - es.query.append(R"(,"query":{"bool":{"should":[{"bool":{"must_not":{"term":{"active_account_auths.keyword":"[]"}}}})"); - es.query.append(R"(,{"bool":{"must_not":{"term":{"owner_account_auths.keyword":"[]"}}}}]}}})"); - auto res = graphene::utilities::simpleQuery(es); - variant json_result = fc::json::from_string(res); - auto count = json_result["hits"]["total"].as_string(); - std::cout << "total:" << count << std::endl; - - es.endpoint = es.index_prefix + "/_search?size=" + count +"&pretty=true"; - res = graphene::utilities::simpleQuery(es); - - // save accounts to file - std::ofstream file(file_path.string(), std::ofstream::out); - std::copy(res.begin(),res.end(), std::ostreambuf_iterator(file)); - } - - variant get_potentially_locked(const fs::path& file_path) const - { - std::ifstream file(file_path.string()); - std::string data((std::istreambuf_iterator(file)), std::istreambuf_iterator()); - return fc::json::from_string(data); - } - - void run() const - { - auto potentially_locked_path = current_path; - potentially_locked_path += potentially_locked_file; - if ( !fs::exists( fs::status(potentially_locked_path) ) ) - { - store_potentially_locked_accounts(potentially_locked_path); - } - auto potentially_locked_accounts = get_potentially_locked(potentially_locked_path); - - const auto total_count = potentially_locked_accounts["hits"]["total"].as_int64(); - - auto locked_path = current_path; - locked_path += "/locked.txt"; - std::ofstream locked_file(locked_path.string(), std::ofstream::out); - - for(auto i = 0; i < total_count; ++i) - { - account_id_type account_id; - const auto v_account_id = potentially_locked_accounts["hits"]["hits"][size_t(i)]["_source"]["object_id"]; - from_variant(v_account_id, account_id); - - auto account = get(account_id); - std::cout << "id: " << v_account_id.as_string() << " checked: " << i << " from: " << total_count << std::endl; - try - { - graphene::chain::detail::check_account_authorities(account.get_id(), *this, account.active, account.owner); - } - catch(const tx_missing_active_auth &) - { - locked_file << v_account_id.as_string() << std::endl; - } - } - } -}; - -BOOST_AUTO_TEST_CASE(find_locked_accounts) { - try { - - locked_account_finder finder(db); - finder.run(); - } - FC_LOG_AND_RETHROW() -} - BOOST_AUTO_TEST_SUITE_END() diff --git a/tests/tests/operation_tests.cpp b/tests/tests/operation_tests.cpp index 05b2ac0966..ffdbeb6f2d 100644 --- a/tests/tests/operation_tests.cpp +++ b/tests/tests/operation_tests.cpp @@ -1146,7 +1146,6 @@ BOOST_AUTO_TEST_CASE( update_account_self_cycled_authority_after_HARDFORK_CYCLED auo.active = authority(1, bob_id, 1); trx.operations.push_back( auo ); sign( trx, alice_private_key ); - //PUSH_TX( db, trx ); GRAPHENE_CHECK_THROW(PUSH_TX( db, trx ), fc::exception); trx.clear(); @@ -1257,7 +1256,7 @@ BOOST_AUTO_TEST_CASE( update_account_make_cycled_authority_before_HARDFORK_CYCLE trx.operations.clear(); trx.operations.push_back(op); - GRAPHENE_CHECK_THROW(PUSH_TX( db, trx, ~0 ), fc::exception); + PUSH_TX( db, trx, ~0 ); } FC_LOG_AND_RETHROW() } @@ -1296,7 +1295,10 @@ BOOST_AUTO_TEST_CASE( update_account_make_cycled_authority_after_HARDFORK_CYCLED trx.operations.clear(); trx.operations.push_back(op); - PUSH_TX( db, trx, ~0 ); + + GRAPHENE_CHECK_THROW( + PUSH_TX( db, trx, ~0 ), + graphene::chain::tx_missing_active_auth); } FC_LOG_AND_RETHROW() } From 3c6048b032b8740063c62eaca39683fac9dfd9a6 Mon Sep 17 00:00:00 2001 From: Dmitry Yakovitsky Date: Fri, 3 May 2019 16:55:09 +0300 Subject: [PATCH 05/14] CORE-978 BSIP-Fix-locked: fix unit tests cli_test --- libraries/chain/account_evaluator.cpp | 10 +++++----- tests/tests/operation_tests.cpp | 17 +++++++++++++++++ 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/libraries/chain/account_evaluator.cpp b/libraries/chain/account_evaluator.cpp index cfc0a5f349..67c3c69120 100644 --- a/libraries/chain/account_evaluator.cpp +++ b/libraries/chain/account_evaluator.cpp @@ -58,10 +58,10 @@ namespace detail { return; } - const auto null_auth = authority::null_authority(); + const auto empty_auth = authority{}; const auto no_account = account_id_type{}; - auto get_active = [&account_id, &db, &active, &owner, &null_auth, &no_account]( account_id_type id ) + auto get_active = [&account_id, &db, &active, &owner, &empty_auth, &no_account]( account_id_type id ) { if ( (no_account == id) || (account_id == id) ) { @@ -69,12 +69,12 @@ namespace detail { { return &(*active); } - return &null_auth; + return &empty_auth; } return &id(db).active; }; - auto get_owner = [&account_id, &db, &active, &owner, &null_auth, &no_account]( account_id_type id ) + auto get_owner = [&account_id, &db, &active, &owner, &empty_auth, &no_account]( account_id_type id ) { if ( (no_account == id) || (account_id == id) ) { @@ -82,7 +82,7 @@ namespace detail { { return &(*owner); } - return &null_auth; + return &empty_auth; } return &id(db).owner; }; diff --git a/tests/tests/operation_tests.cpp b/tests/tests/operation_tests.cpp index ffdbeb6f2d..e5a658a467 100644 --- a/tests/tests/operation_tests.cpp +++ b/tests/tests/operation_tests.cpp @@ -1226,6 +1226,22 @@ BOOST_AUTO_TEST_CASE( update_account_authority_test ) FC_LOG_AND_RETHROW() } +BOOST_AUTO_TEST_CASE( disables_update_to_empty_account_authority ) +{ + try { + ACTOR(nathan); + + account_update_operation op; + op.account = nathan_id; + op.active = authority{}; + op.owner = op.active; + trx.operations.clear(); + trx.operations.push_back(op); + GRAPHENE_CHECK_THROW( PUSH_TX( db, trx, ~0 ), fc::exception ); + } + FC_LOG_AND_RETHROW() +} + BOOST_AUTO_TEST_CASE( update_account_make_cycled_authority_before_HARDFORK_CYCLED_ACCOUNTS_TIME_test) { // make cycle with other accounts in owner auth @@ -1344,6 +1360,7 @@ BOOST_AUTO_TEST_CASE( update_account_max_authority_depth_after_HARDFORK_CYCLED_A account_update_operation op; op.account = nathan_id; op.active = authority(1, bob_id, 1); + op.owner = op.active; trx.operations.clear(); trx.operations.push_back(op); From f7ee13f8c87fb0e7b7aad3c29f0b11dc82eb97e5 Mon Sep 17 00:00:00 2001 From: Alexey Frolov Date: Tue, 21 May 2019 17:55:00 +0300 Subject: [PATCH 06/14] save restore point of account authority unit test: get restore point --- libraries/chain/account_evaluator.cpp | 37 +++++++--- .../graphene/chain/account_evaluator.hpp | 1 + .../include/graphene/chain/account_object.hpp | 1 + tests/tests/operation_tests.cpp | 68 +++++++++++++++++++ 4 files changed, 97 insertions(+), 10 deletions(-) diff --git a/libraries/chain/account_evaluator.cpp b/libraries/chain/account_evaluator.cpp index 67c3c69120..3dfb38fac7 100644 --- a/libraries/chain/account_evaluator.cpp +++ b/libraries/chain/account_evaluator.cpp @@ -53,11 +53,6 @@ namespace detail { const optional& active, const optional& owner) { - if( db.head_block_time() < HARDFORK_CYCLED_ACCOUNTS_TIME ) - { - return; - } - const auto empty_auth = authority{}; const auto no_account = account_id_type{}; @@ -87,7 +82,7 @@ namespace detail { return &id(db).owner; }; - verify_cycled_authority(account_id, get_active, get_owner, db.get_global_properties().parameters.max_authority_depth ); + verify_cycled_authority(account_id, get_active, get_owner, db.get_global_properties().parameters.max_authority_depth); }; } //detail @@ -186,7 +181,10 @@ void_result account_create_evaluator::do_evaluate( const account_create_operatio verify_authority_accounts( d, op.owner ); verify_authority_accounts( d, op.active ); - detail::check_account_authorities({}, d, op.active, op.owner); + if( d.head_block_time() >= HARDFORK_CYCLED_ACCOUNTS_TIME ) + { + detail::check_account_authorities({}, d, op.active, op.owner); + } } GRAPHENE_RECODE_EXC( internal_verify_auth_max_auth_exceeded, account_create_max_auth_exceeded ) GRAPHENE_RECODE_EXC( internal_verify_auth_account_not_found, account_create_auth_account_not_found ) @@ -338,8 +336,22 @@ void_result account_update_evaluator::do_evaluate( const account_update_operatio { verify_authority_accounts( d, *o.active ); } - - detail::check_account_authorities(o.account, d, o.active, o.owner); + + try + { + detail::check_account_authorities(o.account, d, o.active, o.owner); + } + catch (tx_missing_active_auth) + { + if( d.head_block_time() < HARDFORK_CYCLED_ACCOUNTS_TIME ) + { + save_owner = true; + } + else + { + throw; + } + } } GRAPHENE_RECODE_EXC( internal_verify_auth_max_auth_exceeded, account_update_max_auth_exceeded ) GRAPHENE_RECODE_EXC( internal_verify_auth_account_not_found, account_update_auth_account_not_found ) @@ -378,7 +390,12 @@ void_result account_update_evaluator::do_apply( const account_update_operation& } // update account object - d.modify( *acnt, [&o](account_object& a){ + d.modify( *acnt, [&o, this](account_object& a){ + if( save_owner ) + { + a.stable_owner = a.owner; + } + if( o.owner ) { a.owner = *o.owner; diff --git a/libraries/chain/include/graphene/chain/account_evaluator.hpp b/libraries/chain/include/graphene/chain/account_evaluator.hpp index e543c90dab..28c45d946a 100644 --- a/libraries/chain/include/graphene/chain/account_evaluator.hpp +++ b/libraries/chain/include/graphene/chain/account_evaluator.hpp @@ -45,6 +45,7 @@ class account_update_evaluator : public evaluator void_result do_apply( const account_update_operation& o ); const account_object* acnt; + bool save_owner = false; }; class account_upgrade_evaluator : public evaluator diff --git a/libraries/chain/include/graphene/chain/account_object.hpp b/libraries/chain/include/graphene/chain/account_object.hpp index 70aa0f78e9..5cde51e3b0 100644 --- a/libraries/chain/include/graphene/chain/account_object.hpp +++ b/libraries/chain/include/graphene/chain/account_object.hpp @@ -190,6 +190,7 @@ namespace graphene { namespace chain { * update the active authority. */ authority owner; + optional stable_owner; /// The owner authority contains the hot keys of the account. This authority has control over nearly all /// operations the account may perform. authority active; diff --git a/tests/tests/operation_tests.cpp b/tests/tests/operation_tests.cpp index e5a658a467..300cd67150 100644 --- a/tests/tests/operation_tests.cpp +++ b/tests/tests/operation_tests.cpp @@ -1020,6 +1020,74 @@ BOOST_AUTO_TEST_CASE( update_account_lead_to_cycled_authority_before_HARDFORK_CY FC_LOG_AND_RETHROW() } +BOOST_AUTO_TEST_CASE( get_restore_point_of_cycled_account_test ) +{ + /* + see https://github.com/bitshares/bsips/pull/149#discussion_r270407694 + Precondition: + There are three accounts Alice, Bob and Charlie + Steps: + 1. Alice sets Bob to her active and owner authority + 2. Charlie sets Alice to his active and owner authority + 3. Alice sets Charlie to her active and owner authority + 4. Bob sets Charlie to his active and owner authority + Expected: + Restore point created during account_update_operation evaluation + */ + try { + ACTORS( (bob) (alice) (charlie) ); + + auto update_auth = [&](const account_update_operation& op){ + trx.operations.clear(); + trx.operations.push_back(op); + PUSH_TX( db, trx, ~0 ); + }; + + { + account_update_operation op; + op.account = alice_id; + op.active = authority(1, bob_id, 1); + op.owner = op.active; + update_auth(op); + } + + { + account_update_operation op; + op.account = charlie_id; + op.active = authority(1, alice_id, 1); + op.owner = op.active; + update_auth(op); + } + + { + account_update_operation op; + op.account = alice_id; + op.active = authority(1, charlie_id, 1); + op.owner = op.active; + update_auth(op); + } + + const auto alice_stable_owner = alice_id(db).stable_owner; + BOOST_REQUIRE(alice_stable_owner.valid()); + const auto alice_expected = authority(1, bob_id, 1); + BOOST_CHECK(*alice_stable_owner == alice_expected); + + { + account_update_operation op; + op.account = bob_id; + op.active = authority(1, charlie_id, 1); + op.owner = op.active; + update_auth(op); + } + + const auto bob_stable_owner = bob_id(db).stable_owner; + BOOST_REQUIRE(bob_stable_owner.valid()); + const auto bob_expected = authority(123, bob_public_key, 123); + BOOST_CHECK(*bob_stable_owner == bob_expected); + } + FC_LOG_AND_RETHROW() +} + BOOST_AUTO_TEST_CASE( create_account_with_cycled_authority_before_HARDFORK_CYCLED_ACCOUNTS_TIME_test ) { try { From 17488edad7c66cea1b67ae6eae220c16d2ba2395 Mon Sep 17 00:00:00 2001 From: Alexey Frolov Date: Mon, 10 Jun 2019 17:33:45 +0300 Subject: [PATCH 07/14] Modify unit tests - add restore point test - fix authority depth limit test --- tests/tests/operation_tests.cpp | 92 ++++++++++++++++++++++++++++++++- 1 file changed, 90 insertions(+), 2 deletions(-) diff --git a/tests/tests/operation_tests.cpp b/tests/tests/operation_tests.cpp index 300cd67150..668cf93129 100644 --- a/tests/tests/operation_tests.cpp +++ b/tests/tests/operation_tests.cpp @@ -1088,6 +1088,91 @@ BOOST_AUTO_TEST_CASE( get_restore_point_of_cycled_account_test ) FC_LOG_AND_RETHROW() } +BOOST_AUTO_TEST_CASE( get_restore_point_if_exceed_depth_limit_test ) +{ + try { + // Delegation chain with exceeded authority depth limit + // Steps: + // 1. bob->charlie + // 2. alice->bob + // Expected: + // db saves restore point for alice account (alice_public_key) + + ACTORS( (alice) (bob) (charlie) ); + + db.modify( db.get_global_properties(), [&]( global_property_object& gpo ) { + gpo.parameters.max_authority_depth = 1; + }); + + auto update_auth = [&](const account_update_operation& op){ + trx.operations.clear(); + trx.operations.push_back(op); + PUSH_TX( db, trx, ~0 ); + }; + + { + account_update_operation op; + op.account = bob_id; + op.active = authority(1, charlie_id, 1); + op.owner = op.active; + update_auth(op); + } + + { + account_update_operation op; + op.account = alice_id; + op.active = authority(1, bob_id, 1); + op.owner = op.active; + update_auth(op); + } + + const auto alice_stable_owner = alice_id(db).stable_owner; + BOOST_REQUIRE(alice_stable_owner.valid()); + const auto expected = authority(123, alice_public_key, 123); + BOOST_CHECK(*alice_stable_owner == expected); + } + FC_LOG_AND_RETHROW() +} + +BOOST_AUTO_TEST_CASE( restore_point_not_created_if_account_not_locked_test ) +{ + try { + // Delegation chain + // Steps: + // 1. bob->charlie + // 2. alice->bob + // Expected: + // no restore point for alice account saved + + ACTORS( (alice) (bob) (charlie) ); + + auto update_auth = [&](const account_update_operation& op){ + trx.operations.clear(); + trx.operations.push_back(op); + PUSH_TX( db, trx, ~0 ); + }; + + { + account_update_operation op; + op.account = bob_id; + op.active = authority(1, charlie_id, 1); + op.owner = op.active; + update_auth(op); + } + + { + account_update_operation op; + op.account = alice_id; + op.active = authority(1, bob_id, 1); + op.owner = op.active; + update_auth(op); + } + + BOOST_REQUIRE(!alice_id(db).stable_owner.valid()); + } + FC_LOG_AND_RETHROW() +} + BOOST_AUTO_TEST_CASE( create_account_with_cycled_authority_before_HARDFORK_CYCLED_ACCOUNTS_TIME_test ) { try { @@ -1396,7 +1481,7 @@ BOOST_AUTO_TEST_CASE( update_account_max_authority_depth_before_HARDFORK_CYCLED_ GET_ACTOR(bob); db.modify( db.get_global_properties(), [&]( global_property_object& gpo ) { - gpo.parameters.maximum_authority_membership = 1; + gpo.parameters.max_authority_depth = 1; }); account_update_operation op; @@ -1432,7 +1517,10 @@ BOOST_AUTO_TEST_CASE( update_account_max_authority_depth_after_HARDFORK_CYCLED_A trx.operations.clear(); trx.operations.push_back(op); - GRAPHENE_CHECK_THROW(PUSH_TX( db, trx, ~0 ), fc::exception); + + GRAPHENE_CHECK_THROW( + PUSH_TX( db, trx, ~0 ), + graphene::chain::tx_missing_active_auth ); } FC_LOG_AND_RETHROW() } From bdaa7acec2a10810ff561f50bf44ce9137cb4c02 Mon Sep 17 00:00:00 2001 From: Alexey Frolov Date: Mon, 10 Jun 2019 17:37:07 +0300 Subject: [PATCH 08/14] Add account_unlock_operation --- libraries/chain/account_evaluator.cpp | 30 +++++++- libraries/chain/db_init.cpp | 1 + libraries/chain/db_notify.cpp | 4 + .../graphene/chain/account_evaluator.hpp | 13 +++- libraries/chain/proposal_evaluator.cpp | 4 + libraries/protocol/account.cpp | 11 +++ .../include/graphene/protocol/account.hpp | 41 ++++++++++ .../include/graphene/protocol/operations.hpp | 3 +- .../wallet/include/graphene/wallet/wallet.hpp | 11 +++ libraries/wallet/wallet.cpp | 20 +++++ tests/tests/operation_tests.cpp | 74 +++++++++++++++++++ 11 files changed, 208 insertions(+), 4 deletions(-) diff --git a/libraries/chain/account_evaluator.cpp b/libraries/chain/account_evaluator.cpp index 3dfb38fac7..da1115fe90 100644 --- a/libraries/chain/account_evaluator.cpp +++ b/libraries/chain/account_evaluator.cpp @@ -345,7 +345,7 @@ void_result account_update_evaluator::do_evaluate( const account_update_operatio { if( d.head_block_time() < HARDFORK_CYCLED_ACCOUNTS_TIME ) { - save_owner = true; + cycle_detected = true; } else { @@ -391,7 +391,7 @@ void_result account_update_evaluator::do_apply( const account_update_operation& // update account object d.modify( *acnt, [&o, this](account_object& a){ - if( save_owner ) + if( cycle_detected && !a.stable_owner.valid()) { a.stable_owner = a.owner; } @@ -439,6 +439,32 @@ void_result account_update_evaluator::do_apply( const account_update_operation& return void_result(); } FC_CAPTURE_AND_RETHROW( (o) ) } +void_result account_unlock_evaluator::do_evaluate( const account_unlock_operation& o ) +{ try { + database& d = db(); + FC_ASSERT(d.head_block_time() >= HARDFORK_CYCLED_ACCOUNTS_TIME, + "Unlocking account is available after HARDFORK_CYCLED_ACCOUNTS_TIME only!" + ); + + acnt = &o.account_to_unlock(d); + FC_ASSERT( acnt->stable_owner.valid(), "Account ${a} is not unlockable.", ("a", o.account_to_unlock) ); + + return void_result(); +} FC_CAPTURE_AND_RETHROW( (o) ) } + +void_result account_unlock_evaluator::do_apply( const account_unlock_operation& o ) +{ try { + database& d = db(); + + // update account object + d.modify( *acnt, [&o, this](account_object& a){ + a.owner = *a.stable_owner; + a.stable_owner.reset(); + }); + + return void_result(); +} FC_CAPTURE_AND_RETHROW( (o) ) } + void_result account_whitelist_evaluator::do_evaluate(const account_whitelist_operation& o) { try { database& d = db(); diff --git a/libraries/chain/db_init.cpp b/libraries/chain/db_init.cpp index 3d0aaf8c04..be68f2f66c 100644 --- a/libraries/chain/db_init.cpp +++ b/libraries/chain/db_init.cpp @@ -135,6 +135,7 @@ void database::initialize_evaluators() register_evaluator(); register_evaluator(); register_evaluator(); + register_evaluator(); register_evaluator(); register_evaluator(); register_evaluator(); diff --git a/libraries/chain/db_notify.cpp b/libraries/chain/db_notify.cpp index d17bec6821..710112e07b 100644 --- a/libraries/chain/db_notify.cpp +++ b/libraries/chain/db_notify.cpp @@ -80,6 +80,10 @@ struct get_impacted_account_visitor if( op.active ) add_authority_accounts( _impacted, *(op.active) ); } + void operator()( const account_unlock_operation& op ) + { + _impacted.insert( op.fee_payer() ); // account_to_unlock + } void operator()( const account_whitelist_operation& op ) { _impacted.insert( op.fee_payer() ); // authorizing_account diff --git a/libraries/chain/include/graphene/chain/account_evaluator.hpp b/libraries/chain/include/graphene/chain/account_evaluator.hpp index 28c45d946a..c69f30a999 100644 --- a/libraries/chain/include/graphene/chain/account_evaluator.hpp +++ b/libraries/chain/include/graphene/chain/account_evaluator.hpp @@ -45,7 +45,7 @@ class account_update_evaluator : public evaluator void_result do_apply( const account_update_operation& o ); const account_object* acnt; - bool save_owner = false; + bool cycle_detected = false; }; class account_upgrade_evaluator : public evaluator @@ -59,6 +59,17 @@ class account_upgrade_evaluator : public evaluator const account_object* account; }; +class account_unlock_evaluator : public evaluator +{ +public: + typedef account_unlock_operation operation_type; + + void_result do_evaluate( const account_unlock_operation& o ); + void_result do_apply( const account_unlock_operation& o ); + + const account_object* acnt; +}; + class account_whitelist_evaluator : public evaluator { public: diff --git a/libraries/chain/proposal_evaluator.cpp b/libraries/chain/proposal_evaluator.cpp index 4e9a3ea656..4104048b95 100644 --- a/libraries/chain/proposal_evaluator.cpp +++ b/libraries/chain/proposal_evaluator.cpp @@ -110,6 +110,10 @@ struct proposal_operation_hardfork_visitor void operator()(const graphene::chain::htlc_extend_operation &op) const { FC_ASSERT( block_time >= HARDFORK_CORE_1468_TIME, "Not allowed until hardfork 1468" ); } + // hf_cycled_accounts + void operator()(const graphene::chain::account_unlock_operation &op) const { + FC_ASSERT( block_time >= HARDFORK_CYCLED_ACCOUNTS_TIME, "Not allowed until hardfork CYCLED_ACCOUNTS" ); + } // loop and self visit in proposals void operator()(const graphene::chain::proposal_create_operation &v) const { bool already_contains_proposal_update = false; diff --git a/libraries/protocol/account.cpp b/libraries/protocol/account.cpp index f51271405b..9e508ea7fe 100644 --- a/libraries/protocol/account.cpp +++ b/libraries/protocol/account.cpp @@ -258,6 +258,17 @@ void account_update_operation::validate()const validate_special_authority( *extensions.value.active_special_authority ); } +share_type account_unlock_operation::calculate_fee( const fee_parameters_type& k ) const +{ + auto core_fee_required = k.fee; + return core_fee_required; +} + +void account_unlock_operation::validate() const +{ + FC_ASSERT( fee.amount >= 0 ); +} + share_type account_upgrade_operation::calculate_fee(const fee_parameters_type& k) const { if( upgrade_to_lifetime_member ) diff --git a/libraries/protocol/include/graphene/protocol/account.hpp b/libraries/protocol/include/graphene/protocol/account.hpp index 02aba3a3f7..83c3d91f4c 100644 --- a/libraries/protocol/include/graphene/protocol/account.hpp +++ b/libraries/protocol/include/graphene/protocol/account.hpp @@ -166,6 +166,31 @@ namespace graphene { namespace protocol { { if( !is_owner_update() ) a.insert( account ); } }; + /** + * @ingroup operations + * @brief Fix locked account and restore access + * + * This operation is used to unlock account being blocked. + */ + struct account_unlock_operation : public base_operation + { + struct fee_parameters_type + { + share_type fee = 20 * GRAPHENE_BLOCKCHAIN_PRECISION; + }; + + asset fee; + account_id_type account_to_unlock; + extensions_type extensions; + + account_id_type fee_payer() const { return account_to_unlock; } + void validate() const; + share_type calculate_fee( const fee_parameters_type& k ) const; + + void get_required_owner_authorities( flat_set& a )const + { a.insert( account_to_unlock ); } + }; + /** * @brief This operation is used to whitelist and blacklist accounts, primarily for transacting in whitelisted assets * @ingroup operations @@ -288,6 +313,10 @@ FC_REFLECT( graphene::protocol::account_update_operation, (fee)(account)(owner)(active)(new_options)(extensions) ) +FC_REFLECT( graphene::protocol::account_unlock_operation::fee_parameters_type, (fee) ) +FC_REFLECT( graphene::protocol::account_unlock_operation, (fee)(account_to_unlock)(extensions) ) + + FC_REFLECT( graphene::protocol::account_upgrade_operation, (fee)(account_to_upgrade)(upgrade_to_lifetime_member)(extensions) ) @@ -297,6 +326,7 @@ FC_REFLECT( graphene::protocol::account_create_operation::fee_parameters_type, ( FC_REFLECT( graphene::protocol::account_whitelist_operation::fee_parameters_type, (fee) ) FC_REFLECT( graphene::protocol::account_update_operation::fee_parameters_type, (fee)(price_per_kbyte) ) FC_REFLECT( graphene::protocol::account_upgrade_operation::fee_parameters_type, (membership_annual_fee)(membership_lifetime_fee) ) + FC_REFLECT( graphene::protocol::account_transfer_operation::fee_parameters_type, (fee) ) FC_REFLECT( graphene::protocol::account_transfer_operation, (fee)(account_id)(new_owner)(extensions) ) @@ -311,4 +341,15 @@ GRAPHENE_DECLARE_EXTERNAL_SERIALIZATION( graphene::protocol::account_create_oper GRAPHENE_DECLARE_EXTERNAL_SERIALIZATION( graphene::protocol::account_whitelist_operation ) GRAPHENE_DECLARE_EXTERNAL_SERIALIZATION( graphene::protocol::account_update_operation ) GRAPHENE_DECLARE_EXTERNAL_SERIALIZATION( graphene::protocol::account_upgrade_operation ) +GRAPHENE_DECLARE_EXTERNAL_SERIALIZATION( graphene::protocol::account_unlock_operation ) GRAPHENE_DECLARE_EXTERNAL_SERIALIZATION( graphene::protocol::account_transfer_operation ) +// FC_REFLECT( graphene::chain::account_whitelist_operation, (fee)(authorizing_account)(account_to_list)(new_listing)(extensions)) + +// FC_REFLECT( graphene::chain::account_create_operation::fee_parameters_type, (basic_fee)(premium_fee)(price_per_kbyte) ) +// FC_REFLECT( graphene::chain::account_whitelist_operation::fee_parameters_type, (fee) ) +// FC_REFLECT( graphene::chain::account_update_operation::fee_parameters_type, (fee)(price_per_kbyte) ) + +// FC_REFLECT( graphene::chain::account_upgrade_operation::fee_parameters_type, (membership_annual_fee)(membership_lifetime_fee) ) +// FC_REFLECT( graphene::chain::account_transfer_operation::fee_parameters_type, (fee) ) + +// FC_REFLECT( graphene::chain::account_transfer_operation, (fee)(account_id)(new_owner)(extensions) ) diff --git a/libraries/protocol/include/graphene/protocol/operations.hpp b/libraries/protocol/include/graphene/protocol/operations.hpp index 3a603ebf64..9a5569ff5c 100644 --- a/libraries/protocol/include/graphene/protocol/operations.hpp +++ b/libraries/protocol/include/graphene/protocol/operations.hpp @@ -101,7 +101,8 @@ namespace graphene { namespace protocol { htlc_redeem_operation, htlc_redeemed_operation, // VIRTUAL htlc_extend_operation, - htlc_refund_operation // VIRTUAL + htlc_refund_operation, // VIRTUAL + account_unlock_operation > operation; /// @} // operations group diff --git a/libraries/wallet/include/graphene/wallet/wallet.hpp b/libraries/wallet/include/graphene/wallet/wallet.hpp index 1fc432ce1c..a19c20296a 100644 --- a/libraries/wallet/include/graphene/wallet/wallet.hpp +++ b/libraries/wallet/include/graphene/wallet/wallet.hpp @@ -818,6 +818,16 @@ class wallet_api */ signed_transaction upgrade_account(string name, bool broadcast); + /** + * Fix locked account. + * This restores access to locked account. + * + * @param name the name or id of the account to unlock + * @param broadcast true to broadcast the transaction on the network + * @returns the signed transaction unlocking the account + */ + signed_transaction fix_locked_account(string name, bool broadcast = false); + /** Creates a new account and registers it on the blockchain. * * @todo why no referrer_percent here? @@ -1846,6 +1856,7 @@ FC_API( graphene::wallet::wallet_api, (derive_owner_keys_from_brain_key) (register_account) (upgrade_account) + (fix_locked_account) (create_account_with_brain_key) (sell_asset) (borrow_asset) diff --git a/libraries/wallet/wallet.cpp b/libraries/wallet/wallet.cpp index 2f964bb811..0710c2f7ad 100644 --- a/libraries/wallet/wallet.cpp +++ b/libraries/wallet/wallet.cpp @@ -1184,6 +1184,21 @@ class wallet_api_impl return sign_transaction( tx, broadcast ); } FC_CAPTURE_AND_RETHROW( (name) ) } + signed_transaction fix_locked_account(string name, bool broadcast) + { try { + FC_ASSERT( !self.is_locked() ); + account_object account_obj = get_account(name); + + signed_transaction tx; + account_unlock_operation op; + op.account_to_unlock = account_obj.get_id(); + tx.operations = {op}; + set_operation_fees( tx, _remote_db->get_global_properties().parameters.current_fees ); + tx.validate(); + + return sign_transaction( tx, broadcast ); + } FC_CAPTURE_AND_RETHROW( (name) ) } + // This function generates derived keys starting with index 0 and keeps incrementing // the index until it finds a key that isn't registered in the block chain. To be // safer, it continues checking for a few more keys to make sure there wasn't a short gap @@ -4434,6 +4449,11 @@ signed_transaction wallet_api::upgrade_account( string name, bool broadcast ) return my->upgrade_account(name,broadcast); } +signed_transaction wallet_api::fix_locked_account(string name, bool broadcast ) +{ + return my->fix_locked_account(name, broadcast); +} + signed_transaction wallet_api::sell_asset(string seller_account, string amount_to_sell, string symbol_to_sell, diff --git a/tests/tests/operation_tests.cpp b/tests/tests/operation_tests.cpp index 668cf93129..7c6ef8f2e1 100644 --- a/tests/tests/operation_tests.cpp +++ b/tests/tests/operation_tests.cpp @@ -1173,6 +1173,80 @@ BOOST_AUTO_TEST_CASE( restore_point_not_created_if_account_not_locked_test ) FC_LOG_AND_RETHROW() } +BOOST_AUTO_TEST_CASE( unlock_cycled_account_test ) +{ + try { + ACTORS( (bob) (alice) ); + fund(bob); + generate_block(); + + auto update_auth = [&](const account_update_operation& op){ + trx.operations.clear(); + trx.operations.push_back(op); + PUSH_TX( db, trx, ~0 ); + generate_block(); + set_expiration(db, trx); + }; + + // Alice sets Bob to her active and owner authority + { + account_update_operation op; + op.account = alice_id; + op.active = authority(1, bob_id, 1); + op.owner = op.active; + update_auth(op); + } + + // Bob sets Alice to his active and owner authority + { + account_update_operation op; + op.account = bob_id; + op.active = authority(1, alice_id, 1); + op.owner = op.active; + update_auth(op); + } + + generate_blocks(HARDFORK_CYCLED_ACCOUNTS_TIME, true); + + // account locked and can't transfer money + transfer_operation t_op; + t_op.from = bob_id; + t_op.to = alice_id; + t_op.amount = asset(1000); + trx.operations = {t_op}; + GRAPHENE_CHECK_THROW( PUSH_TX( db, trx ), graphene::chain::tx_missing_active_auth ); + + // unlock + { + account_unlock_operation op; + op.account_to_unlock = bob_id; + trx.operations = {op}; + set_expiration(db, trx); + + // TODO sign(trx, bob_private_key); + // TODO remove ~0 and check prev authority + PUSH_TX( db, trx, ~0 ); + } + + // account unlocked and can transfer money + trx.operations = {t_op}; + sign(trx, bob_private_key); + PUSH_TX( db, trx ); + } + FC_LOG_AND_RETHROW() +} + +BOOST_AUTO_TEST_CASE( cant_execute_unlock_operation_before_HARDFORK_CYCLED_ACCOUNTS_TIME_test ) +{ + ACTORS( (alice) ); + + account_unlock_operation op; + op.account_to_unlock = alice_id; + trx.operations = {op}; + sign(trx, alice_private_key); + GRAPHENE_CHECK_THROW(PUSH_TX( db, trx ), fc::assert_exception); +} + BOOST_AUTO_TEST_CASE( create_account_with_cycled_authority_before_HARDFORK_CYCLED_ACCOUNTS_TIME_test ) { try { From 9ab5e25a35e937c5e209e50fc57ed39b98b72c01 Mon Sep 17 00:00:00 2001 From: Alexey Frolov Date: Fri, 7 Jun 2019 13:43:20 +0000 Subject: [PATCH 09/14] Authorize unlock_account_operation --- .../chain/include/graphene/chain/config.hpp | 3 +- .../include/graphene/protocol/account.hpp | 18 ++- libraries/wallet/wallet.cpp | 2 + tests/tests/operation_tests.cpp | 108 ++++++++++++------ 4 files changed, 91 insertions(+), 40 deletions(-) diff --git a/libraries/chain/include/graphene/chain/config.hpp b/libraries/chain/include/graphene/chain/config.hpp index 46a8feb54e..3e1e39dd8c 100644 --- a/libraries/chain/include/graphene/chain/config.hpp +++ b/libraries/chain/include/graphene/chain/config.hpp @@ -30,8 +30,7 @@ #define GRAPHENE_MAX_NESTED_OBJECTS (200) -#define GRAPHENE_CURRENT_DB_VERSION "20190503" +#define GRAPHENE_CURRENT_DB_VERSION "20191231" #define GRAPHENE_RECENTLY_MISSED_COUNT_INCREMENT 4 #define GRAPHENE_RECENTLY_MISSED_COUNT_DECREMENT 3 - diff --git a/libraries/protocol/include/graphene/protocol/account.hpp b/libraries/protocol/include/graphene/protocol/account.hpp index 83c3d91f4c..057db046a5 100644 --- a/libraries/protocol/include/graphene/protocol/account.hpp +++ b/libraries/protocol/include/graphene/protocol/account.hpp @@ -181,14 +181,25 @@ namespace graphene { namespace protocol { asset fee; account_id_type account_to_unlock; + authority previous_authority; extensions_type extensions; account_id_type fee_payer() const { return account_to_unlock; } void validate() const; share_type calculate_fee( const fee_parameters_type& k ) const; - void get_required_owner_authorities( flat_set& a )const - { a.insert( account_to_unlock ); } + void get_required_active_authorities( flat_set& active) const + { + // a little bit tricky, see operations.cpp : operation_get_required_auth + // active.insert( v.fee_payer() ); - fee_payer added explicitly as active authority + // but we needn't it to authorize because it is locked + active.erase( account_to_unlock ); + } + + void get_required_authorities( vector& a ) const + { + a.push_back( previous_authority ); + } }; /** @@ -313,8 +324,9 @@ FC_REFLECT( graphene::protocol::account_update_operation, (fee)(account)(owner)(active)(new_options)(extensions) ) + FC_REFLECT( graphene::protocol::account_unlock_operation::fee_parameters_type, (fee) ) -FC_REFLECT( graphene::protocol::account_unlock_operation, (fee)(account_to_unlock)(extensions) ) +FC_REFLECT( graphene::protocol::account_unlock_operation, (fee)(account_to_unlock)(previous_authority)(extensions) ) FC_REFLECT( graphene::protocol::account_upgrade_operation, diff --git a/libraries/wallet/wallet.cpp b/libraries/wallet/wallet.cpp index 0710c2f7ad..9dc9311325 100644 --- a/libraries/wallet/wallet.cpp +++ b/libraries/wallet/wallet.cpp @@ -1188,10 +1188,12 @@ class wallet_api_impl { try { FC_ASSERT( !self.is_locked() ); account_object account_obj = get_account(name); + FC_ASSERT( account_obj.stable_owner, "Account is not unlockable" ); signed_transaction tx; account_unlock_operation op; op.account_to_unlock = account_obj.get_id(); + op.previous_authority = *account_obj.stable_owner; tx.operations = {op}; set_operation_fees( tx, _remote_db->get_global_properties().parameters.current_fees ); tx.validate(); diff --git a/tests/tests/operation_tests.cpp b/tests/tests/operation_tests.cpp index 7c6ef8f2e1..790148a113 100644 --- a/tests/tests/operation_tests.cpp +++ b/tests/tests/operation_tests.cpp @@ -1173,20 +1173,10 @@ BOOST_AUTO_TEST_CASE( restore_point_not_created_if_account_not_locked_test ) FC_LOG_AND_RETHROW() } -BOOST_AUTO_TEST_CASE( unlock_cycled_account_test ) +BOOST_AUTO_TEST_CASE( make_recursive_authority_test ) { try { ACTORS( (bob) (alice) ); - fund(bob); - generate_block(); - - auto update_auth = [&](const account_update_operation& op){ - trx.operations.clear(); - trx.operations.push_back(op); - PUSH_TX( db, trx, ~0 ); - generate_block(); - set_expiration(db, trx); - }; // Alice sets Bob to her active and owner authority { @@ -1194,7 +1184,8 @@ BOOST_AUTO_TEST_CASE( unlock_cycled_account_test ) op.account = alice_id; op.active = authority(1, bob_id, 1); op.owner = op.active; - update_auth(op); + trx.operations = {op}; + PUSH_TX( db, trx, ~0 ); } // Bob sets Alice to his active and owner authority @@ -1203,48 +1194,95 @@ BOOST_AUTO_TEST_CASE( unlock_cycled_account_test ) op.account = bob_id; op.active = authority(1, alice_id, 1); op.owner = op.active; - update_auth(op); + trx.operations = {op}; + PUSH_TX( db, trx, ~0 ); } + BOOST_REQUIRE( bob_id(db).stable_owner.valid() ); + + // Bob can't update his authority (can't sign any operation) + { + account_update_operation op; + op.account = bob_id; + op.active = authority(123, bob_public_key, 123); + op.owner = op.active; + trx.operations = {op}; + sign(trx, bob_private_key); + sign(trx, alice_private_key); + + GRAPHENE_CHECK_THROW( + PUSH_TX( db, trx ), + graphene::chain::tx_missing_owner_auth ); + } + } + FC_LOG_AND_RETHROW() +} + +BOOST_AUTO_TEST_CASE( unlock_cycled_account_test ) +{ + try { + INVOKE(make_recursive_authority_test); + + GET_ACTOR(bob); + GET_ACTOR(alice); + fund(bob); + generate_blocks(HARDFORK_CYCLED_ACCOUNTS_TIME, true); - - // account locked and can't transfer money - transfer_operation t_op; - t_op.from = bob_id; - t_op.to = alice_id; - t_op.amount = asset(1000); - trx.operations = {t_op}; - GRAPHENE_CHECK_THROW( PUSH_TX( db, trx ), graphene::chain::tx_missing_active_auth ); // unlock { account_unlock_operation op; op.account_to_unlock = bob_id; + op.previous_authority = *bob_id(db).stable_owner; + trx.operations = {op}; + trx.clear_signatures(); set_expiration(db, trx); - - // TODO sign(trx, bob_private_key); - // TODO remove ~0 and check prev authority - PUSH_TX( db, trx, ~0 ); + sign(trx, bob_private_key); + + PUSH_TX( db, trx ); } // account unlocked and can transfer money - trx.operations = {t_op}; - sign(trx, bob_private_key); - PUSH_TX( db, trx ); + auto initial_balance_bob = get_balance(bob_id, asset_id_type()); + auto initial_balance_alice = get_balance(alice_id, asset_id_type()); + + { + transfer_operation t_op; + t_op.from = bob_id; + t_op.to = alice_id; + t_op.amount = asset(1000); + + trx.clear_signatures(); + trx.operations = {t_op}; + sign(trx, bob_private_key); + + PUSH_TX( db, trx ); + } + + BOOST_CHECK_EQUAL( get_balance(bob_id, asset_id_type()), initial_balance_bob - 1000 ); + BOOST_CHECK_EQUAL( get_balance(alice_id, asset_id_type()), initial_balance_alice + 1000 ); } FC_LOG_AND_RETHROW() } BOOST_AUTO_TEST_CASE( cant_execute_unlock_operation_before_HARDFORK_CYCLED_ACCOUNTS_TIME_test ) { - ACTORS( (alice) ); - - account_unlock_operation op; - op.account_to_unlock = alice_id; - trx.operations = {op}; - sign(trx, alice_private_key); - GRAPHENE_CHECK_THROW(PUSH_TX( db, trx ), fc::assert_exception); + try { + ACTORS( (alice) ); + + account_unlock_operation op; + op.account_to_unlock = alice_id; + auto auth = authority(111, alice_public_key, 111); + op.previous_authority = auth; + trx.operations = {op}; + sign(trx, alice_private_key); + + GRAPHENE_CHECK_THROW( + PUSH_TX( db, trx ), + fc::assert_exception ); + } + FC_LOG_AND_RETHROW() } BOOST_AUTO_TEST_CASE( create_account_with_cycled_authority_before_HARDFORK_CYCLED_ACCOUNTS_TIME_test ) From 4bbf0dc51a089ce137294fdab4f602f4dd520dc3 Mon Sep 17 00:00:00 2001 From: Alexey Frolov Date: Tue, 18 Jun 2019 17:45:50 +0300 Subject: [PATCH 10/14] add unlock account unit tests (chain) --- tests/tests/operation_tests.cpp | 73 ++++++++++++++++++++++++++++++++- 1 file changed, 72 insertions(+), 1 deletion(-) diff --git a/tests/tests/operation_tests.cpp b/tests/tests/operation_tests.cpp index 790148a113..c7eb39f5d2 100644 --- a/tests/tests/operation_tests.cpp +++ b/tests/tests/operation_tests.cpp @@ -1285,6 +1285,27 @@ BOOST_AUTO_TEST_CASE( cant_execute_unlock_operation_before_HARDFORK_CYCLED_ACCOU FC_LOG_AND_RETHROW() } +BOOST_AUTO_TEST_CASE( cant_unlock_not_locked_account_test ) +{ + try { + ACTORS( (alice) ); + + generate_blocks(HARDFORK_CYCLED_ACCOUNTS_TIME, true); + + account_unlock_operation op; + op.account_to_unlock = alice_id; + auto auth = authority(111, alice_public_key, 111); + op.previous_authority = auth; + trx.operations = {op}; + sign(trx, alice_private_key); + + GRAPHENE_CHECK_THROW( + PUSH_TX( db, trx ), + fc::assert_exception ); + } + FC_LOG_AND_RETHROW() +} + BOOST_AUTO_TEST_CASE( create_account_with_cycled_authority_before_HARDFORK_CYCLED_ACCOUNTS_TIME_test ) { try { @@ -1333,7 +1354,36 @@ BOOST_AUTO_TEST_CASE( create_account_with_cycled_authority_after_HARDFORK_CYCLED create_op.owner = authority(1, bob_id, 1); create_op.active = authority(1, alice_id, 1); trx.operations.push_back(create_op); - GRAPHENE_CHECK_THROW(PUSH_TX( db, trx, ~0 ), fc::exception); + + GRAPHENE_CHECK_THROW( + PUSH_TX( db, trx, ~0 ), + graphene::chain::tx_missing_active_auth ); + } + FC_LOG_AND_RETHROW() +} + +BOOST_AUTO_TEST_CASE( cant_unlock_account_that_created_locked_test ) +{ + try { + INVOKE(create_account_with_cycled_authority_before_HARDFORK_CYCLED_ACCOUNTS_TIME_test); + + generate_blocks(HARDFORK_CYCLED_ACCOUNTS_TIME); + set_expiration( db, trx ); + + const auto &locked_account = get_account("allowed"); + auto priv = generate_private_key("allowed"); + auto pub = public_key_type(priv.get_public_key()); + + account_unlock_operation op; + op.account_to_unlock = locked_account.get_id(); + auto auth = authority(111, pub, 111); + op.previous_authority = auth; + trx.operations = {op}; + sign(trx, priv); + + GRAPHENE_CHECK_THROW( + PUSH_TX( db, trx ), + fc::assert_exception ); } FC_LOG_AND_RETHROW() } @@ -1740,6 +1790,27 @@ BOOST_AUTO_TEST_CASE( cant_create_auth_chain_longer_than_max_authority_depth_lim FC_LOG_AND_RETHROW() } +BOOST_AUTO_TEST_CASE( cant_create_proposal_with_unlock_account_before_HARDFORK_CYCLED_ACCOUNTS_TIME_test ) +{ + try { + ACTORS( (alice)(bob) ) + + proposal_create_operation pco; + pco.expiration_time = db.head_block_time() + fc::minutes(1); + pco.fee_paying_account = alice_id; + + account_unlock_operation op; + op.account_to_unlock = bob_id; + op.previous_authority = authority(111, bob_public_key, 111); + + pco.proposed_ops.emplace_back( op ); + trx.operations = {pco}; + + GRAPHENE_CHECK_THROW( PUSH_TX( db, trx, ~0 ), fc::assert_exception ); + } + FC_LOG_AND_RETHROW() +} + BOOST_AUTO_TEST_CASE( transfer_core_asset ) { try { From 2e2ad28085e320f8045a0d0a2c98f60056e9c767 Mon Sep 17 00:00:00 2001 From: ddylko Date: Wed, 31 Jul 2019 12:24:54 +0300 Subject: [PATCH 11/14] conflict resolving result --- libraries/chain/account_evaluator.cpp | 8 +++++--- libraries/protocol/account.cpp | 2 ++ .../protocol/include/graphene/protocol/account.hpp | 11 +---------- libraries/wallet/wallet.cpp | 2 +- 4 files changed, 9 insertions(+), 14 deletions(-) diff --git a/libraries/chain/account_evaluator.cpp b/libraries/chain/account_evaluator.cpp index da1115fe90..6303b3a05a 100644 --- a/libraries/chain/account_evaluator.cpp +++ b/libraries/chain/account_evaluator.cpp @@ -38,12 +38,14 @@ #include #include -namespace graphene { namespace chain { - - void verify_cycled_authority( const account_id_type& id, +namespace graphene { + namespace protocol { + void verify_cycled_authority( const account_id_type& id, const std::function& get_active, const std::function& get_owner, uint32_t max_recursion_depth ); + } + namespace chain { namespace detail { diff --git a/libraries/protocol/account.cpp b/libraries/protocol/account.cpp index 9e508ea7fe..5a44c44d11 100644 --- a/libraries/protocol/account.cpp +++ b/libraries/protocol/account.cpp @@ -295,9 +295,11 @@ GRAPHENE_IMPLEMENT_EXTERNAL_SERIALIZATION( graphene::protocol::account_create_op GRAPHENE_IMPLEMENT_EXTERNAL_SERIALIZATION( graphene::protocol::account_whitelist_operation::fee_parameters_type ) GRAPHENE_IMPLEMENT_EXTERNAL_SERIALIZATION( graphene::protocol::account_update_operation::fee_parameters_type ) GRAPHENE_IMPLEMENT_EXTERNAL_SERIALIZATION( graphene::protocol::account_upgrade_operation::fee_parameters_type ) +GRAPHENE_IMPLEMENT_EXTERNAL_SERIALIZATION( graphene::protocol::account_unlock_operation::fee_parameters_type ) GRAPHENE_IMPLEMENT_EXTERNAL_SERIALIZATION( graphene::protocol::account_transfer_operation::fee_parameters_type ) GRAPHENE_IMPLEMENT_EXTERNAL_SERIALIZATION( graphene::protocol::account_create_operation ) GRAPHENE_IMPLEMENT_EXTERNAL_SERIALIZATION( graphene::protocol::account_whitelist_operation ) GRAPHENE_IMPLEMENT_EXTERNAL_SERIALIZATION( graphene::protocol::account_update_operation ) GRAPHENE_IMPLEMENT_EXTERNAL_SERIALIZATION( graphene::protocol::account_upgrade_operation ) +GRAPHENE_IMPLEMENT_EXTERNAL_SERIALIZATION( graphene::protocol::account_unlock_operation ) GRAPHENE_IMPLEMENT_EXTERNAL_SERIALIZATION( graphene::protocol::account_transfer_operation ) diff --git a/libraries/protocol/include/graphene/protocol/account.hpp b/libraries/protocol/include/graphene/protocol/account.hpp index 057db046a5..e0fdddbbcb 100644 --- a/libraries/protocol/include/graphene/protocol/account.hpp +++ b/libraries/protocol/include/graphene/protocol/account.hpp @@ -348,6 +348,7 @@ GRAPHENE_DECLARE_EXTERNAL_SERIALIZATION( graphene::protocol::account_create_oper GRAPHENE_DECLARE_EXTERNAL_SERIALIZATION( graphene::protocol::account_whitelist_operation::fee_parameters_type ) GRAPHENE_DECLARE_EXTERNAL_SERIALIZATION( graphene::protocol::account_update_operation::fee_parameters_type ) GRAPHENE_DECLARE_EXTERNAL_SERIALIZATION( graphene::protocol::account_upgrade_operation::fee_parameters_type ) +GRAPHENE_DECLARE_EXTERNAL_SERIALIZATION( graphene::protocol::account_unlock_operation::fee_parameters_type ) GRAPHENE_DECLARE_EXTERNAL_SERIALIZATION( graphene::protocol::account_transfer_operation::fee_parameters_type ) GRAPHENE_DECLARE_EXTERNAL_SERIALIZATION( graphene::protocol::account_create_operation ) GRAPHENE_DECLARE_EXTERNAL_SERIALIZATION( graphene::protocol::account_whitelist_operation ) @@ -355,13 +356,3 @@ GRAPHENE_DECLARE_EXTERNAL_SERIALIZATION( graphene::protocol::account_update_oper GRAPHENE_DECLARE_EXTERNAL_SERIALIZATION( graphene::protocol::account_upgrade_operation ) GRAPHENE_DECLARE_EXTERNAL_SERIALIZATION( graphene::protocol::account_unlock_operation ) GRAPHENE_DECLARE_EXTERNAL_SERIALIZATION( graphene::protocol::account_transfer_operation ) -// FC_REFLECT( graphene::chain::account_whitelist_operation, (fee)(authorizing_account)(account_to_list)(new_listing)(extensions)) - -// FC_REFLECT( graphene::chain::account_create_operation::fee_parameters_type, (basic_fee)(premium_fee)(price_per_kbyte) ) -// FC_REFLECT( graphene::chain::account_whitelist_operation::fee_parameters_type, (fee) ) -// FC_REFLECT( graphene::chain::account_update_operation::fee_parameters_type, (fee)(price_per_kbyte) ) - -// FC_REFLECT( graphene::chain::account_upgrade_operation::fee_parameters_type, (membership_annual_fee)(membership_lifetime_fee) ) -// FC_REFLECT( graphene::chain::account_transfer_operation::fee_parameters_type, (fee) ) - -// FC_REFLECT( graphene::chain::account_transfer_operation, (fee)(account_id)(new_owner)(extensions) ) diff --git a/libraries/wallet/wallet.cpp b/libraries/wallet/wallet.cpp index 9dc9311325..19053b995c 100644 --- a/libraries/wallet/wallet.cpp +++ b/libraries/wallet/wallet.cpp @@ -1195,7 +1195,7 @@ class wallet_api_impl op.account_to_unlock = account_obj.get_id(); op.previous_authority = *account_obj.stable_owner; tx.operations = {op}; - set_operation_fees( tx, _remote_db->get_global_properties().parameters.current_fees ); + set_operation_fees( tx, _remote_db->get_global_properties().parameters.get_current_fees() ); tx.validate(); return sign_transaction( tx, broadcast ); From e392b20c405668ae7b6c70b2276a1c206d16bac5 Mon Sep 17 00:00:00 2001 From: ddylko Date: Wed, 31 Jul 2019 18:16:22 +0300 Subject: [PATCH 12/14] .fix-locked_accounts main logic impl --- libraries/chain/account_evaluator.cpp | 12 +++++++ libraries/chain/db_notify.cpp | 5 +++ libraries/protocol/account.cpp | 1 + .../include/graphene/protocol/account.hpp | 35 ++++++++++++++----- .../include/graphene/protocol/operations.hpp | 3 +- 5 files changed, 46 insertions(+), 10 deletions(-) diff --git a/libraries/chain/account_evaluator.cpp b/libraries/chain/account_evaluator.cpp index 6303b3a05a..4d0f9c3531 100644 --- a/libraries/chain/account_evaluator.cpp +++ b/libraries/chain/account_evaluator.cpp @@ -464,6 +464,18 @@ void_result account_unlock_evaluator::do_apply( const account_unlock_operation& a.stable_owner.reset(); }); + const auto& bal_idx = d.get_index_type< primary_index< account_balance_index > >().get_secondary_index< balances_by_account_index >(); + for( const auto& entry : bal_idx.get_account_balances( acnt->get_id() ) ) + { + const auto balance = entry.second->get_balance(); + const auto unlock_cost = balance.amount / 10; + const auto penalty = asset(unlock_cost, balance.asset_id); + + d.push_applied_operation(account_unlock_penalty_payment_operation( acnt->get_id(), penalty )); + d.adjust_balance(acnt->get_id(), penalty); + d.adjust_balance(GRAPHENE_COMMITTEE_ACCOUNT, -penalty); + } + return void_result(); } FC_CAPTURE_AND_RETHROW( (o) ) } diff --git a/libraries/chain/db_notify.cpp b/libraries/chain/db_notify.cpp index 710112e07b..e849d70d29 100644 --- a/libraries/chain/db_notify.cpp +++ b/libraries/chain/db_notify.cpp @@ -61,6 +61,11 @@ struct get_impacted_account_visitor { _impacted.insert( op.fee_payer() ); // account_id } + void operator()(const account_unlock_penalty_payment_operation& op) + { + _impacted.insert( op.fee_payer() ); + _impacted.insert( GRAPHENE_COMMITTEE_ACCOUNT ); + } void operator()( const execute_bid_operation& op ) { _impacted.insert( op.fee_payer() ); // bidder diff --git a/libraries/protocol/account.cpp b/libraries/protocol/account.cpp index 5a44c44d11..77b7b6e7c0 100644 --- a/libraries/protocol/account.cpp +++ b/libraries/protocol/account.cpp @@ -303,3 +303,4 @@ GRAPHENE_IMPLEMENT_EXTERNAL_SERIALIZATION( graphene::protocol::account_update_op GRAPHENE_IMPLEMENT_EXTERNAL_SERIALIZATION( graphene::protocol::account_upgrade_operation ) GRAPHENE_IMPLEMENT_EXTERNAL_SERIALIZATION( graphene::protocol::account_unlock_operation ) GRAPHENE_IMPLEMENT_EXTERNAL_SERIALIZATION( graphene::protocol::account_transfer_operation ) +GRAPHENE_IMPLEMENT_EXTERNAL_SERIALIZATION( graphene::protocol::account_unlock_penalty_payment_operation ) diff --git a/libraries/protocol/include/graphene/protocol/account.hpp b/libraries/protocol/include/graphene/protocol/account.hpp index e0fdddbbcb..d126d5c9d7 100644 --- a/libraries/protocol/include/graphene/protocol/account.hpp +++ b/libraries/protocol/include/graphene/protocol/account.hpp @@ -202,6 +202,27 @@ namespace graphene { namespace protocol { } }; + struct account_unlock_penalty_payment_operation : public base_operation + { + struct fee_parameters_type {}; + + account_unlock_penalty_payment_operation(){} + account_unlock_penalty_payment_operation( + account_id_type a, asset p + ) + :account_id(a),penalty(p), fee() {} + + account_id_type account_id; + asset penalty; + asset fee; + + account_id_type fee_payer()const { return account_id; } + void validate()const { FC_ASSERT( !"virtual operation" ); } + + /// This is a virtual operation; there is no fee + share_type calculate_fee(const fee_parameters_type& k)const { return 0; } + }; + /** * @brief This operation is used to whitelist and blacklist accounts, primarily for transacting in whitelisted assets * @ingroup operations @@ -324,24 +345,18 @@ FC_REFLECT( graphene::protocol::account_update_operation, (fee)(account)(owner)(active)(new_options)(extensions) ) - FC_REFLECT( graphene::protocol::account_unlock_operation::fee_parameters_type, (fee) ) FC_REFLECT( graphene::protocol::account_unlock_operation, (fee)(account_to_unlock)(previous_authority)(extensions) ) - - -FC_REFLECT( graphene::protocol::account_upgrade_operation, - (fee)(account_to_upgrade)(upgrade_to_lifetime_member)(extensions) ) - +FC_REFLECT( graphene::protocol::account_upgrade_operation, (fee)(account_to_upgrade)(upgrade_to_lifetime_member)(extensions) ) FC_REFLECT( graphene::protocol::account_whitelist_operation, (fee)(authorizing_account)(account_to_list)(new_listing)(extensions)) - FC_REFLECT( graphene::protocol::account_create_operation::fee_parameters_type, (basic_fee)(premium_fee)(price_per_kbyte) ) FC_REFLECT( graphene::protocol::account_whitelist_operation::fee_parameters_type, (fee) ) FC_REFLECT( graphene::protocol::account_update_operation::fee_parameters_type, (fee)(price_per_kbyte) ) FC_REFLECT( graphene::protocol::account_upgrade_operation::fee_parameters_type, (membership_annual_fee)(membership_lifetime_fee) ) - FC_REFLECT( graphene::protocol::account_transfer_operation::fee_parameters_type, (fee) ) - FC_REFLECT( graphene::protocol::account_transfer_operation, (fee)(account_id)(new_owner)(extensions) ) +FC_REFLECT( graphene::protocol::account_unlock_penalty_payment_operation, (fee)(account_id)(penalty) ) +FC_REFLECT( graphene::protocol::account_unlock_penalty_payment_operation::fee_parameters_type, ) // VIRTUAL GRAPHENE_DECLARE_EXTERNAL_SERIALIZATION( graphene::protocol::account_options ) GRAPHENE_DECLARE_EXTERNAL_SERIALIZATION( graphene::protocol::account_create_operation::fee_parameters_type ) @@ -356,3 +371,5 @@ GRAPHENE_DECLARE_EXTERNAL_SERIALIZATION( graphene::protocol::account_update_oper GRAPHENE_DECLARE_EXTERNAL_SERIALIZATION( graphene::protocol::account_upgrade_operation ) GRAPHENE_DECLARE_EXTERNAL_SERIALIZATION( graphene::protocol::account_unlock_operation ) GRAPHENE_DECLARE_EXTERNAL_SERIALIZATION( graphene::protocol::account_transfer_operation ) +GRAPHENE_DECLARE_EXTERNAL_SERIALIZATION( graphene::protocol::account_unlock_penalty_payment_operation ) + diff --git a/libraries/protocol/include/graphene/protocol/operations.hpp b/libraries/protocol/include/graphene/protocol/operations.hpp index 9a5569ff5c..28934df0d3 100644 --- a/libraries/protocol/include/graphene/protocol/operations.hpp +++ b/libraries/protocol/include/graphene/protocol/operations.hpp @@ -102,7 +102,8 @@ namespace graphene { namespace protocol { htlc_redeemed_operation, // VIRTUAL htlc_extend_operation, htlc_refund_operation, // VIRTUAL - account_unlock_operation + account_unlock_operation, + account_unlock_penalty_payment_operation //VIRTUAL > operation; /// @} // operations group From 3095f2aa5d2533d152f759b194fad4400f1925d2 Mon Sep 17 00:00:00 2001 From: ddylko Date: Thu, 1 Aug 2019 15:24:42 +0300 Subject: [PATCH 13/14] unit tests --- libraries/chain/account_evaluator.cpp | 4 +- tests/tests/operation_tests.cpp | 109 ++++++++++++++++++++++++++ 2 files changed, 111 insertions(+), 2 deletions(-) diff --git a/libraries/chain/account_evaluator.cpp b/libraries/chain/account_evaluator.cpp index 4d0f9c3531..3ce6fcd223 100644 --- a/libraries/chain/account_evaluator.cpp +++ b/libraries/chain/account_evaluator.cpp @@ -472,8 +472,8 @@ void_result account_unlock_evaluator::do_apply( const account_unlock_operation& const auto penalty = asset(unlock_cost, balance.asset_id); d.push_applied_operation(account_unlock_penalty_payment_operation( acnt->get_id(), penalty )); - d.adjust_balance(acnt->get_id(), penalty); - d.adjust_balance(GRAPHENE_COMMITTEE_ACCOUNT, -penalty); + d.adjust_balance(acnt->get_id(), -penalty); + d.adjust_balance(GRAPHENE_COMMITTEE_ACCOUNT, penalty); } return void_result(); diff --git a/tests/tests/operation_tests.cpp b/tests/tests/operation_tests.cpp index c7eb39f5d2..8bd8355494 100644 --- a/tests/tests/operation_tests.cpp +++ b/tests/tests/operation_tests.cpp @@ -1266,6 +1266,115 @@ BOOST_AUTO_TEST_CASE( unlock_cycled_account_test ) FC_LOG_AND_RETHROW() } +BOOST_AUTO_TEST_CASE( unlock_operation_penalty_payment_test ) +{ + try + { + INVOKE(make_recursive_authority_test); + + generate_blocks(HARDFORK_1268_TIME); + generate_block(); + + GET_ACTOR(bob); + fund(bob); + ACTOR(issuer); + + additional_asset_options_t options; + options.value.reward_percent = 2 * GRAPHENE_1_PERCENT; + + const auto usd = create_user_issued_asset( + "USD", + issuer, + charge_market_fee, + price(asset(1, asset_id_type(1)), asset(1)), + 1, + 20 * GRAPHENE_1_PERCENT, + options); + + issue_uia(issuer, usd.amount(2000)); + transfer(issuer, bob, usd.amount(200)); + transfer(issuer, GRAPHENE_COMMITTEE_ACCOUNT(db), usd.amount(200)); + + std::map bob_balance_helper; + std::map committee_balance_helper; + + const auto& bal_idx_before = db.get_index_type< primary_index< account_balance_index > >().get_secondary_index< balances_by_account_index >(); + for( const auto& entry : bal_idx_before.get_account_balances( bob_id ) ) + { + const auto balance = entry.second->get_balance(); + bob_balance_helper.emplace(balance.asset_id, balance.amount); + BOOST_CHECK_GT(balance.amount.value, 0); + } + for( const auto& entry : bal_idx_before.get_account_balances( GRAPHENE_COMMITTEE_ACCOUNT ) ) + { + const auto balance = entry.second->get_balance(); + committee_balance_helper.emplace(balance.asset_id, balance.amount); + BOOST_CHECK_GT(balance.amount.value, 0); + } + + // unlock + { + account_unlock_operation op; + op.account_to_unlock = bob_id; + op.previous_authority = *bob_id(db).stable_owner; + + trx.operations = {op}; + trx.clear_signatures(); + set_expiration(db, trx); + sign(trx, bob_private_key); + + PUSH_TX( db, trx ); + } + + const auto& bal_idx_after = db.get_index_type< primary_index< account_balance_index > >().get_secondary_index< balances_by_account_index >(); + for( const auto& entry : bal_idx_after.get_account_balances( bob_id ) ) + { + const auto balance = entry.second->get_balance(); + BOOST_CHECK_EQUAL(balance.amount.value, bob_balance_helper.at(balance.asset_id).value * 0.9); + } + + for( const auto& entry : bal_idx_after.get_account_balances( GRAPHENE_COMMITTEE_ACCOUNT ) ) + { + const auto balance = entry.second->get_balance(); + BOOST_CHECK_EQUAL(balance.amount.value - committee_balance_helper.at(balance.asset_id).value, bob_balance_helper.at(balance.asset_id).value * 0.1 ); + } + + } FC_LOG_AND_RETHROW() +} + +BOOST_AUTO_TEST_CASE(unloc_account_operation_failed_not_enough_BTS) +{ + try + { + INVOKE(make_recursive_authority_test); + + generate_blocks(HARDFORK_1268_TIME); + generate_block(); + + GET_ACTOR(bob); + enable_fees(); + + std::map bob_balance_helper; + std::map committee_balance_helper; + + const auto& bal_idx_before = db.get_index_type< primary_index< account_balance_index > >().get_secondary_index< balances_by_account_index >(); + BOOST_CHECK( bal_idx_before.get_account_balances( bob_id ).size() == 0); + + // unlock + account_unlock_operation op; + op.account_to_unlock = bob_id; + op.previous_authority = *bob_id(db).stable_owner; + + trx.operations = {op}; + trx.clear_signatures(); + set_expiration(db, trx); + sign(trx, bob_private_key); + + GRAPHENE_CHECK_THROW( PUSH_TX( db, trx ), fc::exception ); + + } FC_LOG_AND_RETHROW() +} + BOOST_AUTO_TEST_CASE( cant_execute_unlock_operation_before_HARDFORK_CYCLED_ACCOUNTS_TIME_test ) { try { From 3256407950e2d375c43419e9d5f77f0e1b7fb3fd Mon Sep 17 00:00:00 2001 From: ddylko Date: Thu, 1 Aug 2019 17:03:39 +0300 Subject: [PATCH 14/14] utests adjustment and hforck time fix --- libraries/chain/account_evaluator.cpp | 8 +-- .../chain/hardfork.d/CYCLED_ACCOUNTS_HF.hf | 4 +- .../include/graphene/protocol/account.hpp | 10 ++- tests/tests/operation_tests.cpp | 64 ++++--------------- 4 files changed, 27 insertions(+), 59 deletions(-) diff --git a/libraries/chain/account_evaluator.cpp b/libraries/chain/account_evaluator.cpp index 3ce6fcd223..4b49a56b4e 100644 --- a/libraries/chain/account_evaluator.cpp +++ b/libraries/chain/account_evaluator.cpp @@ -38,14 +38,14 @@ #include #include -namespace graphene { +namespace graphene { namespace protocol { - void verify_cycled_authority( const account_id_type& id, + void verify_cycled_authority( const account_id_type& id, const std::function& get_active, const std::function& get_owner, uint32_t max_recursion_depth ); } - namespace chain { + namespace chain { namespace detail { @@ -471,9 +471,9 @@ void_result account_unlock_evaluator::do_apply( const account_unlock_operation& const auto unlock_cost = balance.amount / 10; const auto penalty = asset(unlock_cost, balance.asset_id); - d.push_applied_operation(account_unlock_penalty_payment_operation( acnt->get_id(), penalty )); d.adjust_balance(acnt->get_id(), -penalty); d.adjust_balance(GRAPHENE_COMMITTEE_ACCOUNT, penalty); + d.push_applied_operation(account_unlock_penalty_payment_operation( acnt->get_id(), penalty )); } return void_result(); diff --git a/libraries/chain/hardfork.d/CYCLED_ACCOUNTS_HF.hf b/libraries/chain/hardfork.d/CYCLED_ACCOUNTS_HF.hf index bd4ef0a85f..82baae2d69 100644 --- a/libraries/chain/hardfork.d/CYCLED_ACCOUNTS_HF.hf +++ b/libraries/chain/hardfork.d/CYCLED_ACCOUNTS_HF.hf @@ -1,4 +1,4 @@ // #HARDFORK_CYCLED_ACCOUNTS_TIME Prevent to create/update account authorities that locks account. -#ifndef HARDFORK_CYCLED_ACCOUNTS_TIME -#define HARDFORK_CYCLED_ACCOUNTS_TIME (fc::time_point_sec( 1550102400 )) // Thursday, February 14, 2019 12:00:00 AM +#ifndef HARDFORK_CYCLED_ACCOUNTS_TIME +#define HARDFORK_CYCLED_ACCOUNTS_TIME (fc::time_point_sec( 1600000000 )) // Sunday, September 13, 2020 12:26:40 PM #endif diff --git a/libraries/protocol/include/graphene/protocol/account.hpp b/libraries/protocol/include/graphene/protocol/account.hpp index d126d5c9d7..adb9ccfd7c 100644 --- a/libraries/protocol/include/graphene/protocol/account.hpp +++ b/libraries/protocol/include/graphene/protocol/account.hpp @@ -207,10 +207,10 @@ namespace graphene { namespace protocol { struct fee_parameters_type {}; account_unlock_penalty_payment_operation(){} - account_unlock_penalty_payment_operation( + account_unlock_penalty_payment_operation( account_id_type a, asset p ) - :account_id(a),penalty(p), fee() {} + :account_id(a), penalty(p), fee() {} account_id_type account_id; asset penalty; @@ -347,7 +347,11 @@ FC_REFLECT( graphene::protocol::account_update_operation, FC_REFLECT( graphene::protocol::account_unlock_operation::fee_parameters_type, (fee) ) FC_REFLECT( graphene::protocol::account_unlock_operation, (fee)(account_to_unlock)(previous_authority)(extensions) ) -FC_REFLECT( graphene::protocol::account_upgrade_operation, (fee)(account_to_upgrade)(upgrade_to_lifetime_member)(extensions) ) + + +FC_REFLECT( graphene::protocol::account_upgrade_operation, + (fee)(account_to_upgrade)(upgrade_to_lifetime_member)(extensions) ) + FC_REFLECT( graphene::protocol::account_whitelist_operation, (fee)(authorizing_account)(account_to_list)(new_listing)(extensions)) FC_REFLECT( graphene::protocol::account_create_operation::fee_parameters_type, (basic_fee)(premium_fee)(price_per_kbyte) ) FC_REFLECT( graphene::protocol::account_whitelist_operation::fee_parameters_type, (fee) ) diff --git a/tests/tests/operation_tests.cpp b/tests/tests/operation_tests.cpp index 8bd8355494..9e6a3673b1 100644 --- a/tests/tests/operation_tests.cpp +++ b/tests/tests/operation_tests.cpp @@ -1272,46 +1272,23 @@ BOOST_AUTO_TEST_CASE( unlock_operation_penalty_payment_test ) { INVOKE(make_recursive_authority_test); - generate_blocks(HARDFORK_1268_TIME); - generate_block(); - GET_ACTOR(bob); fund(bob); ACTOR(issuer); - additional_asset_options_t options; - options.value.reward_percent = 2 * GRAPHENE_1_PERCENT; - - const auto usd = create_user_issued_asset( - "USD", - issuer, - charge_market_fee, - price(asset(1, asset_id_type(1)), asset(1)), - 1, - 20 * GRAPHENE_1_PERCENT, - options); + const auto usd = create_user_issued_asset("USD"); issue_uia(issuer, usd.amount(2000)); transfer(issuer, bob, usd.amount(200)); transfer(issuer, GRAPHENE_COMMITTEE_ACCOUNT(db), usd.amount(200)); - std::map bob_balance_helper; - std::map committee_balance_helper; + auto initial_balance_bob_BTS = get_balance(bob_id, asset_id_type()); + auto initial_balance_bob_USD = get_balance(bob_id, usd.get_id()); - const auto& bal_idx_before = db.get_index_type< primary_index< account_balance_index > >().get_secondary_index< balances_by_account_index >(); - for( const auto& entry : bal_idx_before.get_account_balances( bob_id ) ) - { - const auto balance = entry.second->get_balance(); - bob_balance_helper.emplace(balance.asset_id, balance.amount); - BOOST_CHECK_GT(balance.amount.value, 0); - } - for( const auto& entry : bal_idx_before.get_account_balances( GRAPHENE_COMMITTEE_ACCOUNT ) ) - { - const auto balance = entry.second->get_balance(); - committee_balance_helper.emplace(balance.asset_id, balance.amount); - BOOST_CHECK_GT(balance.amount.value, 0); - } - + auto initial_balance_committee_BTS = get_balance( GRAPHENE_COMMITTEE_ACCOUNT, asset_id_type()); + auto initial_balance_committee_USD = get_balance( GRAPHENE_COMMITTEE_ACCOUNT, usd.get_id()); + + generate_blocks(HARDFORK_CYCLED_ACCOUNTS_TIME); // unlock { account_unlock_operation op; @@ -1326,40 +1303,27 @@ BOOST_AUTO_TEST_CASE( unlock_operation_penalty_payment_test ) PUSH_TX( db, trx ); } - const auto& bal_idx_after = db.get_index_type< primary_index< account_balance_index > >().get_secondary_index< balances_by_account_index >(); - for( const auto& entry : bal_idx_after.get_account_balances( bob_id ) ) - { - const auto balance = entry.second->get_balance(); - BOOST_CHECK_EQUAL(balance.amount.value, bob_balance_helper.at(balance.asset_id).value * 0.9); - } + BOOST_CHECK_EQUAL( get_balance(bob_id, asset_id_type()), initial_balance_bob_BTS * 0.9); + BOOST_CHECK_EQUAL( get_balance(bob_id, usd.get_id()), initial_balance_bob_USD * 0.9); - for( const auto& entry : bal_idx_after.get_account_balances( GRAPHENE_COMMITTEE_ACCOUNT ) ) - { - const auto balance = entry.second->get_balance(); - BOOST_CHECK_EQUAL(balance.amount.value - committee_balance_helper.at(balance.asset_id).value, bob_balance_helper.at(balance.asset_id).value * 0.1 ); - } + BOOST_CHECK_EQUAL( get_balance(GRAPHENE_COMMITTEE_ACCOUNT, asset_id_type()), initial_balance_committee_BTS + initial_balance_bob_BTS * 0.1); + BOOST_CHECK_EQUAL( get_balance(GRAPHENE_COMMITTEE_ACCOUNT, usd.get_id()), initial_balance_committee_USD + initial_balance_bob_USD * 0.1); } FC_LOG_AND_RETHROW() } -BOOST_AUTO_TEST_CASE(unloc_account_operation_failed_not_enough_BTS) +BOOST_AUTO_TEST_CASE(unlock_account_operation_failed_not_enough_BTS) { try { INVOKE(make_recursive_authority_test); - generate_blocks(HARDFORK_1268_TIME); + generate_blocks(HARDFORK_CYCLED_ACCOUNTS_TIME); generate_block(); GET_ACTOR(bob); enable_fees(); - std::map bob_balance_helper; - std::map committee_balance_helper; - - const auto& bal_idx_before = db.get_index_type< primary_index< account_balance_index > >().get_secondary_index< balances_by_account_index >(); - BOOST_CHECK( bal_idx_before.get_account_balances( bob_id ).size() == 0); - // unlock account_unlock_operation op; op.account_to_unlock = bob_id; @@ -1371,7 +1335,7 @@ BOOST_AUTO_TEST_CASE(unloc_account_operation_failed_not_enough_BTS) sign(trx, bob_private_key); GRAPHENE_CHECK_THROW( PUSH_TX( db, trx ), fc::exception ); - + } FC_LOG_AND_RETHROW() }