From 3e9ca96d539a8a65743bf4d9f34aa0171a1872f4 Mon Sep 17 00:00:00 2001 From: arhag Date: Thu, 28 Mar 2019 11:55:45 -0400 Subject: [PATCH 1/3] implement DISALLOW_EMPTY_PRODUCER_SCHEDULE protocol feature --- libraries/chain/controller.cpp | 4 ++++ .../include/eosio/chain/protocol_feature_manager.hpp | 3 ++- libraries/chain/protocol_feature_manager.cpp | 11 +++++++++++ libraries/chain/wasm_interface.cpp | 7 +++++++ 4 files changed, 24 insertions(+), 1 deletion(-) diff --git a/libraries/chain/controller.cpp b/libraries/chain/controller.cpp index 8bec114d6be..01caab76083 100644 --- a/libraries/chain/controller.cpp +++ b/libraries/chain/controller.cpp @@ -2670,6 +2670,10 @@ int64_t controller::set_proposed_producers( vector producers ) { const auto& gpo = get_global_properties(); auto cur_block_num = head_block_num() + 1; + if( producers.size() == 0 && is_builtin_activated( builtin_protocol_feature_t::disallow_empty_producer_schedule ) ) { + return -1; + } + if( gpo.proposed_schedule_block_num.valid() ) { if( *gpo.proposed_schedule_block_num != cur_block_num ) return -1; // there is already a proposed schedule set in a previous block, wait for it to become pending diff --git a/libraries/chain/include/eosio/chain/protocol_feature_manager.hpp b/libraries/chain/include/eosio/chain/protocol_feature_manager.hpp index 4c7bcf6d95a..f9f55dffb7a 100644 --- a/libraries/chain/include/eosio/chain/protocol_feature_manager.hpp +++ b/libraries/chain/include/eosio/chain/protocol_feature_manager.hpp @@ -17,7 +17,8 @@ enum class builtin_protocol_feature_t : uint32_t { preactivate_feature, only_link_to_existing_permission, replace_deferred, - fix_linkauth_restriction + fix_linkauth_restriction, + disallow_empty_producer_schedule }; struct protocol_feature_subjective_restrictions { diff --git a/libraries/chain/protocol_feature_manager.cpp b/libraries/chain/protocol_feature_manager.cpp index 94d42cfad1d..78c84ff9410 100644 --- a/libraries/chain/protocol_feature_manager.cpp +++ b/libraries/chain/protocol_feature_manager.cpp @@ -62,6 +62,17 @@ Builtin protocol feature: FIX_LINKAUTH_RESTRICTION Removes the restriction on eosio::linkauth for non-native actions named one of the five special action names: updateauth, deleteauth, linkauth, unlinkauth, or canceldelay. +*/ + {} + } ) + ( builtin_protocol_feature_t::disallow_empty_producer_schedule, builtin_protocol_feature_spec{ + "DISALLOW_EMPTY_PRODUCER_SCHEDULE", + fc::variant("2853617cec3eabd41881eb48882e6fc5e81a0db917d375057864b3befbe29acd").as(), + // SHA256 hash of the raw message below within the comment delimiters (do not modify message below). +/* +Builtin protocol feature: DISALLOW_EMPTY_PRODUCER_SCHEDULE + +Disallows proposing an empty producer schedule. */ {} } ) diff --git a/libraries/chain/wasm_interface.cpp b/libraries/chain/wasm_interface.cpp index 98d82bbd335..3414d9dc972 100644 --- a/libraries/chain/wasm_interface.cpp +++ b/libraries/chain/wasm_interface.cpp @@ -168,6 +168,13 @@ class privileged_api : public context_aware_api { datastream ds( packed_producer_schedule, datalen ); vector producers; fc::raw::unpack(ds, producers); + EOS_ASSERT( producers.size() > 0 + || !context.control.is_builtin_activated( + builtin_protocol_feature_t::disallow_empty_producer_schedule + ), + wasm_execution_error, + "Producer schedule cannot be empty" + ); EOS_ASSERT(producers.size() <= config::max_producers, wasm_execution_error, "Producer schedule exceeds the maximum producer count for this chain"); // check that producers are unique std::set unique_producers; From cd28dafad1a7a5290370c50dd26288301f046da0 Mon Sep 17 00:00:00 2001 From: arhag Date: Thu, 28 Mar 2019 14:59:58 -0400 Subject: [PATCH 2/3] fix producer_schedule_tests/empty_producer_schedule_has_no_effect --- unittests/producer_schedule_tests.cpp | 83 ++++++++++++++------------- 1 file changed, 43 insertions(+), 40 deletions(-) diff --git a/unittests/producer_schedule_tests.cpp b/unittests/producer_schedule_tests.cpp index 9003f8555bd..2a91f06a7c1 100644 --- a/unittests/producer_schedule_tests.cpp +++ b/unittests/producer_schedule_tests.cpp @@ -328,81 +328,84 @@ BOOST_FIXTURE_TEST_CASE( producer_schedule_reduction, tester ) try { BOOST_REQUIRE_EQUAL( validate(), true ); } FC_LOG_AND_RETHROW() -BOOST_FIXTURE_TEST_CASE( empty_producer_schedule_has_no_effect, tester ) try { - create_accounts( {N(alice),N(bob),N(carol)} ); - while (control->head_block_num() < 3) { - produce_block(); +BOOST_AUTO_TEST_CASE( empty_producer_schedule_has_no_effect ) try { + validating_tester c( validating_tester::default_config() ); + c.execute_setup_policy( setup_policy::preactivate_feature_and_new_bios ); + + c.create_accounts( {N(alice),N(bob),N(carol)} ); + while (c.control->head_block_num() < 3) { + c.produce_block(); } auto compare_schedules = [&]( const vector& a, const producer_schedule_type& b ) { return std::equal( a.begin(), a.end(), b.producers.begin(), b.producers.end() ); }; - auto res = set_producers( {N(alice),N(bob)} ); + auto res = c.set_producers( {N(alice),N(bob)} ); vector sch1 = { {N(alice), get_public_key(N(alice), "active")}, {N(bob), get_public_key(N(bob), "active")} }; wlog("set producer schedule to [alice,bob]"); - BOOST_REQUIRE_EQUAL( true, control->proposed_producers().valid() ); - BOOST_CHECK_EQUAL( true, compare_schedules( sch1, *control->proposed_producers() ) ); - BOOST_CHECK_EQUAL( control->pending_producers().producers.size(), 0u ); + BOOST_REQUIRE_EQUAL( true, c.control->proposed_producers().valid() ); + BOOST_CHECK_EQUAL( true, compare_schedules( sch1, *c.control->proposed_producers() ) ); + BOOST_CHECK_EQUAL( c.control->pending_producers().producers.size(), 0u ); // Start a new block which promotes the proposed schedule to pending - produce_block(); - BOOST_CHECK_EQUAL( control->pending_producers().version, 1u ); - BOOST_CHECK_EQUAL( true, compare_schedules( sch1, control->pending_producers() ) ); - BOOST_CHECK_EQUAL( control->active_producers().version, 0u ); + c.produce_block(); + BOOST_CHECK_EQUAL( c.control->pending_producers().version, 1u ); + BOOST_CHECK_EQUAL( true, compare_schedules( sch1, c.control->pending_producers() ) ); + BOOST_CHECK_EQUAL( c.control->active_producers().version, 0u ); // Start a new block which promotes the pending schedule to active - produce_block(); - BOOST_CHECK_EQUAL( control->active_producers().version, 1u ); - BOOST_CHECK_EQUAL( true, compare_schedules( sch1, control->active_producers() ) ); - produce_blocks(6); + c.produce_block(); + BOOST_CHECK_EQUAL( c.control->active_producers().version, 1u ); + BOOST_CHECK_EQUAL( true, compare_schedules( sch1, c.control->active_producers() ) ); + c.produce_blocks(6); - res = set_producers( {} ); + res = c.set_producers( {} ); wlog("set producer schedule to []"); - BOOST_REQUIRE_EQUAL( true, control->proposed_producers().valid() ); - BOOST_CHECK_EQUAL( control->proposed_producers()->producers.size(), 0u ); - BOOST_CHECK_EQUAL( control->proposed_producers()->version, 2u ); + BOOST_REQUIRE_EQUAL( true, c.control->proposed_producers().valid() ); + BOOST_CHECK_EQUAL( c.control->proposed_producers()->producers.size(), 0u ); + BOOST_CHECK_EQUAL( c.control->proposed_producers()->version, 2u ); - produce_blocks(12); - BOOST_CHECK_EQUAL( control->pending_producers().version, 1u ); + c.produce_blocks(12); + BOOST_CHECK_EQUAL( c.control->pending_producers().version, 1u ); // Empty producer schedule does get promoted from proposed to pending - produce_block(); - BOOST_CHECK_EQUAL( control->pending_producers().version, 2u ); - BOOST_CHECK_EQUAL( false, control->proposed_producers().valid() ); + c.produce_block(); + BOOST_CHECK_EQUAL( c.control->pending_producers().version, 2u ); + BOOST_CHECK_EQUAL( false, c.control->proposed_producers().valid() ); // However it should not get promoted from pending to active - produce_blocks(24); - BOOST_CHECK_EQUAL( control->active_producers().version, 1u ); - BOOST_CHECK_EQUAL( control->pending_producers().version, 2u ); + c.produce_blocks(24); + BOOST_CHECK_EQUAL( c.control->active_producers().version, 1u ); + BOOST_CHECK_EQUAL( c.control->pending_producers().version, 2u ); // Setting a new producer schedule should still use version 2 - res = set_producers( {N(alice),N(bob),N(carol)} ); + res = c.set_producers( {N(alice),N(bob),N(carol)} ); vector sch2 = { {N(alice), get_public_key(N(alice), "active")}, {N(bob), get_public_key(N(bob), "active")}, {N(carol), get_public_key(N(carol), "active")} }; wlog("set producer schedule to [alice,bob,carol]"); - BOOST_REQUIRE_EQUAL( true, control->proposed_producers().valid() ); - BOOST_CHECK_EQUAL( true, compare_schedules( sch2, *control->proposed_producers() ) ); - BOOST_CHECK_EQUAL( control->proposed_producers()->version, 2u ); + BOOST_REQUIRE_EQUAL( true, c.control->proposed_producers().valid() ); + BOOST_CHECK_EQUAL( true, compare_schedules( sch2, *c.control->proposed_producers() ) ); + BOOST_CHECK_EQUAL( c.control->proposed_producers()->version, 2u ); // Produce enough blocks to promote the proposed schedule to pending, which it can do because the existing pending has zero producers - produce_blocks(24); - BOOST_CHECK_EQUAL( control->active_producers().version, 1u ); - BOOST_CHECK_EQUAL( control->pending_producers().version, 2u ); - BOOST_CHECK_EQUAL( true, compare_schedules( sch2, control->pending_producers() ) ); + c.produce_blocks(24); + BOOST_CHECK_EQUAL( c.control->active_producers().version, 1u ); + BOOST_CHECK_EQUAL( c.control->pending_producers().version, 2u ); + BOOST_CHECK_EQUAL( true, compare_schedules( sch2, c.control->pending_producers() ) ); // Produce enough blocks to promote the pending schedule to active - produce_blocks(24); - BOOST_CHECK_EQUAL( control->active_producers().version, 2u ); - BOOST_CHECK_EQUAL( true, compare_schedules( sch2, control->active_producers() ) ); + c.produce_blocks(24); + BOOST_CHECK_EQUAL( c.control->active_producers().version, 2u ); + BOOST_CHECK_EQUAL( true, compare_schedules( sch2, c.control->active_producers() ) ); - BOOST_REQUIRE_EQUAL( validate(), true ); + BOOST_REQUIRE_EQUAL( c.validate(), true ); } FC_LOG_AND_RETHROW() BOOST_AUTO_TEST_CASE( producer_watermark_test ) try { From 65ca26684299634739f3ff6b950f12bfcde51c0c Mon Sep 17 00:00:00 2001 From: Andrianto Lie Date: Fri, 29 Mar 2019 14:57:32 +0800 Subject: [PATCH 3/3] Add unit test for disallow empty producer schedule protocol feature --- unittests/protocol_feature_tests.cpp | 29 +++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/unittests/protocol_feature_tests.cpp b/unittests/protocol_feature_tests.cpp index eeb26def023..10d9e96e718 100644 --- a/unittests/protocol_feature_tests.cpp +++ b/unittests/protocol_feature_tests.cpp @@ -494,7 +494,7 @@ BOOST_AUTO_TEST_CASE( fix_linkauth_restriction ) { try { chain.create_account(N(currency)); chain.create_account(tester_account); chain.produce_blocks(); - + chain.push_action(config::system_account_name, updateauth::get_name(), tester_account, fc::mutable_variant_object() ("account", name(tester_account).to_string()) ("permission", "first") @@ -555,4 +555,31 @@ BOOST_AUTO_TEST_CASE( fix_linkauth_restriction ) { try { } FC_LOG_AND_RETHROW() } +BOOST_AUTO_TEST_CASE( disallow_empty_producer_schedule_test ) { try { + tester c( setup_policy::preactivate_feature_and_new_bios ); + + const auto& pfm = c.control->get_protocol_feature_manager(); + const auto& d = pfm.get_builtin_digest( builtin_protocol_feature_t::disallow_empty_producer_schedule ); + BOOST_REQUIRE( d ); + + // Before activation, it is allowed to set empty producer schedule + c.set_producers( {} ); + + // After activation, it should not be allowed + c.preactivate_protocol_features( {*d} ); + c.produce_block(); + BOOST_REQUIRE_EXCEPTION( c.set_producers( {} ), + wasm_execution_error, + fc_exception_message_is( "Producer schedule cannot be empty" ) ); + + // Setting non empty producer schedule should still be fine + vector producer_names = {N(alice),N(bob),N(carol)}; + c.create_accounts( producer_names ); + c.set_producers( producer_names ); + c.produce_blocks(2); + const auto& schedule = c.get_producer_keys( producer_names ); + BOOST_CHECK( std::equal( schedule.begin(), schedule.end(), c.control->active_producers().producers.begin()) ); + +} FC_LOG_AND_RETHROW() } + BOOST_AUTO_TEST_SUITE_END()