Merge pull request #2440 from EOSIO/1994-cancel-delay-authorization

Fixes and cleanup of check_authorization for updateauth, deleteauth, linkauth, unlinkauth, canceldelay native actions

contracts/eosio.system/native.hpp

@@ -132,9 +132,10 @@ namespace eosiosystem {
          }
 
          ACTION( SystemAccount, canceldelay ) {
+            permission_level      canceling_auth;
             transaction_id_type   trx_id;
 
-            EOSLIB_SERIALIZE( canceldelay, (trx_id) )
+            EOSLIB_SERIALIZE( canceldelay, (canceling_auth)(trx_id) )
          };
 
          static void on( const canceldelay& ) {

libraries/chain/chain_controller.cpp

@@ -984,13 +984,13 @@ flat_set<public_key_type> chain_controller::get_required_keys(const transaction&
 class permission_visitor {
 public:
    permission_visitor(const chain_controller& controller)
-   : _chain_controller(controller) {
+   : _chain_controller(controller), _track_delay(true) {
       _max_delay_stack.emplace_back();
    }
 
    void operator()(const permission_level& perm_level) {
       const auto obj = _chain_controller.get_permission(perm_level);
-      if( _max_delay_stack.back() < obj.delay )
+      if( _track_delay && _max_delay_stack.back() < obj.delay )
          _max_delay_stack.back() = obj.delay;
    }
 
@@ -1010,16 +1010,150 @@ public:
       _max_delay_stack.back() = delay_to_keep;
    }
 
-   fc::microseconds get_max_delay() const {
+   fc::microseconds get_max_delay()const {
       FC_ASSERT( _max_delay_stack.size() == 1, "invariant failure in permission_visitor" );
       return _max_delay_stack.back();
    }
 
+   void pause_delay_tracking() {
+      _track_delay = false;
+   }
+
+   void resume_delay_tracking() {
+      _track_delay = true;
+   }
+
 private:
    const chain_controller& _chain_controller;
    vector<fc::microseconds> _max_delay_stack;
+   bool _track_delay;
 };
 
+optional<fc::microseconds> chain_controller::check_updateauth_authorization( const contracts::updateauth& update,
+                                                                             const vector<permission_level>& auths )const
+{
+   EOS_ASSERT( auths.size() == 1, tx_irrelevant_auth,
+               "updateauth action should only have one declared authorization" );
+   const auto& auth = auths[0];
+   EOS_ASSERT( auth.actor == update.account, tx_irrelevant_auth,
+               "the owner of the affected permission needs to be the actor of the declared authorization" );
+
+   const auto* min_permission = find_permission({update.account, update.permission});
+   bool ignore_delay = false;
+   if( !min_permission ) { // creating a new permission
+      ignore_delay = true;
+      min_permission = &get_permission({update.account, update.parent});
+
+   }
+   const auto delay = get_permission(auth).satisfies( *min_permission,
+                                                      _db.get_index<permission_index>().indices() );
+   EOS_ASSERT( delay.valid(),
+               tx_irrelevant_auth,
+               "updateauth action declares irrelevant authority '${auth}'; minimum authority is ${min}",
+               ("auth", auth)("min", permission_level{update.account, min_permission->name}) );
+
+   return (ignore_delay ? optional<fc::microseconds>() : *delay);
+}
+
+fc::microseconds chain_controller::check_deleteauth_authorization( const contracts::deleteauth& del,
+                                                                   const vector<permission_level>& auths )const
+{
+   EOS_ASSERT( auths.size() == 1, tx_irrelevant_auth,
+               "deleteauth action should only have one declared authorization" );
+   const auto& auth = auths[0];
+   EOS_ASSERT( auth.actor == del.account, tx_irrelevant_auth,
+               "the owner of the permission to delete needs to be the actor of the declared authorization" );
+
+   const auto& min_permission = get_permission({del.account, del.permission});
+   const auto delay = get_permission(auth).satisfies( min_permission,
+                                                      _db.get_index<permission_index>().indices() );
+   EOS_ASSERT( delay.valid(),
+               tx_irrelevant_auth,
+               "updateauth action declares irrelevant authority '${auth}'; minimum authority is ${min}",
+               ("auth", auth)("min", permission_level{min_permission.owner, min_permission.name}) );
+
+   return *delay;
+}
+
+fc::microseconds chain_controller::check_linkauth_authorization( const contracts::linkauth& link,
+                                                                 const vector<permission_level>& auths )const
+{
+   EOS_ASSERT( auths.size() == 1, tx_irrelevant_auth,
+               "link action should only have one declared authorization" );
+   const auto& auth = auths[0];
+   EOS_ASSERT( auth.actor == link.account, tx_irrelevant_auth,
+               "the owner of the linked permission needs to be the actor of the declared authorization" );
+
+   EOS_ASSERT( link.type != contracts::updateauth::get_name(),  action_validate_exception,
+               "Cannot link eosio::updateauth to a minimum permission" );
+   EOS_ASSERT( link.type != contracts::deleteauth::get_name(),  action_validate_exception,
+               "Cannot link eosio::deleteauth to a minimum permission" );
+   EOS_ASSERT( link.type != contracts::linkauth::get_name(),    action_validate_exception,
+               "Cannot link eosio::linkauth to a minimum permission" );
+   EOS_ASSERT( link.type != contracts::unlinkauth::get_name(),  action_validate_exception,
+               "Cannot link eosio::unlinkauth to a minimum permission" );
+   EOS_ASSERT( link.type != contracts::canceldelay::get_name(), action_validate_exception,
+               "Cannot link eosio::canceldelay to a minimum permission" );
+
+   const auto linked_permission_name = lookup_minimum_permission(link.account, link.code, link.type);
+
+   if( !linked_permission_name ) // if action is linked to eosio.any permission
+      return fc::microseconds(0);
+
+   const auto delay = get_permission(auth).satisfies( get_permission({link.account, *linked_permission_name}),
+                                                      _db.get_index<permission_index>().indices() );
+
+   EOS_ASSERT( delay.valid(),
+               tx_irrelevant_auth,
+               "link action declares irrelevant authority '${auth}'; minimum authority is ${min}",
+               ("auth", auth)("min", permission_level{link.account, *linked_permission_name}) );
+
+   return *delay;
+}
+
+fc::microseconds chain_controller::check_unlinkauth_authorization( const contracts::unlinkauth& unlink,
+                                                                   const vector<permission_level>& auths )const
+{
+   EOS_ASSERT( auths.size() == 1, tx_irrelevant_auth,
+               "unlink action should only have one declared authorization" );
+   const auto& auth = auths[0];
+   EOS_ASSERT( auth.actor == unlink.account, tx_irrelevant_auth,
+               "the owner of the linked permission needs to be the actor of the declared authorization" );
+
+   const auto unlinked_permission_name = lookup_linked_permission(unlink.account, unlink.code, unlink.type);
+   EOS_ASSERT( unlinked_permission_name.valid(), transaction_exception,
+               "cannot unlink non-existent permission link of account '${account}' for actions matching '${code}::${action}'",
+               ("account", unlink.account)("code", unlink.code)("action", unlink.type) );
+
+   if( *unlinked_permission_name == config::eosio_any_name )
+      return fc::microseconds(0);
+
+   const auto delay = get_permission(auth).satisfies( get_permission({unlink.account, *unlinked_permission_name}),
+                                                      _db.get_index<permission_index>().indices() );
+
+   EOS_ASSERT( delay.valid(),
+               tx_irrelevant_auth,
+               "unlink action declares irrelevant authority '${auth}'; minimum authority is ${min}",
+               ("auth", auth)("min", permission_level{unlink.account, *unlinked_permission_name}) );
+
+   return *delay;
+}
+
+void chain_controller::check_canceldelay_authorization( const contracts::canceldelay& cancel,
+                                                        const vector<permission_level>& auths )const
+{
+   EOS_ASSERT( auths.size() == 1, tx_irrelevant_auth,
+               "canceldelay action should only have one declared authorization" );
+   const auto& auth = auths[0];
+
+   const auto delay = get_permission(auth).satisfies( get_permission(cancel.canceling_auth),
+                                                      _db.get_index<permission_index>().indices() );
+   EOS_ASSERT( delay.valid(),
+               tx_irrelevant_auth,
+               "canceldelay action declares irrelevant authority '${auth}'; specified authority to satisfy is ${min}",
+               ("auth", auth)("min", cancel.canceling_auth) );
+}
+
 fc::microseconds chain_controller::check_authorization( const vector<action>& actions,
                                                         const flat_set<public_key_type>& provided_keys,
                                                         bool allow_unused_signatures,
@@ -1034,67 +1168,59 @@ fc::microseconds chain_controller::check_authorization( const vector<action>& ac
    fc::microseconds max_delay;
 
    for( const auto& act : actions ) {
-      for( const auto& declared_auth : act.authorization ) {
-
-         // check a minimum permission if one is set, otherwise assume the contract code will validate
-         auto min_permission_name = lookup_minimum_permission(declared_auth.actor, act.account, act.name);
-         if( !min_permission_name ) {
-            // for updateauth actions, need to determine the permission that is changing
-            if( act.account == config::system_account_name && act.name == contracts::updateauth::get_name() ) {
-               auto update = act.data_as<contracts::updateauth>();
-               const auto permission_to_change = _db.find<permission_object, by_owner>(boost::make_tuple(update.account, update.permission));
-               if( permission_to_change != nullptr ) {
-                  // Only changes to permissions need to possibly be delayed. New permissions can be added immediately.
-                  min_permission_name = update.permission;
-               }
-            }
-         }
-         if( min_permission_name ) {
-            const auto& min_permission = _db.get<permission_object, by_owner>(boost::make_tuple(declared_auth.actor, *min_permission_name));
-            const auto& index = _db.get_index<permission_index>().indices();
-            const optional<fc::microseconds> delay = get_permission(declared_auth).satisfies(min_permission, index);
-            EOS_ASSERT( delay.valid(),
-                        tx_irrelevant_auth,
-                        "action declares irrelevant authority '${auth}'; minimum authority is ${min}",
-                        ("auth", declared_auth)("min", min_permission.name) );
-            if( max_delay < *delay )
-               max_delay = *delay;
+      bool special_case = false;
+      bool ignore_delay = false;
+
+      if( act.account == config::system_account_name ) {
+         special_case = true;
+
+         if( act.name == contracts::updateauth::get_name() ) {
+            const auto delay = check_updateauth_authorization(act.data_as<contracts::updateauth>(), act.authorization);
+            if( delay.valid() ) // update auth is used to modify an existing permission
+               max_delay = std::max( max_delay, *delay );
+            else // updateauth is used to create a new permission
+               ignore_delay = true;
+         } else if( act.name == contracts::deleteauth::get_name() ) {
+            max_delay = std::max( max_delay,
+                                  check_deleteauth_authorization(act.data_as<contracts::deleteauth>(), act.authorization) );
+         } else if( act.name == contracts::linkauth::get_name() ) {
+            max_delay = std::max( max_delay,
+                                  check_linkauth_authorization(act.data_as<contracts::linkauth>(), act.authorization) );
+         } else if( act.name == contracts::unlinkauth::get_name() ) {
+            max_delay = std::max( max_delay,
+                                  check_unlinkauth_authorization(act.data_as<contracts::unlinkauth>(), act.authorization) );
+         } else if( act.name == contracts::canceldelay::get_name() ) {
+            check_canceldelay_authorization(act.data_as<contracts::canceldelay>(), act.authorization);
+            ignore_delay = true;
+         } else {
+            special_case = false;
          }
+      }
 
-         if( act.account == config::system_account_name ) {
-            // for link changes, we need to also determine the delay associated with an existing link that is being
-            // moved or removed
-            if( act.name == contracts::linkauth::get_name() ) {
-               auto link = act.data_as<contracts::linkauth>();
-               if( declared_auth.actor == link.account ) {
-                  const auto linked_permission_name = lookup_linked_permission(link.account, link.code, link.type);
-                  if( linked_permission_name.valid() && *linked_permission_name != config::eosio_any_name ) {
-                     const auto& linked_permission = _db.get<permission_object, by_owner>(boost::make_tuple(link.account, *linked_permission_name));
-                     const auto& index = _db.get_index<permission_index>().indices();
-                     const optional<fc::microseconds> delay = get_permission(declared_auth).satisfies(linked_permission, index);
-                     if( delay.valid() && max_delay < *delay )
-                        max_delay = *delay;
-                  } // else it is only a new link, so don't need to delay
-               }
-            } else if( act.name == contracts::unlinkauth::get_name() ) {
-               auto unlink = act.data_as<contracts::unlinkauth>();
-               if( declared_auth.actor == unlink.account ) {
-                  const auto unlinked_permission_name = lookup_linked_permission(unlink.account, unlink.code, unlink.type);
-                  if( unlinked_permission_name.valid() && *unlinked_permission_name != config::eosio_any_name ) {
-                     const auto& unlinked_permission = _db.get<permission_object, by_owner>(boost::make_tuple(unlink.account, *unlinked_permission_name));
-                     const auto& index = _db.get_index<permission_index>().indices();
-                     const optional<fc::microseconds> delay = get_permission(declared_auth).satisfies(unlinked_permission, index);
-                     if( delay.valid() && max_delay < *delay )
-                        max_delay = *delay;
-                  }
-               }
+      for( const auto& declared_auth : act.authorization ) {
+
+         if( !special_case ) {
+            auto min_permission_name = lookup_minimum_permission(declared_auth.actor, act.account, act.name);
+            if( min_permission_name ) { // since special cases were already handled, it should only be false if the permission is eosio.any
+               const auto& min_permission = get_permission({declared_auth.actor, *min_permission_name});
+               auto delay = get_permission(declared_auth).satisfies( min_permission,
+                                                                     _db.get_index<permission_index>().indices() );
+               EOS_ASSERT( delay.valid(),
+                           tx_irrelevant_auth,
+                           "action declares irrelevant authority '${auth}'; minimum authority is ${min}",
+                           ("auth", declared_auth)("min", permission_level{min_permission.owner, min_permission.name}) );
+               max_delay = std::max( max_delay, *delay );
             }
          }
 
          if( should_check_signatures() ) {
+            if( ignore_delay )
+               checker.get_permission_visitor().pause_delay_tracking();
             EOS_ASSERT(checker.satisfied(declared_auth), tx_missing_sigs,
                        "transaction declares authority '${auth}', but does not have signatures for it.",
                        ("auth", declared_auth));
+            if( ignore_delay )
+               checker.get_permission_visitor().resume_delay_tracking();
          }
       }
    }
@@ -1106,10 +1232,8 @@ fc::microseconds chain_controller::check_authorization( const vector<action>& ac
    }
 
    const auto checker_max_delay = checker.get_permission_visitor().get_max_delay();
-   if( max_delay < checker_max_delay )
-      max_delay = checker_max_delay;
 
-   return max_delay;
+   return std::max(max_delay, checker_max_delay);
 }
 
 bool chain_controller::check_authorization( account_name account, permission_name permission,
@@ -1150,10 +1274,14 @@ optional<permission_name> chain_controller::lookup_minimum_permission(account_na
                                                                     account_name scope,
                                                                     action_name act_name) const {
 #warning TODO: this comment sounds like it is expecting a check ("may") somewhere else, but I have not found anything else
-   // updateauth is a special case where any permission _may_ be suitable depending
-   // on the contents of the action
-   if( scope == config::system_account_name && act_name == contracts::updateauth::get_name() ) {
-      return optional<permission_name>();
+   // Special case native actions cannot be linked to a minimum permission, so there is no need to check.
+   if( scope == config::system_account_name ) {
+       FC_ASSERT( act_name != contracts::updateauth::get_name() &&
+                  act_name != contracts::deleteauth::get_name() &&
+                  act_name != contracts::linkauth::get_name() &&
+                  act_name != contracts::unlinkauth::get_name() &&
+                  act_name != contracts::canceldelay::get_name(),
+                  "cannot call lookup_minimum_permission on native actions that are not allowed to be linked to minimum permissions" );
    }
 
    try {
@@ -1502,8 +1630,7 @@ void chain_controller::_update_producers_authority() {
       active_producers_authority.accounts.push_back({{name.producer_name, config::active_name}, 1});
    }
 
-   auto& po = _db.get<permission_object, by_owner>( boost::make_tuple(config::producers_account_name,
-                                                                      config::active_name ) );
+   auto& po = get_permission({config::producers_account_name, config::active_name});
    _db.modify(po,[active_producers_authority] (permission_object& po) {
       po.auth = active_producers_authority;
    });
@@ -1552,6 +1679,12 @@ const producer_object& chain_controller::get_producer(const account_name& owner_
    return _db.get<producer_object, by_owner>(owner_name);
 } FC_CAPTURE_AND_RETHROW( (owner_name) ) }
 
+const permission_object*   chain_controller::find_permission( const permission_level& level )const
+{ try {
+   FC_ASSERT( !level.actor.empty() && !level.permission.empty(), "Invalid permission" );
+   return _db.find<permission_object, by_owner>( boost::make_tuple(level.actor,level.permission) );
+} EOS_RETHROW_EXCEPTIONS( chain::permission_query_exception, "Failed to retrieve permission: ${level}", ("level", level) ) }
+
 const permission_object&   chain_controller::get_permission( const permission_level& level )const
 { try {
    FC_ASSERT( !level.actor.empty() && !level.permission.empty(), "Invalid permission" );

libraries/chain/contracts/abi_serializer.cpp

@@ -96,6 +96,7 @@ namespace eosio { namespace chain { namespace contracts {
       built_in_types.emplace("permission_name",           pack_unpack<permission_name>());
       built_in_types.emplace("action_name",               pack_unpack<action_name>());
       built_in_types.emplace("scope_name",                pack_unpack<scope_name>());
+      built_in_types.emplace("permission_level",          pack_unpack<permission_level>());
       built_in_types.emplace("producer_schedule",         pack_unpack<producer_schedule_type>());
       built_in_types.emplace("newaccount",                pack_unpack<newaccount>());
    }

libraries/chain/contracts/chain_initializer.cpp

@@ -75,8 +75,8 @@ abi_def chain_initializer::eos_contract_abi(const abi_def& eosio_system_abi)
    eos_abi.actions.push_back( action_def{name("onerror"), "onerror",""} );
    eos_abi.actions.push_back( action_def{name("onblock"), "onblock",""} );
    eos_abi.actions.push_back( action_def{name("canceldelay"), "canceldelay",""} );
-   
-   // TODO add any ricardian_clauses 
+
+   // TODO add any ricardian_clauses
    //
    // ACTION PAYLOADS
 
@@ -163,6 +163,7 @@ abi_def chain_initializer::eos_contract_abi(const abi_def& eosio_system_abi)
 
    eos_abi.structs.emplace_back( struct_def {
       "canceldelay", "", {
+         {"canceling_auth", "permission_level"},
          {"trx_id", "transaction_id_type"},
       }
    });
@@ -270,7 +271,7 @@ abi_def chain_initializer::eos_contract_abi(const abi_def& eosio_system_abi)
    eos_abi.structs.emplace_back( struct_def {
          "clause_pair", "", {
             {"id", "string"},
-            {"body", "string"} 
+            {"body", "string"}
          }
    });
    eos_abi.structs.emplace_back( struct_def {

libraries/chain/contracts/eosio_contract.cpp

@@ -180,12 +180,14 @@ void apply_eosio_setabi(apply_context& context) {
 }
 
 void apply_eosio_updateauth(apply_context& context) {
-   auto& resources = context.mutable_controller.get_mutable_resource_limits_manager();
    context.require_write_lock( config::eosio_auth_scope );
 
+   auto update = context.act.data_as<updateauth>();
+   context.require_authorization(update.account); // only here to mark the single authority on this action as used
+
+   auto& resources = context.mutable_controller.get_mutable_resource_limits_manager();
    auto& db = context.mutable_db;
 
-   auto update = context.act.data_as<updateauth>();
    EOS_ASSERT(!update.permission.empty(), action_validate_exception, "Cannot create authority with empty name");
    EOS_ASSERT( update.permission.to_string().find( "eosio." ) != 0, action_validate_exception,
                "Permission names that start with 'eosio.' are reserved" );
@@ -200,33 +202,6 @@ void apply_eosio_updateauth(apply_context& context) {
    else
       EOS_ASSERT(!update.parent.empty(), action_validate_exception, "Only owner permission can have empty parent" );
 
-   FC_ASSERT(context.act.authorization.size(), "updateauth can only have one action authorization");
-   const auto& act_auth = context.act.authorization.front();
-   // lazy evaluating loop
-   auto permission_is_valid_for_update = [&](){
-      if (act_auth.permission == config::owner_name || act_auth.permission == update.permission) {
-         return true;
-      }
-      const permission_object *current = db.find<permission_object, by_owner>(boost::make_tuple(update.account, update.permission));
-      // Permission doesn't exist yet, check parent permission
-      if (current == nullptr) current = db.find<permission_object, by_owner>(boost::make_tuple(update.account, update.parent));
-      // Ensure either the permission or parent's permission exists
-      EOS_ASSERT(current != nullptr, permission_query_exception,
-                 "Failed to retrieve permission for: {\"actor\": \"${actor}\", \"permission\": \"${permission}\" }",
-                 ("actor", update.account)("permission", update.parent));
-
-      while(current->name != config::owner_name) {
-         if (current->name == act_auth.permission) {
-            return true;
-         }
-         current = &db.get<permission_object>(current->parent);
-      }
-
-      return false;
-   };
-
-   FC_ASSERT(act_auth.actor == update.account && permission_is_valid_for_update(), "updateauth must carry a permission equal to or in the ancestery of permission it updates");
-
    validate_authority_precondition(context, update.data);
 
    auto permission = db.find<permission_object, by_owner>(boost::make_tuple(update.account, update.permission));
@@ -282,17 +257,17 @@ void apply_eosio_updateauth(apply_context& context) {
 }
 
 void apply_eosio_deleteauth(apply_context& context) {
-   auto& resources = context.mutable_controller.get_mutable_resource_limits_manager();
+   context.require_write_lock( config::eosio_auth_scope );
+
    auto remove = context.act.data_as<deleteauth>();
+   context.require_authorization(remove.account); // only here to mark the single authority on this action as used
+
    EOS_ASSERT(remove.permission != config::active_name, action_validate_exception, "Cannot delete active authority");
    EOS_ASSERT(remove.permission != config::owner_name, action_validate_exception, "Cannot delete owner authority");
 
+   auto& resources = context.mutable_controller.get_mutable_resource_limits_manager();
    auto& db = context.mutable_db;
-   context.require_authorization(remove.account);
-   // TODO/QUESTION:
-   //   Inconsistency between permissions that can be satisfied to create/modify (via updateauth) a permission and the
-   //   stricter requirements for deleting the permission using deleteauth.
-   //   If a permission can be updated, shouldn't it also be allowed to delete it without higher permissions required?
+
    const auto& permission = db.get<permission_object, by_owner>(boost::make_tuple(remove.account, remove.permission));
 
    { // Check for children
@@ -317,12 +292,14 @@ void apply_eosio_deleteauth(apply_context& context) {
 }
 
 void apply_eosio_linkauth(apply_context& context) {
+   context.require_write_lock( config::eosio_auth_scope );
+
    auto& resources = context.mutable_controller.get_mutable_resource_limits_manager();
    auto requirement = context.act.data_as<linkauth>();
    try {
       EOS_ASSERT(!requirement.requirement.empty(), action_validate_exception, "Required permission cannot be empty");
 
-      context.require_authorization(requirement.account);
+      context.require_authorization(requirement.account); // only here to mark the single authority on this action as used
 
       auto& db = context.mutable_db;
       const auto *account = db.find<account_object, by_name>(requirement.account);
@@ -363,11 +340,13 @@ void apply_eosio_linkauth(apply_context& context) {
 }
 
 void apply_eosio_unlinkauth(apply_context& context) {
+   context.require_write_lock( config::eosio_auth_scope );
+   
    auto& resources = context.mutable_controller.get_mutable_resource_limits_manager();
    auto& db = context.mutable_db;
    auto unlink = context.act.data_as<unlinkauth>();
 
-   context.require_authorization(unlink.account);
+   context.require_authorization(unlink.account); // only here to mark the single authority on this action as used
 
    auto link_key = boost::make_tuple(unlink.account, unlink.code, unlink.type);
    auto link = db.find<permission_link_object, by_action_name>(link_key);
@@ -556,6 +535,8 @@ void apply_eosio_vetorecovery(apply_context& context) {
 
 void apply_eosio_canceldelay(apply_context& context) {
    auto cancel = context.act.data_as<canceldelay>();
+   context.require_authorization(cancel.canceling_auth.actor); // only here to mark the single authority on this action as used
+
    const auto& trx_id = cancel.trx_id;
 
    const auto& generated_transaction_idx = context.controller.get_database().get_index<generated_transaction_multi_index>();
@@ -565,23 +546,18 @@ void apply_eosio_canceldelay(apply_context& context) {
               "cannot cancel trx_id=${tid}, there is no deferred transaction with that transaction id",("tid", trx_id));
 
    auto dtrx = fc::raw::unpack<deferred_transaction>(itr->packed_trx.data(), itr->packed_trx.size());
-   set<account_name> accounts;
-   for (const auto& act : dtrx.actions) {
-      for (const auto& auth : act.authorization) {
-         accounts.insert(auth.actor);
-      }
-   }
-
    bool found = false;
-   for (const auto& auth : context.act.authorization) {
-      if (auth.permission == config::active_name && accounts.count(auth.actor)) {
-         found = true;
-         break;
+   for( const auto& act : dtrx.actions ) {
+      for( const auto& auth : act.authorization ) {
+         if( auth == cancel.canceling_auth ) {
+            found = true;
+            break;
+         }
       }
+      if( found ) break;
    }
 
-   FC_ASSERT (found, "canceldelay action must be signed with the \"active\" permission for one of the actors"
-                     " provided in the authorizations on the original transaction");
+   FC_ASSERT (found, "canceling_auth in canceldelay action was not found as authorization in the original delayed transaction");
 
    context.cancel_deferred(context.controller.transaction_id_to_sender_id(trx_id));
 }

libraries/chain/include/eosio/chain/authority_checker.hpp

@@ -170,7 +170,7 @@ namespace detail {
             return {range.begin(), range.end()};
          }
 
-         const PermissionVisitorFunc& get_permission_visitor() {
+         PermissionVisitorFunc& get_permission_visitor() {
             return permission_visitor;
          }
 

libraries/chain/include/eosio/chain/chain_controller.hpp

@@ -92,7 +92,7 @@ namespace eosio { namespace chain {
          void push_block( const signed_block& b, uint32_t skip = skip_nothing );
          transaction_trace push_transaction( const packed_transaction& trx, uint32_t skip = skip_nothing );
          vector<transaction_trace> push_deferred_transactions( bool flush = false, uint32_t skip = skip_nothing );
-         
+
          uint128_t transaction_id_to_sender_id( const transaction_id_type& tid )const;
 
       /**
@@ -265,6 +265,7 @@ namespace eosio { namespace chain {
          const global_property_object&          get_global_properties()const;
          const dynamic_global_property_object&  get_dynamic_global_properties()const;
          const producer_object&                 get_producer(const account_name& ownername)const;
+         const permission_object*               find_permission( const permission_level& level )const;
          const permission_object&               get_permission( const permission_level& level )const;
 
          time_point           head_block_time()const;
@@ -300,6 +301,12 @@ namespace eosio { namespace chain {
                                                flat_set<permission_level>       provided_levels = flat_set<permission_level>()
                                              )const;
 
+         optional<fc::microseconds> check_updateauth_authorization( const contracts::updateauth& update, const vector<permission_level>& auths )const;
+         fc::microseconds check_deleteauth_authorization( const contracts::deleteauth& del, const vector<permission_level>& auths )const;
+         fc::microseconds check_linkauth_authorization( const contracts::linkauth& link, const vector<permission_level>& auths )const;
+         fc::microseconds check_unlinkauth_authorization( const contracts::unlinkauth& unlink, const vector<permission_level>& auths )const;
+         void             check_canceldelay_authorization( const contracts::canceldelay& cancel, const vector<permission_level>& auths )const;
+
          /**
           * @param account - the account owner of the permission
           * @param permission - the permission name to check for authorization

libraries/chain/include/eosio/chain/contracts/types.hpp

@@ -73,7 +73,7 @@ struct action_def {
 
    action_name name;
    type_name   type;
-   string      ricardian_contract; 
+   string      ricardian_contract;
 };
 
 struct table_def {
@@ -92,7 +92,7 @@ struct table_def {
 struct clause_pair {
    clause_pair() = default;
    clause_pair( const string& id, const string& body )
-   : id(id), body(body) 
+   : id(id), body(body)
    {}
 
    string id;
@@ -282,6 +282,7 @@ struct vetorecovery {
 };
 
 struct canceldelay {
+   permission_level      canceling_auth;
    transaction_id_type   trx_id;
 
    static account_name get_account() {
@@ -313,4 +314,4 @@ FC_REFLECT( eosio::chain::contracts::unlinkauth                       , (account
 FC_REFLECT( eosio::chain::contracts::postrecovery                     , (account)(data)(memo) )
 FC_REFLECT( eosio::chain::contracts::passrecovery                     , (account) )
 FC_REFLECT( eosio::chain::contracts::vetorecovery                     , (account) )
-FC_REFLECT( eosio::chain::contracts::canceldelay                      , (trx_id) )
+FC_REFLECT( eosio::chain::contracts::canceldelay                      , (canceling_auth)(trx_id) )

libraries/testing/include/eosio/testing/tester.hpp

@@ -83,6 +83,7 @@ namespace eosio { namespace testing {
 
          transaction_trace    push_action( const account_name& code, const action_name& acttype, const account_name& actor, const variant_object& data, uint32_t expiration = DEFAULT_EXPIRATION_DELTA, uint32_t delay_sec = 0 );
          transaction_trace    push_action( const account_name& code, const action_name& acttype, const vector<account_name>& actors, const variant_object& data, uint32_t expiration = DEFAULT_EXPIRATION_DELTA, uint32_t delay_sec = 0 );
+         transaction_trace    push_action( const account_name& code, const action_name& acttype, const vector<permission_level>& auths, const variant_object& data, uint32_t expiration = DEFAULT_EXPIRATION_DELTA, uint32_t delay_sec = 0 );
 
          void                 set_tapos( signed_transaction& trx, uint32_t expiration = DEFAULT_EXPIRATION_DELTA ) const;
 

libraries/testing/tester.cpp

@@ -209,7 +209,9 @@ namespace eosio { namespace testing {
                              uint32_t delay_sec)
 
    { try {
-      return push_action(code, acttype, vector<account_name>{ actor }, data, expiration, delay_sec);
+      vector<permission_level> auths;
+      auths.push_back(permission_level{actor, config::active_name});
+      return push_action(code, acttype, auths, data, expiration, delay_sec);
    } FC_CAPTURE_AND_RETHROW( (code)(acttype)(actor)(data)(expiration) ) }
 
 
@@ -220,6 +222,21 @@ namespace eosio { namespace testing {
                              uint32_t expiration,
                              uint32_t delay_sec)
 
+   { try {
+      vector<permission_level> auths;
+      for (const auto& actor : actors) {
+         auths.push_back(permission_level{actor, config::active_name});
+      }
+      return push_action(code, acttype, auths, data, expiration, delay_sec);
+   } FC_CAPTURE_AND_RETHROW( (code)(acttype)(actors)(data)(expiration) ) }
+
+   transaction_trace base_tester::push_action( const account_name& code,
+                             const action_name& acttype,
+                             const vector<permission_level>& auths,
+                             const variant_object& data,
+                             uint32_t expiration,
+                             uint32_t delay_sec)
+
    { try {
       const auto& acnt = control->get_database().get<account_object,by_name>(code);
       auto abi = acnt.get_abi();
@@ -233,21 +250,18 @@ namespace eosio { namespace testing {
       action act;
       act.account = code;
       act.name = acttype;
-      for (const auto& actor : actors) {
-         act.authorization.push_back(permission_level{actor, config::active_name});
-      }
+      act.authorization = auths;
       act.data = abis.variant_to_binary(action_type_name, data);
 
       signed_transaction trx;
       trx.actions.emplace_back(std::move(act));
       set_transaction_headers(trx, expiration, delay_sec);
-      for (const auto& actor : actors) {
-         trx.sign(get_private_key(actor, "active"), chain_id_type());
+      for (const auto& auth : auths) {
+         trx.sign(get_private_key(auth.actor, auth.permission.to_string()), chain_id_type());
       }
 
       return push_transaction(trx);
-   } FC_CAPTURE_AND_RETHROW( (code)(acttype)(actors)(data)(expiration) ) }
-
+   } FC_CAPTURE_AND_RETHROW( (code)(acttype)(auths)(data)(expiration) ) }
 
    transaction_trace base_tester::push_reqauth( account_name from, const vector<permission_level>& auths, const vector<private_key_type>& keys ) {
       variant pretty_trx = fc::mutable_variant_object()

tests/chain_tests/auth_tests.cpp

@@ -82,6 +82,7 @@ BOOST_AUTO_TEST_CASE(update_auths) {
 try {
    TESTER chain;
    chain.create_account("alice");
+   chain.create_account("bob");
 
    // Deleting active or owner should fail
    BOOST_CHECK_THROW(chain.delete_authority("alice", "active"), action_validate_exception);
@@ -136,6 +137,11 @@ try {
    auto trading_priv_key = chain.get_private_key("alice", "trading");
    auto trading_pub_key = trading_priv_key.get_public_key();
 
+   // Bob attempts to create new spending auth for Alice
+   BOOST_CHECK_THROW( chain.set_authority( "alice", "spending", authority(spending_pub_key), "active",
+                                           { permission_level{"bob", "active"} }, { chain.get_private_key("bob", "active") } ),
+                      transaction_exception );
+
    // Create new spending auth
    chain.set_authority("alice", "spending", authority(spending_pub_key), "active",
                        { permission_level{"alice", "active"} }, { new_active_priv_key });

tests/chain_tests/delay_tests.cpp

@@ -1495,8 +1495,7 @@ BOOST_AUTO_TEST_CASE( canceldelay_test ) { try {
    // send canceldelay for first delayed transaction
    signed_transaction trx;
    trx.actions.emplace_back(vector<permission_level>{{N(tester), config::active_name}},
-                            chain::contracts::canceldelay{ids[0]});
-   trx.actions.back().authorization.push_back({N(tester), config::active_name});
+                            chain::contracts::canceldelay{{N(tester), config::active_name}, ids[0]});
 
    chain.set_transaction_headers(trx);
    trx.sign(chain.get_private_key(N(tester), "active"), chain_id_type());
@@ -1515,7 +1514,7 @@ BOOST_AUTO_TEST_CASE( canceldelay_test ) { try {
    liquid_balance = get_currency_balance(chain, N(tester2));
    BOOST_REQUIRE_EQUAL(asset::from_string("0.0000 CUR"), liquid_balance);
 
-   // first transfer will finally be performed
+   // update auth will finally be performed
    chain.produce_blocks();
 
    liquid_balance = get_currency_balance(chain, N(tester));
@@ -1561,6 +1560,259 @@ BOOST_AUTO_TEST_CASE( canceldelay_test ) { try {
    BOOST_REQUIRE_EQUAL(asset::from_string("85.0000 CUR"), liquid_balance);
    liquid_balance = get_currency_balance(chain, N(tester2));
    BOOST_REQUIRE_EQUAL(asset::from_string("15.0000 CUR"), liquid_balance);
-} FC_LOG_AND_RETHROW() }/// schedule_test
+} FC_LOG_AND_RETHROW() }
+
+// test canceldelay action under different permission levels
+BOOST_AUTO_TEST_CASE( canceldelay_test2 ) { try {
+   TESTER chain;
+
+   const auto& tester_account = N(tester);
+   std::vector<transaction_id_type> ids;
+   chain.set_code(config::system_account_name, eosio_system_wast);
+   chain.set_abi(config::system_account_name, eosio_system_abi);
+
+   chain.produce_blocks();
+   chain.create_account(N(currency));
+   chain.produce_blocks();
+
+   chain.set_code(N(currency), currency_wast);
+   chain.set_abi(N(currency), currency_abi);
+
+   chain.produce_blocks();
+   chain.create_account(N(tester));
+   chain.create_account(N(tester2));
+   chain.produce_blocks();
+
+   chain.push_action(config::system_account_name, contracts::updateauth::get_name(), tester_account, fc::mutable_variant_object()
+           ("account", "tester")
+           ("permission", "first")
+           ("parent", "active")
+           ("data",  authority(chain.get_public_key(tester_account, "first")))
+           ("delay", 5));
+   chain.push_action(config::system_account_name, contracts::updateauth::get_name(), tester_account, fc::mutable_variant_object()
+          ("account", "tester")
+          ("permission", "second")
+          ("parent", "first")
+          ("data",  authority(chain.get_public_key(tester_account, "second")))
+          ("delay", 0));
+   chain.push_action(config::system_account_name, contracts::linkauth::get_name(), tester_account, fc::mutable_variant_object()
+           ("account", "tester")
+           ("code", "currency")
+           ("type", "transfer")
+           ("requirement", "first"));
+
+   chain.produce_blocks();
+   chain.push_action(N(currency), N(create), N(currency), mutable_variant_object()
+           ("issuer", "currency" )
+           ("maximum_supply", "9000000.0000 CUR" )
+           ("can_freeze", 0)
+           ("can_recall", 0)
+           ("can_whitelist", 0)
+   );
+
+   chain.push_action(N(currency), name("issue"), N(currency), fc::mutable_variant_object()
+           ("to",       "currency")
+           ("quantity", "1000000.0000 CUR")
+           ("memo", "for stuff")
+   );
+
+   auto trace = chain.push_action(N(currency), name("transfer"), N(currency), fc::mutable_variant_object()
+       ("from", "currency")
+       ("to", "tester")
+       ("quantity", "100.0000 CUR")
+       ("memo", "hi" )
+   );
+   BOOST_REQUIRE_EQUAL(transaction_receipt::executed, trace.status);
+   BOOST_REQUIRE_EQUAL(0, trace.deferred_transaction_requests.size());
+
+   chain.produce_blocks();
+   auto liquid_balance = get_currency_balance(chain, N(currency));
+   BOOST_REQUIRE_EQUAL(asset::from_string("999900.0000 CUR"), liquid_balance);
+   liquid_balance = get_currency_balance(chain, N(tester));
+   BOOST_REQUIRE_EQUAL(asset::from_string("100.0000 CUR"), liquid_balance);
+
+   ilog("attempting first delayed transfer");
+
+   {
+      // this transaction will be delayed 10 blocks
+      trace = chain.push_action(N(currency), name("transfer"), vector<permission_level>{{N(tester), N(first)}}, fc::mutable_variant_object()
+          ("from", "tester")
+          ("to", "tester2")
+          ("quantity", "1.0000 CUR")
+          ("memo", "hi" ),
+          30, 5
+      );
+      auto trx_id = trace.id;
+      BOOST_REQUIRE_EQUAL(transaction_receipt::delayed, trace.status);
+      BOOST_REQUIRE_EQUAL(1, trace.deferred_transaction_requests.size());
+      BOOST_REQUIRE_EQUAL(0, trace.action_traces.size());
+
+      const auto sender_id_to_cancel = trace.deferred_transaction_requests[0].get<deferred_transaction>().sender_id;
+
+      chain.produce_blocks();
+
+      liquid_balance = get_currency_balance(chain, N(tester));
+      BOOST_REQUIRE_EQUAL(asset::from_string("100.0000 CUR"), liquid_balance);
+      liquid_balance = get_currency_balance(chain, N(tester2));
+      BOOST_REQUIRE_EQUAL(asset::from_string("0.0000 CUR"), liquid_balance);
+
+      // attempt canceldelay with wrong canceling_auth for delayed transfer of 1.0000 CUR
+      {
+         signed_transaction trx;
+         trx.actions.emplace_back(vector<permission_level>{{N(tester), config::active_name}},
+                                  chain::contracts::canceldelay{{N(tester), config::active_name}, trx_id});
+         chain.set_transaction_headers(trx);
+         trx.sign(chain.get_private_key(N(tester), "active"), chain_id_type());
+         BOOST_REQUIRE_THROW( chain.push_transaction(trx), transaction_exception );
+      }
+
+      // attempt canceldelay with "second" permission for delayed transfer of 1.0000 CUR
+      {
+         signed_transaction trx;
+         trx.actions.emplace_back(vector<permission_level>{{N(tester), N(second)}},
+                                  chain::contracts::canceldelay{{N(tester), N(first)}, trx_id});
+         chain.set_transaction_headers(trx);
+         trx.sign(chain.get_private_key(N(tester), "second"), chain_id_type());
+         BOOST_REQUIRE_THROW( chain.push_transaction(trx), tx_irrelevant_auth );
+      }
+
+      // canceldelay with "active" permission for delayed transfer of 1.0000 CUR
+      signed_transaction trx;
+      trx.actions.emplace_back(vector<permission_level>{{N(tester), config::active_name}},
+                               chain::contracts::canceldelay{{N(tester), N(first)}, trx_id});
+      chain.set_transaction_headers(trx);
+      trx.sign(chain.get_private_key(N(tester), "active"), chain_id_type());
+      trace = chain.push_transaction(trx);
+
+      BOOST_REQUIRE_EQUAL(transaction_receipt::executed, trace.status);
+      BOOST_REQUIRE_EQUAL(1, trace.deferred_transaction_requests.size());
+
+      const auto sender_id_canceled = trace.deferred_transaction_requests[0].get<deferred_reference>().sender_id;
+      BOOST_REQUIRE_EQUAL(std::string(uint128(sender_id_to_cancel)), std::string(uint128(sender_id_canceled)));
+
+      chain.produce_blocks(10);
+
+      liquid_balance = get_currency_balance(chain, N(tester));
+      BOOST_REQUIRE_EQUAL(asset::from_string("100.0000 CUR"), liquid_balance);
+      liquid_balance = get_currency_balance(chain, N(tester2));
+      BOOST_REQUIRE_EQUAL(asset::from_string("0.0000 CUR"), liquid_balance);
+   }
+
+   ilog("reset minimum permission of transfer to second permission");
+
+   chain.push_action(config::system_account_name, contracts::linkauth::get_name(), tester_account, fc::mutable_variant_object()
+           ("account", "tester")
+           ("code", "currency")
+           ("type", "transfer")
+           ("requirement", "second"),
+           30, 5
+   );
+
+   chain.produce_blocks(10);
+
+
+   ilog("attempting second delayed transfer");
+   {
+      // this transaction will be delayed 10 blocks
+      trace = chain.push_action(N(currency), name("transfer"), vector<permission_level>{{N(tester), N(second)}}, fc::mutable_variant_object()
+          ("from", "tester")
+          ("to", "tester2")
+          ("quantity", "5.0000 CUR")
+          ("memo", "hi" ),
+          30, 5
+      );
+      auto trx_id = trace.id;
+      BOOST_REQUIRE_EQUAL(transaction_receipt::delayed, trace.status);
+      BOOST_REQUIRE_EQUAL(1, trace.deferred_transaction_requests.size());
+      BOOST_REQUIRE_EQUAL(0, trace.action_traces.size());
+
+      const auto sender_id_to_cancel = trace.deferred_transaction_requests[0].get<deferred_transaction>().sender_id;
+
+      chain.produce_blocks();
+
+      liquid_balance = get_currency_balance(chain, N(tester));
+      BOOST_REQUIRE_EQUAL(asset::from_string("100.0000 CUR"), liquid_balance);
+      liquid_balance = get_currency_balance(chain, N(tester2));
+      BOOST_REQUIRE_EQUAL(asset::from_string("0.0000 CUR"), liquid_balance);
+
+      // canceldelay with "first" permission for delayed transfer of 5.0000 CUR
+      signed_transaction trx;
+      trx.actions.emplace_back(vector<permission_level>{{N(tester), N(first)}},
+                               chain::contracts::canceldelay{{N(tester), N(second)}, trx_id});
+      chain.set_transaction_headers(trx);
+      trx.sign(chain.get_private_key(N(tester), "first"), chain_id_type());
+      trace = chain.push_transaction(trx);
+
+      BOOST_REQUIRE_EQUAL(transaction_receipt::executed, trace.status);
+      BOOST_REQUIRE_EQUAL(1, trace.deferred_transaction_requests.size());
+
+      const auto sender_id_canceled = trace.deferred_transaction_requests[0].get<deferred_reference>().sender_id;
+      BOOST_REQUIRE_EQUAL(std::string(uint128(sender_id_to_cancel)), std::string(uint128(sender_id_canceled)));
+
+      chain.produce_blocks(10);
+
+      liquid_balance = get_currency_balance(chain, N(tester));
+      BOOST_REQUIRE_EQUAL(asset::from_string("100.0000 CUR"), liquid_balance);
+      liquid_balance = get_currency_balance(chain, N(tester2));
+      BOOST_REQUIRE_EQUAL(asset::from_string("0.0000 CUR"), liquid_balance);
+   }
+
+   ilog("attempting third delayed transfer");
+
+   {
+      // this transaction will be delayed 10 blocks
+      trace = chain.push_action(N(currency), name("transfer"), vector<permission_level>{{N(tester), config::owner_name}}, fc::mutable_variant_object()
+          ("from", "tester")
+          ("to", "tester2")
+          ("quantity", "10.0000 CUR")
+          ("memo", "hi" ),
+          30, 5
+      );
+      auto trx_id = trace.id;
+      BOOST_REQUIRE_EQUAL(transaction_receipt::delayed, trace.status);
+      BOOST_REQUIRE_EQUAL(1, trace.deferred_transaction_requests.size());
+      BOOST_REQUIRE_EQUAL(0, trace.action_traces.size());
+
+      const auto sender_id_to_cancel = trace.deferred_transaction_requests[0].get<deferred_transaction>().sender_id;
+
+      chain.produce_blocks();
+
+      liquid_balance = get_currency_balance(chain, N(tester));
+      BOOST_REQUIRE_EQUAL(asset::from_string("100.0000 CUR"), liquid_balance);
+      liquid_balance = get_currency_balance(chain, N(tester2));
+      BOOST_REQUIRE_EQUAL(asset::from_string("0.0000 CUR"), liquid_balance);
+
+      // attempt canceldelay with "active" permission for delayed transfer of 10.0000 CUR
+      {
+         signed_transaction trx;
+         trx.actions.emplace_back(vector<permission_level>{{N(tester), N(active)}},
+                                  chain::contracts::canceldelay{{N(tester), config::owner_name}, trx_id});
+         chain.set_transaction_headers(trx);
+         trx.sign(chain.get_private_key(N(tester), "active"), chain_id_type());
+         BOOST_REQUIRE_THROW( chain.push_transaction(trx), tx_irrelevant_auth );
+      }
+
+      // canceldelay with "owner" permission for delayed transfer of 10.0000 CUR
+      signed_transaction trx;
+      trx.actions.emplace_back(vector<permission_level>{{N(tester), config::owner_name}},
+                               chain::contracts::canceldelay{{N(tester), config::owner_name}, trx_id});
+      chain.set_transaction_headers(trx);
+      trx.sign(chain.get_private_key(N(tester), "owner"), chain_id_type());
+      trace = chain.push_transaction(trx);
+
+      BOOST_REQUIRE_EQUAL(transaction_receipt::executed, trace.status);
+      BOOST_REQUIRE_EQUAL(1, trace.deferred_transaction_requests.size());
+
+      const auto sender_id_canceled = trace.deferred_transaction_requests[0].get<deferred_reference>().sender_id;
+      BOOST_REQUIRE_EQUAL(std::string(uint128(sender_id_to_cancel)), std::string(uint128(sender_id_canceled)));
+
+      chain.produce_blocks(10);
+
+      liquid_balance = get_currency_balance(chain, N(tester));
+      BOOST_REQUIRE_EQUAL(asset::from_string("100.0000 CUR"), liquid_balance);
+      liquid_balance = get_currency_balance(chain, N(tester2));
+      BOOST_REQUIRE_EQUAL(asset::from_string("0.0000 CUR"), liquid_balance);
+   }
+} FC_LOG_AND_RETHROW() }
 
 BOOST_AUTO_TEST_SUITE_END()