From 2b6be2a65e174c1992fb474767c25de205860e30 Mon Sep 17 00:00:00 2001 From: Eric Frias Date: Mon, 10 Apr 2017 15:50:18 -0400 Subject: [PATCH 1/2] Verify all account ids on whitelist are valid accounts --- libraries/chain/tournament_evaluator.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/libraries/chain/tournament_evaluator.cpp b/libraries/chain/tournament_evaluator.cpp index 1f811290..94d64bf6 100644 --- a/libraries/chain/tournament_evaluator.cpp +++ b/libraries/chain/tournament_evaluator.cpp @@ -35,6 +35,11 @@ namespace graphene { namespace chain { FC_ASSERT(op.options.whitelist.size() < maximum_tournament_whitelist_length, "Whitelist must not be longer than ${maximum_tournament_whitelist_length}", ("maximum_tournament_whitelist_length", maximum_tournament_whitelist_length)); + + for (const account_id_type& account_id : op.options.whitelist) + { + account_id(d); + } if (op.options.start_time) { From f7113d3d0580a37f92ccd2347db99b5e9e0b12d1 Mon Sep 17 00:00:00 2001 From: Eric Frias Date: Mon, 10 Apr 2017 23:29:02 -0400 Subject: [PATCH 2/2] [DLN] change parameters to tournament_leave_operation to specify the account canceling the registration instead of the payer. The canceling acount must be either the payer or the player. Don't allow a player to leave a tournament once registration has been closed. Require player to be authorized for the asset being paid as tournament prize winnings during tournament_join_operation. Tests still need to be made for these changes. --- libraries/app/impacted.cpp | 4 +++- .../graphene/chain/protocol/tournament.hpp | 8 +++---- .../graphene/chain/tournament_object.hpp | 2 +- libraries/chain/tournament_evaluator.cpp | 24 ++++++++++++------- libraries/chain/tournament_object.cpp | 13 +++++----- libraries/wallet/wallet.cpp | 6 ++--- tests/tournament/tournament_tests.cpp | 4 ++-- 7 files changed, 34 insertions(+), 27 deletions(-) diff --git a/libraries/app/impacted.cpp b/libraries/app/impacted.cpp index 2ed2169d..55613d27 100644 --- a/libraries/app/impacted.cpp +++ b/libraries/app/impacted.cpp @@ -220,7 +220,9 @@ struct get_impacted_account_visitor } void operator()( const tournament_leave_operation& op ) { - _impacted.erase( op.payer_account_id ); + //if account canceling registration is not the player, it must be the payer + if (op.canceling_account_id != op.player_account_id) + _impacted.erase( op.canceling_account_id ); _impacted.erase( op.player_account_id ); } void operator()( const game_move_operation& op ) diff --git a/libraries/chain/include/graphene/chain/protocol/tournament.hpp b/libraries/chain/include/graphene/chain/protocol/tournament.hpp index 82bb49dd..5c9398d7 100644 --- a/libraries/chain/include/graphene/chain/protocol/tournament.hpp +++ b/libraries/chain/include/graphene/chain/protocol/tournament.hpp @@ -153,8 +153,8 @@ namespace graphene { namespace chain { asset fee; - /// The account that is paying the fee - account_id_type payer_account_id; + /// The account that is unregistering the player from tournament (must be payer or player) + account_id_type canceling_account_id; /// The account that would play in the tournament, would receive any winnings. account_id_type player_account_id; @@ -163,7 +163,7 @@ namespace graphene { namespace chain { tournament_id_type tournament_id; extensions_type extensions; - account_id_type fee_payer()const { return payer_account_id; } + account_id_type fee_payer()const { return canceling_account_id; } share_type calculate_fee(const fee_parameters_type& k)const; void validate()const; }; @@ -253,7 +253,7 @@ FC_REFLECT( graphene::chain::tournament_join_operation, (extensions)) FC_REFLECT( graphene::chain::tournament_leave_operation, (fee) - (payer_account_id) + (canceling_account_id) (player_account_id) (tournament_id) (extensions)) diff --git a/libraries/chain/include/graphene/chain/tournament_object.hpp b/libraries/chain/include/graphene/chain/tournament_object.hpp index 81edabdc..531a0b98 100644 --- a/libraries/chain/include/graphene/chain/tournament_object.hpp +++ b/libraries/chain/include/graphene/chain/tournament_object.hpp @@ -117,7 +117,7 @@ namespace graphene { namespace chain { /// called by database maintenance code when registration for this contest has expired void on_registration_deadline_passed(database& db); void on_player_registered(database& db, account_id_type payer_id, account_id_type player_id); - void on_player_unregistered(database& db, account_id_type payer_id, account_id_type player_id); + void on_player_unregistered(database& db, account_id_type player_id); void on_start_time_arrived(database& db); void on_match_completed(database& db, const match_object& match); diff --git a/libraries/chain/tournament_evaluator.cpp b/libraries/chain/tournament_evaluator.cpp index 94d64bf6..9b02e75c 100644 --- a/libraries/chain/tournament_evaluator.cpp +++ b/libraries/chain/tournament_evaluator.cpp @@ -140,6 +140,7 @@ namespace graphene { namespace chain { fc_ilog(fc::logger::get("tournament"), "details_id = ${id}",("id", _tournament_obj->tournament_details_id)); _tournament_details_obj = &_tournament_obj->tournament_details_id(d); _payer_account = &op.payer_account_id(d); + const account_object& player_account = op.player_account_id(d); //const account_object& player_account = op.player_account_id(d); _buy_in_asset_type = &op.buy_in.asset_id(d); @@ -162,6 +163,12 @@ namespace graphene { namespace chain { "Asset {asset} has transfer_restricted flag enabled", ("asset", op.buy_in.asset_id)); + GRAPHENE_ASSERT(is_authorized_asset(d, player_account, *_buy_in_asset_type), + transfer_from_account_not_whitelisted, + "player account ${player} is not whitelisted for asset ${asset}", + ("player", op.player_account_id) + ("asset", op.buy_in.asset_id)); + GRAPHENE_ASSERT(is_authorized_asset(d, *_payer_account, *_buy_in_asset_type), transfer_from_account_not_whitelisted, "payer account ${payer} is not whitelisted for asset ${asset}", @@ -195,23 +202,22 @@ namespace graphene { namespace chain { _tournament_details_obj = &_tournament_obj->tournament_details_id(d); FC_ASSERT(_tournament_details_obj->registered_players.find(op.player_account_id) != _tournament_details_obj->registered_players.end(), "Player is not registered for this tournament"); - //FC_ASSERT(_tournament_details_obj->payers.find(op.payer_account_id) != _tournament_details_obj->payers.end(), - // "Payer is not registered for this tournament"); - FC_ASSERT(_tournament_obj->get_state() == tournament_state::accepting_registrations || - _tournament_obj->get_state() == tournament_state::awaiting_start); - FC_ASSERT(d.head_block_time() <= _tournament_obj->options.registration_deadline, - "Registration deadline has already passed"); + FC_ASSERT(op.canceling_account_id == op.player_account_id || + op.canceling_account_id == _tournament_details_obj->players_payers.at(op.player_account_id), + "Only player or payer can unregister the player from a tournament"); + + FC_ASSERT(_tournament_obj->get_state() == tournament_state::accepting_registrations, + "Can only leave a tournament during registration period"); + return void_result(); } FC_CAPTURE_AND_RETHROW( (op) ) } void_result tournament_leave_evaluator::do_apply( const tournament_leave_operation& op ) { try { -#if 1 db().modify(*_tournament_obj, [&](tournament_object& tournament_obj){ - tournament_obj.on_player_unregistered(db(), op.payer_account_id, op.player_account_id); + tournament_obj.on_player_unregistered(db(), op.player_account_id); }); -#endif return void_result(); } FC_CAPTURE_AND_RETHROW( (op) ) } diff --git a/libraries/chain/tournament_object.cpp b/libraries/chain/tournament_object.cpp index 4adb5bb7..b71909e2 100644 --- a/libraries/chain/tournament_object.cpp +++ b/libraries/chain/tournament_object.cpp @@ -51,10 +51,9 @@ namespace graphene { namespace chain { struct player_unregistered { database& db; - account_id_type payer_id; account_id_type player_id; - player_unregistered(database& db, account_id_type payer_id, account_id_type player_id) : - db(db), payer_id(payer_id), player_id(player_id) + player_unregistered(database& db, account_id_type player_id) : + db(db), player_id(player_id) {} }; struct registration_deadline_passed @@ -403,10 +402,10 @@ namespace graphene { namespace chain { "In unregister_player action, player_id is ${player_id}", ("player_id", event.player_id)); - event.db.adjust_balance(event.payer_id, tournament_obj->options.buy_in); const tournament_details_object& tournament_details_obj = tournament_obj->tournament_details_id(event.db); + account_id_type payer_id = tournament_details_obj.players_payers.at(event.player_id); + event.db.adjust_balance(payer_id, tournament_obj->options.buy_in); event.db.modify(tournament_details_obj, [&](tournament_details_object& tournament_details_obj){ - account_id_type payer_id = tournament_details_obj.players_payers[event.player_id]; tournament_details_obj.payers[payer_id] -= tournament_obj->options.buy_in.amount; if (tournament_details_obj.payers[payer_id] <= 0) tournament_details_obj.payers.erase(payer_id); @@ -551,9 +550,9 @@ namespace graphene { namespace chain { my->state_machine.process_event(player_registered(db, payer_id, player_id)); } - void tournament_object::on_player_unregistered(database& db, account_id_type payer_id, account_id_type player_id) + void tournament_object::on_player_unregistered(database& db, account_id_type player_id) { - my->state_machine.process_event(player_unregistered(db, payer_id, player_id)); + my->state_machine.process_event(player_unregistered(db, player_id)); } void tournament_object::on_start_time_arrived(database& db) diff --git a/libraries/wallet/wallet.cpp b/libraries/wallet/wallet.cpp index 3e9db0d7..456536c6 100644 --- a/libraries/wallet/wallet.cpp +++ b/libraries/wallet/wallet.cpp @@ -4867,19 +4867,19 @@ signed_transaction wallet_api::tournament_join( string payer_account, return my->sign_transaction( tx, broadcast ); } -signed_transaction wallet_api::tournament_leave( string payer_account, +signed_transaction wallet_api::tournament_leave( string canceling_account, string player_account, tournament_id_type tournament_id, bool broadcast) { FC_ASSERT( !is_locked() ); account_object player_account_obj = get_account(player_account); - account_object payer_account_obj = get_account(payer_account); + account_object canceling_account_obj = get_account(canceling_account); //graphene::chain::tournament_object tournament_obj = my->get_object(tournament_id); signed_transaction tx; tournament_leave_operation op; - op.payer_account_id = payer_account_obj.get_id(); + op.canceling_account_id = canceling_account_obj.get_id(); op.player_account_id = player_account_obj.get_id(); op.tournament_id = tournament_id; diff --git a/tests/tournament/tournament_tests.cpp b/tests/tournament/tournament_tests.cpp index 8889f205..d2d421f8 100644 --- a/tests/tournament/tournament_tests.cpp +++ b/tests/tournament/tournament_tests.cpp @@ -238,7 +238,7 @@ public: void leave_tournament(const tournament_id_type & tournament_id, const account_id_type& player_id, - const account_id_type& payer_id, + const account_id_type& canceling_account_id, const fc::ecc::private_key& sig_priv_key ) { @@ -247,7 +247,7 @@ public: signed_transaction tx; tournament_leave_operation op; - op.payer_account_id = payer_id; + op.canceling_account_id = canceling_account_id; op.player_account_id = player_id; op.tournament_id = tournament_id; tx.operations = {op};