From af57b8e83560e1e0a6e41914c80a255a814c848e Mon Sep 17 00:00:00 2001 From: Matt Witherspoon <32485495+spoonincode@users.noreply.github.com> Date: Fri, 19 Jan 2024 18:15:06 -0500 Subject: [PATCH 1/2] don't use exceptions to indicate a missing permission in auth checker --- libraries/chain/authorization_manager.cpp | 21 +++++++++++++--- .../include/eosio/chain/authority_checker.hpp | 25 +++++++++---------- unittests/misc_tests.cpp | 17 ++++++++----- 3 files changed, 41 insertions(+), 22 deletions(-) diff --git a/libraries/chain/authorization_manager.cpp b/libraries/chain/authorization_manager.cpp index 9cb4580a0c..7b60554161 100644 --- a/libraries/chain/authorization_manager.cpp +++ b/libraries/chain/authorization_manager.cpp @@ -479,7 +479,12 @@ namespace eosio { namespace chain { auto effective_provided_delay = (provided_delay >= delay_max_limit) ? fc::microseconds::maximum() : provided_delay; - auto checker = make_auth_checker( [&](const permission_level& p){ return get_permission(p).auth; }, + auto checker = make_auth_checker( [&](const permission_level& p) -> const shared_authority* { + if(const permission_object* po = find_permission(p)) + return &po->auth; + else + return nullptr; + }, _control.get_global_properties().configuration.max_authority_depth, provided_keys, provided_permissions, @@ -580,7 +585,12 @@ namespace eosio { namespace chain { auto delay_max_limit = fc::seconds( _control.get_global_properties().configuration.max_transaction_delay ); - auto checker = make_auth_checker( [&](const permission_level& p){ return get_permission(p).auth; }, + auto checker = make_auth_checker( [&](const permission_level& p) -> const shared_authority* { + if(const permission_object* po = find_permission(p)) + return &po->auth; + else + return nullptr; + }, _control.get_global_properties().configuration.max_authority_depth, provided_keys, provided_permissions, @@ -611,7 +621,12 @@ namespace eosio { namespace chain { fc::microseconds provided_delay )const { - auto checker = make_auth_checker( [&](const permission_level& p){ return get_permission(p).auth; }, + auto checker = make_auth_checker( [&](const permission_level& p) -> const shared_authority* { + if(const permission_object* po = find_permission(p)) + return &po->auth; + else + return nullptr; + }, _control.get_global_properties().configuration.max_authority_depth, candidate_keys, {}, diff --git a/libraries/chain/include/eosio/chain/authority_checker.hpp b/libraries/chain/include/eosio/chain/authority_checker.hpp index ccda160c99..6d2171a05b 100644 --- a/libraries/chain/include/eosio/chain/authority_checker.hpp +++ b/libraries/chain/include/eosio/chain/authority_checker.hpp @@ -28,8 +28,8 @@ namespace detail { * then determine whether that list of keys is sufficient to satisfy the authority. This class takes a list of keys and * provides the @ref satisfied method to determine whether that list of keys satisfies a provided authority. * - * @tparam F A callable which takes a single argument of type @ref AccountPermission and returns the corresponding - * authority + * @tparam F A callable which takes a single argument of type @ref AccountPermission and returns a pointer to + * the corresponding authority if it exists, or a nullptr. */ template class authority_checker { @@ -225,19 +225,18 @@ namespace detail { bool r = false; typename permission_cache_type::iterator itr = cached_permissions.end(); - bool propagate_error = false; + std::invoke_result_t auth = nullptr; try { - auto&& auth = checker.permission_to_authority( permission.permission ); - propagate_error = true; - auto res = cached_permissions.emplace( permission.permission, being_evaluated ); - itr = res.first; - r = checker.satisfied( std::forward(auth), cached_permissions, recursion_depth + 1 ); - } catch( const permission_query_exception& ) { - if( propagate_error ) - throw; - else - return total_weight; // if the permission doesn't exist, continue without it + auth = checker.permission_to_authority( permission.permission ); } + catch( const permission_query_exception& ) {} + //permission was either invalid (threw permission_query_exception), or wasn't found + if(!auth) + return total_weight; + + auto res = cached_permissions.emplace( permission.permission, being_evaluated ); + itr = res.first; + r = checker.satisfied( std::forward(*auth), cached_permissions, recursion_depth + 1 ); if( r ) { total_weight += permission.weight; diff --git a/unittests/misc_tests.cpp b/unittests/misc_tests.cpp index 2877ca491c..7649dcbfd0 100644 --- a/unittests/misc_tests.cpp +++ b/unittests/misc_tests.cpp @@ -400,7 +400,8 @@ BOOST_AUTO_TEST_CASE(authority_checker) auto b = test.get_public_key(name("b"), "active"); auto c = test.get_public_key(name("c"), "active"); - auto GetNullAuthority = [](auto){abort(); return authority();}; + const authority null_authority; + auto GetNullAuthority = [&null_authority](auto){abort(); return &null_authority;}; auto A = authority(2, {key_weight{a, 1}, key_weight{b, 1}}); { @@ -450,8 +451,9 @@ BOOST_AUTO_TEST_CASE(authority_checker) BOOST_TEST(make_auth_checker(GetNullAuthority, 2, {b}).satisfied(A)); BOOST_TEST(!make_auth_checker(GetNullAuthority, 2, {c}).satisfied(A)); - auto GetCAuthority = [c](auto){ - return authority(1, {key_weight{c, 1}}); + const authority c_authority = authority(1, {key_weight{c, 1}}); + auto GetCAuthority = [&c_authority](auto){ + return &c_authority; }; A = authority(2, {key_weight{a, 2}, key_weight{b, 1}}, {permission_level_weight{{name("hello"), name("world")}, 1}}); @@ -544,10 +546,13 @@ BOOST_AUTO_TEST_CASE(authority_checker) auto d = test.get_public_key(name("d"), "active"); auto e = test.get_public_key(name("e"), "active"); - auto GetAuthority = [d, e] (const permission_level& perm) { + const authority auth_for_top_actor = authority(2, {key_weight{d, 1}}, {permission_level_weight{{name("bottom"), name("bottom")}, 1}}); + const authority auth_for_others = authority{1, {{e, 1}}, {}}; + + auto GetAuthority = [&auth_for_top_actor, &auth_for_others] (const permission_level& perm) { if (perm.actor == name("top")) - return authority(2, {key_weight{d, 1}}, {permission_level_weight{{name("bottom"), name("bottom")}, 1}}); - return authority{1, {{e, 1}}, {}}; + return &auth_for_top_actor; + return &auth_for_others; }; A = authority(5, {key_weight{a, 2}, key_weight{b, 2}, key_weight{c, 2}}, {permission_level_weight{{name("top"), name("top")}, 5}}); From 7ab4d4c10c563b291d5f2d3ae2774c696cdf0bf4 Mon Sep 17 00:00:00 2001 From: Matt Witherspoon <32485495+spoonincode@users.noreply.github.com> Date: Mon, 22 Jan 2024 11:39:40 -0500 Subject: [PATCH 2/2] no longer need the forward here --- libraries/chain/include/eosio/chain/authority_checker.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/chain/include/eosio/chain/authority_checker.hpp b/libraries/chain/include/eosio/chain/authority_checker.hpp index 6d2171a05b..a0d9f2d154 100644 --- a/libraries/chain/include/eosio/chain/authority_checker.hpp +++ b/libraries/chain/include/eosio/chain/authority_checker.hpp @@ -236,7 +236,7 @@ namespace detail { auto res = cached_permissions.emplace( permission.permission, being_evaluated ); itr = res.first; - r = checker.satisfied( std::forward(*auth), cached_permissions, recursion_depth + 1 ); + r = checker.satisfied( *auth, cached_permissions, recursion_depth + 1 ); if( r ) { total_weight += permission.weight;