From 38f3988ac55a810ce51b0e4ea83b211f6409e2b5 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Tue, 7 Feb 2023 08:34:33 -0600 Subject: [PATCH 01/10] GH-677 Move abi serialization off the main thread for get_block --- plugins/chain_api_plugin/chain_api_plugin.cpp | 74 ++++++++++++++++++- plugins/chain_plugin/chain_plugin.cpp | 17 +---- .../eosio/chain_plugin/chain_plugin.hpp | 2 +- plugins/http_plugin/http_plugin.cpp | 5 ++ .../include/eosio/http_plugin/http_plugin.hpp | 2 + 5 files changed, 81 insertions(+), 19 deletions(-) diff --git a/plugins/chain_api_plugin/chain_api_plugin.cpp b/plugins/chain_api_plugin/chain_api_plugin.cpp index db1c25bf28..24d79b9e55 100644 --- a/plugins/chain_api_plugin/chain_api_plugin.cpp +++ b/plugins/chain_api_plugin/chain_api_plugin.cpp @@ -98,20 +98,18 @@ void chain_api_plugin::plugin_startup() { ilog( "starting chain_api_plugin" ); my.reset(new chain_api_plugin_impl(app().get_plugin().chain())); auto& chain = app().get_plugin(); - auto& http = app().get_plugin(); - fc::microseconds max_response_time = http.get_max_response_time(); + auto& _http_plugin = app().get_plugin(); + fc::microseconds max_response_time = _http_plugin.get_max_response_time(); auto ro_api = chain.get_read_only_api(max_response_time); auto rw_api = chain.get_read_write_api(max_response_time); - auto& _http_plugin = app().get_plugin(); ro_api.set_shorten_abi_errors( !http_plugin::verbose_errors() ); _http_plugin.add_api( { CHAIN_RO_CALL(get_info, 200, http_params_types::no_params)}, appbase::priority::medium_high); _http_plugin.add_api({ CHAIN_RO_CALL(get_activated_protocol_features, 200, http_params_types::possible_no_params), - CHAIN_RO_CALL(get_block, 200, http_params_types::params_required), CHAIN_RO_CALL(get_block_info, 200, http_params_types::params_required), CHAIN_RO_CALL(get_block_header_state, 200, http_params_types::params_required), CHAIN_RO_CALL(get_account, 200, http_params_types::params_required), @@ -151,6 +149,74 @@ void chain_api_plugin::plugin_startup() { CHAIN_RO_CALL_WITH_400(get_transaction_status, 200, http_params_types::params_required), }); } + + _http_plugin.add_api({ + { std::string("/v1/chain/get_block"), + [ro_api, &chain, &_http_plugin, max_time=std::min(chain.get_abi_serializer_max_time(),max_response_time)] + ( string, string body, url_response_callback cb ) mutable { + auto deadline = ro_api.start(); + try { + auto start = fc::time_point::now(); + auto params = parse_params(body); + FC_CHECK_DEADLINE( deadline ); + chain::signed_block_ptr block = ro_api.get_block( params, deadline ); + + auto yield = abi_serializer::create_yield_function( max_time ); + std::unordered_map > abi_cache; + auto add_to_cache = [&]( const chain::action& a ) { + auto it = abi_cache.find( a.account ); + if( it == abi_cache.end() ) + abi_cache.emplace_hint( it, a.account, chain.chain().get_abi_serializer( a.account, yield ) ); + }; + for( const auto& receipt: block->transactions ) { + if( std::holds_alternative( receipt.trx ) ) { + const auto& pt = std::get( receipt.trx ); + const auto& t = pt.get_transaction(); + for( const auto& a: t.actions ) + add_to_cache( a ); + for( const auto& a: t.context_free_actions ) + add_to_cache( a ); + } + } + FC_CHECK_DEADLINE( deadline ); + + auto post_time = fc::time_point::now(); + auto remaining_time = max_time - (post_time - start); + _http_plugin.post_http_thread_pool( + [cb, deadline, post_time, remaining_time, abi_cache{std::move(abi_cache)}, block{std::move( block )}]() { + try { + auto new_deadline = deadline + (fc::time_point::now() - post_time); + + auto abi_serializer_resolver = [&abi_cache](const account_name& account) -> std::optional { + auto it = abi_cache.find( account ); + if( it != abi_cache.end() ) + return it->second; + return {}; + }; + + fc::variant pretty_output; + abi_serializer::to_variant( *block, pretty_output, abi_serializer_resolver, + abi_serializer::create_yield_function( remaining_time ) ); + + const auto block_id = block->calculate_id(); + uint32_t ref_block_prefix = block_id._hash[1]; + + fc::variant result = fc::mutable_variant_object( pretty_output.get_object() ) + ( "id", block_id ) + ( "block_num", block->block_num() ) + ( "ref_block_prefix", ref_block_prefix ); + + cb( 200, new_deadline, std::move( result ) ); + } catch( ... ) { + http_plugin::handle_exception( "chain", "get_block", "", cb ); + } + } ); + } catch( ... ) { + http_plugin::handle_exception("chain", "get_block", body, cb); + } + } + } + } ); } void chain_api_plugin::plugin_shutdown() {} diff --git a/plugins/chain_plugin/chain_plugin.cpp b/plugins/chain_plugin/chain_plugin.cpp index 3df14b4e28..50b1fe4ef9 100644 --- a/plugins/chain_plugin/chain_plugin.cpp +++ b/plugins/chain_plugin/chain_plugin.cpp @@ -16,7 +16,6 @@ #include #include #include -#include #include @@ -30,7 +29,6 @@ #include #include -#include #include // reflect chainbase::environment for --print-build-info option @@ -1881,7 +1879,7 @@ read_only::get_scheduled_transactions( const read_only::get_scheduled_transactio return result; } -fc::variant read_only::get_block(const read_only::get_block_params& params, const fc::time_point& deadline) const { +chain::signed_block_ptr read_only::get_block(const read_only::get_block_params& params, const fc::time_point& deadline) const { signed_block_ptr block; std::optional block_num; @@ -1903,18 +1901,9 @@ fc::variant read_only::get_block(const read_only::get_block_params& params, cons } EOS_ASSERT( block, unknown_block_exception, "Could not find block: ${block}", ("block", params.block_num_or_id)); + FC_CHECK_DEADLINE(deadline); - fc::variant pretty_output; - abi_serializer::to_variant(*block, pretty_output, make_resolver(db, abi_serializer::create_yield_function( abi_serializer_max_time )), - abi_serializer::create_yield_function( abi_serializer_max_time )); - - const auto block_id = block->calculate_id(); - uint32_t ref_block_prefix = block_id._hash[1]; - - return fc::mutable_variant_object(pretty_output.get_object()) - ("id", block_id) - ("block_num",block->block_num()) - ("ref_block_prefix", ref_block_prefix); + return block; } fc::variant read_only::get_block_info(const read_only::get_block_info_params& params, const fc::time_point& deadline) const { diff --git a/plugins/chain_plugin/include/eosio/chain_plugin/chain_plugin.hpp b/plugins/chain_plugin/include/eosio/chain_plugin/chain_plugin.hpp index 094c1e28eb..ba28eb5784 100644 --- a/plugins/chain_plugin/include/eosio/chain_plugin/chain_plugin.hpp +++ b/plugins/chain_plugin/include/eosio/chain_plugin/chain_plugin.hpp @@ -354,7 +354,7 @@ class read_only { string block_num_or_id; }; - fc::variant get_block(const get_block_params& params, const fc::time_point& deadline) const; + chain::signed_block_ptr get_block(const get_block_params& params, const fc::time_point& deadline) const; struct get_block_info_params { uint32_t block_num = 0; diff --git a/plugins/http_plugin/http_plugin.cpp b/plugins/http_plugin/http_plugin.cpp index 637cdd84c8..82dda56973 100644 --- a/plugins/http_plugin/http_plugin.cpp +++ b/plugins/http_plugin/http_plugin.cpp @@ -490,6 +490,11 @@ class http_plugin_impl : public std::enable_shared_from_this { my->plugin_state->url_handlers[url] = my->make_http_thread_url_handler(handler); } + void http_plugin::post_http_thread_pool(std::function f) { + if( f ) + boost::asio::post( my->plugin_state->thread_pool.get_executor(), f ); + } + void http_plugin::handle_exception( const char *api_name, const char *call_name, const string& body, const url_response_callback& cb) { try { try { diff --git a/plugins/http_plugin/include/eosio/http_plugin/http_plugin.hpp b/plugins/http_plugin/include/eosio/http_plugin/http_plugin.hpp index 58323adb04..55adf02f11 100644 --- a/plugins/http_plugin/include/eosio/http_plugin/http_plugin.hpp +++ b/plugins/http_plugin/include/eosio/http_plugin/http_plugin.hpp @@ -95,6 +95,8 @@ namespace eosio { // standard exception handling for api handlers static void handle_exception( const char *api_name, const char *call_name, const string& body, const url_response_callback& cb ); + void post_http_thread_pool(std::function f); + bool is_on_loopback() const; bool is_secure() const; From b7fdd1c8840c7f30bbfab6eafa4df22234ec6855 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Tue, 7 Feb 2023 11:56:06 -0600 Subject: [PATCH 02/10] GH-677 Refactor get_block into multiple methods for testing. Also fix issue with invalid abi should not throw exception. --- plugins/chain_api_plugin/chain_api_plugin.cpp | 41 ++------------- plugins/chain_plugin/chain_plugin.cpp | 52 +++++++++++++++++++ .../eosio/chain_plugin/chain_plugin.hpp | 7 +++ tests/chain_plugin_tests.cpp | 8 ++- 4 files changed, 69 insertions(+), 39 deletions(-) diff --git a/plugins/chain_api_plugin/chain_api_plugin.cpp b/plugins/chain_api_plugin/chain_api_plugin.cpp index 24d79b9e55..0588aa1734 100644 --- a/plugins/chain_api_plugin/chain_api_plugin.cpp +++ b/plugins/chain_api_plugin/chain_api_plugin.cpp @@ -152,7 +152,7 @@ void chain_api_plugin::plugin_startup() { _http_plugin.add_api({ { std::string("/v1/chain/get_block"), - [ro_api, &chain, &_http_plugin, max_time=std::min(chain.get_abi_serializer_max_time(),max_response_time)] + [ro_api, &_http_plugin, max_time=std::min(chain.get_abi_serializer_max_time(),max_response_time)] ( string, string body, url_response_callback cb ) mutable { auto deadline = ro_api.start(); try { @@ -161,50 +161,17 @@ void chain_api_plugin::plugin_startup() { FC_CHECK_DEADLINE( deadline ); chain::signed_block_ptr block = ro_api.get_block( params, deadline ); - auto yield = abi_serializer::create_yield_function( max_time ); - std::unordered_map > abi_cache; - auto add_to_cache = [&]( const chain::action& a ) { - auto it = abi_cache.find( a.account ); - if( it == abi_cache.end() ) - abi_cache.emplace_hint( it, a.account, chain.chain().get_abi_serializer( a.account, yield ) ); - }; - for( const auto& receipt: block->transactions ) { - if( std::holds_alternative( receipt.trx ) ) { - const auto& pt = std::get( receipt.trx ); - const auto& t = pt.get_transaction(); - for( const auto& a: t.actions ) - add_to_cache( a ); - for( const auto& a: t.context_free_actions ) - add_to_cache( a ); - } - } + auto abi_cache = ro_api.get_block_serializers( block, max_time ); FC_CHECK_DEADLINE( deadline ); auto post_time = fc::time_point::now(); auto remaining_time = max_time - (post_time - start); _http_plugin.post_http_thread_pool( - [cb, deadline, post_time, remaining_time, abi_cache{std::move(abi_cache)}, block{std::move( block )}]() { + [ro_api, cb, deadline, post_time, remaining_time, abi_cache{std::move(abi_cache)}, block{std::move( block )}]() { try { auto new_deadline = deadline + (fc::time_point::now() - post_time); - auto abi_serializer_resolver = [&abi_cache](const account_name& account) -> std::optional { - auto it = abi_cache.find( account ); - if( it != abi_cache.end() ) - return it->second; - return {}; - }; - - fc::variant pretty_output; - abi_serializer::to_variant( *block, pretty_output, abi_serializer_resolver, - abi_serializer::create_yield_function( remaining_time ) ); - - const auto block_id = block->calculate_id(); - uint32_t ref_block_prefix = block_id._hash[1]; - - fc::variant result = fc::mutable_variant_object( pretty_output.get_object() ) - ( "id", block_id ) - ( "block_num", block->block_num() ) - ( "ref_block_prefix", ref_block_prefix ); + fc::variant result = ro_api.convert_block( block, abi_cache, remaining_time ); cb( 200, new_deadline, std::move( result ) ); } catch( ... ) { diff --git a/plugins/chain_plugin/chain_plugin.cpp b/plugins/chain_plugin/chain_plugin.cpp index 50b1fe4ef9..bdcb642649 100644 --- a/plugins/chain_plugin/chain_plugin.cpp +++ b/plugins/chain_plugin/chain_plugin.cpp @@ -1906,6 +1906,58 @@ chain::signed_block_ptr read_only::get_block(const read_only::get_block_params& return block; } +std::unordered_map> +read_only::get_block_serializers( const chain::signed_block_ptr& block, const fc::microseconds& max_time ) const { + auto yield = abi_serializer::create_yield_function( max_time ); + auto resolver = make_resolver(db, yield ); + std::unordered_map > abi_cache; + auto add_to_cache = [&]( const chain::action& a ) { + auto it = abi_cache.find( a.account ); + if( it == abi_cache.end() ) { + try { + abi_cache.emplace_hint( it, a.account, resolver( a.account ) ); + } catch( ... ) { + // keep behavior of not throwing on invalid abi, will result in hex data + } + } + }; + for( const auto& receipt: block->transactions ) { + if( std::holds_alternative( receipt.trx ) ) { + const auto& pt = std::get( receipt.trx ); + const auto& t = pt.get_transaction(); + for( const auto& a: t.actions ) + add_to_cache( a ); + for( const auto& a: t.context_free_actions ) + add_to_cache( a ); + } + } + return abi_cache; +} + +fc::variant read_only::convert_block( const chain::signed_block_ptr& block, + std::unordered_map> abi_cache, + const fc::microseconds& max_time ) const { + + auto abi_serializer_resolver = [&abi_cache](const account_name& account) -> std::optional { + auto it = abi_cache.find( account ); + if( it != abi_cache.end() ) + return it->second; + return {}; + }; + + fc::variant pretty_output; + abi_serializer::to_variant( *block, pretty_output, abi_serializer_resolver, + abi_serializer::create_yield_function( max_time ) ); + + const auto block_id = block->calculate_id(); + uint32_t ref_block_prefix = block_id._hash[1]; + + return fc::mutable_variant_object( pretty_output.get_object() ) + ( "id", block_id ) + ( "block_num", block->block_num() ) + ( "ref_block_prefix", ref_block_prefix ); +} + fc::variant read_only::get_block_info(const read_only::get_block_info_params& params, const fc::time_point& deadline) const { signed_block_ptr block; diff --git a/plugins/chain_plugin/include/eosio/chain_plugin/chain_plugin.hpp b/plugins/chain_plugin/include/eosio/chain_plugin/chain_plugin.hpp index ba28eb5784..f8b6f1b1be 100644 --- a/plugins/chain_plugin/include/eosio/chain_plugin/chain_plugin.hpp +++ b/plugins/chain_plugin/include/eosio/chain_plugin/chain_plugin.hpp @@ -355,6 +355,13 @@ class read_only { }; chain::signed_block_ptr get_block(const get_block_params& params, const fc::time_point& deadline) const; + // call from app() thread + std::unordered_map> + get_block_serializers( const chain::signed_block_ptr& block, const fc::microseconds& max_time ) const; + // call from any thread + fc::variant convert_block( const chain::signed_block_ptr& block, + std::unordered_map> abi_cache, + const fc::microseconds& max_time ) const; struct get_block_info_params { uint32_t block_num = 0; diff --git a/tests/chain_plugin_tests.cpp b/tests/chain_plugin_tests.cpp index 8d103ade3c..b6e7a8eced 100644 --- a/tests/chain_plugin_tests.cpp +++ b/tests/chain_plugin_tests.cpp @@ -93,7 +93,9 @@ BOOST_FIXTURE_TEST_CASE( get_block_with_invalid_abi, TESTER ) try { chain_apis::read_only plugin(*(this->control), {}, fc::microseconds::maximum(), fc::microseconds::maximum(), {}, {}); // block should be decoded successfully - std::string block_str = json::to_pretty_string(plugin.get_block(param, fc::time_point::maximum())); + auto block = plugin.get_block(param, fc::time_point::maximum()); + auto abi_cache = plugin.get_block_serializers(block, fc::microseconds::maximum()); + std::string block_str = json::to_pretty_string(plugin.convert_block(block, abi_cache, fc::microseconds::maximum())); BOOST_TEST(block_str.find("procassert") != std::string::npos); BOOST_TEST(block_str.find("condition") != std::string::npos); BOOST_TEST(block_str.find("Should Not Assert!") != std::string::npos); @@ -111,7 +113,9 @@ BOOST_FIXTURE_TEST_CASE( get_block_with_invalid_abi, TESTER ) try { BOOST_CHECK_THROW(resolver("asserter"_n), invalid_type_inside_abi); // get the same block as string, results in decode failed(invalid abi) but not exception - std::string block_str2 = json::to_pretty_string(plugin.get_block(param, fc::time_point::maximum())); + auto block2 = plugin.get_block(param, fc::time_point::maximum()); + auto abi_cache2 = plugin.get_block_serializers(block2, fc::microseconds::maximum()); + std::string block_str2 = json::to_pretty_string(plugin.convert_block(block2, abi_cache2, fc::microseconds::maximum())); BOOST_TEST(block_str2.find("procassert") != std::string::npos); BOOST_TEST(block_str2.find("condition") == std::string::npos); // decode failed BOOST_TEST(block_str2.find("Should Not Assert!") == std::string::npos); // decode failed From 62903c04ab510784e255e4e38d67fd7cb5e27ba6 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Tue, 7 Feb 2023 11:58:41 -0600 Subject: [PATCH 03/10] GH-677 Use std::move for abi into abi_serializer to avoid string copies. --- libraries/chain/abi_serializer.cpp | 64 +++++++++++-------- .../include/eosio/chain/abi_serializer.hpp | 8 +-- .../chain/include/eosio/chain/controller.hpp | 2 +- .../testing/include/eosio/testing/tester.hpp | 2 +- libraries/testing/tester.cpp | 2 +- plugins/chain_plugin/chain_plugin.cpp | 47 +++++++------- .../eosio/chain_plugin/chain_plugin.hpp | 8 +-- plugins/trace_api_plugin/abi_data_handler.cpp | 4 +- .../eosio/trace_api/abi_data_handler.hpp | 2 +- .../test/test_data_handlers.cpp | 10 +-- plugins/trace_api_plugin/trace_api_plugin.cpp | 2 +- tests/chain_plugin_tests.cpp | 2 +- tests/test_chain_plugin.cpp | 4 +- unittests/abi_tests.cpp | 30 +++++---- unittests/bootseq_tests.cpp | 4 +- unittests/delay_tests.cpp | 4 +- unittests/eosio.token_tests.cpp | 2 +- unittests/eosio_system_tester.hpp | 6 +- unittests/wasm_tests.cpp | 6 +- 19 files changed, 112 insertions(+), 97 deletions(-) diff --git a/libraries/chain/abi_serializer.cpp b/libraries/chain/abi_serializer.cpp index 45e84e1b16..b99a045dd8 100644 --- a/libraries/chain/abi_serializer.cpp +++ b/libraries/chain/abi_serializer.cpp @@ -70,14 +70,14 @@ namespace eosio { namespace chain { ); } - abi_serializer::abi_serializer( const abi_def& abi, const yield_function_t& yield ) { + abi_serializer::abi_serializer( abi_def&& abi, const yield_function_t& yield ) { configure_built_in_types(); - set_abi(abi, yield); + set_abi(std::move(abi), yield); } - abi_serializer::abi_serializer( const abi_def& abi, const fc::microseconds& max_serialization_time) { + abi_serializer::abi_serializer( abi_def&& abi, const fc::microseconds& max_serialization_time) { configure_built_in_types(); - set_abi(abi, create_yield_function(max_serialization_time)); + set_abi(std::move(abi), create_yield_function(max_serialization_time)); } void abi_serializer::add_specialized_unpack_pack( const string& name, @@ -128,11 +128,19 @@ namespace eosio { namespace chain { built_in_types.emplace("extended_asset", pack_unpack()); } - void abi_serializer::set_abi(const abi_def& abi, const yield_function_t& yield) { + void abi_serializer::set_abi(abi_def&& abi, const yield_function_t& yield) { impl::abi_traverse_context ctx(yield); EOS_ASSERT(starts_with(abi.version, "eosio::abi/1."), unsupported_abi_version_exception, "ABI has an unsupported version"); + size_t types_size = abi.types.size(); + size_t structs_size = abi.structs.size(); + size_t actions_size = abi.actions.size(); + size_t tables_size = abi.tables.size(); + size_t error_messages_size = abi.error_messages.size(); + size_t variants_size = abi.variants.value.size(); + size_t action_results_size = abi.action_results.value.size(); + typedefs.clear(); structs.clear(); actions.clear(); @@ -141,47 +149,47 @@ namespace eosio { namespace chain { variants.clear(); action_results.clear(); - for( const auto& st : abi.structs ) - structs[st.name] = st; + for( auto&& st : abi.structs ) + structs[st.name] = std::move(st); - for( const auto& td : abi.types ) { + for( auto&& td : abi.types ) { EOS_ASSERT(!_is_type(td.new_type_name, ctx), duplicate_abi_type_def_exception, "type already exists", ("new_type_name",impl::limit_size(td.new_type_name))); - typedefs[td.new_type_name] = td.type; + typedefs[td.new_type_name] = std::move(td.type); } - for( const auto& a : abi.actions ) - actions[a.name] = a.type; + for( auto&& a : abi.actions ) + actions[a.name] = std::move(a.type); - for( const auto& t : abi.tables ) - tables[t.name] = t.type; + for( auto&& t : abi.tables ) + tables[t.name] = std::move(t.type); - for( const auto& e : abi.error_messages ) - error_messages[e.error_code] = e.error_msg; + for( auto&& e : abi.error_messages ) + error_messages[e.error_code] = std::move(e.error_msg); - for( const auto& v : abi.variants.value ) - variants[v.name] = v; + for( auto&& v : abi.variants.value ) + variants[v.name] = std::move(v); - for( const auto& r : abi.action_results.value ) - action_results[r.name] = r.result_type; + for( auto&& r : abi.action_results.value ) + action_results[r.name] = std::move(r.result_type); /** * The ABI vector may contain duplicates which would make it * an invalid ABI */ - EOS_ASSERT( typedefs.size() == abi.types.size(), duplicate_abi_type_def_exception, "duplicate type definition detected" ); - EOS_ASSERT( structs.size() == abi.structs.size(), duplicate_abi_struct_def_exception, "duplicate struct definition detected" ); - EOS_ASSERT( actions.size() == abi.actions.size(), duplicate_abi_action_def_exception, "duplicate action definition detected" ); - EOS_ASSERT( tables.size() == abi.tables.size(), duplicate_abi_table_def_exception, "duplicate table definition detected" ); - EOS_ASSERT( error_messages.size() == abi.error_messages.size(), duplicate_abi_err_msg_def_exception, "duplicate error message definition detected" ); - EOS_ASSERT( variants.size() == abi.variants.value.size(), duplicate_abi_variant_def_exception, "duplicate variant definition detected" ); - EOS_ASSERT( action_results.size() == abi.action_results.value.size(), duplicate_abi_action_results_def_exception, "duplicate action results definition detected" ); + EOS_ASSERT( typedefs.size() == types_size, duplicate_abi_type_def_exception, "duplicate type definition detected" ); + EOS_ASSERT( structs.size() == structs_size, duplicate_abi_struct_def_exception, "duplicate struct definition detected" ); + EOS_ASSERT( actions.size() == actions_size, duplicate_abi_action_def_exception, "duplicate action definition detected" ); + EOS_ASSERT( tables.size() == tables_size, duplicate_abi_table_def_exception, "duplicate table definition detected" ); + EOS_ASSERT( error_messages.size() == error_messages_size, duplicate_abi_err_msg_def_exception, "duplicate error message definition detected" ); + EOS_ASSERT( variants.size() == variants_size, duplicate_abi_variant_def_exception, "duplicate variant definition detected" ); + EOS_ASSERT( action_results.size() == action_results_size, duplicate_abi_action_results_def_exception, "duplicate action results definition detected" ); validate(ctx); } - void abi_serializer::set_abi(const abi_def& abi, const fc::microseconds& max_serialization_time) { - return set_abi(abi, create_yield_function(max_serialization_time)); + void abi_serializer::set_abi(abi_def&& abi, const fc::microseconds& max_serialization_time) { + return set_abi(std::move(abi), create_yield_function(max_serialization_time)); } bool abi_serializer::is_builtin_type(const std::string_view& type)const { diff --git a/libraries/chain/include/eosio/chain/abi_serializer.hpp b/libraries/chain/include/eosio/chain/abi_serializer.hpp index 10e876a7ff..f67b9a400f 100644 --- a/libraries/chain/include/eosio/chain/abi_serializer.hpp +++ b/libraries/chain/include/eosio/chain/abi_serializer.hpp @@ -35,12 +35,12 @@ struct abi_serializer { using yield_function_t = fc::optional_delegate; abi_serializer(){ configure_built_in_types(); } - abi_serializer( const abi_def& abi, const yield_function_t& yield ); + abi_serializer( abi_def&& abi, const yield_function_t& yield ); [[deprecated("use the overload with yield_function_t[=create_yield_function(max_serialization_time)]")]] - abi_serializer( const abi_def& abi, const fc::microseconds& max_serialization_time ); - void set_abi( const abi_def& abi, const yield_function_t& yield ); + abi_serializer( abi_def&& abi, const fc::microseconds& max_serialization_time ); + void set_abi( abi_def&& abi, const yield_function_t& yield ); [[deprecated("use the overload with yield_function_t[=create_yield_function(max_serialization_time)]")]] - void set_abi(const abi_def& abi, const fc::microseconds& max_serialization_time); + void set_abi( abi_def&& abi, const fc::microseconds& max_serialization_time); /// @return string_view of `t` or internal string type std::string_view resolve_type(const std::string_view& t)const; diff --git a/libraries/chain/include/eosio/chain/controller.hpp b/libraries/chain/include/eosio/chain/controller.hpp index 016565ef16..25d6d8b15a 100644 --- a/libraries/chain/include/eosio/chain/controller.hpp +++ b/libraries/chain/include/eosio/chain/controller.hpp @@ -350,7 +350,7 @@ namespace eosio { namespace chain { const auto& a = get_account( n ); abi_def abi; if( abi_serializer::to_abi( a.abi, abi )) - return abi_serializer( abi, yield ); + return abi_serializer( std::move(abi), yield ); } FC_CAPTURE_AND_LOG((n)) } return std::optional(); diff --git a/libraries/testing/include/eosio/testing/tester.hpp b/libraries/testing/include/eosio/testing/tester.hpp index a809ee6b5d..f8deb47b4e 100644 --- a/libraries/testing/include/eosio/testing/tester.hpp +++ b/libraries/testing/include/eosio/testing/tester.hpp @@ -343,7 +343,7 @@ namespace eosio { namespace testing { const auto& accnt = control->db().get( name ); abi_def abi; if( abi_serializer::to_abi( accnt.abi, abi )) { - return abi_serializer( abi, abi_serializer::create_yield_function( abi_serializer_max_time ) ); + return abi_serializer( std::move(abi), abi_serializer::create_yield_function( abi_serializer_max_time ) ); } return std::optional(); } FC_RETHROW_EXCEPTIONS( error, "Failed to find or parse ABI for ${name}", ("name", name)) diff --git a/libraries/testing/tester.cpp b/libraries/testing/tester.cpp index 31612835b3..b0e23ab5d8 100644 --- a/libraries/testing/tester.cpp +++ b/libraries/testing/tester.cpp @@ -682,7 +682,7 @@ namespace eosio { namespace testing { const variant_object& data )const { try { const auto& acnt = control->get_account(code); auto abi = acnt.get_abi(); - chain::abi_serializer abis(abi, abi_serializer::create_yield_function( abi_serializer_max_time )); + chain::abi_serializer abis(std::move(abi), abi_serializer::create_yield_function( abi_serializer_max_time )); string action_type_name = abis.get_action_type(acttype); FC_ASSERT( action_type_name != string(), "unknown action type ${a}", ("a",acttype) ); diff --git a/plugins/chain_plugin/chain_plugin.cpp b/plugins/chain_plugin/chain_plugin.cpp index bdcb642649..b127f458ae 100644 --- a/plugins/chain_plugin/chain_plugin.cpp +++ b/plugins/chain_plugin/chain_plugin.cpp @@ -1507,39 +1507,39 @@ string get_table_type( const abi_def& abi, const name& table_name ) { } read_only::get_table_rows_result read_only::get_table_rows( const read_only::get_table_rows_params& p, const fc::time_point& deadline )const { - const abi_def abi = eosio::chain_apis::get_abi( db, p.code ); + abi_def abi = eosio::chain_apis::get_abi( db, p.code ); bool primary = false; auto table_with_index = get_table_index_name( p, primary ); if( primary ) { EOS_ASSERT( p.table == table_with_index, chain::contract_table_query_exception, "Invalid table name ${t}", ( "t", p.table )); auto table_type = get_table_type( abi, p.table ); if( table_type == KEYi64 || p.key_type == "i64" || p.key_type == "name" ) { - return get_table_rows_ex(p,abi,deadline); + return get_table_rows_ex(p,std::move(abi),deadline); } EOS_ASSERT( false, chain::contract_table_query_exception, "Invalid table type ${type}", ("type",table_type)("abi",abi)); } else { EOS_ASSERT( !p.key_type.empty(), chain::contract_table_query_exception, "key type required for non-primary index" ); if (p.key_type == chain_apis::i64 || p.key_type == "name") { - return get_table_rows_by_seckey(p, abi, deadline, [](uint64_t v)->uint64_t { + return get_table_rows_by_seckey(p, std::move(abi), deadline, [](uint64_t v)->uint64_t { return v; }); } else if (p.key_type == chain_apis::i128) { - return get_table_rows_by_seckey(p, abi, deadline, [](uint128_t v)->uint128_t { + return get_table_rows_by_seckey(p, std::move(abi), deadline, [](uint128_t v)->uint128_t { return v; }); } else if (p.key_type == chain_apis::i256) { if ( p.encode_type == chain_apis::hex) { using conv = keytype_converter; - return get_table_rows_by_seckey(p, abi, deadline, conv::function()); + return get_table_rows_by_seckey(p, std::move(abi), deadline, conv::function()); } using conv = keytype_converter; - return get_table_rows_by_seckey(p, abi, deadline, conv::function()); + return get_table_rows_by_seckey(p, std::move(abi), deadline, conv::function()); } else if (p.key_type == chain_apis::float64) { - return get_table_rows_by_seckey(p, abi, deadline, [](double v)->float64_t { + return get_table_rows_by_seckey(p, std::move(abi), deadline, [](double v)->float64_t { float64_t f; double_to_float64(v, f); return f; @@ -1547,13 +1547,13 @@ read_only::get_table_rows_result read_only::get_table_rows( const read_only::get } else if (p.key_type == chain_apis::float128) { if ( p.encode_type == chain_apis::hex) { - return get_table_rows_by_seckey(p, abi, deadline, [](uint128_t v)->float128_t{ + return get_table_rows_by_seckey(p, std::move(abi), deadline, [](uint128_t v)->float128_t{ float128_t f; uint128_to_float128(v, f); return f; }); } - return get_table_rows_by_seckey(p, abi, deadline, [](double v)->float128_t{ + return get_table_rows_by_seckey(p, std::move(abi), deadline, [](double v)->float128_t{ float64_t f; double_to_float64(v, f); float128_t f128; @@ -1563,11 +1563,11 @@ read_only::get_table_rows_result read_only::get_table_rows( const read_only::get } else if (p.key_type == chain_apis::sha256) { using conv = keytype_converter; - return get_table_rows_by_seckey(p, abi, deadline, conv::function()); + return get_table_rows_by_seckey(p, std::move(abi), deadline, conv::function()); } else if(p.key_type == chain_apis::ripemd160) { using conv = keytype_converter; - return get_table_rows_by_seckey(p, abi, deadline, conv::function()); + return get_table_rows_by_seckey(p, std::move(abi), deadline, conv::function()); } EOS_ASSERT(false, chain::contract_table_query_exception, "Unsupported secondary index type: ${t}", ("t", p.key_type)); } @@ -1695,9 +1695,9 @@ fc::variant get_global_row( const database& db, const abi_def& abi, const abi_se read_only::get_producers_result read_only::get_producers( const read_only::get_producers_params& params, const fc::time_point& deadline ) const try { - const abi_def abi = eosio::chain_apis::get_abi(db, config::system_account_name); + abi_def abi = eosio::chain_apis::get_abi(db, config::system_account_name); const auto table_type = get_table_type(abi, "producers"_n); - const abi_serializer abis{ abi, abi_serializer::create_yield_function( abi_serializer_max_time ) }; + const abi_serializer abis{ std::move(abi), abi_serializer::create_yield_function( abi_serializer_max_time ) }; EOS_ASSERT(table_type == KEYi64, chain::contract_table_query_exception, "Invalid table type ${type} for table producers", ("type",table_type)); const auto& d = db.db(); @@ -1790,7 +1790,7 @@ struct resolver_factory { if (accnt != nullptr) { abi_def abi; if (abi_serializer::to_abi(accnt->abi, abi)) { - return abi_serializer(abi, yield); + return abi_serializer(std::move(abi), yield); } } return std::optional(); @@ -2399,7 +2399,7 @@ read_only::get_account_results read_only::get_account( const get_account_params& abi_def abi; if( abi_serializer::to_abi(code_account.abi, abi) ) { - abi_serializer abis( abi, abi_serializer::create_yield_function( abi_serializer_max_time ) ); + abi_serializer abis( std::move(abi), abi_serializer::create_yield_function( abi_serializer_max_time ) ); const auto token_code = "eosio.token"_n; @@ -2481,11 +2481,14 @@ read_only::get_account_results read_only::get_account( const get_account_params& return result; } -static fc::variant action_abi_to_variant( const abi_def& abi, type_name action_type ) { +static fc::variant action_abi_to_variant( const account_object* code_account, type_name action_type ) { fc::variant v; - auto it = std::find_if(abi.structs.begin(), abi.structs.end(), [&](auto& x){return x.name == action_type;}); - if( it != abi.structs.end() ) - to_variant( it->fields, v ); + abi_def abi; + if( abi_serializer::to_abi(code_account->abi, abi) ) { + auto it = std::find_if(abi.structs.begin(), abi.structs.end(), [&](auto& x){return x.name == action_type;}); + if( it != abi.structs.end() ) + to_variant( it->fields, v ); + } return v; }; @@ -2496,14 +2499,14 @@ read_only::abi_json_to_bin_result read_only::abi_json_to_bin( const read_only::a abi_def abi; if( abi_serializer::to_abi(code_account->abi, abi) ) { - abi_serializer abis( abi, abi_serializer::create_yield_function( abi_serializer_max_time ) ); + abi_serializer abis( std::move(abi), abi_serializer::create_yield_function( abi_serializer_max_time ) ); auto action_type = abis.get_action_type(params.action); EOS_ASSERT(!action_type.empty(), action_validate_exception, "Unknown action ${action} in contract ${contract}", ("action", params.action)("contract", params.code)); try { result.binargs = abis.variant_to_binary( action_type, params.args, abi_serializer::create_yield_function( abi_serializer_max_time ), shorten_abi_errors ); } EOS_RETHROW_EXCEPTIONS(chain::invalid_action_args_exception, "'${args}' is invalid args for action '${action}' code '${code}'. expected '${proto}'", - ("args", params.args)("action", params.action)("code", params.code)("proto", action_abi_to_variant(abi, action_type))) + ("args", params.args)("action", params.action)("code", params.code)("proto", action_abi_to_variant(code_account, action_type))) } else { EOS_ASSERT(false, abi_not_found_exception, "No ABI found for ${contract}", ("contract", params.code)); } @@ -2516,7 +2519,7 @@ read_only::abi_bin_to_json_result read_only::abi_bin_to_json( const read_only::a const auto& code_account = db.db().get( params.code ); abi_def abi; if( abi_serializer::to_abi(code_account.abi, abi) ) { - abi_serializer abis( abi, abi_serializer::create_yield_function( abi_serializer_max_time ) ); + abi_serializer abis( std::move(abi), abi_serializer::create_yield_function( abi_serializer_max_time ) ); result.args = abis.binary_to_variant( abis.get_action_type( params.action ), params.binargs, abi_serializer::create_yield_function( abi_serializer_max_time ), shorten_abi_errors ); } else { EOS_ASSERT(false, abi_not_found_exception, "No ABI found for ${contract}", ("contract", params.code)); diff --git a/plugins/chain_plugin/include/eosio/chain_plugin/chain_plugin.hpp b/plugins/chain_plugin/include/eosio/chain_plugin/chain_plugin.hpp index f8b6f1b1be..ce5064829e 100644 --- a/plugins/chain_plugin/include/eosio/chain_plugin/chain_plugin.hpp +++ b/plugins/chain_plugin/include/eosio/chain_plugin/chain_plugin.hpp @@ -523,7 +523,7 @@ class read_only { template read_only::get_table_rows_result get_table_rows_by_seckey( const read_only::get_table_rows_params& p, - const abi_def& abi, + abi_def&& abi, const fc::time_point& deadline, ConvFn conv )const { @@ -536,7 +536,7 @@ class read_only { name scope{ convert_to_type(p.scope, "scope") }; abi_serializer abis; - abis.set_abi(abi, abi_serializer::create_yield_function( abi_serializer_max_time ) ); + abis.set_abi(std::move(abi), abi_serializer::create_yield_function( abi_serializer_max_time ) ); bool primary = false; const uint64_t table_with_index = get_table_index_name(p, primary); const auto* t_id = d.find(boost::make_tuple(p.code, scope, p.table)); @@ -627,7 +627,7 @@ class read_only { template read_only::get_table_rows_result get_table_rows_ex( const read_only::get_table_rows_params& p, - const abi_def& abi, + abi_def&& abi, const fc::time_point& deadline )const { fc::microseconds params_time_limit = p.time_limit_ms ? fc::milliseconds(*p.time_limit_ms) : fc::milliseconds(10); @@ -639,7 +639,7 @@ class read_only { uint64_t scope = convert_to_type(p.scope, "scope"); abi_serializer abis; - abis.set_abi(abi, abi_serializer::create_yield_function( abi_serializer_max_time )); + abis.set_abi(std::move(abi), abi_serializer::create_yield_function( abi_serializer_max_time )); const auto* t_id = d.find(boost::make_tuple(p.code, name(scope), p.table)); if( t_id != nullptr ) { const auto& idx = d.get_index(); diff --git a/plugins/trace_api_plugin/abi_data_handler.cpp b/plugins/trace_api_plugin/abi_data_handler.cpp index 520222d3f4..b0c8b46c49 100644 --- a/plugins/trace_api_plugin/abi_data_handler.cpp +++ b/plugins/trace_api_plugin/abi_data_handler.cpp @@ -3,10 +3,10 @@ namespace eosio::trace_api { - void abi_data_handler::add_abi( const chain::name& name, const chain::abi_def& abi ) { + void abi_data_handler::add_abi( const chain::name& name, chain::abi_def&& abi ) { // currently abis are operator provided so no need to protect against abuse abi_serializer_by_account.emplace(name, - std::make_shared(abi, chain::abi_serializer::create_yield_function(fc::microseconds::maximum()))); + std::make_shared(std::move(abi), chain::abi_serializer::create_yield_function(fc::microseconds::maximum()))); } std::tuple> abi_data_handler::serialize_to_variant(const std::variant & action, const yield_function& yield ) { diff --git a/plugins/trace_api_plugin/include/eosio/trace_api/abi_data_handler.hpp b/plugins/trace_api_plugin/include/eosio/trace_api/abi_data_handler.hpp index 16660e4684..7ff5729786 100644 --- a/plugins/trace_api_plugin/include/eosio/trace_api/abi_data_handler.hpp +++ b/plugins/trace_api_plugin/include/eosio/trace_api/abi_data_handler.hpp @@ -28,7 +28,7 @@ namespace eosio { * @param name - the name of the account/contract that this ABI belongs to * @param abi - the ABI definition of that ABI */ - void add_abi( const chain::name& name, const chain::abi_def& abi ); + void add_abi( const chain::name& name, chain::abi_def&& abi ); /** * Given an action trace, produce a tuple representing the `data` and `return_value` fields in the trace diff --git a/plugins/trace_api_plugin/test/test_data_handlers.cpp b/plugins/trace_api_plugin/test/test_data_handlers.cpp index 6ae4501b08..c869f8156d 100644 --- a/plugins/trace_api_plugin/test/test_data_handlers.cpp +++ b/plugins/trace_api_plugin/test/test_data_handlers.cpp @@ -88,7 +88,7 @@ BOOST_AUTO_TEST_SUITE(abi_data_handler_tests) abi.version = "eosio::abi/1."; abi_data_handler handler(exception_handler{}); - handler.add_abi("alice"_n, abi); + handler.add_abi("alice"_n, std::move(abi)); fc::variant expected = fc::mutable_variant_object() ("a", 0) @@ -122,7 +122,7 @@ BOOST_AUTO_TEST_SUITE(abi_data_handler_tests) abi.version = "eosio::abi/1."; abi_data_handler handler(exception_handler{}); - handler.add_abi("alice"_n, abi); + handler.add_abi("alice"_n, std::move(abi)); fc::variant expected = fc::mutable_variant_object() ("a", 0) @@ -155,7 +155,7 @@ BOOST_AUTO_TEST_SUITE(abi_data_handler_tests) abi.version = "eosio::abi/1."; abi_data_handler handler(exception_handler{}); - handler.add_abi("alice"_n, abi); + handler.add_abi("alice"_n, std::move(abi)); auto expected = fc::variant(); @@ -184,7 +184,7 @@ BOOST_AUTO_TEST_SUITE(abi_data_handler_tests) abi.version = "eosio::abi/1."; abi_data_handler handler(exception_handler{}); - handler.add_abi("alice"_n, abi); + handler.add_abi("alice"_n, std::move(abi)); auto expected = fc::variant(); @@ -214,7 +214,7 @@ BOOST_AUTO_TEST_SUITE(abi_data_handler_tests) bool log_called = false; abi_data_handler handler([&log_called](const exception_with_context& ){log_called = true;}); - handler.add_abi("alice"_n, abi); + handler.add_abi("alice"_n, std::move(abi)); auto expected = fc::variant(); diff --git a/plugins/trace_api_plugin/trace_api_plugin.cpp b/plugins/trace_api_plugin/trace_api_plugin.cpp index adb9534f2b..9a29bac70a 100644 --- a/plugins/trace_api_plugin/trace_api_plugin.cpp +++ b/plugins/trace_api_plugin/trace_api_plugin.cpp @@ -213,7 +213,7 @@ struct trace_api_rpc_plugin_impl : public std::enable_shared_from_thisadd_abi(account, abi); + data_handler->add_abi(account, std::move(abi)); } catch (...) { elog("Malformed trace-rpc-abi provider: \"${val}\"", ("val", entry)); throw; diff --git a/tests/chain_plugin_tests.cpp b/tests/chain_plugin_tests.cpp index b6e7a8eced..e518f0eb39 100644 --- a/tests/chain_plugin_tests.cpp +++ b/tests/chain_plugin_tests.cpp @@ -51,7 +51,7 @@ BOOST_FIXTURE_TEST_CASE( get_block_with_invalid_abi, TESTER ) try { const auto& accnt = this->control->db().get( name ); abi_def abi; if (abi_serializer::to_abi(accnt.abi, abi)) { - return abi_serializer(abi, abi_serializer::create_yield_function( abi_serializer_max_time )); + return abi_serializer(std::move(abi), abi_serializer::create_yield_function( abi_serializer_max_time )); } return std::optional(); } FC_RETHROW_EXCEPTIONS(error, "resolver failed at chain_plugin_tests::abi_invalid_type"); diff --git a/tests/test_chain_plugin.cpp b/tests/test_chain_plugin.cpp index b20c494cc0..371d05f26c 100644 --- a/tests/test_chain_plugin.cpp +++ b/tests/test_chain_plugin.cpp @@ -205,7 +205,7 @@ class chain_plugin_tester : public TESTER { const auto& accnt = control->db().get( "eosio.token"_n ); abi_def abi; BOOST_CHECK_EQUAL(abi_serializer::to_abi(accnt.abi, abi), true); - token_abi_ser.set_abi(abi, abi_serializer::create_yield_function( abi_serializer_max_time )); + token_abi_ser.set_abi(std::move(abi), abi_serializer::create_yield_function( abi_serializer_max_time )); } create_currency( "eosio.token"_n, config::system_account_name, core_from_string("10000000000.0000") ); @@ -224,7 +224,7 @@ class chain_plugin_tester : public TESTER { const auto& accnt = control->db().get( config::system_account_name ); abi_def abi; BOOST_CHECK_EQUAL(abi_serializer::to_abi(accnt.abi, abi), true); - abi_ser.set_abi(abi, abi_serializer::create_yield_function( abi_serializer_max_time )); + abi_ser.set_abi(std::move(abi), abi_serializer::create_yield_function( abi_serializer_max_time )); } } diff --git a/unittests/abi_tests.cpp b/unittests/abi_tests.cpp index 6ad37bd994..64de2887fe 100644 --- a/unittests/abi_tests.cpp +++ b/unittests/abi_tests.cpp @@ -542,7 +542,7 @@ BOOST_AUTO_TEST_CASE(uint_types) auto var = fc::json::from_string(test_data); - verify_byte_round_trip_conversion(abi_serializer{abi, abi_serializer::create_yield_function( max_serialization_time )}, "transfer", var); + verify_byte_round_trip_conversion(abi_serializer{std::move(abi), abi_serializer::create_yield_function( max_serialization_time )}, "transfer", var); } FC_LOG_AND_RETHROW() } @@ -551,8 +551,9 @@ BOOST_AUTO_TEST_CASE(general) { try { auto abi = eosio_contract_abi(fc::json::from_string(my_abi).as()); + auto abi2 = abi; - abi_serializer abis(abi, abi_serializer::create_yield_function( max_serialization_time )); + abi_serializer abis(std::move(abi), abi_serializer::create_yield_function( max_serialization_time )); const char *my_other = R"=====( { @@ -751,7 +752,7 @@ BOOST_AUTO_TEST_CASE(general) )====="; auto var = fc::json::from_string(my_other); - verify_byte_round_trip_conversion(abi_serializer{abi, abi_serializer::create_yield_function( max_serialization_time )}, "A", var); + verify_byte_round_trip_conversion(abi_serializer{std::move(abi2), abi_serializer::create_yield_function( max_serialization_time )}, "A", var); } FC_LOG_AND_RETHROW() } @@ -802,11 +803,11 @@ BOOST_AUTO_TEST_CASE(abi_cycle) auto is_assert_exception = [](const auto& e) -> bool { wlog(e.to_string()); return true; }; - BOOST_CHECK_EXCEPTION( abi_serializer abis(abi, abi_serializer::create_yield_function( max_serialization_time )), duplicate_abi_type_def_exception, is_assert_exception); + BOOST_CHECK_EXCEPTION( abi_serializer abis(std::move(abi), abi_serializer::create_yield_function( max_serialization_time )), duplicate_abi_type_def_exception, is_assert_exception); abi = fc::json::from_string(struct_cycle_abi).as(); abi_serializer abis; - BOOST_CHECK_EXCEPTION( abis.set_abi(abi, abi_serializer::create_yield_function( max_serialization_time )), abi_circular_def_exception, is_assert_exception ); + BOOST_CHECK_EXCEPTION( abis.set_abi(std::move(abi), abi_serializer::create_yield_function( max_serialization_time )), abi_circular_def_exception, is_assert_exception ); } FC_LOG_AND_RETHROW() } @@ -1649,7 +1650,7 @@ BOOST_AUTO_TEST_CASE(abi_type_repeat) auto abi = eosio_contract_abi(fc::json::from_string(repeat_abi).as()); auto is_table_exception = [](fc::exception const & e) -> bool { return e.to_detail_string().find("type already exists") != std::string::npos; }; - BOOST_CHECK_EXCEPTION( abi_serializer abis(abi, abi_serializer::create_yield_function( max_serialization_time )), duplicate_abi_type_def_exception, is_table_exception ); + BOOST_CHECK_EXCEPTION( abi_serializer abis(std::move(abi), abi_serializer::create_yield_function( max_serialization_time )), duplicate_abi_type_def_exception, is_table_exception ); } FC_LOG_AND_RETHROW() } BOOST_AUTO_TEST_CASE(abi_struct_repeat) @@ -1706,7 +1707,7 @@ BOOST_AUTO_TEST_CASE(abi_struct_repeat) )====="; auto abi = eosio_contract_abi(fc::json::from_string(repeat_abi).as()); - BOOST_CHECK_THROW( abi_serializer abis(abi, abi_serializer::create_yield_function( max_serialization_time )), duplicate_abi_struct_def_exception ); + BOOST_CHECK_THROW( abi_serializer abis(std::move(abi), abi_serializer::create_yield_function( max_serialization_time )), duplicate_abi_struct_def_exception ); } FC_LOG_AND_RETHROW() } BOOST_AUTO_TEST_CASE(abi_action_repeat) @@ -1766,7 +1767,7 @@ BOOST_AUTO_TEST_CASE(abi_action_repeat) )====="; auto abi = eosio_contract_abi(fc::json::from_string(repeat_abi).as()); - BOOST_CHECK_THROW( abi_serializer abis(abi, abi_serializer::create_yield_function( max_serialization_time )), duplicate_abi_action_def_exception ); + BOOST_CHECK_THROW( abi_serializer abis(std::move(abi), abi_serializer::create_yield_function( max_serialization_time )), duplicate_abi_action_def_exception ); } FC_LOG_AND_RETHROW() } BOOST_AUTO_TEST_CASE(abi_table_repeat) @@ -1829,7 +1830,7 @@ BOOST_AUTO_TEST_CASE(abi_table_repeat) )====="; auto abi = eosio_contract_abi(fc::json::from_string(repeat_abi).as()); - BOOST_CHECK_THROW( abi_serializer abis(abi, abi_serializer::create_yield_function( max_serialization_time )), duplicate_abi_table_def_exception ); + BOOST_CHECK_THROW( abi_serializer abis(std::move(abi), abi_serializer::create_yield_function( max_serialization_time )), duplicate_abi_table_def_exception ); } FC_LOG_AND_RETHROW() } BOOST_AUTO_TEST_CASE(abi_type_def) @@ -2056,7 +2057,7 @@ BOOST_AUTO_TEST_CASE(abi_account_name_in_eosio_abi) auto abi = eosio_contract_abi(fc::json::from_string(repeat_abi).as()); auto is_type_exception = [](fc::assert_exception const & e) -> bool { return e.to_detail_string().find("type already exists") != std::string::npos; }; - BOOST_CHECK_EXCEPTION( abi_serializer abis(abi, abi_serializer::create_yield_function(max_serialization_time )), duplicate_abi_type_def_exception, is_type_exception ); + BOOST_CHECK_EXCEPTION( abi_serializer abis(std::move(abi), abi_serializer::create_yield_function(max_serialization_time )), duplicate_abi_type_def_exception, is_type_exception ); } FC_LOG_AND_RETHROW() } @@ -2971,7 +2972,8 @@ BOOST_AUTO_TEST_CASE(abi_to_variant__add_action__good_return_value) ] })"; auto abidef = fc::json::from_string(abi).as(); - abi_serializer abis(abidef, abi_serializer::create_yield_function(max_serialization_time)); + auto abidef2 = abidef; + abi_serializer abis(std::move(abidef2), abi_serializer::create_yield_function(max_serialization_time)); mutable_variant_object mvo; eosio::chain::impl::abi_traverse_context ctx(abi_serializer::create_yield_function(max_serialization_time)); @@ -2996,7 +2998,8 @@ BOOST_AUTO_TEST_CASE(abi_to_variant__add_action__bad_return_value) ] })"; auto abidef = fc::json::from_string(abi).as(); - abi_serializer abis(abidef, abi_serializer::create_yield_function(max_serialization_time)); + auto abidef2 = abidef; + abi_serializer abis(std::move(abidef2), abi_serializer::create_yield_function(max_serialization_time)); mutable_variant_object mvo; eosio::chain::impl::abi_traverse_context ctx(abi_serializer::create_yield_function(max_serialization_time)); @@ -3031,7 +3034,8 @@ BOOST_AUTO_TEST_CASE(abi_to_variant__add_action__no_return_value) ] })"; auto abidef = fc::json::from_string(abi).as(); - abi_serializer abis(abidef, abi_serializer::create_yield_function(max_serialization_time)); + auto abidef2 = abidef; + abi_serializer abis(std::move(abidef2), abi_serializer::create_yield_function(max_serialization_time)); mutable_variant_object mvo; eosio::chain::impl::abi_traverse_context ctx(abi_serializer::create_yield_function(max_serialization_time)); diff --git a/unittests/bootseq_tests.cpp b/unittests/bootseq_tests.cpp index bd283db072..e06b27dbac 100644 --- a/unittests/bootseq_tests.cpp +++ b/unittests/bootseq_tests.cpp @@ -79,7 +79,7 @@ class bootseq_tester : public TESTER { const auto& accnt = control->db().get( config::system_account_name ); abi_def abi; BOOST_REQUIRE_EQUAL(abi_serializer::to_abi(accnt.abi, abi), true); - abi_ser.set_abi(abi, abi_serializer::create_yield_function( abi_serializer_max_time )); + abi_ser.set_abi(std::move(abi), abi_serializer::create_yield_function( abi_serializer_max_time )); } fc::variant get_global_state() { @@ -175,7 +175,7 @@ class bootseq_tester : public TESTER { const auto& accnt = control->db().get( account ); abi_def abi_definition; BOOST_REQUIRE_EQUAL(abi_serializer::to_abi(accnt.abi, abi_definition), true); - abi_ser.set_abi(abi_definition, abi_serializer::create_yield_function( abi_serializer_max_time )); + abi_ser.set_abi(std::move(abi_definition), abi_serializer::create_yield_function( abi_serializer_max_time )); } produce_blocks(); } diff --git a/unittests/delay_tests.cpp b/unittests/delay_tests.cpp index e30349b027..a6b86799b0 100644 --- a/unittests/delay_tests.cpp +++ b/unittests/delay_tests.cpp @@ -1684,8 +1684,8 @@ BOOST_AUTO_TEST_CASE( mindelay_test ) { try { // send transfer with delay_sec set to 10 const auto& acnt = chain.control->db().get("eosio.token"_n); - const auto abi = acnt.get_abi(); - chain::abi_serializer abis(abi, abi_serializer::create_yield_function( chain.abi_serializer_max_time )); + auto abi = acnt.get_abi(); + chain::abi_serializer abis(std::move(abi), abi_serializer::create_yield_function( chain.abi_serializer_max_time )); const auto a = chain.control->db().get("eosio.token"_n).get_abi(); string action_type_name = abis.get_action_type(name("transfer")); diff --git a/unittests/eosio.token_tests.cpp b/unittests/eosio.token_tests.cpp index f3272fca99..f23a13e407 100644 --- a/unittests/eosio.token_tests.cpp +++ b/unittests/eosio.token_tests.cpp @@ -36,7 +36,7 @@ class eosio_token_tester : public tester { const auto& accnt = control->db().get( "eosio.token"_n ); abi_def abi; BOOST_REQUIRE_EQUAL(abi_serializer::to_abi(accnt.abi, abi), true); - abi_ser.set_abi(abi, abi_serializer::create_yield_function( abi_serializer_max_time )); + abi_ser.set_abi(std::move(abi), abi_serializer::create_yield_function( abi_serializer_max_time )); } action_result push_action( const account_name& signer, const action_name &name, const variant_object &data ) { diff --git a/unittests/eosio_system_tester.hpp b/unittests/eosio_system_tester.hpp index 72b3abcd1b..bca9be4938 100644 --- a/unittests/eosio_system_tester.hpp +++ b/unittests/eosio_system_tester.hpp @@ -48,7 +48,7 @@ class eosio_system_tester : public TESTER { const auto& accnt = control->db().get( "eosio.token"_n ); abi_def abi; BOOST_REQUIRE_EQUAL(abi_serializer::to_abi(accnt.abi, abi), true); - token_abi_ser.set_abi(abi, abi_serializer::create_yield_function( abi_serializer_max_time )); + token_abi_ser.set_abi(std::move(abi), abi_serializer::create_yield_function( abi_serializer_max_time )); } create_currency( "eosio.token"_n, config::system_account_name, core_from_string("10000000000.0000") ); @@ -67,7 +67,7 @@ class eosio_system_tester : public TESTER { const auto& accnt = control->db().get( config::system_account_name ); abi_def abi; BOOST_REQUIRE_EQUAL(abi_serializer::to_abi(accnt.abi, abi), true); - abi_ser.set_abi(abi, abi_serializer::create_yield_function( abi_serializer_max_time )); + abi_ser.set_abi(std::move(abi), abi_serializer::create_yield_function( abi_serializer_max_time )); } produce_blocks(); @@ -429,7 +429,7 @@ class eosio_system_tester : public TESTER { const auto& accnt = control->db().get( "eosio.msig"_n ); abi_def msig_abi; BOOST_REQUIRE_EQUAL(abi_serializer::to_abi(accnt.abi, msig_abi), true); - msig_abi_ser.set_abi(msig_abi, abi_serializer::create_yield_function( abi_serializer_max_time )); + msig_abi_ser.set_abi(std::move(msig_abi), abi_serializer::create_yield_function( abi_serializer_max_time )); } return msig_abi_ser; } diff --git a/unittests/wasm_tests.cpp b/unittests/wasm_tests.cpp index 4a64a11ed5..3b10a9bbe2 100644 --- a/unittests/wasm_tests.cpp +++ b/unittests/wasm_tests.cpp @@ -185,7 +185,7 @@ BOOST_FIXTURE_TEST_CASE( abi_from_variant, TESTER ) try { const auto& accnt = this->control->db().get( name ); abi_def abi; if (abi_serializer::to_abi(accnt.abi, abi)) { - return abi_serializer(abi, abi_serializer::create_yield_function( abi_serializer_max_time )); + return abi_serializer(std::move(abi), abi_serializer::create_yield_function( abi_serializer_max_time )); } return std::optional(); } FC_RETHROW_EXCEPTIONS(error, "Failed to find or parse ABI for ${name}", ("name", name)) @@ -1049,7 +1049,7 @@ BOOST_FIXTURE_TEST_CASE(noop, TESTER) try { const auto& accnt = control->db().get("noop"_n); abi_def abi; BOOST_REQUIRE_EQUAL(abi_serializer::to_abi(accnt.abi, abi), true); - abi_serializer abi_ser(abi, abi_serializer::create_yield_function( abi_serializer_max_time )); + abi_serializer abi_ser(std::move(abi), abi_serializer::create_yield_function( abi_serializer_max_time )); { produce_blocks(5); @@ -1112,7 +1112,7 @@ BOOST_FIXTURE_TEST_CASE(eosio_abi, TESTER) try { const auto& accnt = control->db().get(config::system_account_name); abi_def abi; BOOST_REQUIRE_EQUAL(abi_serializer::to_abi(accnt.abi, abi), true); - abi_serializer abi_ser(abi, abi_serializer::create_yield_function( abi_serializer_max_time )); + abi_serializer abi_ser(std::move(abi), abi_serializer::create_yield_function( abi_serializer_max_time )); signed_transaction trx; name a = "alice"_n; From eb0d3dc0909c245686710f37032f30cc8b23beb2 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Tue, 7 Feb 2023 13:19:35 -0600 Subject: [PATCH 04/10] GH-677 Minor cleanup --- libraries/chain/abi_serializer.cpp | 26 ++++++------ .../chain/include/eosio/chain/controller.hpp | 3 +- .../testing/include/eosio/testing/tester.hpp | 3 +- plugins/chain_plugin/chain_plugin.cpp | 40 +++++++------------ tests/chain_plugin_tests.cpp | 3 +- unittests/wasm_tests.cpp | 3 +- 6 files changed, 31 insertions(+), 47 deletions(-) diff --git a/libraries/chain/abi_serializer.cpp b/libraries/chain/abi_serializer.cpp index b99a045dd8..2af9c4d3b6 100644 --- a/libraries/chain/abi_serializer.cpp +++ b/libraries/chain/abi_serializer.cpp @@ -149,29 +149,29 @@ namespace eosio { namespace chain { variants.clear(); action_results.clear(); - for( auto&& st : abi.structs ) + for( auto& st : abi.structs ) structs[st.name] = std::move(st); - for( auto&& td : abi.types ) { + for( auto& td : abi.types ) { EOS_ASSERT(!_is_type(td.new_type_name, ctx), duplicate_abi_type_def_exception, "type already exists", ("new_type_name",impl::limit_size(td.new_type_name))); - typedefs[td.new_type_name] = std::move(td.type); + typedefs[std::move(td.new_type_name)] = std::move(td.type); } - for( auto&& a : abi.actions ) - actions[a.name] = std::move(a.type); + for( auto& a : abi.actions ) + actions[std::move(a.name)] = std::move(a.type); - for( auto&& t : abi.tables ) - tables[t.name] = std::move(t.type); + for( auto& t : abi.tables ) + tables[std::move(t.name)] = std::move(t.type); - for( auto&& e : abi.error_messages ) - error_messages[e.error_code] = std::move(e.error_msg); + for( auto& e : abi.error_messages ) + error_messages[std::move(e.error_code)] = std::move(e.error_msg); - for( auto&& v : abi.variants.value ) - variants[v.name] = std::move(v); + for( auto& v : abi.variants.value ) + variants[std::move(v.name)] = std::move(v); - for( auto&& r : abi.action_results.value ) - action_results[r.name] = std::move(r.result_type); + for( auto& r : abi.action_results.value ) + action_results[std::move(r.name)] = std::move(r.result_type); /** * The ABI vector may contain duplicates which would make it diff --git a/libraries/chain/include/eosio/chain/controller.hpp b/libraries/chain/include/eosio/chain/controller.hpp index 25d6d8b15a..103b235132 100644 --- a/libraries/chain/include/eosio/chain/controller.hpp +++ b/libraries/chain/include/eosio/chain/controller.hpp @@ -348,8 +348,7 @@ namespace eosio { namespace chain { if( n.good() ) { try { const auto& a = get_account( n ); - abi_def abi; - if( abi_serializer::to_abi( a.abi, abi )) + if( abi_def abi; abi_serializer::to_abi( a.abi, abi )) return abi_serializer( std::move(abi), yield ); } FC_CAPTURE_AND_LOG((n)) } diff --git a/libraries/testing/include/eosio/testing/tester.hpp b/libraries/testing/include/eosio/testing/tester.hpp index f8deb47b4e..5781a5fc9c 100644 --- a/libraries/testing/include/eosio/testing/tester.hpp +++ b/libraries/testing/include/eosio/testing/tester.hpp @@ -341,8 +341,7 @@ namespace eosio { namespace testing { return [this]( const account_name& name ) -> std::optional { try { const auto& accnt = control->db().get( name ); - abi_def abi; - if( abi_serializer::to_abi( accnt.abi, abi )) { + if( abi_def abi; abi_serializer::to_abi( accnt.abi, abi )) { return abi_serializer( std::move(abi), abi_serializer::create_yield_function( abi_serializer_max_time ) ); } return std::optional(); diff --git a/plugins/chain_plugin/chain_plugin.cpp b/plugins/chain_plugin/chain_plugin.cpp index b127f458ae..66327693e5 100644 --- a/plugins/chain_plugin/chain_plugin.cpp +++ b/plugins/chain_plugin/chain_plugin.cpp @@ -1783,23 +1783,16 @@ read_only::get_producer_schedule_result read_only::get_producer_schedule( const } -struct resolver_factory { - static auto make(const controller& control, abi_serializer::yield_function_t yield) { - return [&control, yield{std::move(yield)}](const account_name &name) -> std::optional { - const auto* accnt = control.db().template find(name); - if (accnt != nullptr) { - abi_def abi; - if (abi_serializer::to_abi(accnt->abi, abi)) { - return abi_serializer(std::move(abi), yield); - } - } - return std::optional(); - }; - } -}; - auto make_resolver(const controller& control, abi_serializer::yield_function_t yield) { - return resolver_factory::make(control, std::move( yield )); + return [&control, yield{std::move(yield)}](const account_name &name) -> std::optional { + const auto* accnt = control.db().template find(name); + if (accnt != nullptr) { + if (abi_def abi; abi_serializer::to_abi(accnt->abi, abi)) { + return abi_serializer(std::move(abi), yield); + } + } + return {}; + }; } read_only::get_scheduled_transactions_result @@ -2235,8 +2228,7 @@ read_only::get_abi_results read_only::get_abi( const get_abi_params& params, con const auto& d = db.db(); const auto& accnt = d.get( params.account_name ); - abi_def abi; - if( abi_serializer::to_abi(accnt.abi, abi) ) { + if( abi_def abi; abi_serializer::to_abi(accnt.abi, abi) ) { result.abi = std::move(abi); } @@ -2258,8 +2250,7 @@ read_only::get_code_results read_only::get_code( const get_code_params& params, result.code_hash = code_obj.code_hash; } - abi_def abi; - if( abi_serializer::to_abi(accnt_obj.abi, abi) ) { + if( abi_def abi; abi_serializer::to_abi(accnt_obj.abi, abi) ) { result.abi = std::move(abi); } @@ -2397,8 +2388,7 @@ read_only::get_account_results read_only::get_account( const get_account_params& const auto& code_account = db.db().get( config::system_account_name ); - abi_def abi; - if( abi_serializer::to_abi(code_account.abi, abi) ) { + if( abi_def abi; abi_serializer::to_abi(code_account.abi, abi) ) { abi_serializer abis( std::move(abi), abi_serializer::create_yield_function( abi_serializer_max_time ) ); const auto token_code = "eosio.token"_n; @@ -2497,8 +2487,7 @@ read_only::abi_json_to_bin_result read_only::abi_json_to_bin( const read_only::a const auto code_account = db.db().find( params.code ); EOS_ASSERT(code_account != nullptr, contract_query_exception, "Contract can't be found ${contract}", ("contract", params.code)); - abi_def abi; - if( abi_serializer::to_abi(code_account->abi, abi) ) { + if( abi_def abi; abi_serializer::to_abi(code_account->abi, abi) ) { abi_serializer abis( std::move(abi), abi_serializer::create_yield_function( abi_serializer_max_time ) ); auto action_type = abis.get_action_type(params.action); EOS_ASSERT(!action_type.empty(), action_validate_exception, "Unknown action ${action} in contract ${contract}", ("action", params.action)("contract", params.code)); @@ -2517,8 +2506,7 @@ read_only::abi_json_to_bin_result read_only::abi_json_to_bin( const read_only::a read_only::abi_bin_to_json_result read_only::abi_bin_to_json( const read_only::abi_bin_to_json_params& params, const fc::time_point& deadline )const { abi_bin_to_json_result result; const auto& code_account = db.db().get( params.code ); - abi_def abi; - if( abi_serializer::to_abi(code_account.abi, abi) ) { + if( abi_def abi; abi_serializer::to_abi(code_account.abi, abi) ) { abi_serializer abis( std::move(abi), abi_serializer::create_yield_function( abi_serializer_max_time ) ); result.args = abis.binary_to_variant( abis.get_action_type( params.action ), params.binargs, abi_serializer::create_yield_function( abi_serializer_max_time ), shorten_abi_errors ); } else { diff --git a/tests/chain_plugin_tests.cpp b/tests/chain_plugin_tests.cpp index e518f0eb39..914c83c1d9 100644 --- a/tests/chain_plugin_tests.cpp +++ b/tests/chain_plugin_tests.cpp @@ -49,8 +49,7 @@ BOOST_FIXTURE_TEST_CASE( get_block_with_invalid_abi, TESTER ) try { auto resolver = [&,this]( const account_name& name ) -> std::optional { try { const auto& accnt = this->control->db().get( name ); - abi_def abi; - if (abi_serializer::to_abi(accnt.abi, abi)) { + if (abi_def abi; abi_serializer::to_abi(accnt.abi, abi)) { return abi_serializer(std::move(abi), abi_serializer::create_yield_function( abi_serializer_max_time )); } return std::optional(); diff --git a/unittests/wasm_tests.cpp b/unittests/wasm_tests.cpp index 3b10a9bbe2..a6abc323f4 100644 --- a/unittests/wasm_tests.cpp +++ b/unittests/wasm_tests.cpp @@ -183,8 +183,7 @@ BOOST_FIXTURE_TEST_CASE( abi_from_variant, TESTER ) try { auto resolver = [&,this]( const account_name& name ) -> std::optional { try { const auto& accnt = this->control->db().get( name ); - abi_def abi; - if (abi_serializer::to_abi(accnt.abi, abi)) { + if (abi_def abi; abi_serializer::to_abi(accnt.abi, abi)) { return abi_serializer(std::move(abi), abi_serializer::create_yield_function( abi_serializer_max_time )); } return std::optional(); From ed2e91d68d92cc872af0934f4e2d98674f4ed0e5 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Tue, 7 Feb 2023 13:23:23 -0600 Subject: [PATCH 05/10] GH-677 Constrain scope --- plugins/chain_plugin/chain_plugin.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/plugins/chain_plugin/chain_plugin.cpp b/plugins/chain_plugin/chain_plugin.cpp index 66327693e5..882d54f54f 100644 --- a/plugins/chain_plugin/chain_plugin.cpp +++ b/plugins/chain_plugin/chain_plugin.cpp @@ -2473,8 +2473,7 @@ read_only::get_account_results read_only::get_account( const get_account_params& static fc::variant action_abi_to_variant( const account_object* code_account, type_name action_type ) { fc::variant v; - abi_def abi; - if( abi_serializer::to_abi(code_account->abi, abi) ) { + if( abi_def abi; abi_serializer::to_abi(code_account->abi, abi) ) { auto it = std::find_if(abi.structs.begin(), abi.structs.end(), [&](auto& x){return x.name == action_type;}); if( it != abi.structs.end() ) to_variant( it->fields, v ); From cd3aaf24c1a1daf090b7624b3c02ec385ac45c9b Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Tue, 7 Feb 2023 15:57:44 -0600 Subject: [PATCH 06/10] GH-677 Fix invalid move --- libraries/chain/abi_serializer.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/chain/abi_serializer.cpp b/libraries/chain/abi_serializer.cpp index 2af9c4d3b6..27473f61c7 100644 --- a/libraries/chain/abi_serializer.cpp +++ b/libraries/chain/abi_serializer.cpp @@ -168,7 +168,7 @@ namespace eosio { namespace chain { error_messages[std::move(e.error_code)] = std::move(e.error_msg); for( auto& v : abi.variants.value ) - variants[std::move(v.name)] = std::move(v); + variants[v.name] = std::move(v); for( auto& r : abi.action_results.value ) action_results[std::move(r.name)] = std::move(r.result_type); From b1af6cf1010b204eba7e18067aeb59d97bc38ec7 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Tue, 7 Feb 2023 15:58:50 -0600 Subject: [PATCH 07/10] GH-677 Use unnamed copy --- unittests/abi_tests.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/unittests/abi_tests.cpp b/unittests/abi_tests.cpp index 64de2887fe..1d662bdd37 100644 --- a/unittests/abi_tests.cpp +++ b/unittests/abi_tests.cpp @@ -551,9 +551,8 @@ BOOST_AUTO_TEST_CASE(general) { try { auto abi = eosio_contract_abi(fc::json::from_string(my_abi).as()); - auto abi2 = abi; - abi_serializer abis(std::move(abi), abi_serializer::create_yield_function( max_serialization_time )); + abi_serializer abis(abi_def(abi), abi_serializer::create_yield_function( max_serialization_time )); const char *my_other = R"=====( { @@ -752,7 +751,7 @@ BOOST_AUTO_TEST_CASE(general) )====="; auto var = fc::json::from_string(my_other); - verify_byte_round_trip_conversion(abi_serializer{std::move(abi2), abi_serializer::create_yield_function( max_serialization_time )}, "A", var); + verify_byte_round_trip_conversion(abi_serializer{std::move(abi), abi_serializer::create_yield_function( max_serialization_time )}, "A", var); } FC_LOG_AND_RETHROW() } From a10d6bc3be02a59f70806bd60ad78c2fa4ea411a Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Tue, 7 Feb 2023 16:22:24 -0600 Subject: [PATCH 08/10] GH-677 Fix some uses of std::move --- libraries/chain/abi_serializer.cpp | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/libraries/chain/abi_serializer.cpp b/libraries/chain/abi_serializer.cpp index 27473f61c7..36ec7a2050 100644 --- a/libraries/chain/abi_serializer.cpp +++ b/libraries/chain/abi_serializer.cpp @@ -149,8 +149,11 @@ namespace eosio { namespace chain { variants.clear(); action_results.clear(); - for( auto& st : abi.structs ) - structs[st.name] = std::move(st); + for( auto& st : abi.structs ) { + // side effect rules indicate std::move can happen before st.name is accessed. + auto n = st.name; + structs[std::move(n)] = std::move(st); + } for( auto& td : abi.types ) { EOS_ASSERT(!_is_type(td.new_type_name, ctx), duplicate_abi_type_def_exception, @@ -167,8 +170,11 @@ namespace eosio { namespace chain { for( auto& e : abi.error_messages ) error_messages[std::move(e.error_code)] = std::move(e.error_msg); - for( auto& v : abi.variants.value ) - variants[v.name] = std::move(v); + for( auto& v : abi.variants.value ) { + // Not strictly necessary since trivially copyable, but error on safe side in case name becomes std::string + auto n = v.name; + variants[std::move(n)] = std::move(v); + } for( auto& r : abi.action_results.value ) action_results[std::move(r.name)] = std::move(r.result_type); From 2eb0b43a65866bec01bed467e0e4fa258cd27150 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Tue, 7 Feb 2023 20:01:36 -0600 Subject: [PATCH 09/10] GH-677 Use unnamed copy --- unittests/abi_tests.cpp | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/unittests/abi_tests.cpp b/unittests/abi_tests.cpp index 1d662bdd37..10399bbe25 100644 --- a/unittests/abi_tests.cpp +++ b/unittests/abi_tests.cpp @@ -2971,8 +2971,7 @@ BOOST_AUTO_TEST_CASE(abi_to_variant__add_action__good_return_value) ] })"; auto abidef = fc::json::from_string(abi).as(); - auto abidef2 = abidef; - abi_serializer abis(std::move(abidef2), abi_serializer::create_yield_function(max_serialization_time)); + abi_serializer abis(abi_def(abidef), abi_serializer::create_yield_function(max_serialization_time)); mutable_variant_object mvo; eosio::chain::impl::abi_traverse_context ctx(abi_serializer::create_yield_function(max_serialization_time)); @@ -2997,8 +2996,7 @@ BOOST_AUTO_TEST_CASE(abi_to_variant__add_action__bad_return_value) ] })"; auto abidef = fc::json::from_string(abi).as(); - auto abidef2 = abidef; - abi_serializer abis(std::move(abidef2), abi_serializer::create_yield_function(max_serialization_time)); + abi_serializer abis(abi_def(abidef), abi_serializer::create_yield_function(max_serialization_time)); mutable_variant_object mvo; eosio::chain::impl::abi_traverse_context ctx(abi_serializer::create_yield_function(max_serialization_time)); @@ -3033,8 +3031,7 @@ BOOST_AUTO_TEST_CASE(abi_to_variant__add_action__no_return_value) ] })"; auto abidef = fc::json::from_string(abi).as(); - auto abidef2 = abidef; - abi_serializer abis(std::move(abidef2), abi_serializer::create_yield_function(max_serialization_time)); + abi_serializer abis(abi_def(abidef), abi_serializer::create_yield_function(max_serialization_time)); mutable_variant_object mvo; eosio::chain::impl::abi_traverse_context ctx(abi_serializer::create_yield_function(max_serialization_time)); From 34baed929b36c13c36db3cb1c55b06d87f785f2d Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Tue, 7 Feb 2023 20:02:17 -0600 Subject: [PATCH 10/10] GH-677 Revert unneeded explicit copy --- libraries/chain/abi_serializer.cpp | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/libraries/chain/abi_serializer.cpp b/libraries/chain/abi_serializer.cpp index 36ec7a2050..27473f61c7 100644 --- a/libraries/chain/abi_serializer.cpp +++ b/libraries/chain/abi_serializer.cpp @@ -149,11 +149,8 @@ namespace eosio { namespace chain { variants.clear(); action_results.clear(); - for( auto& st : abi.structs ) { - // side effect rules indicate std::move can happen before st.name is accessed. - auto n = st.name; - structs[std::move(n)] = std::move(st); - } + for( auto& st : abi.structs ) + structs[st.name] = std::move(st); for( auto& td : abi.types ) { EOS_ASSERT(!_is_type(td.new_type_name, ctx), duplicate_abi_type_def_exception, @@ -170,11 +167,8 @@ namespace eosio { namespace chain { for( auto& e : abi.error_messages ) error_messages[std::move(e.error_code)] = std::move(e.error_msg); - for( auto& v : abi.variants.value ) { - // Not strictly necessary since trivially copyable, but error on safe side in case name becomes std::string - auto n = v.name; - variants[std::move(n)] = std::move(v); - } + for( auto& v : abi.variants.value ) + variants[v.name] = std::move(v); for( auto& r : abi.action_results.value ) action_results[std::move(r.name)] = std::move(r.result_type);