[move] rotate_authentication_key_call should not modify OriginatingAddress (aptos-labs#12108)

Co-authored-by: Alin Tomescu <[email protected]>

Author: davidiw

aptos-move/framework/aptos-framework/doc/account.md

@@ -1143,8 +1143,11 @@ is returned. This way, the caller of this function can publish additional resour
 
 ## Function `rotate_authentication_key_internal`
 
-This function is used to rotate a resource account's authentication key to 0, so that no private key can control
-the resource account.
+This function is used to rotate a resource account's authentication key to <code>new_auth_key</code>. This is done in
+many contexts:
+1. During normal key rotation via <code>rotate_authentication_key</code> or <code>rotate_authentication_key_call</code>
+2. During resource account initialization so that no private key can control the resource account
+3. During multisig_v2 account creation
 
 
 <pre><code><b>public</b>(<b>friend</b>) <b>fun</b> <a href="account.md#0x1_account_rotate_authentication_key_internal">rotate_authentication_key_internal</a>(<a href="account.md#0x1_account">account</a>: &<a href="../../aptos-stdlib/../move-stdlib/doc/signer.md#0x1_signer">signer</a>, new_auth_key: <a href="../../aptos-stdlib/../move-stdlib/doc/vector.md#0x1_vector">vector</a>&lt;u8&gt;)
@@ -1176,7 +1179,11 @@ the resource account.
 
 ## Function `rotate_authentication_key_call`
 
-Entry function-only rotation key function that allows the signer update their authentication_key.
+Private entry function for key rotation that allows the signer to update their authentication key.
+Note that this does not update the <code><a href="account.md#0x1_account_OriginatingAddress">OriginatingAddress</a></code> table because the <code>new_auth_key</code> is not "verified": it
+does not come with a proof-of-knowledge of the underlying SK. Nonetheless, we need this functionality due to
+the introduction of non-standard key algorithms, such as passkeys, which cannot produce proofs-of-knowledge in
+the format expected in <code>rotate_authentication_key</code>.
 
 
 <pre><code>entry <b>fun</b> <a href="account.md#0x1_account_rotate_authentication_key_call">rotate_authentication_key_call</a>(<a href="account.md#0x1_account">account</a>: &<a href="../../aptos-stdlib/../move-stdlib/doc/signer.md#0x1_signer">signer</a>, new_auth_key: <a href="../../aptos-stdlib/../move-stdlib/doc/vector.md#0x1_vector">vector</a>&lt;u8&gt;)
@@ -1188,15 +1195,8 @@ Entry function-only rotation key function that allows the signer update their au
 <summary>Implementation</summary>
 
 
-<pre><code>entry <b>fun</b> <a href="account.md#0x1_account_rotate_authentication_key_call">rotate_authentication_key_call</a>(<a href="account.md#0x1_account">account</a>: &<a href="../../aptos-stdlib/../move-stdlib/doc/signer.md#0x1_signer">signer</a>, new_auth_key: <a href="../../aptos-stdlib/../move-stdlib/doc/vector.md#0x1_vector">vector</a>&lt;u8&gt;) <b>acquires</b> <a href="account.md#0x1_account_Account">Account</a>, <a href="account.md#0x1_account_OriginatingAddress">OriginatingAddress</a> {
-    <b>let</b> addr = <a href="../../aptos-stdlib/../move-stdlib/doc/signer.md#0x1_signer_address_of">signer::address_of</a>(<a href="account.md#0x1_account">account</a>);
-    <b>assert</b>!(<a href="account.md#0x1_account_exists_at">exists_at</a>(addr), <a href="../../aptos-stdlib/../move-stdlib/doc/error.md#0x1_error_not_found">error::not_found</a>(<a href="account.md#0x1_account_EACCOUNT_DOES_NOT_EXIST">EACCOUNT_DOES_NOT_EXIST</a>));
-    <b>assert</b>!(
-        <a href="../../aptos-stdlib/../move-stdlib/doc/vector.md#0x1_vector_length">vector::length</a>(&new_auth_key) == 32,
-        <a href="../../aptos-stdlib/../move-stdlib/doc/error.md#0x1_error_invalid_argument">error::invalid_argument</a>(<a href="account.md#0x1_account_EMALFORMED_AUTHENTICATION_KEY">EMALFORMED_AUTHENTICATION_KEY</a>)
-    );
-    <b>let</b> account_resource = <b>borrow_global_mut</b>&lt;<a href="account.md#0x1_account_Account">Account</a>&gt;(addr);
-    <a href="account.md#0x1_account_update_auth_key_and_originating_address_table">update_auth_key_and_originating_address_table</a>(addr, account_resource, new_auth_key);
+<pre><code>entry <b>fun</b> <a href="account.md#0x1_account_rotate_authentication_key_call">rotate_authentication_key_call</a>(<a href="account.md#0x1_account">account</a>: &<a href="../../aptos-stdlib/../move-stdlib/doc/signer.md#0x1_signer">signer</a>, new_auth_key: <a href="../../aptos-stdlib/../move-stdlib/doc/vector.md#0x1_vector">vector</a>&lt;u8&gt;) <b>acquires</b> <a href="account.md#0x1_account_Account">Account</a> {
+    <a href="account.md#0x1_account_rotate_authentication_key_internal">rotate_authentication_key_internal</a>(<a href="account.md#0x1_account">account</a>, new_auth_key);
 }
 </code></pre>
 
@@ -2526,29 +2526,15 @@ The length of new_auth_key is 32.
 </code></pre>
 
 
-The Account existed under the signer before the call.
-The length of new_auth_key is 32.
 
 
 <pre><code><b>let</b> addr = <a href="../../aptos-stdlib/../move-stdlib/doc/signer.md#0x1_signer_address_of">signer::address_of</a>(<a href="account.md#0x1_account">account</a>);
+// This enforces <a id="high-level-req-10" href="#high-level-req">high-level requirement 10</a>:
+<b>let</b> <b>post</b> account_resource = <b>global</b>&lt;<a href="account.md#0x1_account_Account">Account</a>&gt;(addr);
 <b>aborts_if</b> !<b>exists</b>&lt;<a href="account.md#0x1_account_Account">Account</a>&gt;(addr);
 <b>aborts_if</b> <a href="../../aptos-stdlib/../move-stdlib/doc/vector.md#0x1_vector_length">vector::length</a>(new_auth_key) != 32;
-<b>let</b> account_resource = <b>global</b>&lt;<a href="account.md#0x1_account_Account">Account</a>&gt;(addr);
-<b>let</b> curr_auth_key = <a href="../../aptos-stdlib/doc/from_bcs.md#0x1_from_bcs_deserialize">from_bcs::deserialize</a>&lt;<b>address</b>&gt;(account_resource.authentication_key);
-<b>let</b> originating_addr = addr;
-<b>let</b> address_map = <b>global</b>&lt;<a href="account.md#0x1_account_OriginatingAddress">OriginatingAddress</a>&gt;(@aptos_framework).address_map;
-<b>let</b> new_auth_key_addr = <a href="../../aptos-stdlib/doc/from_bcs.md#0x1_from_bcs_deserialize">from_bcs::deserialize</a>&lt;<b>address</b>&gt;(new_auth_key);
-<b>aborts_if</b> !<b>exists</b>&lt;<a href="account.md#0x1_account_OriginatingAddress">OriginatingAddress</a>&gt;(@aptos_framework);
-<b>aborts_if</b> !<a href="../../aptos-stdlib/doc/from_bcs.md#0x1_from_bcs_deserializable">from_bcs::deserializable</a>&lt;<b>address</b>&gt;(account_resource.authentication_key);
-<b>aborts_if</b> <a href="../../aptos-stdlib/doc/table.md#0x1_table_spec_contains">table::spec_contains</a>(address_map, curr_auth_key) &&
-    <a href="../../aptos-stdlib/doc/table.md#0x1_table_spec_get">table::spec_get</a>(address_map, curr_auth_key) != originating_addr;
-<b>aborts_if</b> curr_auth_key != new_auth_key_addr && <a href="../../aptos-stdlib/doc/table.md#0x1_table_spec_contains">table::spec_contains</a>(address_map, new_auth_key_addr);
-<b>include</b> <a href="account.md#0x1_account_UpdateAuthKeyAndOriginatingAddressTableAbortsIf">UpdateAuthKeyAndOriginatingAddressTableAbortsIf</a> {
-    originating_addr: addr,
-    new_auth_key_vector: new_auth_key,
-};
-<b>let</b> <b>post</b> auth_key = <b>global</b>&lt;<a href="account.md#0x1_account_Account">Account</a>&gt;(addr).authentication_key;
-<b>ensures</b> auth_key == new_auth_key;
+<b>modifies</b> <b>global</b>&lt;<a href="account.md#0x1_account_Account">Account</a>&gt;(addr);
+<b>ensures</b> account_resource.authentication_key == new_auth_key;
 </code></pre>
 
 

aptos-move/framework/aptos-framework/sources/account.move

@@ -265,8 +265,11 @@ module aptos_framework::account {
         borrow_global<Account>(addr).authentication_key
     }
 
-    /// This function is used to rotate a resource account's authentication key to 0, so that no private key can control
-    /// the resource account.
+    /// This function is used to rotate a resource account's authentication key to `new_auth_key`. This is done in
+    /// many contexts:
+    /// 1. During normal key rotation via `rotate_authentication_key` or `rotate_authentication_key_call`
+    /// 2. During resource account initialization so that no private key can control the resource account
+    /// 3. During multisig_v2 account creation
     public(friend) fun rotate_authentication_key_internal(account: &signer, new_auth_key: vector<u8>) acquires Account {
         let addr = signer::address_of(account);
         assert!(exists_at(addr), error::not_found(EACCOUNT_DOES_NOT_EXIST));
@@ -278,16 +281,13 @@ module aptos_framework::account {
         account_resource.authentication_key = new_auth_key;
     }
 
-    /// Entry function-only rotation key function that allows the signer update their authentication_key.
-    entry fun rotate_authentication_key_call(account: &signer, new_auth_key: vector<u8>) acquires Account, OriginatingAddress {
-        let addr = signer::address_of(account);
-        assert!(exists_at(addr), error::not_found(EACCOUNT_DOES_NOT_EXIST));
-        assert!(
-            vector::length(&new_auth_key) == 32,
-            error::invalid_argument(EMALFORMED_AUTHENTICATION_KEY)
-        );
-        let account_resource = borrow_global_mut<Account>(addr);
-        update_auth_key_and_originating_address_table(addr, account_resource, new_auth_key);
+    /// Private entry function for key rotation that allows the signer to update their authentication key.
+    /// Note that this does not update the `OriginatingAddress` table because the `new_auth_key` is not "verified": it
+    /// does not come with a proof-of-knowledge of the underlying SK. Nonetheless, we need this functionality due to
+    /// the introduction of non-standard key algorithms, such as passkeys, which cannot produce proofs-of-knowledge in
+    /// the format expected in `rotate_authentication_key`.
+    entry fun rotate_authentication_key_call(account: &signer, new_auth_key: vector<u8>) acquires Account {
+        rotate_authentication_key_internal(account, new_auth_key);
     }
 
     /// Generic authentication key rotation function that allows the user to rotate their authentication key from any scheme to any scheme.
@@ -1337,7 +1337,7 @@ module aptos_framework::account {
 
 
     #[test(account = @aptos_framework)]
-    public entry fun test_simple_rotation(account: &signer) acquires Account, OriginatingAddress {
+    public entry fun test_simple_rotation(account: &signer) acquires Account {
         initialize(account);
 
         let alice_addr = @0x1234;
@@ -1349,10 +1349,6 @@ module aptos_framework::account {
         let new_addr = from_bcs::to_address(new_auth_key);
 
         rotate_authentication_key_call(&alice, new_auth_key);
-
-        let address_map = &mut borrow_global_mut<OriginatingAddress>(@aptos_framework).address_map;
-        let expected_originating_address = table::borrow(address_map, new_addr);
-        assert!(*expected_originating_address == alice_addr, 0);
         assert!(borrow_global<Account>(alice_addr).authentication_key == new_auth_key, 0);
     }
 

aptos-move/framework/aptos-framework/sources/account.spec.move

@@ -203,35 +203,14 @@ spec aptos_framework::account {
         ensures account_resource.authentication_key == new_auth_key;
     }
 
-    /// The Account existed under the signer before the call.
-    /// The length of new_auth_key is 32.
     spec rotate_authentication_key_call(account: &signer, new_auth_key: vector<u8>) {
         let addr = signer::address_of(account);
+        /// [high-level-req-10]
+        let post account_resource = global<Account>(addr);
         aborts_if !exists<Account>(addr);
         aborts_if vector::length(new_auth_key) != 32;
-        let account_resource = global<Account>(addr);
-        let curr_auth_key = from_bcs::deserialize<address>(account_resource.authentication_key);
-
-        // Verify all properties in update_auth_key_and_originating_address_table
-        let originating_addr = addr;
-
-        let address_map = global<OriginatingAddress>(@aptos_framework).address_map;
-        let new_auth_key_addr = from_bcs::deserialize<address>(new_auth_key);
-
-        aborts_if !exists<OriginatingAddress>(@aptos_framework);
-        aborts_if !from_bcs::deserializable<address>(account_resource.authentication_key);
-        aborts_if table::spec_contains(address_map, curr_auth_key) &&
-            table::spec_get(address_map, curr_auth_key) != originating_addr;
-
-        aborts_if curr_auth_key != new_auth_key_addr && table::spec_contains(address_map, new_auth_key_addr);
-
-        include UpdateAuthKeyAndOriginatingAddressTableAbortsIf {
-            originating_addr: addr,
-            new_auth_key_vector: new_auth_key,
-        };
-
-        let post auth_key = global<Account>(addr).authentication_key;
-        ensures auth_key == new_auth_key;
+        modifies global<Account>(addr);
+        ensures account_resource.authentication_key == new_auth_key;
     }
 
     spec fun spec_assert_valid_rotation_proof_signature_and_get_auth_key(scheme: u8, public_key_bytes: vector<u8>, signature: vector<u8>, challenge: RotationProofChallenge): vector<u8>;

aptos-move/framework/cached-packages/src/aptos_framework_sdk_builder.rs

@@ -131,7 +131,11 @@ pub enum EntryFunctionCall {
         cap_update_table: Vec<u8>,
     },
 
-    /// Entry function-only rotation key function that allows the signer update their authentication_key.
+    /// Private entry function for key rotation that allows the signer to update their authentication key.
+    /// Note that this does not update the `OriginatingAddress` table because the `new_auth_key` is not "verified": it
+    /// does not come with a proof-of-knowledge of the underlying SK. Nonetheless, we need this functionality due to
+    /// the introduction of non-standard key algorithms, such as passkeys, which cannot produce proofs-of-knowledge in
+    /// the format expected in `rotate_authentication_key`.
     AccountRotateAuthenticationKeyCall {
         new_auth_key: Vec<u8>,
     },
@@ -1676,7 +1680,11 @@ pub fn account_rotate_authentication_key(
     ))
 }
 
-/// Entry function-only rotation key function that allows the signer update their authentication_key.
+/// Private entry function for key rotation that allows the signer to update their authentication key.
+/// Note that this does not update the `OriginatingAddress` table because the `new_auth_key` is not "verified": it
+/// does not come with a proof-of-knowledge of the underlying SK. Nonetheless, we need this functionality due to
+/// the introduction of non-standard key algorithms, such as passkeys, which cannot produce proofs-of-knowledge in
+/// the format expected in `rotate_authentication_key`.
 pub fn account_rotate_authentication_key_call(new_auth_key: Vec<u8>) -> TransactionPayload {
     TransactionPayload::EntryFunction(EntryFunction::new(
         ModuleId::new(