Fix verify_zklogin_id and issuer functions (MystenLabs#14100)

## Description Fix `verify_zklogin_id` and `verify_zklogin_issuer` functions such that they transfer the verified id or issuer objects to the sender. ## Test Plan Unit tests ### Type of Change (Check all that apply) - [x] protocol change - [x] user-visible impact - [ ] breaking change for a client SDKs - [x] breaking change for FNs (FN binary must upgrade) - [x] breaking change for validators or node operators (must upgrade binaries) - [ ] breaking change for on-chain data layout - [ ] necessitate either a data wipe or data migration ### Release notes Fix bug in `verify_zklogin_id` and `verify_zklogin_issuer` functions.
lz1998 · Oct 4, 2023 · 31007e2 · 31007e2
1 parent 295211e
commit 31007e2
Show file tree

Hide file tree

Showing 5 changed files with 128 additions and 30 deletions.
diff --git a/crates/sui-framework/packages/sui-framework/sources/crypto/zklogin_verified_id.move b/crates/sui-framework/packages/sui-framework/sources/crypto/zklogin_verified_id.move
@@ -5,6 +5,7 @@ module sui::zklogin_verified_id {
     use std::string::String;
     use sui::object;
     use sui::object::UID;
+    use sui::transfer;
     use sui::tx_context::TxContext;
     use sui::tx_context;
 
@@ -69,9 +70,20 @@ module sui::zklogin_verified_id {
         audience: String,
         pin_hash: u256,
         ctx: &mut TxContext,
-    ): VerifiedID {
-        assert!(check_zklogin_id(tx_context::sender(ctx), &key_claim_name, &key_claim_value, &issuer, &audience, pin_hash), EInvalidProof);
-        VerifiedID { id: object::new(ctx), owner: tx_context::sender(ctx), key_claim_name, key_claim_value, issuer, audience}
+    ) {
+        let sender = tx_context::sender(ctx);
+        assert!(check_zklogin_id(sender, &key_claim_name, &key_claim_value, &issuer, &audience, pin_hash), EInvalidProof);
+        transfer::transfer(
+            VerifiedID {
+                id: object::new(ctx),
+                owner: sender,
+                key_claim_name,
+                key_claim_value,
+                issuer,
+                audience
+            },
+            sender
+        );
     }
 
     /// Returns true if `address` was created using zklogin and the given parameters.

diff --git a/crates/sui-framework/packages/sui-framework/sources/crypto/zklogin_verified_issuer.move b/crates/sui-framework/packages/sui-framework/sources/crypto/zklogin_verified_issuer.move
@@ -3,6 +3,7 @@
 
 module sui::zklogin_verified_issuer {
     use std::string::String;
+    use sui::transfer;
     use sui::object;
     use sui::object::UID;
     use sui::tx_context::TxContext;
@@ -43,9 +44,17 @@ module sui::zklogin_verified_issuer {
         address_seed: u256,
         issuer: String,
         ctx: &mut TxContext,
-    ): VerifiedIssuer {
-        assert!(check_zklogin_issuer(tx_context::sender(ctx), address_seed, &issuer), EInvalidProof);
-        VerifiedIssuer {id: object::new(ctx), owner: tx_context::sender(ctx), issuer}
+    ) {
+        let sender = tx_context::sender(ctx);
+        assert!(check_zklogin_issuer(sender, address_seed, &issuer), EInvalidProof);
+        transfer::transfer(
+            VerifiedIssuer {
+                id: object::new(ctx),
+                owner: sender,
+                issuer
+            },
+            sender
+        )
     }
 
     /// Returns true if `address` was created using zklogin with the given issuer and address seed.

diff --git a/crates/sui-framework/packages/sui-framework/tests/crypto/zklogin_verified_id_tests.move b/crates/sui-framework/packages/sui-framework/tests/crypto/zklogin_verified_id_tests.move
@@ -3,13 +3,15 @@
 
 #[test_only]
 module sui::zklogin_verified_id_tests {
-    use sui::zklogin_verified_id::check_zklogin_id;
+    use sui::zklogin_verified_id::{check_zklogin_id, verify_zklogin_id, VerifiedID};
     use sui::address;
     use std::string::utf8;
+    use sui::bag::add;
+    use sui::test_scenario;
 
     #[test]
     fun test_check_zklogin_id() {
-        let address = address::from_bytes(x"1c6b623a2f2c91333df730c98d220f11484953b391a3818680f922c264cc0c6b");
+        let address = @0x1c6b623a2f2c91333df730c98d220f11484953b391a3818680f922c264cc0c6b;
         let kc_name = utf8(b"sub");
         let kc_value = utf8(b"106294049240999307923");
         let aud = utf8(b"575519204237-msop9ep45u2uo98hapqmngv8d84qdc8k.apps.googleusercontent.com");
@@ -41,7 +43,7 @@ module sui::zklogin_verified_id_tests {
     fun test_check_zklogin_id_upper_bounds() {
         // Set kc_name, kc_value and aud to be as long as they can be (32, 115 and 145 bytes respectively) and verify
         // that the check function doesn't abort.
-        let address = address::from_bytes(x"1c6b623a2f2c91333df730c98d220f11484953b391a3818680f922c264cc0c6b");
+        let address = @0x1c6b623a2f2c91333df730c98d220f11484953b391a3818680f922c264cc0c6b;
         let kc_name = utf8(b"qvKbuwvu6LTnYocFPwz1EiIClFUAuMC3");
         let kc_value = utf8(b"BA7SREzYLKG5opithujfrU8SFaSpKI4zezu8Vb2GBPVpZsMUpYVeXl6oEAo84ryIlbHOqrMWpI7mlTfvr7HYxiF70jdyIY4rPOOpuJIYWwN3o1olTP2");
         let aud = utf8(b"munO2fnn2XnBNq5fXRmSmhC4GPL3yX4Rv9fGoECXTsmniR8dwkPTefbmLF08zh7BnVcaxriii4dEPNwzEF2puLIHmJoeuYbQxV84J9of4NRaL3IhwImVGubgPHWfMfCuGuedCdLn6KUdJsgG1");
@@ -53,7 +55,7 @@ module sui::zklogin_verified_id_tests {
     #[test]
     #[expected_failure(abort_code = sui::zklogin_verified_id::EInvalidInput)]
     fun test_check_zklogin_id_long_kc_name() {
-        let address = address::from_bytes(x"1c6b623a2f2c91333df730c98d220f11484953b391a3818680f922c264cc0c6b");
+        let address = @0x1c6b623a2f2c91333df730c98d220f11484953b391a3818680f922c264cc0c6b;
         // Should at most be 32 bytes
         let long_kc_name = utf8(b"qvKbuwvu6LTnYocFPwz1EiIClFUAuMC3G");
         let kc_value = utf8(b"106294049240999307923");
@@ -66,7 +68,7 @@ module sui::zklogin_verified_id_tests {
     #[test]
     #[expected_failure(abort_code = sui::zklogin_verified_id::EInvalidInput)]
     fun test_check_zklogin_id_long_aud() {
-        let address = address::from_bytes(x"1c6b623a2f2c91333df730c98d220f11484953b391a3818680f922c264cc0c6b");
+        let address = @0x1c6b623a2f2c91333df730c98d220f11484953b391a3818680f922c264cc0c6b;
         let kc_name = utf8(b"sub");
         // Should at most be 115 bytes
         let long_kc_value = utf8(b"BA7SREzYLKG5opithujfrU8SFaSpKI4zezu8Vb2GBPVpZsMUpYVeXl6oEAo84ryIlbHOqrMWpI7mlTfvr7HYxiF70jdyIY4rPOOpuJIYWwN3o1olTP2c");
@@ -79,7 +81,7 @@ module sui::zklogin_verified_id_tests {
     #[test]
     #[expected_failure(abort_code = sui::zklogin_verified_id::EInvalidInput)]
     fun test_check_zklogin_id_long_kc_value() {
-        let address = address::from_bytes(x"1c6b623a2f2c91333df730c98d220f11484953b391a3818680f922c264cc0c6b");
+        let address = @0x1c6b623a2f2c91333df730c98d220f11484953b391a3818680f922c264cc0c6b;
         let kc_name = utf8(b"sub");
         let kc_value = utf8(b"106294049240999307923");
         // Should be at most 145 bytes
@@ -88,4 +90,45 @@ module sui::zklogin_verified_id_tests {
         let salt_hash = 15232766888716517538274372547598053531354666056102343895255590477425668733026u256;
         check_zklogin_id(address, &kc_name, &kc_value, &iss, &long_aud, salt_hash);
     }
+
+    #[test]
+    fun test_verified_id() {
+        let address = @0x1c6b623a2f2c91333df730c98d220f11484953b391a3818680f922c264cc0c6b;
+
+        let kc_name = utf8(b"sub");
+        let kc_value = utf8(b"106294049240999307923");
+        let aud = utf8(b"575519204237-msop9ep45u2uo98hapqmngv8d84qdc8k.apps.googleusercontent.com");
+        let iss = utf8(b"https://accounts.google.com");
+        let salt_hash = 15232766888716517538274372547598053531354666056102343895255590477425668733026u256;
+
+        let scenario_val = test_scenario::begin(address);
+        let scenario = &mut scenario_val;
+        {
+            verify_zklogin_id(kc_name, kc_value, iss, aud, salt_hash, test_scenario::ctx(scenario));
+        };
+        test_scenario::next_tx(scenario, address);
+        {
+            assert!(test_scenario::has_most_recent_for_sender<VerifiedID>(scenario), 0);
+        };
+        test_scenario::end(scenario_val);
+    }
+
+    #[test]
+    #[expected_failure(abort_code = sui::zklogin_verified_id::EInvalidProof)]
+    fun test_invalid_verified_issuer() {
+        let other_address = @0x1;
+
+        let kc_name = utf8(b"sub");
+        let kc_value = utf8(b"106294049240999307923");
+        let aud = utf8(b"575519204237-msop9ep45u2uo98hapqmngv8d84qdc8k.apps.googleusercontent.com");
+        let iss = utf8(b"https://accounts.google.com");
+        let salt_hash = 15232766888716517538274372547598053531354666056102343895255590477425668733026u256;
+
+        let scenario_val = test_scenario::begin(other_address);
+        let scenario = &mut scenario_val;
+        {
+            verify_zklogin_id(kc_name, kc_value, iss, aud, salt_hash, test_scenario::ctx(scenario));
+        };
+        test_scenario::end(scenario_val);
+    }
 }
diff --git a/crates/sui-framework/packages/sui-framework/tests/crypto/zklogin_verified_issuer_tests.move b/crates/sui-framework/packages/sui-framework/tests/crypto/zklogin_verified_issuer_tests.move
@@ -3,18 +3,18 @@
 
 #[test_only]
 module sui::zklogin_verified_issuer_tests {
-    use sui::zklogin_verified_issuer::check_zklogin_issuer;
-    use sui::address;
+    use sui::zklogin_verified_issuer::{check_zklogin_issuer, verify_zklogin_issuer, VerifiedIssuer};
     use std::string::utf8;
+    use sui::test_scenario;
 
     #[test]
     fun test_check_zklogin_issuer() {
-        let address = address::from_bytes(x"1c6b623a2f2c91333df730c98d220f11484953b391a3818680f922c264cc0c6b");
+        let address = @0x1c6b623a2f2c91333df730c98d220f11484953b391a3818680f922c264cc0c6b;
         let iss = utf8(b"https://accounts.google.com");
         let address_seed = 3006596378422062745101035755700472756930796952630484939867684134047976874601u256;
         assert!(check_zklogin_issuer(address,  address_seed, &iss,), 0);
 
-        let other_address = address::from_bytes(x"006b623a2f2c91333df730c98d220f11484953b391a3818680f922c264cc0c6b");
+        let other_address = @0x006b623a2f2c91333df730c98d220f11484953b391a3818680f922c264cc0c6b;
         assert!(!check_zklogin_issuer(other_address, address_seed, &iss), 1);
 
         let other_address_seed = 1234u256;
@@ -23,4 +23,38 @@ module sui::zklogin_verified_issuer_tests {
         let other_iss = utf8(b"https://other.issuer.com");
         assert!(!check_zklogin_issuer(address, address_seed, &other_iss), 3);
     }
+
+    #[test]
+    fun test_verified_issuer() {
+        let address = @0x1c6b623a2f2c91333df730c98d220f11484953b391a3818680f922c264cc0c6b;
+        let iss = utf8(b"https://accounts.google.com");
+        let address_seed = 3006596378422062745101035755700472756930796952630484939867684134047976874601u256;
+
+        assert!(check_zklogin_issuer(address,  address_seed, &iss), 0);
+
+        let scenario_val = test_scenario::begin(address);
+        let scenario = &mut scenario_val;
+        {
+            verify_zklogin_issuer(address_seed, iss, test_scenario::ctx(scenario));
+        };
+        test_scenario::next_tx(scenario, address);
+        {
+            assert!(test_scenario::has_most_recent_for_sender<VerifiedIssuer>(scenario), 1);
+        };
+        test_scenario::end(scenario_val);
+    }
+
+    #[test]
+    #[expected_failure(abort_code = sui::zklogin_verified_issuer::EInvalidProof)]
+    fun test_invalid_verified_issuer() {
+        let other_address = @0x1;
+        let iss = utf8(b"https://accounts.google.com");
+        let address_seed = 3006596378422062745101035755700472756930796952630484939867684134047976874601u256;
+        let scenario_val = test_scenario::begin(other_address);
+        let scenario = &mut scenario_val;
+        {
+            verify_zklogin_issuer(address_seed, iss, test_scenario::ctx(scenario));
+        };
+        test_scenario::end(scenario_val);
+    }
 }
diff --git a/...ui-swarm-config/tests/snapshots/snapshot_tests__populated_genesis_snapshot_matches-2.snap b/...ui-swarm-config/tests/snapshots/snapshot_tests__populated_genesis_snapshot_matches-2.snap
@@ -240,56 +240,56 @@ validators:
         next_epoch_worker_address: ~
         extra_fields:
           id:
-            id: "0x1ce4d26025fafb17ecefce1819bfe9d0ab7ab50acebc7643d4c369889471cfa0"
+            id: "0xe0d4d91bbeda526b29ce5bc5155a3c39e47046aa8fdfa4b3a02fea5d24dd5310"
           size: 0
       voting_power: 10000
-      operation_cap_id: "0x222e14762b7e5c8efce495b92c828f923ce8f69bfb7550879097e6ff8cd59874"
+      operation_cap_id: "0x3d53df75e0d6c75724bf8f10599e7adbc5df02b8eedf2d270916868e3086bcca"
       gas_price: 1000
       staking_pool:
-        id: "0xb4e66c283aba08950b0f7572cf1464eb16e469a511618a23e98c320d5e6268f9"
+        id: "0xea9cfd1d1b79c7d1fdafe07e52bd725b780fdaeac3c8cc4df0be11c413376e35"
         activation_epoch: 0
         deactivation_epoch: ~
         sui_balance: 20000000000000000
         rewards_pool:
           value: 0
         pool_token_balance: 20000000000000000
         exchange_rates:
-          id: "0x6725868ad5a3106cd62e3372dbfec20bf811fcb8416497c9ca5d4df659ce8702"
+          id: "0x98d39527d95f28cad749a5ea5ea6f36276fbc995450c8324d2afa108abb760f8"
           size: 1
         pending_stake: 0
         pending_total_sui_withdraw: 0
         pending_pool_token_withdraw: 0
         extra_fields:
           id:
-            id: "0xb02da3c000c5c9867c86e129fcd38ac8cda347243ec415424eba19b1133feeb6"
+            id: "0x296e8e89079146ddff5677f558629482ef343ea0f8ac69e5b2a2478c3889ec93"
           size: 0
       commission_rate: 200
       next_epoch_stake: 20000000000000000
       next_epoch_gas_price: 1000
       next_epoch_commission_rate: 200
       extra_fields:
         id:
-          id: "0x86e65e2b90330d3b95b9c9713ea9eff950edfb280273dba90f5088f7f9503240"
+          id: "0x4adb29aafefd7d669140ac7fa77c56970caee2b8910d3895fc79a8ce8a0df2e8"
         size: 0
   pending_active_validators:
     contents:
-      id: "0x6ce6601c07f9490497b6a4b0fd5990aff5d254198c8f87eddb55dbf05ed22a2a"
+      id: "0xbd69b5645a1c30f65922886ea384312786764143132831b09bcc5aeb1fb1ddad"
       size: 0
   pending_removals: []
   staking_pool_mappings:
-    id: "0x5ffae1a50608be06fcbd8dd300eb29229c7d7ccb437ca50b400c5d7c3d2d53df"
+    id: "0xb76d89f8114ab5e5bc127238a7ed5d3feccc6718fb7316ef95336ff329af5149"
     size: 1
   inactive_validators:
-    id: "0x208295852c01a9c3eae6693c6558cad8575c4a0fac630367192465df6f655c6f"
+    id: "0x301364bf9e1e0a9e3b997addc7b8313b07dfe0e2791aa92810e4f87518331ff6"
     size: 0
   validator_candidates:
-    id: "0xd61e4a41849ab22843aec860ad63fee5aac549035c04beed7e065887ff461025"
+    id: "0x11bc1a25d1caf058ebe1d308c011424099ae2b221f17d0049b0b52868a5091dd"
     size: 0
   at_risk_validators:
     contents: []
   extra_fields:
     id:
-      id: "0x36c534d35f22393724a3cca982d135aad4927068015a89aba6e72b98ee0dbbdb"
+      id: "0x4d17f9acbad182bb9f5f4107657bfa69442ac11779be446bdb5e536d7e7d7bcd"
     size: 0
 storage_fund:
   total_object_storage_rebates:
@@ -306,7 +306,7 @@ parameters:
   validator_low_stake_grace_period: 7
   extra_fields:
     id:
-      id: "0x76a62f2d7709a720c24d30507aaf514f02e2314102304cf428d1767eb159a344"
+      id: "0x61dd90bd030e2e5249396ffb05acee0549c9e262a1a4e33e21ab7b59f7066ff6"
     size: 0
 reference_gas_price: 1000
 validator_report_records:
@@ -320,7 +320,7 @@ stake_subsidy:
   stake_subsidy_decrease_rate: 1000
   extra_fields:
     id:
-      id: "0x9ea1df23de1c63c2998c8982099b2bd92ee52531b16e159b0995cb9d356cdb1a"
+      id: "0xaf8ff2fbc89140df5f50cb18fe0319edcb5821ab67bbefb6601f854f82bc3d04"
     size: 0
 safe_mode: false
 safe_mode_storage_rewards:
@@ -332,6 +332,6 @@ safe_mode_non_refundable_storage_fee: 0
 epoch_start_timestamp_ms: 10
 extra_fields:
   id:
-    id: "0x445d57619c17979d501adc79ed39d36a4d74bab0de188a1a35ccf23f2f040d0b"
+    id: "0x8c78540b9b149a41f2baf65f5a5d83bea578249669ef7fa08b8fa48b9b978c2f"
   size: 0