From 38bb9226ec1fabc4939108f76809c84de999c3e2 Mon Sep 17 00:00:00 2001 From: pbattu123 Date: Thu, 5 Dec 2019 13:46:13 -0400 Subject: [PATCH] Move GPOS withdraw logic to have single transaction(also single fee) and update API --- libraries/app/database_api.cpp | 21 +++++ .../app/include/graphene/app/database_api.hpp | 6 +- .../chain/protocol/chain_parameters.hpp | 2 +- .../graphene/chain/protocol/vesting.hpp | 3 +- libraries/chain/vesting_balance_evaluator.cpp | 89 +++++++++++++++---- libraries/wallet/wallet.cpp | 51 +++-------- tests/tests/gpos_tests.cpp | 65 ++++++++++++++ 7 files changed, 176 insertions(+), 61 deletions(-) diff --git a/libraries/app/database_api.cpp b/libraries/app/database_api.cpp index 3987f192..df6458f3 100644 --- a/libraries/app/database_api.cpp +++ b/libraries/app/database_api.cpp @@ -2278,7 +2278,28 @@ graphene::app::gpos_info database_api_impl::get_gpos_info(const account_id_type } #endif + vector account_vbos; + const time_point_sec now = _db.head_block_time(); + auto vesting_range = _db.get_index_type().indices().get().equal_range(account); + std::for_each(vesting_range.first, vesting_range.second, + [&account_vbos, now](const vesting_balance_object& balance) { + if(balance.balance.amount > 0 && balance.balance_type == vesting_balance_type::gpos + && balance.balance.asset_id == asset_id_type()) + account_vbos.emplace_back(balance); + }); + + share_type allowed_withdraw_amount = 0, account_vested_balance = 0; + + for (const vesting_balance_object& vesting_balance_obj : account_vbos) + { + account_vested_balance += vesting_balance_obj.balance.amount; + if(vesting_balance_obj.is_withdraw_allowed(_db.head_block_time(), vesting_balance_obj.balance.amount)) + allowed_withdraw_amount += vesting_balance_obj.balance.amount; + } + result.total_amount = total_amount; + result.allowed_withdraw_amount = allowed_withdraw_amount; + result.account_vested_balance = account_vested_balance; return result; } diff --git a/libraries/app/include/graphene/app/database_api.hpp b/libraries/app/include/graphene/app/database_api.hpp index 378a4aea..dc8aba52 100644 --- a/libraries/app/include/graphene/app/database_api.hpp +++ b/libraries/app/include/graphene/app/database_api.hpp @@ -119,7 +119,9 @@ struct gpos_info { asset award; share_type total_amount; uint32_t current_subperiod; - fc::time_point_sec last_voted_time; + fc::time_point_sec last_voted_time; + share_type allowed_withdraw_amount; + share_type account_vested_balance; }; /** @@ -722,7 +724,7 @@ FC_REFLECT( graphene::app::order_book, (base)(quote)(bids)(asks) ); FC_REFLECT( graphene::app::market_ticker, (base)(quote)(latest)(lowest_ask)(highest_bid)(percent_change)(base_volume)(quote_volume) ); FC_REFLECT( graphene::app::market_volume, (base)(quote)(base_volume)(quote_volume) ); FC_REFLECT( graphene::app::market_trade, (date)(price)(amount)(value) ); -FC_REFLECT( graphene::app::gpos_info, (vesting_factor)(award)(total_amount)(current_subperiod)(last_voted_time) ); +FC_REFLECT( graphene::app::gpos_info, (vesting_factor)(award)(total_amount)(current_subperiod)(last_voted_time)(allowed_withdraw_amount)(account_vested_balance) ); FC_API(graphene::app::database_api, diff --git a/libraries/chain/include/graphene/chain/protocol/chain_parameters.hpp b/libraries/chain/include/graphene/chain/protocol/chain_parameters.hpp index 091da0c9..3f2c5a22 100644 --- a/libraries/chain/include/graphene/chain/protocol/chain_parameters.hpp +++ b/libraries/chain/include/graphene/chain/protocol/chain_parameters.hpp @@ -47,7 +47,7 @@ namespace graphene { namespace chain { /* gpos parameters */ optional < uint32_t > gpos_period = GPOS_PERIOD; optional < uint32_t > gpos_subperiod = GPOS_SUBPERIOD; - optional < uint32_t > gpos_period_start; + optional < uint32_t > gpos_period_start = HARDFORK_GPOS_TIME.sec_since_epoch(); optional < uint32_t > gpos_vesting_lockin_period = GPOS_VESTING_LOCKIN_PERIOD; }; diff --git a/libraries/chain/include/graphene/chain/protocol/vesting.hpp b/libraries/chain/include/graphene/chain/protocol/vesting.hpp index 1d83b90f..7c53b378 100644 --- a/libraries/chain/include/graphene/chain/protocol/vesting.hpp +++ b/libraries/chain/include/graphene/chain/protocol/vesting.hpp @@ -112,6 +112,7 @@ namespace graphene { namespace chain { vesting_balance_id_type vesting_balance; account_id_type owner; ///< Must be vesting_balance.owner asset amount; + vesting_balance_type balance_type; account_id_type fee_payer()const { return owner; } void validate()const @@ -127,7 +128,7 @@ FC_REFLECT( graphene::chain::vesting_balance_create_operation::fee_parameters_ty FC_REFLECT( graphene::chain::vesting_balance_withdraw_operation::fee_parameters_type, (fee) ) FC_REFLECT( graphene::chain::vesting_balance_create_operation, (fee)(creator)(owner)(amount)(policy)(balance_type) ) -FC_REFLECT( graphene::chain::vesting_balance_withdraw_operation, (fee)(vesting_balance)(owner)(amount) ) +FC_REFLECT( graphene::chain::vesting_balance_withdraw_operation, (fee)(vesting_balance)(owner)(amount)(balance_type) ) FC_REFLECT(graphene::chain::linear_vesting_policy_initializer, (begin_timestamp)(vesting_cliff_seconds)(vesting_duration_seconds) ) FC_REFLECT(graphene::chain::cdd_vesting_policy_initializer, (start_claim)(vesting_seconds) ) diff --git a/libraries/chain/vesting_balance_evaluator.cpp b/libraries/chain/vesting_balance_evaluator.cpp index ec974600..7d78b54d 100644 --- a/libraries/chain/vesting_balance_evaluator.cpp +++ b/libraries/chain/vesting_balance_evaluator.cpp @@ -143,11 +143,33 @@ void_result vesting_balance_withdraw_evaluator::do_evaluate( const vesting_balan const database& d = db(); const time_point_sec now = d.head_block_time(); - const vesting_balance_object& vbo = op.vesting_balance( d ); - FC_ASSERT( op.owner == vbo.owner, "", ("op.owner", op.owner)("vbo.owner", vbo.owner) ); - FC_ASSERT( vbo.is_withdraw_allowed( now, op.amount ), "Account has either insufficient ${balance_type} Vested Balance or lock-in period is not matured", - ("balance_type", get_vesting_balance_type(vbo.balance_type))("now", now)("op", op)("vbo", vbo) ); - assert( op.amount <= vbo.balance ); // is_withdraw_allowed should fail before this check is reached + if(op.balance_type == vesting_balance_type::gpos) + { + const account_id_type account_id = op.owner; + vector vbos; + auto vesting_range = d.get_index_type().indices().get().equal_range(account_id); + std::for_each(vesting_range.first, vesting_range.second, + [&vbos, now](const vesting_balance_object& balance) { + if(balance.balance.amount > 0 && balance.balance_type == vesting_balance_type::gpos + && balance.is_withdraw_allowed(now, balance.balance.amount) && balance.balance.asset_id == asset_id_type()) + vbos.emplace_back(balance); + }); + + asset total_amount; + for (const vesting_balance_object& vesting_balance_obj : vbos) + { + total_amount += vesting_balance_obj.balance.amount; + } + FC_ASSERT( op.amount <= total_amount, "Account has either insufficient GPOS Vested Balance or lock-in period is not matured"); + } + else + { + const vesting_balance_object& vbo = op.vesting_balance( d ); + FC_ASSERT( op.owner == vbo.owner, "", ("op.owner", op.owner)("vbo.owner", vbo.owner) ); + FC_ASSERT( vbo.is_withdraw_allowed( now, op.amount ), "Account has either insufficient ${balance_type} Vested Balance to withdraw", + ("balance_type", get_vesting_balance_type(vbo.balance_type))("now", now)("op", op)("vbo", vbo) ); + assert( op.amount <= vbo.balance ); // is_withdraw_allowed should fail before this check is reached + } /* const account_object& owner_account = op.owner( d ); */ // TODO: Check asset authorizations and withdrawals @@ -159,21 +181,54 @@ void_result vesting_balance_withdraw_evaluator::do_apply( const vesting_balance_ database& d = db(); const time_point_sec now = d.head_block_time(); - - const vesting_balance_object& vbo = op.vesting_balance( d ); - - // Allow zero balance objects to stick around, (1) to comply - // with the chain's "objects live forever" design principle, (2) - // if it's cashback or worker, it'll be filled up again. - - d.modify( vbo, [&]( vesting_balance_object& vbo ) + //Handling all GPOS withdrawls separately from normal and SONs(future extension). + // One request/transaction would be sufficient to withdraw from multiple vesting balance ids + if(op.balance_type == vesting_balance_type::gpos) { - vbo.withdraw( now, op.amount ); - } ); + const account_id_type account_id = op.owner; + vector ids; + auto vesting_range = d.get_index_type().indices().get().equal_range(account_id); + std::for_each(vesting_range.first, vesting_range.second, + [&ids, now](const vesting_balance_object& balance) { + if(balance.balance.amount > 0 && balance.balance_type == vesting_balance_type::gpos + && balance.balance.asset_id == asset_id_type()) + ids.emplace_back(balance.id); + }); - d.adjust_balance( op.owner, op.amount ); + asset total_withdraw_amount = op.amount; + for (const vesting_balance_id_type& id : ids) + { + const vesting_balance_object& vbo = id( d ); + if(total_withdraw_amount.amount > vbo.balance.amount) + { + total_withdraw_amount.amount -= vbo.balance.amount; + d.modify( vbo, [&]( vesting_balance_object& vbo ) {vbo.withdraw( now, vbo.balance );} ); + d.adjust_balance( op.owner, vbo.balance ); + } + else + { + d.modify( vbo, [&]( vesting_balance_object& vbo ) {vbo.withdraw( now, total_withdraw_amount );} ); + d.adjust_balance( op.owner, total_withdraw_amount); + break; + } + } + } + else + { + const vesting_balance_object& vbo = op.vesting_balance( d ); + + // Allow zero balance objects to stick around, (1) to comply + // with the chain's "objects live forever" design principle, (2) + // if it's cashback or worker, it'll be filled up again. + + d.modify( vbo, [&]( vesting_balance_object& vbo ) + { + vbo.withdraw( now, op.amount ); + } ); + + d.adjust_balance( op.owner, op.amount ); + } - // TODO: Check asset authorizations and withdrawals return void_result(); } FC_CAPTURE_AND_RETHROW( (op) ) } diff --git a/libraries/wallet/wallet.cpp b/libraries/wallet/wallet.cpp index bdb756c2..ae64ff04 100644 --- a/libraries/wallet/wallet.cpp +++ b/libraries/wallet/wallet.cpp @@ -2133,47 +2133,18 @@ public: if(!vbos.size()) vbos.emplace_back( get_object(*vbid) ); + const vesting_balance_object& vbo = vbos.front(); + + vesting_balance_withdraw_operation vesting_balance_withdraw_op; + + vesting_balance_withdraw_op.vesting_balance = vbo.id; + vesting_balance_withdraw_op.owner = vbo.owner; + vesting_balance_withdraw_op.amount = asset_obj.amount_from_string(amount); + vesting_balance_withdraw_op.balance_type = vesting_balance_type::gpos; + signed_transaction tx; - asset withdraw_amount = asset_obj.amount_from_string(amount); - bool onetime_fee_paid = false; - - for(const vesting_balance_object& vbo: vbos ) - { - if((vbo.balance_type == vesting_balance_type::gpos) && vbo.balance.amount > 0) - { - fc::optional vest_id = vbo.id; - vesting_balance_withdraw_operation vesting_balance_withdraw_op; - - // Since there are multiple vesting objects, below logic with vesting_balance_evaluator.cpp changes will - // deduct fee from single object and set withdrawl fee to 0 for rest of objects based on requested amount. - if(onetime_fee_paid) - vesting_balance_withdraw_op.fee = asset( 0, asset_id_type() ); - else - vesting_balance_withdraw_op.fee = _remote_db->get_global_properties().parameters.current_fees->calculate_fee(vesting_balance_withdraw_op); - - vesting_balance_withdraw_op.vesting_balance = *vest_id; - vesting_balance_withdraw_op.owner = vbo.owner; - if(withdraw_amount.amount > vbo.balance.amount) - { - vesting_balance_withdraw_op.amount = vbo.balance.amount; - withdraw_amount.amount -= vbo.balance.amount; - } - else - { - vesting_balance_withdraw_op.amount = withdraw_amount.amount; - tx.operations.push_back( vesting_balance_withdraw_op ); - withdraw_amount.amount -= vbo.balance.amount; - break; - } - - tx.operations.push_back( vesting_balance_withdraw_op ); - onetime_fee_paid = true; - } - } - - if( withdraw_amount.amount > 0) - FC_THROW("Account has NO or Insufficient balance to withdraw"); - + tx.operations.push_back( vesting_balance_withdraw_op ); + set_operation_fees( tx, _remote_db->get_global_properties().parameters.current_fees ); tx.validate(); return sign_transaction( tx, broadcast ); diff --git a/tests/tests/gpos_tests.cpp b/tests/tests/gpos_tests.cpp index 196fe7ee..5deaddd4 100644 --- a/tests/tests/gpos_tests.cpp +++ b/tests/tests/gpos_tests.cpp @@ -26,6 +26,7 @@ #include #include +#include #include #include #include @@ -70,6 +71,23 @@ struct gpos_fixture: database_fixture return db.get(ptx.operation_results[0].get()); } + void withdraw_gpos_vesting(const vesting_balance_id_type v_bid, const account_id_type owner, const asset amount, + const vesting_balance_type type, const fc::ecc::private_key& key) + { + vesting_balance_withdraw_operation op; + op.vesting_balance = v_bid; + op.owner = owner; + op.amount = amount; + op.balance_type = type; + + trx.operations.push_back(op); + set_expiration(db, trx); + trx.validate(); + sign(trx, key); + PUSH_TX(db, trx); + trx.clear(); + } + void update_payout_interval(std::string asset_name, fc::time_point start, uint32_t interval) { auto dividend_holder_asset_object = get_asset(asset_name); @@ -90,10 +108,12 @@ struct gpos_fixture: database_fixture p.parameters.extensions.value.gpos_period = vesting_period; p.parameters.extensions.value.gpos_subperiod = vesting_subperiod; p.parameters.extensions.value.gpos_period_start = period_start.sec_since_epoch(); + p.parameters.extensions.value.gpos_vesting_lockin_period = vesting_subperiod; }); BOOST_CHECK_EQUAL(db.get_global_properties().parameters.gpos_period(), vesting_period); BOOST_CHECK_EQUAL(db.get_global_properties().parameters.gpos_subperiod(), vesting_subperiod); BOOST_CHECK_EQUAL(db.get_global_properties().parameters.gpos_period_start(), period_start.sec_since_epoch()); + BOOST_CHECK_EQUAL(db.get_global_properties().parameters.gpos_vesting_lockin_period(), vesting_subperiod); } void update_maintenance_interval(uint32_t new_interval) @@ -919,6 +939,51 @@ BOOST_AUTO_TEST_CASE( account_multiple_vesting ) throw; } } + +BOOST_AUTO_TEST_CASE( Withdraw_gpos_vesting_balance ) +{ + try { + // advance to HF + generate_blocks(HARDFORK_GPOS_TIME); + generate_block(); + set_expiration(db, trx); + + // update default gpos global parameters to 4 days + auto now = db.head_block_time(); + update_gpos_global(345600, 86400, now); + + ACTORS((alice)(bob)); + + const auto& core = asset_id_type()(db); + + + transfer( committee_account, alice_id, core.amount( 500 ) ); + transfer( committee_account, bob_id, core.amount( 99 ) ); + + // add some vesting to Alice, Bob + vesting_balance_object vbo; + create_vesting(alice_id, core.amount(150), vesting_balance_type::gpos); + create_vesting(bob_id, core.amount(99), vesting_balance_type::gpos); + generate_blocks(db.get_dynamic_global_properties().next_maintenance_time); + + generate_block(); + + generate_blocks(db.get_dynamic_global_properties().next_maintenance_time); + generate_blocks(db.get_global_properties().parameters.gpos_vesting_lockin_period()); + BOOST_CHECK_EQUAL(get_balance(alice_id(db), core), 350); + withdraw_gpos_vesting(vbo.id, alice_id, core.amount(50), vesting_balance_type::gpos, alice_private_key); + withdraw_gpos_vesting(vbo.id, bob_id, core.amount(99), vesting_balance_type::gpos, bob_private_key); + generate_block(); + // verify charles balance + BOOST_CHECK_EQUAL(get_balance(alice_id(db), core), 400); + BOOST_CHECK_EQUAL(get_balance(bob_id(db), core), 99); + } + catch (fc::exception &e) { + edump((e.to_detail_string())); + throw; + } +} + /* BOOST_AUTO_TEST_CASE( competing_proposals ) {