Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor basewallet and corresponding consumers #843

Merged
merged 3 commits into from
Jun 9, 2023
Merged

Refactor basewallet and corresponding consumers #843

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
31e0346
Refactor basewallet and corresponding consumers
Apr 30, 2023
339c045
Merge branch 'main' into refactor/base_wallet
tech-bash May 25, 2023
ed38d6a
Merge branch 'main' into refactor/base_wallet
tech-bash May 31, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 10 additions & 4 deletions aries_vcx/src/utils/mockdata/profile/mock_wallet.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use aries_vcx_core::wallet::base_wallet::BaseWallet;
use async_trait::async_trait;

use crate::utils::{self};
use std::collections::HashMap;

#[derive(Debug)]
pub(crate) struct MockWallet;
Expand Down Expand Up @@ -38,12 +39,12 @@ impl BaseWallet for MockWallet {
xtype: &str,
id: &str,
value: &str,
tags_json: Option<&str>,
tags: Option<HashMap<String, String>>,
) -> VcxCoreResult<()> {
Ok(())
}

async fn get_wallet_record(&self, xtype: &str, id: &str, options_json: &str) -> VcxCoreResult<String> {
async fn get_wallet_record(&self, xtype: &str, id: &str, options: &str) -> VcxCoreResult<String> {
Ok(r#"{"id":"123","type":"record type","value":"record value","tags":null}"#.to_string())
}

Expand All @@ -55,11 +56,16 @@ impl BaseWallet for MockWallet {
Ok(())
}

async fn add_wallet_record_tags(&self, xtype: &str, id: &str, tags_json: &str) -> VcxCoreResult<()> {
async fn add_wallet_record_tags(&self, xtype: &str, id: &str, tags: HashMap<String, String>) -> VcxCoreResult<()> {
Ok(())
}

async fn update_wallet_record_tags(&self, xtype: &str, id: &str, tags_json: &str) -> VcxCoreResult<()> {
async fn update_wallet_record_tags(
&self,
xtype: &str,
id: &str,
tags: HashMap<String, String>,
) -> VcxCoreResult<()> {
Ok(())
}

Expand Down
4 changes: 2 additions & 2 deletions aries_vcx_core/src/anoncreds/credx_anoncreds.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -606,10 +606,10 @@ impl BaseAnonCreds for IndyCredxAnonCreds {
let credential_id = cred_id.map_or(Uuid::new_v4().to_string(), String::from);

let record_value = serde_json::to_string(&credential)?;
let tags_json = serde_json::to_string(&tags)?;
let tags_json: HashMap<String, String> = serde_json::from_value(tags)?;

self.wallet
.add_wallet_record(CATEGORY_CREDENTIAL, &credential_id, &record_value, Some(&tags_json))
.add_wallet_record(CATEGORY_CREDENTIAL, &credential_id, &record_value, Some(tags_json))
.await?;

Ok(credential_id)
Expand Down
17 changes: 11 additions & 6 deletions aries_vcx_core/src/wallet/agency_client_wallet.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use std::sync::Arc;

use crate::utils::async_fn_iterator::AsyncFnIterator;
use std::collections::HashMap;
use std::sync::Arc;

use super::base_wallet::BaseWallet;
use crate::errors::error::{AriesVcxCoreError, AriesVcxCoreErrorKind, VcxCoreResult};
Expand Down Expand Up @@ -43,12 +43,12 @@ impl BaseWallet for AgencyClientWallet {
xtype: &str,
id: &str,
value: &str,
tags_json: Option<&str>,
tags: Option<HashMap<String, String>>,
) -> VcxCoreResult<()> {
Err(unimplemented_agency_client_wallet_method("add_wallet_record"))
}

async fn get_wallet_record(&self, xtype: &str, id: &str, options_json: &str) -> VcxCoreResult<String> {
async fn get_wallet_record(&self, xtype: &str, id: &str, options: &str) -> VcxCoreResult<String> {
Err(unimplemented_agency_client_wallet_method("get_wallet_record"))
}

Expand All @@ -60,15 +60,20 @@ impl BaseWallet for AgencyClientWallet {
Err(unimplemented_agency_client_wallet_method("update_wallet_record_value"))
}

async fn add_wallet_record_tags(&self, xtype: &str, id: &str, tags_json: &str) -> VcxCoreResult<()> {
async fn add_wallet_record_tags(&self, xtype: &str, id: &str, tags: HashMap<String, String>) -> VcxCoreResult<()> {
Err(unimplemented_agency_client_wallet_method("add_wallet_record_tags"))
}

async fn delete_wallet_record_tags(&self, xtype: &str, id: &str, tag_names: &str) -> VcxCoreResult<()> {
Err(unimplemented_agency_client_wallet_method("delete_wallet_record_tags"))
}

async fn update_wallet_record_tags(&self, xtype: &str, id: &str, tags_json: &str) -> VcxCoreResult<()> {
async fn update_wallet_record_tags(
&self,
xtype: &str,
id: &str,
tags: HashMap<String, String>,
) -> VcxCoreResult<()> {
Err(unimplemented_agency_client_wallet_method("update_wallet_record_tags"))
}

Expand Down
22 changes: 17 additions & 5 deletions aries_vcx_core/src/wallet/base_wallet.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ use async_trait::async_trait;
use crate::errors::error::VcxCoreResult;
use crate::utils::async_fn_iterator::AsyncFnIterator;

use std::collections::HashMap;

/// Trait defining standard 'wallet' related functionality. The APIs, including
/// input and output types are loosely based off the indy Wallet API:
/// see: <https://github.com/hyperledger/indy-sdk/blob/main/libindy/src/api/wallet.rs>
Expand All @@ -26,18 +28,28 @@ pub trait BaseWallet: std::fmt::Debug + Send + Sync {

// ---- records

async fn add_wallet_record(&self, xtype: &str, id: &str, value: &str, tags_json: Option<&str>)
-> VcxCoreResult<()>;
async fn add_wallet_record(
&self,
xtype: &str,
id: &str,
value: &str,
tags: Option<HashMap<String, String>>,
) -> VcxCoreResult<()>;

async fn get_wallet_record(&self, xtype: &str, id: &str, options_json: &str) -> VcxCoreResult<String>;
async fn get_wallet_record(&self, xtype: &str, id: &str, options: &str) -> VcxCoreResult<String>;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The options could actually be well typed, but its type depends on what kind of options the underlying implementation accepts.

This is slightly more advanced, but we could use associated types here. You would defined something like

 pub trait BaseWallet: std::fmt::Debug + Send + Sync {
    type GetRecordOptions;
    type AddRecordOptions;

   ...
   ...
}

Then in particular implementation of the trait, for example in impl IndySdkWallet , you would inject the actual types. And of course, you would have to create those types (you can find out what the option objects should look like if you dig into libvdrtools code)

So not a hard requirement, but if you could do this it would be really great!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

alright!


async fn delete_wallet_record(&self, xtype: &str, id: &str) -> VcxCoreResult<()>;

async fn update_wallet_record_value(&self, xtype: &str, id: &str, value: &str) -> VcxCoreResult<()>;

async fn add_wallet_record_tags(&self, xtype: &str, id: &str, tags_json: &str) -> VcxCoreResult<()>;
async fn add_wallet_record_tags(&self, xtype: &str, id: &str, tags: HashMap<String, String>) -> VcxCoreResult<()>;

async fn update_wallet_record_tags(&self, xtype: &str, id: &str, tags_json: &str) -> VcxCoreResult<()>;
async fn update_wallet_record_tags(
&self,
xtype: &str,
id: &str,
tags: HashMap<String, String>,
) -> VcxCoreResult<()>;

async fn delete_wallet_record_tags(&self, xtype: &str, id: &str, tag_names: &str) -> VcxCoreResult<()>;

Expand Down
24 changes: 17 additions & 7 deletions aries_vcx_core/src/wallet/indy_wallet.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use std::collections::HashMap;
use std::thread;

use async_trait::async_trait;
Expand Down Expand Up @@ -52,13 +53,15 @@ impl BaseWallet for IndySdkWallet {
xtype: &str,
id: &str,
value: &str,
tags_json: Option<&str>,
tags: Option<HashMap<String, String>>,
) -> VcxCoreResult<()> {
let res = tags.map(|x| serde_json::to_string(&x)).transpose()?.to_owned();
let tags_json = res.as_deref();
indy::wallet::add_wallet_record(self.wallet_handle, xtype, id, value, tags_json).await
}

async fn get_wallet_record(&self, xtype: &str, id: &str, options_json: &str) -> VcxCoreResult<String> {
indy::wallet::get_wallet_record(self.wallet_handle, xtype, id, options_json).await
async fn get_wallet_record(&self, xtype: &str, id: &str, options: &str) -> VcxCoreResult<String> {
indy::wallet::get_wallet_record(self.wallet_handle, xtype, id, options).await
}

async fn delete_wallet_record(&self, xtype: &str, id: &str) -> VcxCoreResult<()> {
Expand All @@ -69,12 +72,19 @@ impl BaseWallet for IndySdkWallet {
indy::wallet::update_wallet_record_value(self.wallet_handle, xtype, id, value).await
}

async fn update_wallet_record_tags(&self, xtype: &str, id: &str, tags_json: &str) -> VcxCoreResult<()> {
indy::wallet::update_wallet_record_tags(self.wallet_handle, xtype, id, tags_json).await
async fn update_wallet_record_tags(
&self,
xtype: &str,
id: &str,
tags: HashMap<String, String>,
) -> VcxCoreResult<()> {
let tags_json = serde_json::to_string(&tags)?;
indy::wallet::update_wallet_record_tags(self.wallet_handle, xtype, id, &tags_json).await
}

async fn add_wallet_record_tags(&self, xtype: &str, id: &str, tags_json: &str) -> VcxCoreResult<()> {
indy::wallet::add_wallet_record_tags(self.wallet_handle, xtype, id, tags_json).await
async fn add_wallet_record_tags(&self, xtype: &str, id: &str, tags: HashMap<String, String>) -> VcxCoreResult<()> {
let tags_json = serde_json::to_string(&tags)?;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We serialize hashamp to string, just to deserialize it inside subsequent add_wallet_record_tags into Hashmap again. Can we avoid this?

Copy link
Contributor Author

@tech-bash tech-bash May 27, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are taking the typed parameters and then converting them back to JSON before passing them to indy::wallet::abc.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are you intending to say that we should only have hashmap as the parameter type and not &str?

indy::wallet::add_wallet_record_tags(self.wallet_handle, xtype, id, &tags_json).await
}

async fn delete_wallet_record_tags(&self, xtype: &str, id: &str, tag_names: &str) -> VcxCoreResult<()> {
Expand Down
11 changes: 8 additions & 3 deletions libvcx_core/src/api_vcx/api_global/wallet.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ use crate::errors::error::LibvcxResult;
use crate::errors::mapping_from_ariesvcx::map_ariesvcx_result;
use crate::errors::mapping_from_ariesvcxcore::map_ariesvcx_core_result;

use std::collections::HashMap;

pub static mut WALLET_HANDLE: WalletHandle = INVALID_WALLET_HANDLE;

pub fn set_main_wallet_handle(handle: WalletHandle) -> WalletHandle {
Expand Down Expand Up @@ -113,7 +115,8 @@ pub async fn wallet_configure_issuer(enterprise_seed: &str) -> LibvcxResult<Issu

pub async fn wallet_add_wallet_record(type_: &str, id: &str, value: &str, option: Option<&str>) -> LibvcxResult<()> {
let wallet = get_main_wallet();
map_ariesvcx_core_result(wallet.add_wallet_record(type_, id, value, option).await)
let tags: Option<HashMap<String, String>> = option.map(|tags| serde_json::from_str(tags)).transpose()?;
map_ariesvcx_core_result(wallet.add_wallet_record(type_, id, value, tags).await)
}

pub async fn wallet_update_wallet_record_value(xtype: &str, id: &str, value: &str) -> LibvcxResult<()> {
Expand All @@ -123,12 +126,14 @@ pub async fn wallet_update_wallet_record_value(xtype: &str, id: &str, value: &st

pub async fn wallet_update_wallet_record_tags(xtype: &str, id: &str, tags_json: &str) -> LibvcxResult<()> {
let wallet = get_main_wallet();
map_ariesvcx_core_result(wallet.update_wallet_record_tags(xtype, id, tags_json).await)
let tags: HashMap<String, String> = serde_json::from_str(tags_json)?;
map_ariesvcx_core_result(wallet.update_wallet_record_tags(xtype, id, tags).await)
}

pub async fn wallet_add_wallet_record_tags(xtype: &str, id: &str, tags_json: &str) -> LibvcxResult<()> {
let wallet = get_main_wallet();
map_ariesvcx_core_result(wallet.add_wallet_record_tags(xtype, id, tags_json).await)
let tags: HashMap<String, String> = serde_json::from_str(tags_json)?;
map_ariesvcx_core_result(wallet.add_wallet_record_tags(xtype, id, tags).await)
}

pub async fn wallet_delete_wallet_record_tags(xtype: &str, id: &str, tags_json: &str) -> LibvcxResult<()> {
Expand Down