Apply review comments

teonite · Aug 17, 2023 · 0ddcf85 · 0ddcf85
1 parent e009bed
commit 0ddcf85
Show file tree

Hide file tree

Showing 9 changed files with 32 additions and 44 deletions.
diff --git a/execution_engine/src/engine_state/mod.rs b/execution_engine/src/engine_state/mod.rs
@@ -2553,6 +2553,7 @@ fn should_charge_for_errors_in_wasm(execution_result: &ExecutionResult) -> bool
                 | ExecError::NoActiveContractVersions(_)
                 | ExecError::InvalidContractVersion(_)
                 | ExecError::NoSuchMethod(_)
+                | ExecError::AbstractMethod(_)
                 | ExecError::KeyIsNotAURef(_)
                 | ExecError::UnexpectedStoredValueVariant
                 | ExecError::LockedContract(_)

diff --git a/execution_engine/src/execution/error.rs b/execution_engine/src/execution/error.rs
@@ -128,6 +128,9 @@ pub enum Error {
     /// Contract does not have specified entry point.
     #[error("No such method: {}", _0)]
     NoSuchMethod(String),
+    /// Contract does
+    #[error("Error calling an abstract entry point: {}", _0)]
+    AbstractMethod(String),
     /// Error processing WASM bytes.
     #[error("Wasm preprocessing error: {}", _0)]
     WasmPreprocessing(PreprocessingError),

diff --git a/execution_engine/src/runtime/mod.rs b/execution_engine/src/runtime/mod.rs
@@ -1075,13 +1075,16 @@ where
                 // Session code can't be called from Contract code for security reasons.
                 Err(Error::InvalidContext)
             }
+            (EntryPointType::Install, EntryPointType::Session) => {
+                // Session code can't be called from Installer code for security reasons.
+                Err(Error::InvalidContext)
+            }
             (EntryPointType::Session, EntryPointType::Session) => {
                 // Session code called from session reuses current base key
                 Ok(self.context.get_entity_address())
             }
             (EntryPointType::Session, EntryPointType::Contract)
             | (EntryPointType::Contract, EntryPointType::Contract) => Ok(contract_hash.into()),
-
             _ => {
                 // Any other combination (installer, normal, etc.) is a contract context.
                 Ok(contract_hash.into())
@@ -1228,11 +1231,7 @@ where
                 self.context.entity().named_keys().clone(),
                 self.context.entity().extract_access_rights(contract_hash),
             ),
-            EntryPointType::Contract => (
-                contract.named_keys().clone(),
-                contract.extract_access_rights(contract_hash),
-            ),
-            EntryPointType::Install | EntryPointType::Normal => (
+            EntryPointType::Contract | EntryPointType::Install => (
                 contract.named_keys().clone(),
                 contract.extract_access_rights(contract_hash),
             ),
@@ -1255,12 +1254,10 @@ where
                     contract.contract_package_hash(),
                     contract_hash,
                 ),
-                EntryPointType::Install | EntryPointType::Normal => {
-                    CallStackElement::stored_contract(
-                        contract.contract_package_hash(),
-                        contract_hash,
-                    )
-                }
+                EntryPointType::Install => CallStackElement::stored_contract(
+                    contract.contract_package_hash(),
+                    contract_hash,
+                ),
             };
             stack.push(call_stack_element)?;
 
@@ -2668,7 +2665,7 @@ where
 
                 Ok(())
             }
-            EntryPointAccess::Abstract => Err(Error::NoSuchMethod(name.to_string())),
+            EntryPointAccess::Abstract => Err(Error::AbstractMethod(name.to_string())),
         }
     }
 

diff --git a/execution_engine_testing/tests/src/test/contract_api/get_call_stack.rs b/execution_engine_testing/tests/src/test/contract_api/get_call_stack.rs
@@ -200,7 +200,7 @@ fn assert_each_context_has_correct_call_stack_info(
         let stored_call_stack_key = format!("call_stack-{}", i);
         // we need to know where to look for the call stack information
         let call_stack = match call.entry_point_type {
-            EntryPointType::Contract | EntryPointType::Install | EntryPointType::Normal => builder
+            EntryPointType::Contract | EntryPointType::Install => builder
                 .get_call_stack_from_contract_context(
                     &stored_call_stack_key,
                     current_contract_package_hash,
@@ -262,14 +262,14 @@ fn assert_each_context_has_correct_call_stack_info_module_bytes(
         let stored_call_stack_key = format!("call_stack-{}", i);
         // we need to know where to look for the call stack information
         let call_stack = match call.entry_point_type {
-            EntryPointType::Contract => builder.get_call_stack_from_contract_context(
-                &stored_call_stack_key,
-                current_contract_package_hash,
-            ),
+            EntryPointType::Contract | EntryPointType::Install => builder
+                .get_call_stack_from_contract_context(
+                    &stored_call_stack_key,
+                    current_contract_package_hash,
+                ),
             EntryPointType::Session => {
                 builder.get_call_stack_from_session_context(&stored_call_stack_key)
             }
-            _ => unreachable!(),
         };
         let (head, rest) = call_stack.split_at(usize::one());
         assert_eq!(

diff --git a/execution_engine_testing/tests/src/test/counter_factory.rs b/execution_engine_testing/tests/src/test/counter_factory.rs
@@ -56,7 +56,7 @@ fn should_not_call_undefined_entrypoints_on_factory() {
     let no_such_method_2 = builder.get_error().expect("should have error");
 
     assert!(
-        matches!(&no_such_method_2, Error::Exec(execution::Error::NoSuchMethod(function_name)) if function_name == INCREASE_ENTRY_POINT),
+        matches!(&no_such_method_2, Error::Exec(execution::Error::AbstractMethod(function_name)) if function_name == INCREASE_ENTRY_POINT),
         "{:?}",
         &no_such_method_2
     );
@@ -76,7 +76,7 @@ fn should_not_call_undefined_entrypoints_on_factory() {
     let no_such_method_3 = builder.get_error().expect("should have error");
 
     assert!(
-        matches!(&no_such_method_3, Error::Exec(execution::Error::NoSuchMethod(function_name)) if function_name == DECREASE_ENTRY_POINT),
+        matches!(&no_such_method_3, Error::Exec(execution::Error::AbstractMethod(function_name)) if function_name == DECREASE_ENTRY_POINT),
         "{:?}",
         &no_such_method_3
     );

diff --git a/smart_contracts/contracts/test/counter-factory/src/main.rs b/smart_contracts/contracts/test/counter-factory/src/main.rs
@@ -85,15 +85,15 @@ fn installer(name: String, initial_value: U512) {
             Parameters::new(),
             CLType::Unit,
             EntryPointAccess::Public,
-            EntryPointType::Normal,
+            EntryPointType::Contract,
         );
         entry_points.add_entry_point(entry_point);
         let entry_point: EntryPoint = EntryPoint::new(
             DECREASE_ENTRY_POINT.to_string(),
             Parameters::new(),
             CLType::Unit,
             EntryPointAccess::Public,
-            EntryPointType::Normal,
+            EntryPointType::Contract,
         );
         entry_points.add_entry_point(entry_point);
 
@@ -137,15 +137,15 @@ pub extern "C" fn call() {
             Parameters::new(),
             CLType::Unit,
             EntryPointAccess::Abstract,
-            EntryPointType::Normal,
+            EntryPointType::Contract,
         );
         entry_points.add_entry_point(entry_point);
         let entry_point: EntryPoint = EntryPoint::new(
             DECREASE_ENTRY_POINT.to_string(),
             Parameters::new(),
             CLType::Unit,
             EntryPointAccess::Abstract,
-            EntryPointType::Normal,
+            EntryPointType::Contract,
         );
         entry_points.add_entry_point(entry_point);
 

diff --git a/types/src/access_rights.rs b/types/src/access_rights.rs
@@ -21,8 +21,8 @@ pub const ACCESS_RIGHTS_SERIALIZED_LENGTH: usize = 1;
 bitflags! {
     /// A struct which behaves like a set of bitflags to define access rights associated with a
     /// [`URef`](crate::URef).
-    // #[derive(Debug, Copy, PartialEq, Eq, Clone, PartialOrd, Ord, Hash)]
-   pub struct AccessRights: u8 {
+    #[cfg_attr(feature = "datasize", derive(DataSize))]
+    pub struct AccessRights: u8 {
         /// No permissions
         const NONE = 0;
         /// Permission to read the value under the associated `URef`.
@@ -42,17 +42,6 @@ bitflags! {
     }
 }
 
-#[cfg(feature = "datasize")]
-impl DataSize for AccessRights {
-    const IS_DYNAMIC: bool = u8::IS_DYNAMIC;
-
-    const STATIC_HEAP_SIZE: usize = u8::STATIC_HEAP_SIZE;
-
-    fn estimate_heap_size(&self) -> usize {
-        self.bits().estimate_heap_size()
-    }
-}
-
 impl Default for AccessRights {
     fn default() -> Self {
         AccessRights::NONE

diff --git a/types/src/addressable_entity.rs b/types/src/addressable_entity.rs
@@ -1024,17 +1024,16 @@ pub enum EntryPointType {
     Session = 0b00000000,
     /// Runs within contract's context
     Contract = 0b00000001,
-    /// Pattern of entry point types introduced in 2.0.
-    ///
-    /// If this bit is missing, that means given entry point type was defined in pre-2.0 world.
     /// Installer entry point.
     Install = 0b10000000,
-    /// Normal entry point.
-    Normal = 0b10000001,
 }
 
 impl EntryPointType {
     /// Checks if entry point type is introduced before 2.0.
+    ///
+    /// This method checks if there is a bit pattern for entry point types introduced in 2.0.
+    ///
+    /// If this bit is missing, that means given entry point type was defined in pre-2.0 world.
     pub fn is_legacy_pattern(&self) -> bool {
         (*self as u8) & 0b10000000 == 0
     }

diff --git a/types/src/gens.rs b/types/src/gens.rs
@@ -300,7 +300,6 @@ pub fn entry_point_type_arb() -> impl Strategy<Value = EntryPointType> {
         Just(EntryPointType::Session),
         Just(EntryPointType::Contract),
         Just(EntryPointType::Install),
-        Just(EntryPointType::Normal),
     ]
 }