Repository: hyperledger/indy-vdr (language: python)

Issue:
Refactor request builder
Nothing urgent, but creating this to track a potential improvement, mainly when consuming `indy-vdr` from another Rust project.

Function `build_nym_request` takes argument `role: Option<String>` which can have variety of values to represent a role - for example if you want to express "endorser", you can pass in any of following strings: `"TRUST_ANCHOR", "ENDORSER", "101"`. 

Ideally there would be defined user facing enum, something like 
```
pub enum IndyRole {
    Steward,
    Trustee,
    Endorser,
    NetworkMonitor,
}
```

To maintain backwards compatibility for FFI, values `"TRUST_ANCHOR", "ENDORSER", "101"` should be converted to appropriate enum variant on the FFI layer. (some kind of unification conversion currently happens inside of `build_nym_request`)

There might be more instances of the same pattern, but this is a particular one I've run into. 



Discussion / hints:
Agreed, this is probably something we can sneak into the next minor release.

Fix patch:
diff --git a/libindy_vdr/src/ffi/ledger.rs b/libindy_vdr/src/ffi/ledger.rs
index 22935e9d..70390c6d 100644
--- a/libindy_vdr/src/ffi/ledger.rs
+++ b/libindy_vdr/src/ffi/ledger.rs
@@ -1,4 +1,7 @@
+use std::str::FromStr;
+
 use crate::common::error::prelude::*;
+use crate::ledger::constants::UpdateRole;
 #[cfg(any(feature = "rich_schema", test))]
 use crate::ledger::identifiers::RichSchemaId;
 use crate::ledger::identifiers::{CredentialDefinitionId, RevocationRegistryId, SchemaId};
@@ -419,14 +422,8 @@ pub extern "C" fn indy_vdr_build_nym_request(
         let dest = DidValue::from_str(dest.as_str())?;
         let verkey = verkey.into_opt_string();
         let alias = alias.into_opt_string();
-        let role = role.into_opt_string();
-        let diddoc_content = match diddoc_content.as_opt_str() {
-            Some(s) => {
-                let js = serde_json::from_str(s).with_input_err("Error deserializing raw value as JSON")?;
-                Some(js)
-            }
-            None => None,
-        };
+        let role = role.as_opt_str().map(UpdateRole::from_str).transpose()?;
+        let diddoc_content = diddoc_content.as_opt_str().map(serde_json::from_str).transpose().with_input_err("Error deserializing raw value as JSON")?;
         let version = if version == -1 { None } else { Some(version) };
         let req = builder.build_nym_request(&identifier, &dest, verkey, alias, role, diddoc_content.as_ref(), version)?;
         let handle = add_request(req)?;
diff --git a/libindy_vdr/src/ledger/constants.rs b/libindy_vdr/src/ledger/constants.rs
index 2ce03859..93852811 100644
--- a/libindy_vdr/src/ledger/constants.rs
+++ b/libindy_vdr/src/ledger/constants.rs
@@ -1,3 +1,8 @@
+use serde::{de::Visitor, Deserialize, Deserializer, Serialize, Serializer};
+use std::{fmt, str::FromStr};
+
+use crate::common::error::{input_err, VdrError};
+
 pub const NODE: &str = "0";
 pub const NYM: &str = "1";
 pub const GET_TXN: &str = "3";
@@ -29,6 +34,7 @@ pub const GET_AUTH_RULE: &str = "121";
 pub const AUTH_RULES: &str = "122";
 pub const FLAG: &str = "130";
 pub const GET_FLAG: &str = "131";
+
 // RichSchema objects
 pub const RICH_SCHEMA_CTX: &str = "200";
 pub const RICH_SCHEMA: &str = "201";
@@ -96,11 +102,11 @@ pub const READ_REQUESTS: [&str; 14] = [
     GET_RICH_SCHEMA_BY_METADATA,
 ];
 
-pub const TRUSTEE: &str = "0";
-pub const STEWARD: &str = "2";
-pub const ENDORSER: &str = "101";
-pub const NETWORK_MONITOR: &str = "201";
-pub const ROLE_REMOVE: &str = "";
+pub const ROLE_TRUSTEE: usize = 0;
+pub const ROLE_STEWARD: usize = 2;
+pub const ROLE_ENDORSER: usize = 101;
+pub const ROLE_NETWORK_MONITOR: usize = 201;
+
 pub const RS_SCHEMA_TYPE_VALUE: &str = "sch";
 pub const RS_ENCODING_TYPE_VALUE: &str = "enc";
 pub const RS_CONTEXT_TYPE_VALUE: &str = "ctx";
@@ -108,14 +114,191 @@ pub const RS_MAPPING_TYPE_VALUE: &str = "map";
 pub const RS_CRED_DEF_TYPE_VALUE: &str = "cdf";
 pub const RS_PRES_DEF_TYPE_VALUE: &str = "pdf";
 
-pub const ROLES: [&str; 4] = [TRUSTEE, STEWARD, ENDORSER, NETWORK_MONITOR];
-
 // Method/version of self-certification
-pub const DEFAULT: i32 = 0; // No (enforced) self-certification
-pub const DID_SOV: i32 = 1; // Legacy self-certification
-pub const DID_INDY: i32 = 2; // Self-certification based on did:indy method spec
+/// No (enforced) self-certification
+pub const CERT_DEFAULT: i32 = 0;
+/// Legacy self-certification
+pub const CERT_DID_SOV: i32 = 1;
+/// Self-certification based on did:indy method spec
+pub const CERT_DID_INDY: i32 = 2;
+
+pub const SELF_CERT_VERSIONS: [i32; 3] = [CERT_DEFAULT, CERT_DID_SOV, CERT_DID_INDY];
+
+#[derive(Clone, Copy, Debug, PartialEq, Eq)]
+#[repr(usize)]
+pub enum LedgerRole {
+    Trustee,
+    Steward,
+    Endorser,
+    NetworkMonitor,
+    Custom(usize),
+}
+
+impl LedgerRole {
+    pub fn to_usize(self) -> usize {
+        match self {
+            Self::Trustee => ROLE_TRUSTEE,
+            Self::Steward => ROLE_STEWARD,
+            Self::Endorser => ROLE_ENDORSER,
+            Self::NetworkMonitor => ROLE_NETWORK_MONITOR,
+            Self::Custom(val) => val,
+        }
+    }
+
+    pub fn to_code(self) -> String {
+        self.to_usize().to_string()
+    }
+}
+
+impl fmt::Display for LedgerRole {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        let ident = match self {
+            Self::Trustee => "TRUSTEE",
+            Self::Steward => "STEWARD",
+            Self::Endorser => "ENDORSER",
+            Self::NetworkMonitor => "NETWORK_MONITOR",
+            Self::Custom(role) => return write!(f, "{}", role),
+        };
+        f.write_str(ident)
+    }
+}
+
+impl FromStr for LedgerRole {
+    type Err = VdrError;
+
+    fn from_str(value: &str) -> Result<Self, Self::Err> {
+        match value {
+            "TRUSTEE" => Ok(Self::Trustee),
+            "STEWARD" => Ok(Self::Steward),
+            "TRUST_ANCHOR" | "ENDORSER" => Ok(Self::Endorser),
+            "NETWORK_MONITOR" => Ok(Self::NetworkMonitor),
+            _ => {
+                if let Ok(role) = value.parse::<usize>() {
+                    Ok(Self::from(role))
+                } else {
+                    Err(input_err(format!("Invalid ledger role: {value}")))
+                }
+            }
+        }
+    }
+}
+
+impl From<usize> for LedgerRole {
+    fn from(value: usize) -> Self {
+        match value {
+            ROLE_TRUSTEE => Self::Trustee,
+            ROLE_STEWARD => Self::Steward,
+            ROLE_ENDORSER => Self::Endorser,
+            ROLE_NETWORK_MONITOR => Self::NetworkMonitor,
+            other => Self::Custom(other),
+        }
+    }
+}
+
+impl From<LedgerRole> for usize {
+    fn from(value: LedgerRole) -> Self {
+        value.to_usize()
+    }
+}
 
-pub const SELF_CERT_VERSIONS: [i32; 3] = [DEFAULT, DID_SOV, DID_INDY];
+impl<'d> Deserialize<'d> for LedgerRole {
+    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
+    where
+        D: Deserializer<'d>,
+    {
+        struct Vis;
+
+        impl<'v> Visitor<'v> for Vis {
+            type Value = LedgerRole;
+
+            fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result {
+                formatter.write_str("expected a ledger role identifier")
+            }
+
+            fn visit_str<E>(self, v: &str) -> Result<Self::Value, E>
+            where
+                E: serde::de::Error,
+            {
+                LedgerRole::from_str(v).map_err(|e| E::custom(e.to_string()))
+            }
+        }
+
+        deserializer.deserialize_any(Vis)
+    }
+}
+
+impl Serialize for LedgerRole {
+    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
+    where
+        S: Serializer,
+    {
+        serializer.collect_str(&self.to_usize())
+    }
+}
+
+#[derive(Clone, Copy, Debug, PartialEq, Eq)]
+pub enum UpdateRole {
+    Reset,
+    Set(LedgerRole),
+}
+
+impl FromStr for UpdateRole {
+    type Err = VdrError;
+
+    fn from_str(value: &str) -> Result<Self, Self::Err> {
+        match value {
+            "" => Ok(Self::Reset),
+            _ => LedgerRole::from_str(value).map(Self::Set),
+        }
+    }
+}
+
+impl<'d> Deserialize<'d> for UpdateRole {
+    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
+    where
+        D: Deserializer<'d>,
+    {
+        struct Vis;
+
+        impl<'v> Visitor<'v> for Vis {
+            type Value = UpdateRole;
+
+            fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result {
+                formatter.write_str("expected a ledger role identifier")
+            }
+
+            fn visit_none<E>(self) -> Result<Self::Value, E>
+            where
+                E: serde::de::Error,
+            {
+                Ok(UpdateRole::Reset)
+            }
+
+            fn visit_str<E>(self, v: &str) -> Result<Self::Value, E>
+            where
+                E: serde::de::Error,
+            {
+                LedgerRole::from_str(v)
+                    .map_err(|e| E::custom(e.to_string()))
+                    .map(UpdateRole::Set)
+            }
+        }
+
+        deserializer.deserialize_any(Vis)
+    }
+}
+
+impl Serialize for UpdateRole {
+    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
+    where
+        S: Serializer,
+    {
+        match self {
+            Self::Reset => serializer.serialize_none(),
+            Self::Set(role) => serializer.collect_str(&role.to_usize()),
+        }
+    }
+}
 
 pub fn txn_name_to_code(txn: &str) -> Option<&str> {
     if REQUESTS.contains(&txn) {
diff --git a/libindy_vdr/src/ledger/request_builder.rs b/libindy_vdr/src/ledger/request_builder.rs
index bd94fdf5..c37e3dc2 100644
--- a/libindy_vdr/src/ledger/request_builder.rs
+++ b/libindy_vdr/src/ledger/request_builder.rs
@@ -23,7 +23,7 @@ use super::requests::cred_def::{CredDefOperation, CredentialDefinition, GetCredD
 use super::requests::flag::{FlagOperation, GetFlagOperation};
 use super::requests::ledgers_freeze::{GetFrozenLedgersOperation, LedgersFreezeOperation};
 use super::requests::node::{NodeOperation, NodeOperationData};
-use super::requests::nym::{role_to_code, GetNymOperation, NymOperation};
+use super::requests::nym::{GetNymOperation, NymOperation};
 use super::requests::pool::{
     PoolConfigOperation, PoolRestartOperation, PoolUpgradeOperation, Schedule,
 };
@@ -47,7 +47,7 @@ use super::requests::txn::GetTxnOperation;
 use super::requests::validator_info::GetValidatorInfoOperation;
 use super::requests::{Request, RequestType};
 
-use super::constants::txn_name_to_code;
+use super::constants::{txn_name_to_code, UpdateRole};
 
 fn datetime_to_date_timestamp(time: u64) -> u64 {
     const SEC_IN_DAY: u64 = 86400;
@@ -156,11 +156,10 @@ impl RequestBuilder {
         dest: &DidValue,
         verkey: Option<String>,
         alias: Option<String>,
-        role: Option<String>,
+        role: Option<UpdateRole>,
         diddoc_content: Option<&SJsonValue>,
         version: Option<i32>,
     ) -> VdrResult<PreparedRequest> {
-        let role = role_to_code(role)?;
         let operation = NymOperation::new(
             dest.to_short(),
             verkey,
diff --git a/libindy_vdr/src/ledger/requests/auth_rule.rs b/libindy_vdr/src/ledger/requests/auth_rule.rs
index 6a2ac063..80960836 100644
--- a/libindy_vdr/src/ledger/requests/auth_rule.rs
+++ b/libindy_vdr/src/ledger/requests/auth_rule.rs
@@ -4,6 +4,7 @@ use std::ops::Not;
 use super::constants::{AUTH_RULE, AUTH_RULES, GET_AUTH_RULE};
 use super::{ProtocolVersion, RequestType};
 use crate::common::error::prelude::*;
+use crate::ledger::constants::LedgerRole;
 
 #[allow(non_camel_case_types)]
 #[derive(Deserialize, Debug, Serialize, PartialEq)]
@@ -54,7 +55,7 @@ pub enum Constraint {
 #[derive(Serialize, Deserialize, PartialEq, Debug, Clone)]
 pub struct RoleConstraint {
     pub sig_count: u32,
-    pub role: Option<String>,
+    pub role: Option<LedgerRole>,
     #[serde(skip_serializing_if = "Option::is_none")]
     pub metadata: Option<Value>,
     #[serde(default)]
diff --git a/libindy_vdr/src/ledger/requests/nym.rs b/libindy_vdr/src/ledger/requests/nym.rs
index 33b3eec7..3a1e7d2c 100644
--- a/libindy_vdr/src/ledger/requests/nym.rs
+++ b/libindy_vdr/src/ledger/requests/nym.rs
@@ -3,8 +3,8 @@ use sha2::{Digest, Sha256};
 use super::constants::{GET_NYM, NYM};
 use super::did::ShortDidValue;
 use super::{ProtocolVersion, RequestType};
-use crate::common::error::{input_err, VdrResult};
-use crate::ledger::constants::{ENDORSER, NETWORK_MONITOR, ROLES, ROLE_REMOVE, STEWARD, TRUSTEE};
+use crate::common::error::VdrResult;
+use crate::ledger::constants::UpdateRole;
 
 #[derive(Serialize, PartialEq, Debug)]
 #[serde(rename_all = "camelCase")]
@@ -16,7 +16,7 @@ pub struct NymOperation {
     #[serde(skip_serializing_if = "Option::is_none")]
     pub alias: Option<String>,
     #[serde(skip_serializing_if = "Option::is_none")]
-    pub role: Option<::serde_json::Value>,
+    pub role: Option<UpdateRole>,
     #[serde(skip_serializing_if = "Option::is_none")]
     pub diddoc_content: Option<String>,
     #[serde(skip_serializing_if = "Option::is_none")]
@@ -28,7 +28,7 @@ impl NymOperation {
         dest: ShortDidValue,
         verkey: Option<String>,
         alias: Option<String>,
-        role: Option<::serde_json::Value>,
+        role: Option<UpdateRole>,
         diddoc_content: Option<String>,
         version: Option<i32>,
     ) -> NymOperation {
@@ -86,22 +86,3 @@ impl RequestType for GetNymOperation {
         Ok(Some(hash))
     }
 }
-
-pub fn role_to_code(role: Option<String>) -> VdrResult<Option<serde_json::Value>> {
-    if let Some(r) = role {
-        Ok(Some(if r == ROLE_REMOVE {
-            serde_json::Value::Null
-        } else {
-            json!(match r.as_str() {
-                "STEWARD" => STEWARD,
-                "TRUSTEE" => TRUSTEE,
-                "TRUST_ANCHOR" | "ENDORSER" => ENDORSER,
-                "NETWORK_MONITOR" => NETWORK_MONITOR,
-                role if ROLES.contains(&role) => role,
-                role => return Err(input_err(format!("Invalid role: {}", role))),
-            })
-        }))
-    } else {
-        Ok(None)
-    }
-}


Test patch:
diff --git a/libindy_vdr/tests/auth_rule.rs b/libindy_vdr/tests/auth_rule.rs
index 95689c80..de771e14 100644
--- a/libindy_vdr/tests/auth_rule.rs
+++ b/libindy_vdr/tests/auth_rule.rs
@@ -19,7 +19,7 @@ const VALUE: &str = "0";
 fn _role_constraint() -> Constraint {
     Constraint::RoleConstraint(RoleConstraint {
         sig_count: 1,
-        role: Some(constants::TRUSTEE.to_string()),
+        role: Some(constants::LedgerRole::Trustee),
         metadata: Some(json!({})),
         need_to_be_owner: false,
         off_ledger_signature: false,
@@ -35,7 +35,7 @@ fn _complex_constraint() -> Constraint {
                     _role_constraint(),
                     Constraint::RoleConstraint(RoleConstraint {
                         sig_count: 2,
-                        role: Some("2".to_string()),
+                        role: Some(constants::LedgerRole::Custom(2)),
                         metadata: None,
                         need_to_be_owner: true,
                         off_ledger_signature: false,
diff --git a/libindy_vdr/tests/cred_def.rs b/libindy_vdr/tests/cred_def.rs
index 3c92b860..f1df1c7e 100644
--- a/libindy_vdr/tests/cred_def.rs
+++ b/libindy_vdr/tests/cred_def.rs
@@ -89,7 +89,7 @@ mod send_cred_def {
 
     #[rstest]
     fn test_pool_send_cred_def_requests(pool: TestPool) {
-        let identity = helpers::new_ledger_identity(&pool, Some(String::from("TRUSTEE")));
+        let identity = helpers::new_ledger_identity(&pool, Some(constants::LedgerRole::Trustee));
 
         let schema = helpers::schema::default_schema(&identity.did);
         let (_schema_id, schema_seq_no) = helpers::schema::publish(&identity, &pool, &schema);
diff --git a/libindy_vdr/tests/endorse_transaction.rs b/libindy_vdr/tests/endorse_transaction.rs
index 4a7595bb..8c982763 100644
--- a/libindy_vdr/tests/endorse_transaction.rs
+++ b/libindy_vdr/tests/endorse_transaction.rs
@@ -16,10 +16,11 @@ mod endorse_transaction {
     use crate::utils::fixtures::*;
     use crate::utils::helpers;
     use crate::utils::pool::TestPool;
+    use indy_vdr::ledger::constants;
 
     #[rstest]
     fn test_pool_send_request_by_endorser(pool: TestPool) {
-        let endorser = helpers::new_ledger_identity(&pool, Some(String::from("ENDORSER")));
+        let endorser = helpers::new_ledger_identity(&pool, Some(constants::LedgerRole::Endorser));
 
         // Endorse Schema.  Multi sign + Multi Sign.
         let author = helpers::new_ledger_identity(&pool, None);
@@ -48,7 +49,7 @@ mod endorse_transaction {
     #[rstest]
     fn test_pool_send_request_by_endorser_for_missed_one_of_signatures(pool: TestPool) {
         let author = helpers::new_ledger_identity(&pool, None);
-        let endorser = helpers::new_ledger_identity(&pool, Some(String::from("ENDORSER")));
+        let endorser = helpers::new_ledger_identity(&pool, Some(constants::LedgerRole::Endorser));
 
         // Endorse Schema signed by author only
 
diff --git a/libindy_vdr/tests/flag.rs b/libindy_vdr/tests/flag.rs
index dd36eac2..dd2d3226 100644
--- a/libindy_vdr/tests/flag.rs
+++ b/libindy_vdr/tests/flag.rs
@@ -99,6 +99,6 @@ mod send_flag {
             .send_request_with_retries(&get_flag_request, &flag_response)
             .unwrap();
         let data = helpers::get_response_data(&response);
-        assert_eq!(true, data.is_err())
+        assert!(data.is_err())
     }
 }
diff --git a/libindy_vdr/tests/nym.rs b/libindy_vdr/tests/nym.rs
index 5b5c69fe..2f7b4a63 100644
--- a/libindy_vdr/tests/nym.rs
+++ b/libindy_vdr/tests/nym.rs
@@ -4,7 +4,7 @@ mod utils;
 inject_dependencies!();
 
 use indy_vdr::ledger::constants;
-use indy_vdr::ledger::requests::nym::role_to_code;
+use indy_vdr::ledger::constants::LedgerRole;
 use indy_vdr::utils::did::DidValue;
 
 use crate::utils::crypto::Identity;
@@ -12,7 +12,7 @@ use crate::utils::fixtures::*;
 use crate::utils::helpers;
 
 const ALIAS: &str = "alias";
-const ROLE: &str = "TRUSTEE";
+const ROLE: LedgerRole = LedgerRole::Trustee;
 
 #[test]
 fn empty() {
@@ -25,6 +25,8 @@ mod builder {
     use indy_vdr::ledger::RequestBuilder;
 
     mod nym {
+        use indy_vdr::ledger::constants::UpdateRole;
+
         use super::*;
 
         #[rstest]
@@ -59,7 +61,7 @@ mod builder {
                     &identity.did,
                     Some(identity.verkey.clone()),
                     Some(ALIAS.to_string()),
-                    Some(ROLE.to_string()),
+                    Some(UpdateRole::Set(ROLE)),
                     Some(&diddoc_content),
                     None,
                 )
@@ -70,7 +72,7 @@ mod builder {
                 "dest": identity.did,
                 "verkey": identity.verkey,
                 "alias": ALIAS,
-                "role": role_to_code(Some(String::from(ROLE))).unwrap(),
+                "role": ROLE.to_code(),
                 "diddocContent": diddoc_content.to_string(),
             });
 
@@ -89,7 +91,7 @@ mod builder {
                     &my_did,
                     None,
                     None,
-                    Some(String::from("")),
+                    Some(UpdateRole::Reset),
                     None,
                     None,
                 )
@@ -122,27 +124,6 @@ mod builder {
 
             helpers::check_request_operation(&nym_request, expected_result);
         }
-
-        #[rstest]
-        fn test_build_nym_request_works_for_invalid_role(
-            request_builder: RequestBuilder,
-            trustee_did: DidValue,
-            identity: Identity,
-        ) {
-            let role = "INALID_ROLE_ALIAS";
-
-            let _err = request_builder
-                .build_nym_request(
-                    &trustee_did,
-                    &identity.did,
-                    Some(identity.verkey),
-                    None,
-                    Some(role.to_string()),
-                    None,
-                    None,
-                )
-                .unwrap_err();
-        }
     }
 
     mod get_nym {
@@ -192,7 +173,8 @@ mod builder {
 mod send {
     use super::*;
     use crate::utils::pool::TestPool;
-    use indy_vdr::ledger::{constants::ROLE_REMOVE, responses::GetNymResultV1};
+    use indy_vdr::ledger::{constants::UpdateRole, responses::GetNymResultV1};
+    use std::str::FromStr;
 
     #[rstest]
     fn test_pool_send_nym_request(pool: TestPool, trustee: Identity, identity: Identity) {
@@ -245,7 +227,7 @@ mod send {
                 &identity.did,
                 Some(identity.verkey.to_string()),
                 Some(ALIAS.to_string()),
-                Some(ROLE.to_string()),
+                Some(UpdateRole::Set(ROLE)),
                 None,
                 None,
             )
@@ -267,7 +249,7 @@ mod send {
         let expected_data = json!({
             "dest": &identity.did,
             "verkey": &identity.verkey,
-            "role": role_to_code(Some(String::from(ROLE))).unwrap(),
+            "role": ROLE.to_code(),
         });
         assert_eq!(expected_data, parse_get_nym_response(&response));
     }
@@ -287,7 +269,7 @@ mod send {
                 &identity.did,
                 Some(identity.verkey.to_string()),
                 Some(ALIAS.to_string()),
-                Some(ROLE.to_string()),
+                Some(UpdateRole::Set(ROLE)),
                 Some(&diddoc_content),
                 None,
             )
@@ -310,7 +292,7 @@ mod send {
             "identifier": &trustee.did,
             "dest": &identity.did,
             "verkey": &identity.verkey,
-            "role": role_to_code(Some(String::from(ROLE))).unwrap(),
+            "role": ROLE.to_code(),
             "diddocContent": &diddoc_content.to_string()
         });
 
@@ -339,7 +321,7 @@ mod send {
                 &identity.did, // Self-cert version 1 identifier
                 Some(identity.verkey.to_string()),
                 Some(ALIAS.to_string()),
-                Some(ROLE.to_string()),
+                Some(UpdateRole::Set(ROLE)),
                 None,
                 Some(2), // Claim Self-cert version 2
             )
@@ -363,7 +345,7 @@ mod send {
                 &identity.did, // Non self-cert identifier
                 Some(identity.verkey.to_string()),
                 Some(ALIAS.to_string()),
-                Some(ROLE.to_string()),
+                Some(UpdateRole::Set(ROLE)),
                 None,
                 Some(2), // Claim Self-cert version 2
             )
@@ -388,7 +370,7 @@ mod send {
                 &identity.did, // Non self-cert identifier
                 Some(identity.verkey.to_string()),
                 Some(ALIAS.to_string()),
-                Some(ROLE.to_string()),
+                Some(UpdateRole::Set(ROLE)),
                 None,
                 None,
             )
@@ -411,7 +393,7 @@ mod send {
                 &identity.did, // Self-cert version 1 identifier
                 Some(identity.verkey.to_string()),
                 Some(ALIAS.to_string()),
-                Some(ROLE.to_string()),
+                Some(UpdateRole::Set(ROLE)),
                 None,
                 Some(0), // Claim Self-cert version 0
             )
@@ -434,7 +416,7 @@ mod send {
             "identifier": &trustee.did,
             "dest": &identity.did,
             "verkey": &identity.verkey,
-            "role": role_to_code(Some(String::from(ROLE))).unwrap(),
+            "role": ROLE.to_code(),
             "version": 0,
         });
 
@@ -465,7 +447,7 @@ mod send {
                 &identity.did,
                 Some(identity.verkey.to_string()),
                 Some(ALIAS.to_string()),
-                Some(ROLE.to_string()),
+                Some(UpdateRole::Set(ROLE)),
                 Some(&diddoc_content),
                 Some(2),
             )
@@ -488,7 +470,7 @@ mod send {
             "identifier": &trustee.did,
             "dest": &identity.did,
             "verkey": &identity.verkey,
-            "role": role_to_code(Some(String::from(ROLE))).unwrap(),
+            "role": ROLE.to_code(),
             "diddocContent": &diddoc_content.to_string(),
             "version": 2,
         });
@@ -518,6 +500,7 @@ mod send {
         role: &str,
     ) {
         let new_identity = Identity::new(None, None);
+        let upd_role = UpdateRole::from_str(role).unwrap();
 
         // Send NYM
         let mut nym_request = pool
@@ -527,7 +510,7 @@ mod send {
                 &new_identity.did,
                 Some(new_identity.verkey.to_string()),
                 None,
-                Some(role.to_string()),
+                Some(upd_role),
                 None,
                 None,
             )
@@ -549,7 +532,7 @@ mod send {
         let expected_data = json!({
             "dest": &new_identity.did,
             "verkey": &new_identity.verkey,
-            "role": role_to_code(Some(role.to_string())).unwrap(),
+            "role": LedgerRole::from_str(role).unwrap().to_code(),
         });
         assert_eq!(expected_data, parse_get_nym_response(&response));
     }
@@ -568,7 +551,7 @@ mod send {
                 &identity.did,
                 Some(identity.verkey.to_string()),
                 None,
-                Some(ROLE.to_string()),
+                Some(UpdateRole::Set(ROLE)),
                 None,
                 None,
             )
@@ -590,7 +573,7 @@ mod send {
         let expected_data = json!({
             "dest": &identity.did,
             "verkey": &identity.verkey,
-            "role": role_to_code(Some(String::from(ROLE))).unwrap(),
+            "role": ROLE.to_code(),
         });
         assert_eq!(expected_data, parse_get_nym_response(&response));
 
@@ -602,7 +585,7 @@ mod send {
                 &identity.did,
                 None,
                 None,
-                Some(ROLE_REMOVE.to_string()),
+                Some(UpdateRole::Reset),
                 None,
                 None,
             )
diff --git a/libindy_vdr/tests/resolver.rs b/libindy_vdr/tests/resolver.rs
index b518274f..aa4a7d46 100644
--- a/libindy_vdr/tests/resolver.rs
+++ b/libindy_vdr/tests/resolver.rs
@@ -14,8 +14,6 @@ mod send_resolver {
     use futures_executor::block_on;
     use indy_vdr::resolver::PoolResolver as Resolver;
 
-    use super::*;
-
     use crate::utils::crypto::Identity;
     use crate::utils::fixtures::*;
     use crate::utils::helpers;
diff --git a/libindy_vdr/tests/revoc_reg.rs b/libindy_vdr/tests/revoc_reg.rs
index d7c4b09c..9f52eca8 100644
--- a/libindy_vdr/tests/revoc_reg.rs
+++ b/libindy_vdr/tests/revoc_reg.rs
@@ -181,7 +181,7 @@ mod send_revoc_reg {
 
     #[rstest]
     fn test_pool_send_revoc_reg_def_def_requests(pool: TestPool) {
-        let identity = helpers::new_ledger_identity(&pool, Some(String::from("TRUSTEE")));
+        let identity = helpers::new_ledger_identity(&pool, Some(constants::LedgerRole::Trustee));
 
         let schema = helpers::schema::default_schema(&identity.did);
         let (_schema_id, schema_seq_no) = helpers::schema::publish(&identity, &pool, &schema);
@@ -349,13 +349,11 @@ mod send_revoc_reg {
     ) -> String {
         let get_revoc_reg = pool
             .request_builder()
-            .build_get_revoc_reg_request(None, &revoc_reg_id, timestamp)
+            .build_get_revoc_reg_request(None, revoc_reg_id, timestamp)
             .unwrap();
 
-        let response = pool
-            .send_request_with_retries(&get_revoc_reg, revoc_reg_entry_response)
-            .unwrap();
-        response
+        pool.send_request_with_retries(&get_revoc_reg, revoc_reg_entry_response)
+            .unwrap()
     }
 
     fn _get_revocation_registry_delta(
@@ -367,13 +365,11 @@ mod send_revoc_reg {
     ) -> String {
         let get_revoc_reg_delta = pool
             .request_builder()
-            .build_get_revoc_reg_delta_request(None, &revoc_reg_id, from, to)
+            .build_get_revoc_reg_delta_request(None, revoc_reg_id, from, to)
             .unwrap();
 
-        let response = pool
-            .send_request_with_retries(&get_revoc_reg_delta, revoc_reg_entry_response)
-            .unwrap();
-        response
+        pool.send_request_with_retries(&get_revoc_reg_delta, revoc_reg_entry_response)
+            .unwrap()
     }
 
     fn _extract_txn_time_from_reply(reply: &str) -> u64 {
diff --git a/libindy_vdr/tests/schema.rs b/libindy_vdr/tests/schema.rs
index ff8df964..ed78a3be 100644
--- a/libindy_vdr/tests/schema.rs
+++ b/libindy_vdr/tests/schema.rs
@@ -130,7 +130,7 @@ mod send_schema {
 
     #[rstest]
     fn test_pool_schema_requests(pool: TestPool) {
-        let identity = helpers::new_ledger_identity(&pool, Some(String::from("TRUSTEE")));
+        let identity = helpers::new_ledger_identity(&pool, Some(constants::LedgerRole::Endorser));
         let schema = helpers::schema::default_schema(&identity.did);
 
         // Send Schema
diff --git a/libindy_vdr/tests/utils/helpers.rs b/libindy_vdr/tests/utils/helpers.rs
index f1d3862b..3f206840 100644
--- a/libindy_vdr/tests/utils/helpers.rs
+++ b/libindy_vdr/tests/utils/helpers.rs
@@ -1,6 +1,7 @@
 use crate::utils::crypto::Identity;
 use crate::utils::pool::TestPool;
 use indy_vdr::common::error::VdrResult;
+use indy_vdr::ledger::constants::{LedgerRole, UpdateRole};
 use indy_vdr::pool::{NodeReplies, PreparedRequest};
 use indy_vdr::utils::did::DidValue;
 use rand::distributions::Alphanumeric;
@@ -31,7 +32,7 @@ pub fn get_response_data(response: &str) -> Result<serde_json::Value, String> {
     Err(String::from("Cannot get response data"))
 }
 
-pub fn new_ledger_identity(pool: &TestPool, role: Option<String>) -> Identity {
+pub fn new_ledger_identity(pool: &TestPool, role: Option<LedgerRole>) -> Identity {
     let trustee = Identity::trustee();
     let new_identity = Identity::new(None, None);
 
@@ -43,7 +44,7 @@ pub fn new_ledger_identity(pool: &TestPool, role: Option<String>) -> Identity {
             &new_identity.did,
             Some(new_identity.verkey.to_string()),
             None,
-            role,
+            role.map(UpdateRole::Set),
             None,
             None,
         )