Repository: stellar/rs-soroban-env
Pull request #424 (2022-09-10 04:36:48)

Move the OK, UNKNOWN_ERROR consts into Symbol
### What
Move the OK, UNKNOWN_ERROR consts into Symbol.

### Why
The constants are presented without context in the exported API of the soroban-env-* crates. Code without context is easily misunderstood, or maybe more accurately put, more difficult to understand. Moving the consts into the Symbol type gives them the perfect context because their values are of type Symbol. This improves the ergonomics of using the OK error type in contracts.


Patch:
diff --git a/soroban-env-common/src/lib.rs b/soroban-env-common/src/lib.rs
index 673b4d34..94ee9b0a 100644
--- a/soroban-env-common/src/lib.rs
+++ b/soroban-env-common/src/lib.rs
@@ -58,7 +58,7 @@ pub use vmcaller_checked_env::{VmCaller, VmCallerCheckedEnv};
 pub use bitset::{BitSet, BitSetError};
 pub use object::Object;
 pub use r#static::Static;
-pub use status::{Status, OK, UNKNOWN_ERROR};
+pub use status::Status;
 pub use symbol::{Symbol, SymbolError, SymbolIter, SymbolStr};
 
 #[inline(always)]
diff --git a/soroban-env-common/src/status.rs b/soroban-env-common/src/status.rs
index e3eede42..cb7086c9 100644
--- a/soroban-env-common/src/status.rs
+++ b/soroban-env-common/src/status.rs
@@ -22,9 +22,11 @@ pub struct Status(RawVal);
 
 decl_tagged_val_wrapper_methods!(Status);
 
-pub const UNKNOWN_ERROR: Status =
-    unsafe { Status::from_major_minor(0, ScStatusType::UnknownError as u32) };
-pub const OK: Status = unsafe { Status::from_major_minor(0, ScStatusType::Ok as u32) };
+impl Status {
+    pub const UNKNOWN_ERROR: Status =
+        unsafe { Status::from_major_minor(0, ScStatusType::UnknownError as u32) };
+    pub const OK: Status = unsafe { Status::from_major_minor(0, ScStatusType::Ok as u32) };
+}
 
 impl Hash for Status {
     #[inline(always)]
diff --git a/soroban-env-host/src/host.rs b/soroban-env-host/src/host.rs
index 650aeace..94217117 100644
--- a/soroban-env-host/src/host.rs
+++ b/soroban-env-host/src/host.rs
@@ -8,7 +8,7 @@ use core::fmt::Debug;
 use im_rc::{OrdMap, Vector};
 use num_bigint::Sign;
 use soroban_env_common::{
-    EnvVal, TryConvert, TryFromVal, TryIntoVal, VmCaller, VmCallerCheckedEnv, OK, UNKNOWN_ERROR,
+    EnvVal, Status, TryConvert, TryFromVal, TryIntoVal, VmCaller, VmCallerCheckedEnv,
 };
 
 use soroban_env_common::xdr::{
@@ -807,7 +807,7 @@ impl Host {
         let topics = self.event_topics_from_host_obj(topics)?;
         let data = self.from_host_val(data)?;
         self.record_contract_event(ContractEventType::System, topics, data)?;
-        Ok(OK.into())
+        Ok(Status::OK.into())
     }
 }
 
@@ -993,7 +993,7 @@ impl VmCallerCheckedEnv for Host {
         let topics = self.event_topics_from_host_obj(topics)?;
         let data = self.from_host_val(data)?;
         self.record_contract_event(ContractEventType::Contract, topics, data)?;
-        Ok(OK.into())
+        Ok(Status::OK.into())
     }
 
     // Notes on metering: covered by the components.
@@ -1115,11 +1115,11 @@ impl VmCallerCheckedEnv for Host {
                     {
                         Ok(pk2.to_raw())
                     } else {
-                        Ok(UNKNOWN_ERROR.to_raw()) //FIXME: replace with the actual status code
+                        Ok(Status::UNKNOWN_ERROR.to_raw()) //FIXME: replace with the actual status code
                     }
                 }
             } else {
-                Ok(UNKNOWN_ERROR.to_raw()) //FIXME: replace with the actual status code
+                Ok(Status::UNKNOWN_ERROR.to_raw()) //FIXME: replace with the actual status code
             }
         })
     }
@@ -1145,11 +1145,11 @@ impl VmCallerCheckedEnv for Host {
                     {
                         Ok(pk2.to_raw())
                     } else {
-                        Ok(UNKNOWN_ERROR.to_raw()) //FIXME: replace with the actual status code
+                        Ok(Status::UNKNOWN_ERROR.to_raw()) //FIXME: replace with the actual status code
                     }
                 }
             } else {
-                Ok(UNKNOWN_ERROR.to_raw()) //FIXME: replace with the actual status code
+                Ok(Status::UNKNOWN_ERROR.to_raw()) //FIXME: replace with the actual status code
             }
         })
     }
@@ -1158,7 +1158,7 @@ impl VmCallerCheckedEnv for Host {
         self.visit_obj(m, |hm: &HostMap| {
             match hm.get_min()? {
                 Some((pk, pv)) => Ok(pk.to_raw()),
-                None => Ok(UNKNOWN_ERROR.to_raw()), //FIXME: replace with the actual status code
+                None => Ok(Status::UNKNOWN_ERROR.to_raw()), //FIXME: replace with the actual status code
             }
         })
     }
@@ -1167,7 +1167,7 @@ impl VmCallerCheckedEnv for Host {
         self.visit_obj(m, |hm: &HostMap| {
             match hm.get_max()? {
                 Some((pk, pv)) => Ok(pk.to_raw()),
-                None => Ok(UNKNOWN_ERROR.to_raw()), //FIXME: replace with the actual status code
+                None => Ok(Status::UNKNOWN_ERROR.to_raw()), //FIXME: replace with the actual status code
             }
         })
     }


Test patch:
diff --git a/soroban-env-host/src/test/contract_event.rs b/soroban-env-host/src/test/contract_event.rs
index 0e81ead0..ff58edf9 100644
--- a/soroban-env-host/src/test/contract_event.rs
+++ b/soroban-env-host/src/test/contract_event.rs
@@ -4,7 +4,7 @@ use crate::{
         ContractEvent, ContractEventBody, ContractEventType, ContractEventV0, ExtensionPoint, Hash,
         ScMap, ScMapEntry, ScObject::Map, ScVal,
     },
-    ContractFunctionSet, Env, EnvBase, Host, HostError, RawVal, Symbol, OK,
+    ContractFunctionSet, Env, EnvBase, Host, HostError, RawVal, Status, Symbol,
 };
 use std::rc::Rc;
 
@@ -34,7 +34,7 @@ fn contract_event() -> Result<(), HostError> {
     host.register_test_contract(id, test_contract)?;
     assert_eq!(
         host.call(id, sym.into(), args.into()).get_payload(),
-        OK.to_raw().get_payload()
+        Status::OK.to_raw().get_payload()
     );
 
     let event_ref = ContractEvent {
diff --git a/soroban-env-host/src/test/map.rs b/soroban-env-host/src/test/map.rs
index ade26c86..cf2b2d06 100644
--- a/soroban-env-host/src/test/map.rs
+++ b/soroban-env-host/src/test/map.rs
@@ -1,6 +1,6 @@
 use crate::{
     xdr::{ScMap, ScMapEntry, ScObject, ScVal, ScVec},
-    CheckedEnv, Host, HostError, RawVal, RawValConvertible, Symbol, UNKNOWN_ERROR,
+    CheckedEnv, Host, HostError, RawVal, RawValConvertible, Status, Symbol,
 };
 
 #[test]
@@ -55,12 +55,12 @@ fn map_prev_and_next() -> Result<(), HostError> {
         assert_eq!(
             host.map_prev_key(obj.to_object(), 0_u32.into())?
                 .get_payload(),
-            UNKNOWN_ERROR.to_raw().get_payload()
+            Status::UNKNOWN_ERROR.to_raw().get_payload()
         );
         assert_eq!(
             host.map_prev_key(obj.to_object(), 1_u32.into())?
                 .get_payload(),
-            UNKNOWN_ERROR.to_raw().get_payload()
+            Status::UNKNOWN_ERROR.to_raw().get_payload()
         );
         assert_eq!(
             host.map_prev_key(obj.to_object(), 2_u32.into())?
@@ -83,12 +83,12 @@ fn map_prev_and_next() -> Result<(), HostError> {
         assert_eq!(
             host.map_next_key(obj.to_object(), 5_u32.into())?
                 .get_payload(),
-            UNKNOWN_ERROR.to_raw().get_payload()
+            Status::UNKNOWN_ERROR.to_raw().get_payload()
         );
         assert_eq!(
             host.map_next_key(obj.to_object(), 4_u32.into())?
                 .get_payload(),
-            UNKNOWN_ERROR.to_raw().get_payload()
+            Status::UNKNOWN_ERROR.to_raw().get_payload()
         );
         assert_eq!(
             host.map_next_key(obj.to_object(), 3_u32.into())?
@@ -138,7 +138,7 @@ fn map_prev_and_next_heterogeneous() -> Result<(), HostError> {
     {
         assert_eq!(
             host.map_prev_key(test_map, 0_u32.into())?.get_payload(),
-            UNKNOWN_ERROR.to_raw().get_payload()
+            Status::UNKNOWN_ERROR.to_raw().get_payload()
         );
         assert_eq!(
             host.map_prev_key(test_map, 4_u32.into())?.get_payload(),
@@ -183,7 +183,7 @@ fn map_prev_and_next_heterogeneous() -> Result<(), HostError> {
         assert_eq!(
             host.map_next_key(test_map, sym.clone().into())?
                 .get_payload(),
-            UNKNOWN_ERROR.to_raw().get_payload()
+            Status::UNKNOWN_ERROR.to_raw().get_payload()
         );
     }