Merge pull request #2117 from AntelopeIO/no_except_missing_perm

improve `authority_checker`'s performance: don't use exceptions as flow control for non-existing permissions

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,
                                         {},

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<typename PermissionToAuthorityFunc>
    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<decltype(checker.permission_to_authority), const permission_level> 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<decltype(auth)>(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( *auth, cached_permissions, recursion_depth + 1 );
 
                      if( r ) {
                         total_weight += permission.weight;

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}});