Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[3.2] Fix incorrect serializing of std::optional when value is not provided - (GH #1189) #1364

Merged
merged 8 commits into from
Jul 5, 2023
Merged

[3.2] Fix incorrect serializing of std::optional when value is not provided - (GH #1189) #1364

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
86c3883
Output flag marking empty `std::optional` in `abi_serializer::_varian…
greg7mdp Jun 23, 2023
f9e91f1
Use value from `numeric_limits` rather than hard coded hex value.
greg7mdp Jun 23, 2023
93fe429
Update `abi_tests.cpp` to not enforce deadline in debug mode.
greg7mdp Jun 23, 2023
f1eb13a
Add test case for using `std::optional` in abi type definitions.
greg7mdp Jun 23, 2023
05fe45a
Add a couple more test patterns.
greg7mdp Jun 23, 2023
1111fee
Merge branch 'release/3.2' of github.com:AntelopeIO/leap into gh-1189…
greg7mdp Jul 3, 2023
7988549
Address PR comment (removed change unneeded in backport).
greg7mdp Jul 3, 2023
f41ca84
Update comments.
greg7mdp Jul 5, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions libraries/chain/abi_serializer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -535,14 +535,15 @@ namespace eosio { namespace chain {
bool disallow_additional_fields = false;
for( uint32_t i = 0; i < st.fields.size(); ++i ) {
const auto& field = st.fields[i];
if( vo.contains( string(field.name).c_str() ) ) {
bool present = vo.contains(string(field.name).c_str());
if( present || is_optional(field.type) ) {
if( disallow_additional_fields )
EOS_THROW( pack_exception, "Unexpected field '${f}' found in input object while processing struct '${p}'",
("f", ctx.maybe_shorten(field.name))("p", ctx.get_path_string()) );
{
auto h1 = ctx.push_to_path( impl::field_path_item{ .parent_struct_itr = s_itr, .field_ordinal = i } );
auto h2 = ctx.disallow_extensions_unless( &field == &st.fields.back() );
_variant_to_binary(_remove_bin_extension(field.type), vo[field.name], ds, ctx);
_variant_to_binary(_remove_bin_extension(field.type), present ? vo[field.name] : fc::variant(nullptr), ds, ctx);
}
} else if( ends_with(field.type, "$") && ctx.extensions_allowed() ) {
disallow_additional_fields = true;
Expand Down
129 changes: 122 additions & 7 deletions unittests/abi_tests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,17 @@ FC_REFLECT(act_sig, (sig) )

BOOST_AUTO_TEST_SUITE(abi_tests)

#ifdef NDEBUG
fc::microseconds max_serialization_time = fc::seconds(1); // some test machines are very slow
#else
fc::microseconds max_serialization_time = fc::microseconds::maximum(); // don't check in debug builds
#endif
heifner marked this conversation as resolved.
Show resolved Hide resolved

static fc::time_point get_deadline() {
if (max_serialization_time == fc::microseconds::maximum())
return fc::time_point(fc::microseconds::maximum());
return fc::time_point::now() + max_serialization_time;
}

// verify that round trip conversion, via bytes, reproduces the exact same data
fc::variant verify_byte_round_trip_conversion( const abi_serializer& abis, const type_name& type, const fc::variant& var )
Expand All @@ -49,8 +59,6 @@ fc::variant verify_byte_round_trip_conversion( const abi_serializer& abis, const

auto var2 = abis.binary_to_variant(type, bytes, abi_serializer::create_yield_function( max_serialization_time ));

std::string r = fc::json::to_string(var2, fc::time_point::now() + max_serialization_time);

auto bytes2 = abis.variant_to_binary(type, var2, abi_serializer::create_yield_function( max_serialization_time ));

BOOST_TEST( fc::to_hex(bytes) == fc::to_hex(bytes2) );
Expand All @@ -64,7 +72,7 @@ void verify_round_trip_conversion( const abi_serializer& abis, const type_name&
auto bytes = abis.variant_to_binary(type, var, abi_serializer::create_yield_function( max_serialization_time ));
BOOST_REQUIRE_EQUAL(fc::to_hex(bytes), hex);
auto var2 = abis.binary_to_variant(type, bytes, abi_serializer::create_yield_function( max_serialization_time ));
BOOST_REQUIRE_EQUAL(fc::json::to_string(var2, fc::time_point::now() + max_serialization_time), expected_json);
BOOST_REQUIRE_EQUAL(fc::json::to_string(var2, get_deadline()), expected_json);
auto bytes2 = abis.variant_to_binary(type, var2, abi_serializer::create_yield_function( max_serialization_time ));
BOOST_REQUIRE_EQUAL(fc::to_hex(bytes2), hex);
}
Expand Down Expand Up @@ -94,7 +102,7 @@ fc::variant verify_type_round_trip_conversion( const abi_serializer& abis, const
fc::variant var2;
abi_serializer::to_variant(obj, var2, get_resolver(), abi_serializer::create_yield_function( max_serialization_time ));

std::string r = fc::json::to_string(var2, fc::time_point::now() + max_serialization_time);
std::string r = fc::json::to_string(var2, get_deadline());


auto bytes2 = abis.variant_to_binary(type, var2, abi_serializer::create_yield_function( max_serialization_time ));
Expand Down Expand Up @@ -1929,6 +1937,113 @@ BOOST_AUTO_TEST_CASE(abi_type_loop)

} FC_LOG_AND_RETHROW() }

BOOST_AUTO_TEST_CASE(abi_std_optional)
{ try {
const char* repeat_abi = R"=====(
{
"version": "eosio::abi/1.2",
"types": [],
"structs": [
{
"name": "fees",
"base": "",
"fields": [
{
"name": "gas_price",
"type": "uint64?"
},
{
"name": "miner_cut",
"type": "uint32?"
},
{
"name": "bridge_fee",
"type": "uint32?"
}
]
}
],
"actions": [
{
"name": "fees",
"type": "fees",
"ricardian_contract": ""
}
],
"tables": [],
"ricardian_clauses": [],
"variants": [],
"action_results": []
}
)=====";

abi_serializer abis(fc::json::from_string(repeat_abi).as<abi_def>(), abi_serializer::create_yield_function( max_serialization_time ));
{
// check conversion when all optional members are provided
std::string test_data = R"=====(
{
"gas_price" : "42",
"miner_cut" : "2",
"bridge_fee" : "2"
}
)=====";

auto var = fc::json::from_string(test_data);
verify_byte_round_trip_conversion(abis, "fees", var);
}

{
// check conversion when the first optional member is missing
std::string test_data = R"=====(
{
"miner_cut" : "2",
"bridge_fee" : "2"
}
)=====";

auto var = fc::json::from_string(test_data);
verify_byte_round_trip_conversion(abis, "fees", var);
}

{
// check conversion when the second optional member is missing
std::string test_data = R"=====(
{
"gas_price" : "42",
"bridge_fee" : "2"
}
)=====";

auto var = fc::json::from_string(test_data);
verify_byte_round_trip_conversion(abis, "fees", var);
}

{
// check conversion when the last optional member is missing
std::string test_data = R"=====(
{
"gas_price" : "42",
"miner_cut" : "2",
}
)=====";

auto var = fc::json::from_string(test_data);
verify_byte_round_trip_conversion(abis, "fees", var);
}

{
// check conversion when all optional members are missing
std::string test_data = R"=====(
{
}
)=====";

auto var = fc::json::from_string(test_data);
verify_byte_round_trip_conversion(abis, "fees", var);
}

} FC_LOG_AND_RETHROW() }

BOOST_AUTO_TEST_CASE(abi_type_redefine)
{ try {
// inifinite loop in types
Expand Down Expand Up @@ -2976,7 +3091,7 @@ BOOST_AUTO_TEST_CASE(abi_to_variant__add_action__good_return_value)
mutable_variant_object mvo;
eosio::chain::impl::abi_traverse_context ctx(abi_serializer::create_yield_function(max_serialization_time));
eosio::chain::impl::abi_to_variant::add(mvo, "action_traces", at, get_resolver(abidef), ctx);
std::string res = fc::json::to_string(mvo, fc::time_point::now() + max_serialization_time);
std::string res = fc::json::to_string(mvo, get_deadline());

BOOST_CHECK_EQUAL(res, expected_json);
}
Expand All @@ -3001,7 +3116,7 @@ BOOST_AUTO_TEST_CASE(abi_to_variant__add_action__bad_return_value)
mutable_variant_object mvo;
eosio::chain::impl::abi_traverse_context ctx(abi_serializer::create_yield_function(max_serialization_time));
eosio::chain::impl::abi_to_variant::add(mvo, "action_traces", at, get_resolver(abidef), ctx);
std::string res = fc::json::to_string(mvo, fc::time_point::now() + max_serialization_time);
std::string res = fc::json::to_string(mvo, get_deadline());

BOOST_CHECK_EQUAL(res, expected_json);
}
Expand Down Expand Up @@ -3036,7 +3151,7 @@ BOOST_AUTO_TEST_CASE(abi_to_variant__add_action__no_return_value)
mutable_variant_object mvo;
eosio::chain::impl::abi_traverse_context ctx(abi_serializer::create_yield_function(max_serialization_time));
eosio::chain::impl::abi_to_variant::add(mvo, "action_traces", at, get_resolver(abidef), ctx);
std::string res = fc::json::to_string(mvo, fc::time_point::now() + max_serialization_time);
std::string res = fc::json::to_string(mvo, get_deadline());

BOOST_CHECK_EQUAL(res, expected_json);
}
Expand Down