Feature gate permissioned signer (#15622)

aptos-labs · Jan 15, 2025 · e00cb12 · e00cb12
1 parent c7452c7
commit e00cb12
Show file tree

Hide file tree

Showing 8 changed files with 218 additions and 0 deletions.
diff --git a/aptos-move/aptos-release-builder/src/components/feature_flags.rs b/aptos-move/aptos-release-builder/src/components/feature_flags.rs
@@ -135,6 +135,7 @@ pub enum FeatureFlag {
     EnableLoaderV2,
     DisallowInitModuleToPublishModules,
     EnableCallTreeAndInstructionVMCache,
+    PermissionedSigner,
 }
 
 fn generate_features_blob(writer: &CodeWriter, data: &[u64]) {
@@ -361,6 +362,7 @@ impl From<FeatureFlag> for AptosFeatureFlag {
             FeatureFlag::EnableCallTreeAndInstructionVMCache => {
                 AptosFeatureFlag::ENABLE_CALL_TREE_AND_INSTRUCTION_VM_CACHE
             },
+            FeatureFlag::PermissionedSigner => AptosFeatureFlag::PERMISSIONED_SIGNER,
         }
     }
 }
@@ -514,6 +516,7 @@ impl From<AptosFeatureFlag> for FeatureFlag {
             AptosFeatureFlag::ENABLE_CALL_TREE_AND_INSTRUCTION_VM_CACHE => {
                 FeatureFlag::EnableCallTreeAndInstructionVMCache
             },
+            AptosFeatureFlag::PERMISSIONED_SIGNER => FeatureFlag::PermissionedSigner,
         }
     }
 }

diff --git a/aptos-move/framework/aptos-framework/doc/permissioned_signer.md b/aptos-move/framework/aptos-framework/doc/permissioned_signer.md
@@ -78,6 +78,7 @@ for blind signing.
 <b>use</b> <a href="../../aptos-stdlib/doc/copyable_any.md#0x1_copyable_any">0x1::copyable_any</a>;
 <b>use</b> <a href="create_signer.md#0x1_create_signer">0x1::create_signer</a>;
 <b>use</b> <a href="../../aptos-stdlib/../move-stdlib/doc/error.md#0x1_error">0x1::error</a>;
+<b>use</b> <a href="../../aptos-stdlib/../move-stdlib/doc/features.md#0x1_features">0x1::features</a>;
 <b>use</b> <a href="../../aptos-stdlib/../move-stdlib/doc/option.md#0x1_option">0x1::option</a>;
 <b>use</b> <a href="../../aptos-stdlib/../move-stdlib/doc/signer.md#0x1_signer">0x1::signer</a>;
 <b>use</b> <a href="timestamp.md#0x1_timestamp">0x1::timestamp</a>;
@@ -391,6 +392,16 @@ Access permission information from a master signer.
 
 
 
+<a id="0x1_permissioned_signer_EPERMISSION_SIGNER_DISABLED"></a>
+
+Permissioned signer feature is not activated.
+
+
+<pre><code><b>const</b> <a href="permissioned_signer.md#0x1_permissioned_signer_EPERMISSION_SIGNER_DISABLED">EPERMISSION_SIGNER_DISABLED</a>: u64 = 9;
+</code></pre>
+
+
+
 <a id="0x1_permissioned_signer_E_NOT_ACTIVE"></a>
 
 destroying permission handle that has already been revoked or not owned by the
@@ -461,6 +472,11 @@ the current transaction.
 
 
 <pre><code><b>public</b> <b>fun</b> <a href="permissioned_signer.md#0x1_permissioned_signer_create_permissioned_handle">create_permissioned_handle</a>(master: &<a href="../../aptos-stdlib/../move-stdlib/doc/signer.md#0x1_signer">signer</a>): <a href="permissioned_signer.md#0x1_permissioned_signer_PermissionedHandle">PermissionedHandle</a> {
+    <b>assert</b>!(
+        <a href="../../aptos-stdlib/../move-stdlib/doc/features.md#0x1_features_is_permissioned_signer_enabled">features::is_permissioned_signer_enabled</a>(),
+        <a href="../../aptos-stdlib/../move-stdlib/doc/error.md#0x1_error_permission_denied">error::permission_denied</a>(<a href="permissioned_signer.md#0x1_permissioned_signer_EPERMISSION_SIGNER_DISABLED">EPERMISSION_SIGNER_DISABLED</a>)
+    );
+
     <a href="permissioned_signer.md#0x1_permissioned_signer_assert_master_signer">assert_master_signer</a>(master);
     <b>let</b> permissions_storage_addr = generate_auid_address();
     <b>let</b> master_account_addr = <a href="../../aptos-stdlib/../move-stdlib/doc/signer.md#0x1_signer_address_of">signer::address_of</a>(master);
@@ -492,6 +508,10 @@ Destroys an ephermeral permission handle. Clean up the permission stored in that
 
 
 <pre><code><b>public</b> <b>fun</b> <a href="permissioned_signer.md#0x1_permissioned_signer_destroy_permissioned_handle">destroy_permissioned_handle</a>(p: <a href="permissioned_signer.md#0x1_permissioned_signer_PermissionedHandle">PermissionedHandle</a>) <b>acquires</b> <a href="permissioned_signer.md#0x1_permissioned_signer_PermissionStorage">PermissionStorage</a> {
+    <b>assert</b>!(
+        <a href="../../aptos-stdlib/../move-stdlib/doc/features.md#0x1_features_is_permissioned_signer_enabled">features::is_permissioned_signer_enabled</a>(),
+        <a href="../../aptos-stdlib/../move-stdlib/doc/error.md#0x1_error_permission_denied">error::permission_denied</a>(<a href="permissioned_signer.md#0x1_permissioned_signer_EPERMISSION_SIGNER_DISABLED">EPERMISSION_SIGNER_DISABLED</a>)
+    );
     <b>let</b> PermissionedHandle::V1 { master_account_addr: _, permissions_storage_addr } =
         p;
     <a href="permissioned_signer.md#0x1_permissioned_signer_destroy_permissions_storage_address">destroy_permissions_storage_address</a>(permissions_storage_addr);
@@ -523,6 +543,10 @@ and would abort if check fails.
 
 
 <pre><code><b>public</b> <b>fun</b> <a href="permissioned_signer.md#0x1_permissioned_signer_signer_from_permissioned_handle">signer_from_permissioned_handle</a>(p: &<a href="permissioned_signer.md#0x1_permissioned_signer_PermissionedHandle">PermissionedHandle</a>): <a href="../../aptos-stdlib/../move-stdlib/doc/signer.md#0x1_signer">signer</a> {
+    <b>assert</b>!(
+        <a href="../../aptos-stdlib/../move-stdlib/doc/features.md#0x1_features_is_permissioned_signer_enabled">features::is_permissioned_signer_enabled</a>(),
+        <a href="../../aptos-stdlib/../move-stdlib/doc/error.md#0x1_error_permission_denied">error::permission_denied</a>(<a href="permissioned_signer.md#0x1_permissioned_signer_EPERMISSION_SIGNER_DISABLED">EPERMISSION_SIGNER_DISABLED</a>)
+    );
     <a href="permissioned_signer.md#0x1_permissioned_signer_signer_from_permissioned_handle_impl">signer_from_permissioned_handle_impl</a>(
         p.master_account_addr, p.permissions_storage_addr
     )
@@ -550,6 +574,12 @@ Returns true if <code>s</code> is a permissioned signer.
 
 
 <pre><code><b>public</b> <b>fun</b> <a href="permissioned_signer.md#0x1_permissioned_signer_is_permissioned_signer">is_permissioned_signer</a>(s: &<a href="../../aptos-stdlib/../move-stdlib/doc/signer.md#0x1_signer">signer</a>): bool {
+    // When the permissioned <a href="../../aptos-stdlib/../move-stdlib/doc/signer.md#0x1_signer">signer</a> is disabled, no one is able <b>to</b> construct a permissioned
+    // <a href="../../aptos-stdlib/../move-stdlib/doc/signer.md#0x1_signer">signer</a>. Thus we should <b>return</b> <b>false</b> here, <b>as</b> other on chain permission checks will
+    // depend on this checks.
+    <b>if</b>(!<a href="../../aptos-stdlib/../move-stdlib/doc/features.md#0x1_features_is_permissioned_signer_enabled">features::is_permissioned_signer_enabled</a>()) {
+        <b>return</b> <b>false</b>;
+    };
     <a href="permissioned_signer.md#0x1_permissioned_signer_is_permissioned_signer_impl">is_permissioned_signer_impl</a>(s)
 }
 </code></pre>
@@ -579,6 +609,10 @@ its address.
     master: &<a href="../../aptos-stdlib/../move-stdlib/doc/signer.md#0x1_signer">signer</a>,
     permissioned: &<a href="../../aptos-stdlib/../move-stdlib/doc/signer.md#0x1_signer">signer</a>,
 ) <b>acquires</b> <a href="permissioned_signer.md#0x1_permissioned_signer_PermissionStorage">PermissionStorage</a> {
+    <b>assert</b>!(
+        <a href="../../aptos-stdlib/../move-stdlib/doc/features.md#0x1_features_is_permissioned_signer_enabled">features::is_permissioned_signer_enabled</a>(),
+        <a href="../../aptos-stdlib/../move-stdlib/doc/error.md#0x1_error_permission_denied">error::permission_denied</a>(<a href="permissioned_signer.md#0x1_permissioned_signer_EPERMISSION_SIGNER_DISABLED">EPERMISSION_SIGNER_DISABLED</a>)
+    );
     <a href="permissioned_signer.md#0x1_permissioned_signer_authorize_unlimited">authorize_unlimited</a>(master, permissioned, <a href="permissioned_signer.md#0x1_permissioned_signer_RevokePermissionHandlePermission">RevokePermissionHandlePermission</a> {});
 }
 </code></pre>
@@ -607,6 +641,10 @@ the storable permission handle to derive signer from it anymore.
 <pre><code><b>public</b> entry <b>fun</b> <a href="permissioned_signer.md#0x1_permissioned_signer_revoke_permission_storage_address">revoke_permission_storage_address</a>(
     s: &<a href="../../aptos-stdlib/../move-stdlib/doc/signer.md#0x1_signer">signer</a>, permissions_storage_addr: <b>address</b>
 ) <b>acquires</b> <a href="permissioned_signer.md#0x1_permissioned_signer_GrantedPermissionHandles">GrantedPermissionHandles</a>, <a href="permissioned_signer.md#0x1_permissioned_signer_PermissionStorage">PermissionStorage</a> {
+    <b>assert</b>!(
+        <a href="../../aptos-stdlib/../move-stdlib/doc/features.md#0x1_features_is_permissioned_signer_enabled">features::is_permissioned_signer_enabled</a>(),
+        <a href="../../aptos-stdlib/../move-stdlib/doc/error.md#0x1_error_permission_denied">error::permission_denied</a>(<a href="permissioned_signer.md#0x1_permissioned_signer_EPERMISSION_SIGNER_DISABLED">EPERMISSION_SIGNER_DISABLED</a>)
+    );
     <b>assert</b>!(
         <a href="permissioned_signer.md#0x1_permissioned_signer_check_permission_exists">check_permission_exists</a>(s, <a href="permissioned_signer.md#0x1_permissioned_signer_RevokePermissionHandlePermission">RevokePermissionHandlePermission</a> {}),
         <a href="../../aptos-stdlib/../move-stdlib/doc/error.md#0x1_error_permission_denied">error::permission_denied</a>(<a href="permissioned_signer.md#0x1_permissioned_signer_ENOT_MASTER_SIGNER">ENOT_MASTER_SIGNER</a>)
@@ -648,6 +686,10 @@ Revoke all storable permission handle of the signer immediately.
 
 
 <pre><code><b>public</b> entry <b>fun</b> <a href="permissioned_signer.md#0x1_permissioned_signer_revoke_all_handles">revoke_all_handles</a>(s: &<a href="../../aptos-stdlib/../move-stdlib/doc/signer.md#0x1_signer">signer</a>) <b>acquires</b> <a href="permissioned_signer.md#0x1_permissioned_signer_GrantedPermissionHandles">GrantedPermissionHandles</a>, <a href="permissioned_signer.md#0x1_permissioned_signer_PermissionStorage">PermissionStorage</a> {
+    <b>assert</b>!(
+        <a href="../../aptos-stdlib/../move-stdlib/doc/features.md#0x1_features_is_permissioned_signer_enabled">features::is_permissioned_signer_enabled</a>(),
+        <a href="../../aptos-stdlib/../move-stdlib/doc/error.md#0x1_error_permission_denied">error::permission_denied</a>(<a href="permissioned_signer.md#0x1_permissioned_signer_EPERMISSION_SIGNER_DISABLED">EPERMISSION_SIGNER_DISABLED</a>)
+    );
     <b>assert</b>!(
         <a href="permissioned_signer.md#0x1_permissioned_signer_check_permission_exists">check_permission_exists</a>(s, <a href="permissioned_signer.md#0x1_permissioned_signer_RevokePermissionHandlePermission">RevokePermissionHandlePermission</a> {}),
         <a href="../../aptos-stdlib/../move-stdlib/doc/error.md#0x1_error_permission_denied">error::permission_denied</a>(<a href="permissioned_signer.md#0x1_permissioned_signer_ENOT_MASTER_SIGNER">ENOT_MASTER_SIGNER</a>)
@@ -726,6 +768,11 @@ The caller should check if <code>expiration_time</code> is not too far in the fu
 <pre><code><b>public</b>(<b>package</b>) <b>fun</b> <a href="permissioned_signer.md#0x1_permissioned_signer_create_storable_permissioned_handle">create_storable_permissioned_handle</a>(
     master: &<a href="../../aptos-stdlib/../move-stdlib/doc/signer.md#0x1_signer">signer</a>, expiration_time: u64
 ): <a href="permissioned_signer.md#0x1_permissioned_signer_StorablePermissionedHandle">StorablePermissionedHandle</a> <b>acquires</b> <a href="permissioned_signer.md#0x1_permissioned_signer_GrantedPermissionHandles">GrantedPermissionHandles</a> {
+    <b>assert</b>!(
+        <a href="../../aptos-stdlib/../move-stdlib/doc/features.md#0x1_features_is_permissioned_signer_enabled">features::is_permissioned_signer_enabled</a>(),
+        <a href="../../aptos-stdlib/../move-stdlib/doc/error.md#0x1_error_permission_denied">error::permission_denied</a>(<a href="permissioned_signer.md#0x1_permissioned_signer_EPERMISSION_SIGNER_DISABLED">EPERMISSION_SIGNER_DISABLED</a>)
+    );
+
     <a href="permissioned_signer.md#0x1_permissioned_signer_assert_master_signer">assert_master_signer</a>(master);
     <b>let</b> permissions_storage_addr = generate_auid_address();
     <b>let</b> master_account_addr = <a href="../../aptos-stdlib/../move-stdlib/doc/signer.md#0x1_signer_address_of">signer::address_of</a>(master);
@@ -856,6 +903,10 @@ Generate the permissioned signer based on the storable permission handle.
 <pre><code><b>public</b>(<b>package</b>) <b>fun</b> <a href="permissioned_signer.md#0x1_permissioned_signer_signer_from_storable_permissioned_handle">signer_from_storable_permissioned_handle</a>(
     p: &<a href="permissioned_signer.md#0x1_permissioned_signer_StorablePermissionedHandle">StorablePermissionedHandle</a>
 ): <a href="../../aptos-stdlib/../move-stdlib/doc/signer.md#0x1_signer">signer</a> {
+    <b>assert</b>!(
+        <a href="../../aptos-stdlib/../move-stdlib/doc/features.md#0x1_features_is_permissioned_signer_enabled">features::is_permissioned_signer_enabled</a>(),
+        <a href="../../aptos-stdlib/../move-stdlib/doc/error.md#0x1_error_permission_denied">error::permission_denied</a>(<a href="permissioned_signer.md#0x1_permissioned_signer_EPERMISSION_SIGNER_DISABLED">EPERMISSION_SIGNER_DISABLED</a>)
+    );
     <b>assert</b>!(
         <a href="timestamp.md#0x1_timestamp_now_seconds">timestamp::now_seconds</a>() &lt; p.expiration_time,
         <a href="../../aptos-stdlib/../move-stdlib/doc/error.md#0x1_error_permission_denied">error::permission_denied</a>(<a href="permissioned_signer.md#0x1_permissioned_signer_E_PERMISSION_EXPIRED">E_PERMISSION_EXPIRED</a>)

diff --git a/aptos-move/framework/aptos-framework/sources/permissioned_signer.move b/aptos-move/framework/aptos-framework/sources/permissioned_signer.move
@@ -14,6 +14,7 @@
 /// After introducing the core functionality, examples are provided for withdraw limit on accounts, and
 /// for blind signing.
 module aptos_framework::permissioned_signer {
+    use std::features;
     use std::signer;
     use std::error;
     use std::vector;
@@ -49,6 +50,9 @@ module aptos_framework::permissioned_signer {
     /// given master signer.
     const E_NOT_ACTIVE: u64 = 8;
 
+    /// Permissioned signer feature is not activated.
+    const EPERMISSION_SIGNER_DISABLED: u64 = 9;
+
     const U256_MAX: u256 =
         115792089237316195423570985008687907853269984665640564039457584007913129639935;
 
@@ -113,6 +117,11 @@ module aptos_framework::permissioned_signer {
     /// This handle can be used to derive a signer that can be used in the context of
     /// the current transaction.
     public fun create_permissioned_handle(master: &signer): PermissionedHandle {
+        assert!(
+            features::is_permissioned_signer_enabled(),
+            error::permission_denied(EPERMISSION_SIGNER_DISABLED)
+        );
+
         assert_master_signer(master);
         let permissions_storage_addr = generate_auid_address();
         let master_account_addr = signer::address_of(master);
@@ -124,6 +133,10 @@ module aptos_framework::permissioned_signer {
 
     /// Destroys an ephermeral permission handle. Clean up the permission stored in that handle
     public fun destroy_permissioned_handle(p: PermissionedHandle) acquires PermissionStorage {
+        assert!(
+            features::is_permissioned_signer_enabled(),
+            error::permission_denied(EPERMISSION_SIGNER_DISABLED)
+        );
         let PermissionedHandle::V1 { master_account_addr: _, permissions_storage_addr } =
             p;
         destroy_permissions_storage_address(permissions_storage_addr);
@@ -135,13 +148,23 @@ module aptos_framework::permissioned_signer {
     /// signer interacts with various framework functions, it would subject to permission checks
     /// and would abort if check fails.
     public fun signer_from_permissioned_handle(p: &PermissionedHandle): signer {
+        assert!(
+            features::is_permissioned_signer_enabled(),
+            error::permission_denied(EPERMISSION_SIGNER_DISABLED)
+        );
         signer_from_permissioned_handle_impl(
             p.master_account_addr, p.permissions_storage_addr
         )
     }
 
     /// Returns true if `s` is a permissioned signer.
     public fun is_permissioned_signer(s: &signer): bool {
+        // When the permissioned signer is disabled, no one is able to construct a permissioned
+        // signer. Thus we should return false here, as other on chain permission checks will
+        // depend on this checks.
+        if(!features::is_permissioned_signer_enabled()) {
+            return false;
+        };
         is_permissioned_signer_impl(s)
     }
 
@@ -151,6 +174,10 @@ module aptos_framework::permissioned_signer {
         master: &signer,
         permissioned: &signer,
     ) acquires PermissionStorage {
+        assert!(
+            features::is_permissioned_signer_enabled(),
+            error::permission_denied(EPERMISSION_SIGNER_DISABLED)
+        );
         authorize_unlimited(master, permissioned, RevokePermissionHandlePermission {});
     }
 
@@ -159,6 +186,10 @@ module aptos_framework::permissioned_signer {
     public entry fun revoke_permission_storage_address(
         s: &signer, permissions_storage_addr: address
     ) acquires GrantedPermissionHandles, PermissionStorage {
+        assert!(
+            features::is_permissioned_signer_enabled(),
+            error::permission_denied(EPERMISSION_SIGNER_DISABLED)
+        );
         assert!(
             check_permission_exists(s, RevokePermissionHandlePermission {}),
             error::permission_denied(ENOT_MASTER_SIGNER)
@@ -180,6 +211,10 @@ module aptos_framework::permissioned_signer {
 
     /// Revoke all storable permission handle of the signer immediately.
     public entry fun revoke_all_handles(s: &signer) acquires GrantedPermissionHandles, PermissionStorage {
+        assert!(
+            features::is_permissioned_signer_enabled(),
+            error::permission_denied(EPERMISSION_SIGNER_DISABLED)
+        );
         assert!(
             check_permission_exists(s, RevokePermissionHandlePermission {}),
             error::permission_denied(ENOT_MASTER_SIGNER)
@@ -218,6 +253,11 @@ module aptos_framework::permissioned_signer {
     public(package) fun create_storable_permissioned_handle(
         master: &signer, expiration_time: u64
     ): StorablePermissionedHandle acquires GrantedPermissionHandles {
+        assert!(
+            features::is_permissioned_signer_enabled(),
+            error::permission_denied(EPERMISSION_SIGNER_DISABLED)
+        );
+
         assert_master_signer(master);
         let permissions_storage_addr = generate_auid_address();
         let master_account_addr = signer::address_of(master);
@@ -288,6 +328,10 @@ module aptos_framework::permissioned_signer {
     public(package) fun signer_from_storable_permissioned_handle(
         p: &StorablePermissionedHandle
     ): signer {
+        assert!(
+            features::is_permissioned_signer_enabled(),
+            error::permission_denied(EPERMISSION_SIGNER_DISABLED)
+        );
         assert!(
             timestamp::now_seconds() < p.expiration_time,
             error::permission_denied(E_PERMISSION_EXPIRED)

diff --git a/aptos-move/framework/aptos-framework/tests/permissioned_signer_tests.move b/aptos-move/framework/aptos-framework/tests/permissioned_signer_tests.move
@@ -1,6 +1,7 @@
 #[test_only]
 module aptos_framework::permissioned_signer_tests {
     use std::bcs;
+    use std::features;
     use aptos_framework::account::create_signer_for_test;
     use aptos_framework::permissioned_signer;
     use aptos_framework::timestamp;
@@ -243,6 +244,28 @@ module aptos_framework::permissioned_signer_tests {
         );
     }
 
+    // Making sure master signer always have all permissions even when feature is disabled.
+    #[test(creator = @aptos_framework)]
+    fun test_master_signer_permission(creator: &signer) {
+        assert!(
+            permissioned_signer::check_permission_exists(creator, OnePermission {}),
+            1
+        );
+
+        // Disable the permissioned signer feature.
+        features::change_feature_flags_for_testing(
+            creator,
+            vector[],
+            vector[features::get_permissioned_signer_feature()]
+        );
+
+        // Master signer should still have permission after feature is disabled.
+        assert!(
+            permissioned_signer::check_permission_exists(creator, OnePermission {}),
+            1
+        );
+    }
+
     // creating permission using a permissioned signer
     #[test(creator = @0xcafe)]
     #[expected_failure(