From e0ced5d46514debbcf6a3713c6de96e9824e1a03 Mon Sep 17 00:00:00 2001 From: maodaishan Date: Thu, 9 May 2019 14:58:49 +0800 Subject: [PATCH] update eosio.msig contract. add new actions: oppose //oppose to current proposal unoppose //undo previous oppose abstain //abstain to current proposal unabstain //undo previous abstain you could only approve or oppose or abstain at a time. ex. if you approved to a proposal,now you wish to oppose it, you need to unapprove first, then you can oppose. oppose and abstain only show attitude, won't affect exec/cancel/invalidate. if you get enough approvals in an proposal, you can exec it, it will ignore the oppose/abstain when doing exec. --- .../include/eosio.msig/eosio.msig.hpp | 21 ++ contracts/eosio.msig/src/eosio.msig.cpp | 151 ++++++++++- tests/eosio.msig_tests.cpp | 254 +++++++++++++++++- 3 files changed, 416 insertions(+), 10 deletions(-) diff --git a/contracts/eosio.msig/include/eosio.msig/eosio.msig.hpp b/contracts/eosio.msig/include/eosio.msig/eosio.msig.hpp index 9589fe52a..8339e5312 100755 --- a/contracts/eosio.msig/include/eosio.msig/eosio.msig.hpp +++ b/contracts/eosio.msig/include/eosio.msig/eosio.msig.hpp @@ -23,6 +23,16 @@ namespace eosio { void exec( name proposer, name proposal_name, name executer ); [[eosio::action]] void invalidate( name account ); + [[eosio::action]] + void oppose( name proposer, name proposal_name, permission_level level, + const eosio::binary_extension& proposal_hash ); + [[eosio::action]] + void unoppose( name proposer, name proposal_name, permission_level level ); + [[eosio::action]] + void abstain( name proposer, name proposal_name, permission_level level, + const eosio::binary_extension& proposal_hash ); + [[eosio::action]] + void unabstain( name proposer, name proposal_name, permission_level level ); using propose_action = eosio::action_wrapper<"propose"_n, &multisig::propose>; using approve_action = eosio::action_wrapper<"approve"_n, &multisig::approve>; @@ -67,6 +77,17 @@ namespace eosio { }; typedef eosio::multi_index< "approvals2"_n, approvals_info > approvals; + //one can only approval, or oppose, or abstain. + //if he/she wants to change opinion of oppose, needs to unoppose first. + //oppose/abstain won't be counted in when trying exec. only approvals are counted. + struct [[eosio::table]] opposes_info { + name proposal_name; + std::vector opposed_approvals; + std::vector abstained_approvals; + uint64_t primary_key() const { return proposal_name.value; } + }; + typedef eosio::multi_index< "opposes"_n, opposes_info > opposes; + struct [[eosio::table]] invalidation { name account; time_point last_invalidation_time; diff --git a/contracts/eosio.msig/src/eosio.msig.cpp b/contracts/eosio.msig/src/eosio.msig.cpp index c0baad73c..01383051f 100755 --- a/contracts/eosio.msig/src/eosio.msig.cpp +++ b/contracts/eosio.msig/src/eosio.msig.cpp @@ -69,6 +69,16 @@ void multisig::approve( name proposer, name proposal_name, permission_level leve assert_sha256( prop.packed_transaction.data(), prop.packed_transaction.size(), *proposal_hash ); } + //check whether opposed or abstained to this proposal. + opposes oppotable(_self, proposer.value ); + auto oppos_it = oppotable.find(proposal_name.value ); + if(oppos_it != oppotable.end() ){ + auto check_itr = std::find( oppos_it->opposed_approvals.begin(), oppos_it->opposed_approvals.end(), level ); + check( check_itr == oppos_it->opposed_approvals.end(), "you already opposed this proposal" ); + check_itr = std::find( oppos_it->abstained_approvals.begin(), oppos_it->abstained_approvals.end(), level ); + check( check_itr == oppos_it->abstained_approvals.end(), "you already abstained this proposal" ); + } + approvals apptable( _self, proposer.value ); auto apps_it = apptable.find( proposal_name.value ); if ( apps_it != apptable.end() ) { @@ -139,6 +149,138 @@ void multisig::cancel( name proposer, name proposal_name, name canceler ) { check( apps_it != old_apptable.end(), "proposal not found" ); old_apptable.erase(apps_it); } + + //remove from oppose table + opposes oppotable(_self, proposer.value ); + auto oppo_it = oppotable.find( proposal_name.value ); + if( oppo_it != oppotable.end() ){ + oppotable.erase(oppo_it); + } +} + +void multisig::oppose( name proposer, name proposal_name, permission_level level, + const eosio::binary_extension& proposal_hash ){ + require_auth( level ); + if( proposal_hash ) { + proposals proptable( _self, proposer.value ); + auto& prop = proptable.get( proposal_name.value, "proposal not found" ); + assert_sha256( prop.packed_transaction.data(), prop.packed_transaction.size(), *proposal_hash ); + } + + //check whether level is in requested approvals or already approved to it + approvals apptable( _self, proposer.value ); + auto app_it = apptable.find(proposal_name.value ); + if( app_it != apptable.end() ){ + auto requested_it = std::find_if( app_it->requested_approvals.begin(), app_it->requested_approvals.end(), [&](const approval& a) { return a.level == level; } ); + bool requested = (requested_it != app_it->requested_approvals.end()); + auto approved_it = std::find_if( app_it->provided_approvals.begin(), app_it->provided_approvals.end(), [&](const approval &a){ return a.level == level;} ); + bool approved = (approved_it != app_it->provided_approvals.end()); + check( requested && !approved , "provided permission not requested, or you have approved to this proposal" ); + }else{ + old_approvals old_apptable( _self, proposer.value ); + auto old_app_it = old_apptable.find( proposal_name.value ); + if( old_app_it != old_apptable.end() ){ + auto requested_it = std::find( old_app_it->requested_approvals.begin(), old_app_it->requested_approvals.end(), level ); + bool requested = (requested_it != old_app_it->requested_approvals.end()); + auto approved_it = std::find( old_app_it->provided_approvals.begin(), old_app_it->provided_approvals.end(), level ); + bool approved = (approved_it != old_app_it->provided_approvals.end()); + check( requested && !approved , "provided permission not requested, or you have approved to this proposal" ); + } + } + + //check whether abstained/opposed to this proposal + //update opposed + opposes oppotable( _self, proposer.value ); + auto oppo_it = oppotable.find( proposal_name.value ); + if ( oppo_it != oppotable.end() ) { + auto abs_check_it = std::find( oppo_it->abstained_approvals.begin(), oppo_it->abstained_approvals.end(), level ); + check( abs_check_it == oppo_it->abstained_approvals.end(), "you have abstained to this proposal" ); + auto oppo_check_it = std::find( oppo_it->opposed_approvals.begin(), oppo_it->opposed_approvals.end(), level); + check( oppo_check_it == oppo_it->opposed_approvals.end(), "you have opposed to this proposal" ); + oppotable.modify( oppo_it, proposer, [&]( auto& o ) { + o.opposed_approvals.push_back( level ); + }); + }else{ + oppotable.emplace( proposer, [&]( auto& o ) { + o.proposal_name = proposal_name; + o.opposed_approvals.push_back( level ); + }); + } +} + +void multisig::unoppose( name proposer, name proposal_name, permission_level level ){ + require_auth( level ); + + opposes oppotable( _self, proposer.value ); + auto& oppo_it = oppotable.get( proposal_name.value, "can't find this proposal in opposes table" ); + auto itr = std::find( oppo_it.opposed_approvals.begin(), oppo_it.opposed_approvals.end(), level ); + check( itr != oppo_it.opposed_approvals.end(), "no oppose found" ); + oppotable.modify( oppo_it, proposer, [&]( auto& o ) { + o.opposed_approvals.erase( itr ); + }); +} + +void multisig::abstain( name proposer, name proposal_name, permission_level level, + const eosio::binary_extension& proposal_hash ){ + require_auth( level ); + + if( proposal_hash ) { + proposals proptable( _self, proposer.value ); + auto& prop = proptable.get( proposal_name.value, "proposal not found" ); + assert_sha256( prop.packed_transaction.data(), prop.packed_transaction.size(), *proposal_hash ); + } + + //check whether level is in requested approvals or already approved to it + approvals apptable( _self, proposer.value ); + auto app_it = apptable.find(proposal_name.value ); + if( app_it != apptable.end() ){ + auto requested_it = std::find_if( app_it->requested_approvals.begin(), app_it->requested_approvals.end(), [&](const approval& a) { return a.level == level; } ); + bool requested = (requested_it != app_it->requested_approvals.end()); + auto approved_it = std::find_if( app_it->provided_approvals.begin(), app_it->provided_approvals.end(), [&](const approval &a){ return a.level == level;} ); + bool approved = (approved_it != app_it->provided_approvals.end()); + check( requested && !approved , "provided permission not requested, or you have approved to this proposal" ); + }else{ + old_approvals old_apptable( _self, proposer.value ); + auto old_app_it = old_apptable.find( proposal_name.value ); + if( old_app_it != old_apptable.end() ){ + auto requested_it = std::find( old_app_it->requested_approvals.begin(), old_app_it->requested_approvals.end(), level ); + bool requested = (requested_it != old_app_it->requested_approvals.end()); + auto approved_it = std::find( old_app_it->provided_approvals.begin(), old_app_it->provided_approvals.end(), level ); + bool approved = (approved_it != old_app_it->provided_approvals.end()); + check( requested && !approved , "provided permission not requested, or you have approved to this proposal" ); + } + } + + //check whether abstained/opposed to this proposal + //update abstained + opposes oppotable( _self, proposer.value ); + auto abs_it = oppotable.find( proposal_name.value ); + if ( abs_it != oppotable.end() ) { + auto abs_check_it = std::find( abs_it->abstained_approvals.begin(), abs_it->abstained_approvals.end(), level ); + check( abs_check_it == abs_it->abstained_approvals.end(), "you have abstained to this proposal" ); + auto oppo_check_it = std::find( abs_it->opposed_approvals.begin(), abs_it->opposed_approvals.end(), level); + check( oppo_check_it == abs_it->opposed_approvals.end(), "you have opposed to this proposal" ); + oppotable.modify( abs_it, proposer, [&]( auto& a ) { + a.abstained_approvals.push_back( level ); + }); + }else{ + oppotable.emplace( proposer, [&]( auto& a ) { + a.proposal_name = proposal_name; + a.abstained_approvals.push_back( level ); + }); + } +} + +void multisig::unabstain( name proposer, name proposal_name, permission_level level ){ + require_auth( level ); + + opposes oppotable( _self, proposer.value ); + auto& abs_it = oppotable.get( proposal_name.value, "can't find this proposal in opposes table" ); + auto itr = std::find( abs_it.abstained_approvals.begin(), abs_it.abstained_approvals.end(), level ); + check( itr != abs_it.abstained_approvals.end(), "no abstain found" ); + oppotable.modify( abs_it, proposer, [&]( auto& a ) { + a.abstained_approvals.erase( itr ); + }); } void multisig::exec( name proposer, name proposal_name, name executer ) { @@ -186,6 +328,13 @@ void multisig::exec( name proposer, name proposal_name, name executer ) { prop.packed_transaction.data(), prop.packed_transaction.size() ); proptable.erase(prop); + + //clear opposes + opposes oppotable(_self, proposer.value); + auto oppos_it = oppotable.find( proposal_name.value ); + if( oppos_it != oppotable.end() ){ + oppotable.erase( oppos_it ); + } } void multisig::invalidate( name account ) { @@ -206,4 +355,4 @@ void multisig::invalidate( name account ) { } /// namespace eosio -EOSIO_DISPATCH( eosio::multisig, (propose)(approve)(unapprove)(cancel)(exec)(invalidate) ) +EOSIO_DISPATCH( eosio::multisig, (propose)(approve)(unapprove)(cancel)(oppose)(unoppose)(abstain)(unabstain)(exec)(invalidate) ) diff --git a/tests/eosio.msig_tests.cpp b/tests/eosio.msig_tests.cpp index cdc6dfc2f..5e00a5aff 100644 --- a/tests/eosio.msig_tests.cpp +++ b/tests/eosio.msig_tests.cpp @@ -131,18 +131,17 @@ class eosio_msig_tester : public tester { produce_block(); BOOST_REQUIRE_EQUAL( true, chain_has_transaction(trace->id) ); return trace; + } - /* - string action_type_name = abi_ser.get_action_type(name); + action_result push_action_get_result( const account_name& signer, const action_name &name, const variant_object &data ) { + string action_type_name = abi_ser.get_action_type(name); - action act; - act.account = N(eosio.msig); - act.name = name; - act.data = abi_ser.variant_to_binary( action_type_name, data, abi_serializer_max_time ); - //std::cout << "test:\n" << fc::to_hex(act.data.data(), act.data.size()) << " size = " << act.data.size() << std::endl; + action act; + act.account = N(eosio.msig); + act.name = name; + act.data = abi_ser.variant_to_binary( action_type_name, data,abi_serializer_max_time ); - return base_tester::push_action( std::move(act), auth ? uint64_t(signer) : 0 ); - */ + return base_tester::push_action( std::move(act), uint64_t(signer)); } transaction reqauth( account_name from, const vector& auths, const fc::microseconds& max_serialization_time ); @@ -954,4 +953,241 @@ BOOST_FIXTURE_TEST_CASE( switch_proposal_and_fail_approve_with_hash, eosio_msig_ ); } FC_LOG_AND_RETHROW() +BOOST_FIXTURE_TEST_CASE( propose_approve_oppose_abstain, eosio_msig_tester ) try { + //what should be tested: + //1. if you approved, you can't oppose/abstain. the same with oppose/abstain + //2. if you approved, unoppose/unabstain should fail, the same with oppose/abstain + //3. if you approved, unapprove should success, the same with unoppose and unabstain + //4. after unapprove, you can oppose/abstain, the same with unoppose/unabstain + //5. without all approved, exec will fail + //6. after all approved, exec will success + auto trx = reqauth("alice", + vector{ + { N(alice), config::active_name }, + { N(bob), config::active_name }, + { N(carol), config::active_name }}, + abi_serializer_max_time ); + + //propose should success + BOOST_REQUIRE_EQUAL(success(), + push_action_get_result( N(alice), N(propose), mvo() + ("proposer", "alice") + ("proposal_name", "first") + ("trx", trx) + ("requested", vector{ { N(alice), config::active_name }, { N(bob), config::active_name },{ N(carol), config::active_name } }) + )); + + //1. if you approved, you can't oppose/abstain. the same with oppose/abstain + //approve by alice should success + BOOST_REQUIRE_EQUAL(success(), + push_action_get_result( N(alice), N(approve), mvo() + ("proposer", "alice") + ("proposal_name", "first") + ("level", permission_level{ N(alice), config::active_name }) + )); + + //oppose by alice should fail + BOOST_REQUIRE_EQUAL( wasm_assert_msg("provided permission not requested, or you have approved to this proposal" ), + push_action_get_result( N(alice), N(oppose), mvo() + ("proposer", "alice") + ("proposal_name", "first") + ("level", permission_level{ N(alice), config::active_name }) + )); + + //abstain by alice should fail + BOOST_REQUIRE_EQUAL( wasm_assert_msg("provided permission not requested, or you have approved to this proposal" ), + push_action_get_result( N(alice), N(abstain), mvo() + ("proposer", "alice") + ("proposal_name", "first") + ("level", permission_level{ N(alice), config::active_name }) + )); + + //oppose by bob should success + BOOST_REQUIRE_EQUAL(success(), + push_action_get_result(N(bob), N(oppose), mvo() + ("proposer", "alice") + ("proposal_name", "first") + ("level", permission_level{N(bob), config::active_name }) + )); + + //approve by bob should fail + BOOST_REQUIRE_EQUAL(wasm_assert_msg("you already opposed this proposal" ), + push_action_get_result(N(bob), N(approve), mvo() + ("proposer", "alice") + ("proposal_name", "first") + ("level", permission_level{N(bob), config::active_name }) + )); + + //abstain by bob should fail + BOOST_REQUIRE_EQUAL(wasm_assert_msg("you have opposed to this proposal" ), + push_action_get_result(N(bob), N(abstain), mvo() + ("proposer", "alice") + ("proposal_name", "first") + ("level", permission_level{N(bob), config::active_name }) + )); + + //abstain by carol should success + BOOST_REQUIRE_EQUAL(success(), + push_action_get_result(N(carol), N(abstain), mvo() + ("proposer", "alice") + ("proposal_name", "first") + ("level", permission_level{N(carol), config::active_name }) + )); + + //approve by carol should fail + BOOST_REQUIRE_EQUAL(wasm_assert_msg("you already abstained this proposal"), + push_action_get_result(N(carol), N(approve), mvo() + ("proposer", "alice") + ("proposal_name", "first") + ("level", permission_level{N(carol), config::active_name }) + )); + + //oppose by carol should fail + BOOST_REQUIRE_EQUAL(wasm_assert_msg("you have abstained to this proposal"), + push_action_get_result(N(carol), N(oppose), mvo() + ("proposer", "alice") + ("proposal_name", "first") + ("level", permission_level{N(carol), config::active_name }) + )); + + //2. if you approved, unoppose/unabstain should fail, the same with oppose/abstain + //bob do unapprove will faill + BOOST_REQUIRE_EQUAL(wasm_assert_msg("no approval previously granted"), + push_action_get_result(N(bob), N(unapprove), mvo() + ("proposer", "alice") + ("proposal_name", "first") + ("level", permission_level{N(bob), config::active_name }) + )); + + //carol do unapprove will fail + BOOST_REQUIRE_EQUAL(wasm_assert_msg("no approval previously granted"), + push_action_get_result(N(carol), N(unapprove), mvo() + ("proposer", "alice") + ("proposal_name", "first") + ("level", permission_level{N(carol), config::active_name }) + )); + + //alice do unoppose will fail + BOOST_REQUIRE_EQUAL(wasm_assert_msg("no oppose found"), + push_action_get_result(N(alice), N(unoppose), mvo() + ("proposer", "alice") + ("proposal_name", "first") + ("level", permission_level{N(alice), config::active_name }) + )); + + //carol do unoppose will fail + BOOST_REQUIRE_EQUAL(wasm_assert_msg("no oppose found"), + push_action_get_result(N(carol), N(unoppose), mvo() + ("proposer", "alice") + ("proposal_name", "first") + ("level", permission_level{N(carol), config::active_name }) + )); + + //alice do unabstain will fail + BOOST_REQUIRE_EQUAL(wasm_assert_msg("no abstain found"), + push_action_get_result(N(alice), N(unabstain), mvo() + ("proposer", "alice") + ("proposal_name", "first") + ("level", permission_level{N(alice), config::active_name }) + )); + + //bob do unabstain will fail + BOOST_REQUIRE_EQUAL(wasm_assert_msg("no abstain found"), + push_action_get_result(N(bob), N(unabstain), mvo() + ("proposer", "alice") + ("proposal_name", "first") + ("level", permission_level{N(bob), config::active_name }) + )); + + //3. if you approved, unapprove should success, the same with unoppose and unabstain + //alice unapprove will success + BOOST_REQUIRE_EQUAL(success(), + push_action_get_result(N(alice), N(unapprove), mvo() + ("proposer", "alice") + ("proposal_name", "first") + ("level", permission_level{N(alice), config::active_name }) + )); + + //bob unoppose will success + BOOST_REQUIRE_EQUAL(success(), + push_action_get_result(N(bob), N(unoppose), mvo() + ("proposer", "alice") + ("proposal_name", "first") + ("level", permission_level{N(bob), config::active_name }) + )); + + //carol unoppose will success + BOOST_REQUIRE_EQUAL(success(), + push_action_get_result(N(carol), N(unabstain), mvo() + ("proposer", "alice") + ("proposal_name", "first") + ("level", permission_level{N(carol), config::active_name }) + )); + + //4. after unapprove, you can oppose/abstain, the same with unoppose/unabstain + //alice do abstain will success + BOOST_REQUIRE_EQUAL(success(), + push_action_get_result(N(alice), N(abstain), mvo() + ("proposer", "alice") + ("proposal_name", "first") + ("level", permission_level{N(alice), config::active_name }) + )); + //bob do approve will success + BOOST_REQUIRE_EQUAL(success(), + push_action_get_result(N(bob), N(approve), mvo() + ("proposer", "alice") + ("proposal_name", "first") + ("level", permission_level{N(bob), config::active_name }) + )); + //carol do oppose will success + BOOST_REQUIRE_EQUAL(success(), + push_action_get_result(N(carol), N(oppose), mvo() + ("proposer", "alice") + ("proposal_name", "first") + ("level", permission_level{N(carol), config::active_name }) + )); + + //5. without all approved, exec will fail + BOOST_REQUIRE_EQUAL(wasm_assert_msg("transaction authorization failed"), + push_action_get_result(N(alice), N(exec), mvo() + ("proposer", "alice") + ("proposal_name", "first") + ("executer", "alice"))); + + //6. after all approved, exec will success + //alice unabstain and approve will success + BOOST_REQUIRE_EQUAL(success(), + push_action_get_result(N(alice), N(unabstain), mvo() + ("proposer", "alice") + ("proposal_name", "first") + ("level", permission_level{N(alice), config::active_name }) + )); + BOOST_REQUIRE_EQUAL(success(), + push_action_get_result(N(alice), N(approve), mvo() + ("proposer", "alice") + ("proposal_name", "first") + ("level", permission_level{N(alice), config::active_name }) + )); + //carol unoppose and approve will success + BOOST_REQUIRE_EQUAL(success(), + push_action_get_result(N(carol), N(unoppose), mvo() + ("proposer", "alice") + ("proposal_name", "first") + ("level", permission_level{N(carol), config::active_name }) + )); + BOOST_REQUIRE_EQUAL(success(), + push_action_get_result(N(carol), N(approve), mvo() + ("proposer", "alice") + ("proposal_name", "first") + ("level", permission_level{N(carol), config::active_name }) + )); + + //exec will success + BOOST_REQUIRE_EQUAL(success(), + push_action_get_result(N(alice), N(exec), mvo() + ("proposer", "alice") + ("proposal_name", "first") + ("executer", "alice"))); +}FC_LOG_AND_RETHROW() + BOOST_AUTO_TEST_SUITE_END()