[Aptos Framework] Update coin::register to not fail on duplicate regi…

…strations
aptos-labs · Dec 19, 2022 · ab1e0bd · ab1e0bd
1 parent d264c3b
commit ab1e0bd
Show file tree

Hide file tree

Showing 7 changed files with 41 additions and 29 deletions.
diff --git a/...ions_test__test_get_transactions_output_user_transaction_with_entry_function_payload.json b/...ions_test__test_get_transactions_output_user_transaction_with_entry_function_payload.json
@@ -114,7 +114,7 @@
     "state_change_hash": "",
     "event_root_hash": "",
     "state_checkpoint_hash": null,
-    "gas_used": "1537",
+    "gas_used": "1536",
     "success": true,
     "vm_status": "Executed successfully",
     "accumulator_root_hash": "",

diff --git a/...ns_test__test_get_transactions_returns_last_page_when_start_version_is_not_specified.json b/...ns_test__test_get_transactions_returns_last_page_when_start_version_is_not_specified.json
@@ -119,7 +119,7 @@
     "state_change_hash": "",
     "event_root_hash": "",
     "state_checkpoint_hash": null,
-    "gas_used": "1537",
+    "gas_used": "1536",
     "success": true,
     "vm_status": "Executed successfully",
     "accumulator_root_hash": "",
@@ -397,7 +397,7 @@
     "state_change_hash": "",
     "event_root_hash": "",
     "state_checkpoint_hash": null,
-    "gas_used": "1537",
+    "gas_used": "1536",
     "success": true,
     "vm_status": "Executed successfully",
     "accumulator_root_hash": "",
@@ -675,7 +675,7 @@
     "state_change_hash": "",
     "event_root_hash": "",
     "state_checkpoint_hash": null,
-    "gas_used": "1537",
+    "gas_used": "1536",
     "success": true,
     "vm_status": "Executed successfully",
     "accumulator_root_hash": "",
@@ -953,7 +953,7 @@
     "state_change_hash": "",
     "event_root_hash": "",
     "state_checkpoint_hash": null,
-    "gas_used": "1537",
+    "gas_used": "1536",
     "success": true,
     "vm_status": "Executed successfully",
     "accumulator_root_hash": "",
@@ -1231,7 +1231,7 @@
     "state_change_hash": "",
     "event_root_hash": "",
     "state_checkpoint_hash": null,
-    "gas_used": "1537",
+    "gas_used": "1536",
     "success": true,
     "vm_status": "Executed successfully",
     "accumulator_root_hash": "",
@@ -1509,7 +1509,7 @@
     "state_change_hash": "",
     "event_root_hash": "",
     "state_checkpoint_hash": null,
-    "gas_used": "1537",
+    "gas_used": "1536",
     "success": true,
     "vm_status": "Executed successfully",
     "accumulator_root_hash": "",
@@ -1787,7 +1787,7 @@
     "state_change_hash": "",
     "event_root_hash": "",
     "state_checkpoint_hash": null,
-    "gas_used": "1537",
+    "gas_used": "1536",
     "success": true,
     "vm_status": "Executed successfully",
     "accumulator_root_hash": "",
@@ -2065,7 +2065,7 @@
     "state_change_hash": "",
     "event_root_hash": "",
     "state_checkpoint_hash": null,
-    "gas_used": "1537",
+    "gas_used": "1536",
     "success": true,
     "vm_status": "Executed successfully",
     "accumulator_root_hash": "",

diff --git a/...-move/e2e-tests/goldens/language_e2e_testsuite__tests__create_account__create_account.exp b/...-move/e2e-tests/goldens/language_e2e_testsuite__tests__create_account__create_account.exp
@@ -21,7 +21,7 @@ Ok(
             events: [
                 ContractEvent { key: EventKey { creation_number: 0, account_address: f5b9d6f01a99e74c790e2f330c092fa05455a8193f1dfc1b113ecc54d067afe1 }, index: 0, type: Struct(StructTag { address: 0000000000000000000000000000000000000000000000000000000000000001, module: Identifier("account"), name: Identifier("CoinRegisterEvent"), type_params: [] }), event_data: "00000000000000000000000000000000000000000000000000000000000000010a6170746f735f636f696e094170746f73436f696e" },
             ],
-            gas_used: 1537,
+            gas_used: 1536,
             status: Keep(
                 Success,
             ),

diff --git a/aptos-move/framework/aptos-framework/doc/coin.md b/aptos-move/framework/aptos-framework/doc/coin.md
@@ -499,7 +499,7 @@ Name of the coin is too long
 
 <a name="0x1_coin_ECOIN_STORE_ALREADY_PUBLISHED"></a>
 
-Account already has <code><a href="coin.md#0x1_coin_CoinStore">CoinStore</a></code> registered for <code>CoinType</code>
+Deprecated. Account already has <code><a href="coin.md#0x1_coin_CoinStore">CoinStore</a></code> registered for <code>CoinType</code>
 
 
 <pre><code><b>const</b> <a href="coin.md#0x1_coin_ECOIN_STORE_ALREADY_PUBLISHED">ECOIN_STORE_ALREADY_PUBLISHED</a>: u64 = 4;
@@ -1523,10 +1523,10 @@ Returns minted <code><a href="coin.md#0x1_coin_Coin">Coin</a></code>.
 
 <pre><code><b>public</b> <b>fun</b> <a href="coin.md#0x1_coin_register">register</a>&lt;CoinType&gt;(<a href="account.md#0x1_account">account</a>: &<a href="../../aptos-stdlib/../move-stdlib/doc/signer.md#0x1_signer">signer</a>) {
     <b>let</b> account_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="coin.md#0x1_coin_is_account_registered">is_account_registered</a>&lt;CoinType&gt;(account_addr),
-        <a href="../../aptos-stdlib/../move-stdlib/doc/error.md#0x1_error_already_exists">error::already_exists</a>(<a href="coin.md#0x1_coin_ECOIN_STORE_ALREADY_PUBLISHED">ECOIN_STORE_ALREADY_PUBLISHED</a>),
-    );
+    // Short-circuit and do nothing <b>if</b> <a href="account.md#0x1_account">account</a> is already registered for CoinType.
+    <b>if</b> (<a href="coin.md#0x1_coin_is_account_registered">is_account_registered</a>&lt;CoinType&gt;(account_addr)) {
+        <b>return</b>
+    };
 
     <a href="account.md#0x1_account_register_coin">account::register_coin</a>&lt;CoinType&gt;(account_addr);
     <b>let</b> coin_store = <a href="coin.md#0x1_coin_CoinStore">CoinStore</a>&lt;CoinType&gt; {

diff --git a/aptos-move/framework/aptos-framework/sources/coin.move b/aptos-move/framework/aptos-framework/sources/coin.move
@@ -31,7 +31,7 @@ module aptos_framework::coin {
     /// `CoinType` hasn't been initialized as a coin
     const ECOIN_INFO_NOT_PUBLISHED: u64 = 3;
 
-    /// Account already has `CoinStore` registered for `CoinType`
+    /// Deprecated. Account already has `CoinStore` registered for `CoinType`
     const ECOIN_STORE_ALREADY_PUBLISHED: u64 = 4;
 
     /// Account hasn't registered `CoinStore` for `CoinType`
@@ -488,10 +488,10 @@ module aptos_framework::coin {
 
     public fun register<CoinType>(account: &signer) {
         let account_addr = signer::address_of(account);
-        assert!(
-            !is_account_registered<CoinType>(account_addr),
-            error::already_exists(ECOIN_STORE_ALREADY_PUBLISHED),
-        );
+        // Short-circuit and do nothing if account is already registered for CoinType.
+        if (is_account_registered<CoinType>(account_addr)) {
+            return
+        };
 
         account::register_coin<CoinType>(account_addr);
         let coin_store = CoinStore<CoinType> {
@@ -1037,6 +1037,24 @@ module aptos_framework::coin {
         aggregator::destroy(value);
     }
 
+    #[test(framework = @aptos_framework)]
+    public entry fun test_register_twice_should_not_fail(framework: &signer) {
+        let framework_addr = signer::address_of(framework);
+        account::create_account_for_test(framework_addr);
+        let (burn_cap, freeze_cap, mint_cap) = initialize_and_register_fake_money(framework, 1, true);
+
+        // Registering twice should not fail.
+        assert!(is_account_registered<FakeMoney>(@0x1), 0);
+        register<FakeMoney>(framework);
+        assert!(is_account_registered<FakeMoney>(@0x1), 1);
+
+        move_to(framework, FakeMoneyCapabilities {
+            burn_cap,
+            freeze_cap,
+            mint_cap,
+        });
+    }
+
     #[test(framework = @aptos_framework)]
     public entry fun test_collect_from_and_drain(
         framework: signer,

diff --git a/aptos-move/framework/aptos-framework/sources/stake.move b/aptos-move/framework/aptos-framework/sources/stake.move
@@ -1425,12 +1425,8 @@ module aptos_framework::stake {
 
     #[test_only]
     public fun mint(account: &signer, amount: u64) acquires AptosCoinCapabilities {
-        let account_address = signer::address_of(account);
-        if (!coin::is_account_registered<AptosCoin>(account_address)) {
-            coin::register<AptosCoin>(account);
-        };
-
-        coin::deposit(account_address, mint_coins(amount));
+        coin::register<AptosCoin>(account);
+        coin::deposit(signer::address_of(account), mint_coins(amount));
     }
 
     #[test_only]

diff --git a/aptos-move/move-examples/marketplace/sources/marketplace_auction_example.move b/aptos-move/move-examples/marketplace/sources/marketplace_auction_example.move
@@ -202,9 +202,7 @@ module marketplace::marketplace_auction_example {
         );
 
         // initialized coin store when listing
-        if (!coin::is_account_registered<CoinType>(signer::address_of(owner))) {
-            coin::register<CoinType>(owner);
-        };
+        coin::register<CoinType>(owner);
 
         let auctions = borrow_global_mut<Auctions<CoinType>>(@marketplace);
         event::emit_event<ListingEvent>(