From 2a10c5d0db8877febfb6cc34412518df1c70d364 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Tue, 20 Feb 2024 11:29:39 -0600 Subject: [PATCH 1/2] GH-2228 Correctly handle action return value conversion by ABI --- plugins/trace_api_plugin/abi_data_handler.cpp | 5 +- plugins/trace_api_plugin/request_handler.cpp | 7 +-- .../test/test_data_handlers.cpp | 57 ++++++++++++++++++- 3 files changed, 61 insertions(+), 8 deletions(-) diff --git a/plugins/trace_api_plugin/abi_data_handler.cpp b/plugins/trace_api_plugin/abi_data_handler.cpp index 520222d3f4..7ce0d24db6 100644 --- a/plugins/trace_api_plugin/abi_data_handler.cpp +++ b/plugins/trace_api_plugin/abi_data_handler.cpp @@ -31,7 +31,10 @@ namespace eosio::trace_api { auto params = serializer_p->binary_to_variant(type_name, action.data, abi_yield); if constexpr (std::is_same_v) { if(action.return_value.size() > 0) { - ret_data = serializer_p->binary_to_variant(type_name, action.return_value, abi_yield); + auto return_type_name = serializer_p->get_action_result_type(action_name); + if (!return_type_name.empty()) { + ret_data = serializer_p->binary_to_variant(return_type_name, action.return_value, abi_yield); + } } } return {params, ret_data}; diff --git a/plugins/trace_api_plugin/request_handler.cpp b/plugins/trace_api_plugin/request_handler.cpp index 20755cf79e..1af8d6eaee 100644 --- a/plugins/trace_api_plugin/request_handler.cpp +++ b/plugins/trace_api_plugin/request_handler.cpp @@ -41,25 +41,22 @@ namespace { yield(); const auto& a = actions.at(index); - auto common_mvo = fc::mutable_variant_object(); + auto action_variant = fc::mutable_variant_object(); - common_mvo("global_sequence", a.global_sequence) + action_variant("global_sequence", a.global_sequence) ("receiver", a.receiver.to_string()) ("account", a.account.to_string()) ("action", a.action.to_string()) ("authorization", process_authorizations(a.authorization, yield)) ("data", fc::to_hex(a.data.data(), a.data.size())); - auto action_variant = fc::mutable_variant_object(); if constexpr(std::is_same_v){ - action_variant(std::move(common_mvo)); auto [params, return_data] = data_handler(a, yield); if (!params.is_null()) { action_variant("params", params); } } else if constexpr(std::is_same_v){ - action_variant(std::move(common_mvo)); action_variant("return_value", fc::to_hex(a.return_value.data(),a.return_value.size())) ; auto [params, return_data] = data_handler(a, yield); if (!params.is_null()) { diff --git a/plugins/trace_api_plugin/test/test_data_handlers.cpp b/plugins/trace_api_plugin/test/test_data_handlers.cpp index ee63ab50fb..3b3779320c 100644 --- a/plugins/trace_api_plugin/test/test_data_handlers.cpp +++ b/plugins/trace_api_plugin/test/test_data_handlers.cpp @@ -22,6 +22,7 @@ BOOST_AUTO_TEST_SUITE(abi_data_handler_tests) auto actual = handler.serialize_to_variant(action_trace_t, [](){}); BOOST_TEST(to_kv(expected) == to_kv(std::get<0>(actual)), boost::test_tools::per_element()); + BOOST_REQUIRE(!std::get<1>(actual)); } BOOST_AUTO_TEST_CASE(empty_data_v1) @@ -37,6 +38,7 @@ BOOST_AUTO_TEST_SUITE(abi_data_handler_tests) auto actual = handler.serialize_to_variant(action_trace_t, [](){}); BOOST_TEST(to_kv(expected) == to_kv(std::get<0>(actual)), boost::test_tools::per_element()); + BOOST_REQUIRE(!std::get<1>(actual)); } BOOST_AUTO_TEST_CASE(no_abi) @@ -51,6 +53,7 @@ BOOST_AUTO_TEST_SUITE(abi_data_handler_tests) auto actual = handler.serialize_to_variant(action_trace_t, [](){}); BOOST_TEST(to_kv(expected) == to_kv(std::get<0>(actual)), boost::test_tools::per_element()); + BOOST_REQUIRE(!std::get<1>(actual)); } BOOST_AUTO_TEST_CASE(no_abi_v1) @@ -66,6 +69,7 @@ BOOST_AUTO_TEST_SUITE(abi_data_handler_tests) auto actual = handler.serialize_to_variant(action_trace_t, [](){}); BOOST_TEST(to_kv(expected) == to_kv(std::get<0>(actual)), boost::test_tools::per_element()); + BOOST_REQUIRE(!std::get<1>(actual)); } BOOST_AUTO_TEST_CASE(basic_abi) @@ -99,20 +103,22 @@ BOOST_AUTO_TEST_SUITE(abi_data_handler_tests) auto actual = handler.serialize_to_variant(action_trace_t, [](){}); BOOST_TEST(to_kv(expected) == to_kv(std::get<0>(actual)), boost::test_tools::per_element()); + BOOST_REQUIRE(!std::get<1>(actual)); } BOOST_AUTO_TEST_CASE(basic_abi_v1) { auto action = action_trace_v1 { { 0, "alice"_n, "alice"_n, "foo"_n, {}, {0x00, 0x01, 0x02, 0x03}}, - {0x04, 0x05, 0x06, 0x07} + {0x04, 0x05, 0x06} }; std::variant action_trace_t = action; auto abi = chain::abi_def ( {}, { - { "foo", "", { {"a", "varuint32"}, {"b", "varuint32"}, {"c", "varuint32"}, {"d", "varuint32"} } } + { "foo", "", { {"a", "varuint32"}, {"b", "varuint32"}, {"c", "varuint32"}, {"d", "varuint32"} } }, + { "foor", "", { {"e", "varuint32"}, {"f", "varuint32"}, {"g", "varuint32"} } } }, { { "foo"_n, "foo", ""} @@ -120,6 +126,7 @@ BOOST_AUTO_TEST_SUITE(abi_data_handler_tests) {}, {}, {} ); abi.version = "eosio::abi/1."; + abi.action_results = { std::vector{ chain::action_result_def{ "foo"_n, "foor"} } }; abi_data_handler handler(exception_handler{}); handler.add_abi("alice"_n, abi); @@ -129,10 +136,16 @@ BOOST_AUTO_TEST_SUITE(abi_data_handler_tests) ("b", 1) ("c", 2) ("d", 3); + fc::variant expected_return = fc::mutable_variant_object() + ("e", 4) + ("f", 5) + ("g", 6); auto actual = handler.serialize_to_variant(action_trace_t, [](){}); BOOST_TEST(to_kv(expected) == to_kv(std::get<0>(actual)), boost::test_tools::per_element()); + BOOST_REQUIRE(std::get<1>(actual)); + BOOST_TEST(to_kv(expected_return) == to_kv(*std::get<1>(actual)), boost::test_tools::per_element()); } BOOST_AUTO_TEST_CASE(abi_fail_yield) @@ -209,6 +222,7 @@ BOOST_AUTO_TEST_SUITE(abi_data_handler_tests) auto actual = handler.serialize_to_variant(action_trace_t, [](){}); BOOST_TEST(to_kv(expected) == to_kv(std::get<0>(actual)), boost::test_tools::per_element()); + BOOST_REQUIRE(!std::get<1>(actual)); } BOOST_AUTO_TEST_CASE(basic_abi_wrong_type_v1) @@ -238,6 +252,7 @@ BOOST_AUTO_TEST_SUITE(abi_data_handler_tests) auto actual = handler.serialize_to_variant(action_trace_t, [](){}); BOOST_TEST(to_kv(expected) == to_kv(std::get<0>(actual)), boost::test_tools::per_element()); + BOOST_REQUIRE(!std::get<1>(actual)); } BOOST_AUTO_TEST_CASE(basic_abi_insufficient_data) @@ -269,6 +284,44 @@ BOOST_AUTO_TEST_SUITE(abi_data_handler_tests) BOOST_TEST(to_kv(expected) == to_kv(std::get<0>(actual)), boost::test_tools::per_element()); BOOST_TEST(log_called); + BOOST_REQUIRE(!std::get<1>(actual)); + } + + // If no ABI provided for return type then do not attempt to decode it + BOOST_AUTO_TEST_CASE(basic_abi_no_return_abi_when_return_value_provided) + { + auto action = action_trace_v1 { + { 0, "alice"_n, "alice"_n, "foo"_n, {}, {0x00, 0x01, 0x02, 0x03}}, + {0x04, 0x05, 0x06} + }; + + std::variant action_trace_t = action; + + auto abi = chain::abi_def ( {}, + { + { "foo", "", { {"a", "varuint32"}, {"b", "varuint32"}, {"c", "varuint32"}, {"d", "varuint32"} } }, + }, + { + { "foo"_n, "foo", ""} + }, + {}, {}, {} + ); + abi.version = "eosio::abi/1."; + + abi_data_handler handler(exception_handler{}); + handler.add_abi("alice"_n, std::move(abi)); + + fc::variant expected = fc::mutable_variant_object() + ("a", 0) + ("b", 1) + ("c", 2) + ("d", 3); + + auto actual = handler.serialize_to_variant(action_trace_t); + + BOOST_TEST(to_kv(expected) == to_kv(std::get<0>(actual)), boost::test_tools::per_element()); + BOOST_REQUIRE(!std::get<1>(actual)); } + BOOST_AUTO_TEST_SUITE_END() From 5e34f3e78aa0a35e6c82d07ace9002ca2c773048 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Tue, 20 Feb 2024 11:56:50 -0600 Subject: [PATCH 2/2] Fix merge --- plugins/trace_api_plugin/test/test_data_handlers.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/trace_api_plugin/test/test_data_handlers.cpp b/plugins/trace_api_plugin/test/test_data_handlers.cpp index 3b3779320c..a2783b188e 100644 --- a/plugins/trace_api_plugin/test/test_data_handlers.cpp +++ b/plugins/trace_api_plugin/test/test_data_handlers.cpp @@ -317,7 +317,7 @@ BOOST_AUTO_TEST_SUITE(abi_data_handler_tests) ("c", 2) ("d", 3); - auto actual = handler.serialize_to_variant(action_trace_t); + auto actual = handler.serialize_to_variant(action_trace_t, [](){}); BOOST_TEST(to_kv(expected) == to_kv(std::get<0>(actual)), boost::test_tools::per_element()); BOOST_REQUIRE(!std::get<1>(actual));