GH-737 Should never std::move from a signed_block as the signed_block…

… still might be in use. For example if still in the forkdb. Would have preferred to `delete` the move constructor/operator= but unfortunately those are needed for our unused http push_block.

libraries/chain/include/eosio/chain/block.hpp

@@ -93,9 +93,9 @@ namespace eosio { namespace chain {
    public:
       signed_block() = default;
       explicit signed_block( const signed_block_header& h ):signed_block_header(h){}
-      signed_block( signed_block&& ) = default;
+      signed_block( signed_block&& ) = default;                // can be =delete once http push_block removed
       signed_block& operator=(const signed_block&) = delete;
-      signed_block& operator=(signed_block&&) = default;
+      signed_block& operator=(signed_block&&) = default;       // can be =delete once http push_block removed
       signed_block clone() const { return *this; }
 
       deque<transaction_receipt>   transactions; /// new or generated transactions

plugins/chain_plugin/chain_plugin.cpp

@@ -2115,7 +2115,7 @@ fc::variant read_only::get_block_header_state(const get_block_header_state_param
 
 void read_write::push_block(read_write::push_block_params&& params, next_function<read_write::push_block_results> next) {
    try {
-      auto b = std::make_shared<signed_block>( std::move(params) );
+      auto b = std::make_shared<signed_block>( params.clone() );
       app().get_method<incoming::methods::block_sync>()(b, b->calculate_id(), std::optional<block_handle>{});
    } catch ( boost::interprocess::bad_alloc& ) {
       handle_db_exhaustion();

unittests/block_tests.cpp

@@ -17,7 +17,7 @@ BOOST_AUTO_TEST_CASE_TEMPLATE( block_with_invalid_tx_test, T, testers )
    auto b = main.produce_block();
 
    // Make a copy of the valid block and corrupt the transaction
-   auto copy_b = std::make_shared<signed_block>(std::move(*b));
+   auto copy_b = std::make_shared<signed_block>(b->clone());
    auto signed_tx = std::get<packed_transaction>(copy_b->transactions.back().trx).get_signed_transaction();
    auto& act = signed_tx.actions.back();
    auto act_data = act.template data_as<newaccount>();
@@ -72,7 +72,7 @@ BOOST_AUTO_TEST_CASE_TEMPLATE( block_with_invalid_tx_mroot_test, T, testers )
    auto b = main.produce_block();
 
    // Make a copy of the valid block and corrupt the transaction
-   auto copy_b = std::make_shared<signed_block>(std::move(*b));
+   auto copy_b = std::make_shared<signed_block>(b->clone());
    const auto& packed_trx = std::get<packed_transaction>(copy_b->transactions.back().trx);
    auto signed_tx = packed_trx.get_signed_transaction();
 

unittests/forked_tests.cpp

@@ -242,10 +242,10 @@ BOOST_AUTO_TEST_CASE( forking ) try {
    }
    wlog( "end push c2 blocks to c1" );
    wlog( "now push dan's block to c1 but first corrupt it so it is a bad block" );
-   signed_block bad_block = std::move(*b);
+   signed_block bad_block{b->clone()};
    bad_block.action_mroot = bad_block.previous;
    auto bad_id = bad_block.calculate_id();
-   auto bad_block_btf = c.control->create_block_handle_future( bad_id, std::make_shared<signed_block>(std::move(bad_block)) );
+   auto bad_block_btf = c.control->create_block_handle_future( bad_id, std::make_shared<signed_block>(bad_block.clone()) );
    c.control->abort_block();
    controller::block_report br;
    BOOST_REQUIRE_EXCEPTION(c.control->push_block( br, bad_block_btf.get(), {}, trx_meta_cache_lookup{} ), fc::exception,

unittests/protocol_feature_tests.cpp

@@ -2219,7 +2219,7 @@ BOOST_AUTO_TEST_CASE( block_validation_after_stage_1_test ) { try {
    auto b = tester1.produce_block();
 
    // Make a copy of the block
-   auto copy_b = std::make_shared<signed_block>(std::move(*b));
+   auto copy_b = std::make_shared<signed_block>(b->clone());
    // Retrieve the last transaction
    auto signed_tx = std::get<packed_transaction>(copy_b->transactions.back().trx).get_signed_transaction();
    // Make a delayed transaction by forcing delay_sec greater than 0