[framework] Fix session key create session key bug (#1690)

crates/rooch-framework-tests/src/tests/transaction_validator_tests.rs

@@ -6,7 +6,7 @@ use move_core_types::account_address::AccountAddress;
 use move_core_types::ident_str;
 use move_core_types::language_storage::ModuleId;
 use move_core_types::value::MoveValue;
-use move_core_types::vm_status::{AbortLocation, VMStatus};
+use move_core_types::vm_status::{AbortLocation, KeptVMStatus, VMStatus};
 use moveos_types::module_binding::MoveFunctionCaller;
 use moveos_types::move_std::ascii::MoveAsciiString;
 use moveos_types::move_std::string::MoveString;
@@ -31,18 +31,13 @@ fn test_session_key_rooch() {
 
     let session_auth_key = keystore.generate_session_key(&sender, None).unwrap();
 
-    let session_scope = SessionScope::new(
-        ROOCH_FRAMEWORK_ADDRESS,
-        Empty::MODULE_NAME.as_str(),
-        Empty::EMPTY_FUNCTION_NAME.as_str(),
-    )
-    .unwrap();
+    let session_scope = SessionScope::new(ROOCH_FRAMEWORK_ADDRESS, "*", "*").unwrap();
     let app_name = MoveString::from_str("test").unwrap();
     let app_url = MoveAsciiString::from_str("https:://test.rooch.network").unwrap();
     let max_inactive_interval = 100;
     let action = rooch_types::framework::session_key::SessionKeyModule::create_session_key_action(
-        app_name,
-        app_url,
+        app_name.clone(),
+        app_url.clone(),
         session_auth_key.as_ref().to_vec(),
         session_scope.clone(),
         max_inactive_interval,
@@ -59,11 +54,11 @@ fn test_session_key_rooch() {
     assert!(session_key_option.is_some(), "Session key not found");
     let session_key = session_key_option.unwrap();
     assert_eq!(&session_key.authentication_key, session_auth_key.as_ref());
-    assert_eq!(session_key.scopes, vec![session_scope]);
+    assert_eq!(session_key.scopes, vec![session_scope.clone()]);
     assert_eq!(session_key.max_inactive_interval, max_inactive_interval);
     keystore.binding_session_key(sender, session_key).unwrap();
 
-    // send transaction via session key
+    // send transaction via session key, it in the scop of session key, so it should success.
 
     let action = MoveAction::new_function_call(Empty::empty_function_id(), vec![], vec![]);
     let tx_data = RoochTransactionData::new_for_test(sender, sequence_number + 1, action);
@@ -73,11 +68,11 @@ fn test_session_key_rooch() {
 
     binding_test.execute(tx).unwrap();
 
-    // test the session key call function is out the scope.
+    // test the session key call function which not in the scope of session key, it should be rejected.
 
     let action = MoveAction::new_function_call(
         FunctionId::new(
-            ModuleId::new(ROOCH_FRAMEWORK_ADDRESS, ident_str!("empty").to_owned()),
+            ModuleId::new(AccountAddress::random(), ident_str!("empty").to_owned()),
             ident_str!("empty_with_signer").to_owned(),
         ),
         vec![],
@@ -113,17 +108,55 @@ fn test_session_key_rooch() {
         }
     }
 
+    // test create session key with session key, it should fail.
+
+    let new_session_auth_key = keystore.generate_session_key(&sender, None).unwrap();
+    let action = rooch_types::framework::session_key::SessionKeyModule::create_session_key_action(
+        app_name,
+        app_url,
+        new_session_auth_key.as_ref().to_vec(),
+        session_scope.clone(),
+        max_inactive_interval,
+    );
+    // because previous transaction is failed, so the sequence number is not increased.
+    let tx_data = RoochTransactionData::new_for_test(sender, sequence_number + 2, action);
+    // sign with the old session key
+    let tx = keystore
+        .sign_transaction_via_session_key(&sender, tx_data, &session_auth_key, None)
+        .unwrap();
+
+    let execute_result = binding_test.execute_as_result(tx).unwrap();
+    match execute_result.output.status {
+        KeptVMStatus::MoveAbort(l, code) => {
+            match l {
+                AbortLocation::Module(module_id) => {
+                    assert_eq!(
+                        module_id,
+                        SessionKeyModule::module_id(),
+                        "expect session key module"
+                    );
+                }
+                _ => panic!("expect move abort in module"),
+            }
+            // ErrorSessionKeyCreatePermissionDenied = 5
+            assert_eq!(code, 1, "expect ErrorSessionKeyCreatePermissionDenied");
+        }
+        _ => {
+            panic!("Expect move abort")
+        }
+    }
+
     // test session key expired
     let update_time_action =
         TimestampModule::create_fast_forward_seconds_for_local_action(max_inactive_interval + 1);
-    // because previous transaction is failed, so the sequence number is not increased.
+
     let tx_data =
-        RoochTransactionData::new_for_test(sender, sequence_number + 2, update_time_action);
+        RoochTransactionData::new_for_test(sender, sequence_number + 3, update_time_action);
     let tx = keystore.sign_transaction(&sender, tx_data, None).unwrap();
     binding_test.execute(tx).unwrap();
 
     let action = MoveAction::new_function_call(Empty::empty_function_id(), vec![], vec![]);
-    let tx_data = RoochTransactionData::new_for_test(sender, sequence_number + 3, action);
+    let tx_data = RoochTransactionData::new_for_test(sender, sequence_number + 4, action);
     let tx = keystore
         .sign_transaction_via_session_key(&sender, tx_data, &session_auth_key, None)
         .unwrap();

crates/rooch-types/src/framework/auth_validator.rs

@@ -1,8 +1,6 @@
 // Copyright (c) RoochNetwork
 // SPDX-License-Identifier: Apache-2.0
 
-use super::ethereum_validator::EthereumValidatorModule;
-use super::native_validator::NativeValidatorModule;
 use super::transaction_validator::TransactionValidator;
 use crate::address::MultiChainAddress;
 use crate::addresses::ROOCH_FRAMEWORK_ADDRESS;
@@ -15,7 +13,6 @@ use move_core_types::{
 };
 use moveos_types::function_return_value::DecodedFunctionResult;
 use moveos_types::move_std::option::MoveOption;
-use moveos_types::transaction::MoveAction;
 use moveos_types::{
     module_binding::MoveFunctionCaller,
     move_std::ascii::MoveAsciiString,
@@ -27,6 +24,8 @@ use moveos_types::{
 use serde::{Deserialize, Serialize};
 use strum_macros::{Display, EnumString};
 
+pub const MODULE_NAME: &IdentStr = ident_str!("auth_validator");
+
 /// The Authenticator auth validator which has builtin Rooch and Ethereum
 #[derive(
     Copy,
@@ -46,16 +45,19 @@ use strum_macros::{Display, EnumString};
 pub enum BuiltinAuthValidator {
     Rooch,
     Ethereum,
+    Bitcoin,
 }
 
 impl BuiltinAuthValidator {
     const ROOCH_FLAG: u8 = 0x00;
     const ETHEREUM_FLAG: u8 = 0x01;
+    const BITCOIN_FLAG: u8 = 0x02;
 
     pub fn flag(&self) -> u8 {
         match self {
             BuiltinAuthValidator::Rooch => Self::ROOCH_FLAG,
             BuiltinAuthValidator::Ethereum => Self::ETHEREUM_FLAG,
+            BuiltinAuthValidator::Bitcoin => Self::BITCOIN_FLAG,
         }
     }
 
@@ -69,38 +71,13 @@ impl BuiltinAuthValidator {
     pub fn from_flag_byte(byte_int: u8) -> Result<BuiltinAuthValidator, RoochError> {
         match byte_int {
             Self::ROOCH_FLAG => Ok(BuiltinAuthValidator::Rooch),
+            Self::ETHEREUM_FLAG => Ok(BuiltinAuthValidator::Ethereum),
+            Self::BITCOIN_FLAG => Ok(BuiltinAuthValidator::Bitcoin),
             _ => Err(RoochError::KeyConversionError(
                 "Invalid key auth validator".to_owned(),
             )),
         }
     }
-
-    pub fn create_rotate_authentication_key_action(
-        &self,
-        public_key: Vec<u8>,
-    ) -> Result<MoveAction, RoochError> {
-        let action = match self {
-            BuiltinAuthValidator::Rooch => {
-                NativeValidatorModule::rotate_authentication_key_action(public_key)
-            }
-            BuiltinAuthValidator::Ethereum => {
-                EthereumValidatorModule::rotate_authentication_key_action(public_key)
-            }
-        };
-        Ok(action)
-    }
-
-    pub fn create_remove_authentication_key_action(&self) -> Result<MoveAction, RoochError> {
-        let action = match self {
-            BuiltinAuthValidator::Rooch => {
-                NativeValidatorModule::remove_authentication_key_action()
-            }
-            BuiltinAuthValidator::Ethereum => {
-                EthereumValidatorModule::remove_authentication_key_action()
-            }
-        };
-        Ok(action)
-    }
 }
 
 #[derive(Debug, Clone, Serialize, Deserialize)]
@@ -112,7 +89,7 @@ pub struct AuthValidator {
 
 impl MoveStructType for AuthValidator {
     const ADDRESS: AccountAddress = ROOCH_FRAMEWORK_ADDRESS;
-    const MODULE_NAME: &'static IdentStr = ident_str!("auth_validator");
+    const MODULE_NAME: &'static IdentStr = MODULE_NAME;
     const STRUCT_NAME: &'static IdentStr = ident_str!("AuthValidator");
 }
 
@@ -158,7 +135,7 @@ pub struct TxValidateResult {
 
 impl MoveStructType for TxValidateResult {
     const ADDRESS: AccountAddress = ROOCH_FRAMEWORK_ADDRESS;
-    const MODULE_NAME: &'static IdentStr = ident_str!("transaction_validtor");
+    const MODULE_NAME: &'static IdentStr = MODULE_NAME;
     const STRUCT_NAME: &'static IdentStr = ident_str!("TxValidateResult");
 }
 

crates/rooch-types/src/framework/ethereum_validator.rs

@@ -11,7 +11,7 @@ use moveos_types::{
     module_binding::{ModuleBinding, MoveFunctionCaller},
     moveos_std::tx_context::TxContext,
     state::MoveStructType,
-    transaction::{FunctionCall, MoveAction},
+    transaction::FunctionCall,
 };
 
 pub struct EthereumValidator {}
@@ -35,10 +35,6 @@ pub struct EthereumValidatorModule<'a> {
 
 impl<'a> EthereumValidatorModule<'a> {
     const VALIDATE_FUNCTION_NAME: &'static IdentStr = ident_str!("validate");
-    const ROTATE_AUTHENTICATION_KEY_ENTRY_FUNCTION_NAME: &'static IdentStr =
-        ident_str!("rotate_authentication_key_entry");
-    const REMOVE_AUTHENTICATION_KEY_ENTRY_FUNCTION_NAME: &'static IdentStr =
-        ident_str!("remove_authentication_key_entry");
 
     pub fn validate(&self, ctx: &TxContext, payload: Vec<u8>) -> Result<()> {
         let auth_validator_call = FunctionCall::new(
@@ -54,22 +50,6 @@ impl<'a> EthereumValidatorModule<'a> {
             })?;
         Ok(())
     }
-
-    pub fn rotate_authentication_key_action(public_key: Vec<u8>) -> MoveAction {
-        Self::create_move_action(
-            Self::ROTATE_AUTHENTICATION_KEY_ENTRY_FUNCTION_NAME,
-            vec![],
-            vec![MoveValue::vector_u8(public_key)],
-        )
-    }
-
-    pub fn remove_authentication_key_action() -> MoveAction {
-        Self::create_move_action(
-            Self::REMOVE_AUTHENTICATION_KEY_ENTRY_FUNCTION_NAME,
-            vec![],
-            vec![],
-        )
-    }
 }
 
 impl<'a> ModuleBinding<'a> for EthereumValidatorModule<'a> {

crates/rooch-types/src/framework/session_key.rs

@@ -23,6 +23,8 @@ use serde_with::DisplayFromStr;
 use std::fmt::Display;
 use std::str::FromStr;
 
+pub const MODULE_NAME: &IdentStr = ident_str!("session_key");
+
 #[derive(Debug, Clone, Serialize, Deserialize, Eq, PartialEq, Hash)]
 pub struct SessionScope {
     pub module_address: AccountAddress,
@@ -71,7 +73,7 @@ impl SessionScope {
 
 impl MoveStructType for SessionScope {
     const ADDRESS: AccountAddress = ROOCH_FRAMEWORK_ADDRESS;
-    const MODULE_NAME: &'static IdentStr = ident_str!("session_key");
+    const MODULE_NAME: &'static IdentStr = MODULE_NAME;
     const STRUCT_NAME: &'static IdentStr = ident_str!("SessionScope");
 }
 
@@ -163,7 +165,7 @@ impl SessionKey {
 
 impl MoveStructType for SessionKey {
     const ADDRESS: AccountAddress = ROOCH_FRAMEWORK_ADDRESS;
-    const MODULE_NAME: &'static IdentStr = ident_str!("session_key");
+    const MODULE_NAME: &'static IdentStr = MODULE_NAME;
     const STRUCT_NAME: &'static IdentStr = ident_str!("SessionKey");
 }