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.

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

feat: add new Wallet trait, implement for indy wallet #1084

Merged
merged 2 commits into from
Jan 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions aries/aries_vcx/src/errors/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,9 @@ pub enum AriesVcxErrorKind {
#[error("Could not parse a value")]
ParsingError,

#[error("Unexpected wallet error")]
WalletError,

// A2A
#[error("Invalid HTTP response.")]
InvalidHttpResponse,
Expand Down
1 change: 1 addition & 0 deletions aries/aries_vcx/src/errors/mapping_others.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ impl From<AriesVcxCoreError> for AriesVcxError {
AriesVcxErrorKind::DuplicationMasterSecret
}
AriesVcxCoreErrorKind::DuplicationDid => AriesVcxErrorKind::DuplicationDid,
AriesVcxCoreErrorKind::WalletError => AriesVcxErrorKind::WalletError,
AriesVcxCoreErrorKind::LoggingError => AriesVcxErrorKind::LoggingError,
AriesVcxCoreErrorKind::EncodeError => AriesVcxErrorKind::EncodeError,
AriesVcxCoreErrorKind::UnknownError => AriesVcxErrorKind::UnknownError,
Expand Down
1 change: 1 addition & 0 deletions aries/aries_vcx_core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ tokio = { version = "1.20" }
indy-vdr-proxy-client = { git = "https://github.com/hyperledger/indy-vdr.git", rev = "c143268", optional = true }
indy-ledger-response-parser = { path = "../misc/indy_ledger_response_parser" }
lru = { version = "0.12.0" }
public_key = { path = "../../did_core/public_key"}

[dev-dependencies]
tokio = { version = "1.20", features = ["rt", "macros", "rt-multi-thread"] }
3 changes: 3 additions & 0 deletions aries/aries_vcx_core/src/errors/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,9 @@ pub enum AriesVcxCoreErrorKind {
#[error("Attempted to add a DID to wallet when that DID already exists in wallet")]
DuplicationDid,

#[error("Unexpected wallet error")]
WalletError,

// Logger
#[error("Logging Error")]
LoggingError,
Expand Down
1 change: 1 addition & 0 deletions aries/aries_vcx_core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ pub mod global;
pub mod ledger;
pub mod utils;
pub mod wallet;
pub mod wallet2;

pub use indy_ledger_response_parser::ResponseParser;
pub use indy_vdr::config::PoolConfig;
Expand Down
62 changes: 62 additions & 0 deletions aries/aries_vcx_core/src/wallet2/entry_tag.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
use serde::{Deserialize, Serialize};

#[derive(Clone, Debug, Serialize, Deserialize, Eq, PartialEq, PartialOrd, Ord)]
pub enum EntryTag {
Copy link
Contributor

Choose a reason for hiding this comment

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

This enum is a little confusing - what is the purpose of an encrypted tag? Why is it a key-value pair instead of a value? How is it always possible to, given a string, infer whether it is "encrypted" or not (whatever that means)? Why would the wallet return record with encrypted tag, shouldn't they all be decrypted?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

All good questions, I am afraid I do not have answers to all of them.
For some reason, the tags are key-value pairs in both vdrtools wallet and askar. Determining whether the tag value is encrypted is a responsibility of the wallet implementation.

Encrypted(String, String),
Plaintext(String, String),
}

#[derive(Debug, Default, Clone, PartialEq)]
pub struct EntryTags {
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder what is the purpose of this struct. Why encapsulate the vec and reimplement a subset of vec functionality for it, instead of using the vec itself? I assume it is something like we want to keep open the possibility of changing the Vec for e.g. a HashSet? If so, why can't the decision of using a Vec vs HashSet be made now, what are the unknowns?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The underlying implementation is irrelevant. I needed a type that would represent a collection of tags which would be independent on any implementation as vdrtools wallet uses HashMap<String, String> and askar Vec<AskarEntryTag>. I started with Vec<EntryTag>, but I can't implement From for that.

inner: Vec<EntryTag>,
}

impl EntryTags {
pub fn new(inner: Vec<EntryTag>) -> Self {
Self { inner }
}

pub fn add(&mut self, tag: EntryTag) {
self.inner.push(tag)
}

pub fn is_empty(&self) -> bool {
self.inner.is_empty()
}
}

impl IntoIterator for EntryTags {
type Item = EntryTag;

type IntoIter = std::vec::IntoIter<Self::Item>;

fn into_iter(self) -> Self::IntoIter {
self.inner.into_iter()
}
}

impl FromIterator<EntryTag> for EntryTags {
fn from_iter<T: IntoIterator<Item = EntryTag>>(iter: T) -> Self {
let mut tags = Self::default();

for item in iter {
tags.add(item);
}
tags
}
}

impl From<Vec<EntryTag>> for EntryTags {
fn from(value: Vec<EntryTag>) -> Self {
value.into_iter().fold(Self::default(), |mut memo, item| {
memo.add(item);
memo
})
}
}

impl From<EntryTags> for Vec<EntryTag> {
fn from(value: EntryTags) -> Self {
value.inner
}
}
120 changes: 120 additions & 0 deletions aries/aries_vcx_core/src/wallet2/indy_wallet/indy_did_wallet.rs
Copy link
Contributor

@Patrik-Stas Patrik-Stas Jan 3, 2024

Choose a reason for hiding this comment

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

Probably the most important comment within this review:

You placed the tests into indy implementations directly, but the tests themselves are not necessarily indy specific - they rely on the DidWallet, RecordWallet traits in general, regardless of the underlying implementations. Am I perhaps missing something?

The current approach would indicate that given you have test test_indy_should_sign_and_verify, you will likewise have test_askar_should_sign_and_verify where the test case would only differ by its setup.
Also, you are using create_indy_test_wallet_handle for all of the tests you have added, which sets up mysql wallet - that's somewhat unexpected, most of the test for sake of simplicity are using the default sqlite version of vdrtools wallet. There's only 1 particular test which specifically aims to verify that mysql works (test_mysql_wallet.rs).

So in my view, solving that boils down to:

  • Make tests agnostic of underlying implementations - test against interfaces, not specific implementation
    • For that you can use dev_build_featured_wallet(seed).await; (all existing tests, except for the mysql one, are using this)
  • Because dev_build_featured_wallet is choosing wallet implementation based on feature flags, it will be easy to use the same test cases for testing askar implementation too
  • Consequently you shouldn't need create_indy_test_wallet_handle, which sets up mysql wallet, anymore. If it for some reason should stay, it would be nice to deduplicate it with test_mysql_init_issuer_with_mysql_wallet

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good, there is one small complication though. dev_build_featured_wallet cannot really be used in aries_vcx_core, because that means adding test_utils crate as a dependency for aries_vcs_core thus creating a dependency cycle. While cargo initially does not object, having cycles in deps is generally not a good idea and problems start to surface quite rapidly with failing type checks.

I created a custom setup for now, any thoughts on this are appreciated.

Copy link
Contributor

Choose a reason for hiding this comment

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

As agreed to, we will look into removing aries_vcx_core as test_utils dependency and use the setup code in aries_vcx_core tests directly.

Copy link
Contributor

Choose a reason for hiding this comment

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

Tracking issue #1101 .

Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
use async_trait::async_trait;
use public_key::KeyType;
use vdrtools::{DidMethod, DidValue, KeyInfo, Locator, MyDidInfo};

use crate::{
errors::error::{AriesVcxCoreError, AriesVcxCoreErrorKind, VcxCoreResult},
wallet::{indy::IndySdkWallet, structs_io::UnpackMessageOutput},
wallet2::{DidData, DidWallet, Key},
};

#[async_trait]
impl DidWallet for IndySdkWallet {
async fn create_and_store_my_did(
&self,
seed: Option<&str>,
method_name: Option<&str>,
) -> VcxCoreResult<DidData> {
let res = Locator::instance()
.did_controller
.create_and_store_my_did(
self.wallet_handle,
MyDidInfo {
method_name: method_name.map(|m| DidMethod(m.into())),
seed: seed.map(Into::into),
..MyDidInfo::default()
},
)
.await?;

Ok(DidData {
did: res.0,
verkey: Key::from_base58(&res.1, KeyType::Ed25519).map_err(|err| {
AriesVcxCoreError::from_msg(AriesVcxCoreErrorKind::WalletError, err)
})?,
})
}

async fn did_key(&self, did: &str) -> VcxCoreResult<Key> {
let res = Locator::instance()
.did_controller
.key_for_local_did(self.wallet_handle, DidValue(did.into()))
.await?;

Key::from_base58(&res, KeyType::Ed25519)
.map_err(|err| AriesVcxCoreError::from_msg(AriesVcxCoreErrorKind::WalletError, err))
}

async fn replace_did_key_start(&self, did: &str, seed: Option<&str>) -> VcxCoreResult<Key> {
let key_info = KeyInfo {
seed: seed.map(Into::into),
..Default::default()
};

let key_string = Locator::instance()
.did_controller
.replace_keys_start(self.wallet_handle, key_info, DidValue(did.into()))
.await?;

let key = Key::from_base58(&key_string, KeyType::Ed25519)
.map_err(|err| AriesVcxCoreError::from_msg(AriesVcxCoreErrorKind::WalletError, err))?;

Ok(key)
}

async fn replace_did_key_apply(&self, did: &str) -> VcxCoreResult<()> {
Ok(Locator::instance()
.did_controller
.replace_keys_apply(self.wallet_handle, DidValue(did.into()))
.await?)
}

async fn sign(&self, key: &Key, msg: &[u8]) -> VcxCoreResult<Vec<u8>> {
Locator::instance()
.crypto_controller
.crypto_sign(self.wallet_handle, &key.base58(), msg)
.await
.map_err(From::from)
}

async fn verify(&self, key: &Key, msg: &[u8], signature: &[u8]) -> VcxCoreResult<bool> {
Locator::instance()
.crypto_controller
.crypto_verify(&key.base58(), msg, signature)
.await
.map_err(From::from)
}

async fn pack_message(
&self,
sender_vk: Option<Key>,
receiver_keys: Vec<Key>,
msg: &[u8],
) -> VcxCoreResult<Vec<u8>> {
let receiver_keys_str = receiver_keys.into_iter().map(|key| key.base58()).collect();

Ok(Locator::instance()
.crypto_controller
.pack_msg(
msg.into(),
receiver_keys_str,
sender_vk.map(|key| key.base58()),
self.wallet_handle,
)
.await?)
}

async fn unpack_message(&self, msg: &[u8]) -> VcxCoreResult<UnpackMessageOutput> {
let unpacked_bytes = Locator::instance()
.crypto_controller
.unpack_msg(serde_json::from_slice(msg)?, self.wallet_handle)
.await?;

let res: UnpackMessageOutput =
serde_json::from_slice(&unpacked_bytes[..]).map_err(|err| {
AriesVcxCoreError::from_msg(AriesVcxCoreErrorKind::ParsingError, err.to_string())
})?;

Ok(res)
}
}
138 changes: 138 additions & 0 deletions aries/aries_vcx_core/src/wallet2/indy_wallet/indy_record_wallet.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,138 @@
use async_trait::async_trait;
use indy_api_types::domain::wallet::Record as IndyRecord;
use serde::Deserialize;
use serde_json::Value;
use vdrtools::Locator;

use super::{SEARCH_OPTIONS, WALLET_OPTIONS};
use crate::{
errors::error::{AriesVcxCoreError, VcxCoreResult},
wallet::indy::IndySdkWallet,
wallet2::{entry_tag::EntryTags, Record, RecordWallet, SearchFilter},
};

#[async_trait]
impl RecordWallet for IndySdkWallet {
async fn add_record(&self, record: Record) -> VcxCoreResult<()> {
let tags_map = if record.tags.is_empty() {
None
} else {
Some(record.tags.into())
};

Ok(Locator::instance()
.non_secret_controller
.add_record(
self.wallet_handle,
record.category,
record.name,
record.value,
tags_map,
)
.await?)
}

async fn get_record(&self, name: &str, category: &str) -> VcxCoreResult<Record> {
let res = Locator::instance()
.non_secret_controller
.get_record(
self.wallet_handle,
category.into(),
name.into(),
WALLET_OPTIONS.into(),
)
.await?;

let indy_record: IndyRecord = serde_json::from_str(&res)?;

Ok(indy_record.into())
}

async fn update_record_tags(
&self,
name: &str,
category: &str,
new_tags: EntryTags,
) -> VcxCoreResult<()> {
Ok(Locator::instance()
.non_secret_controller
.update_record_tags(
self.wallet_handle,
category.into(),
name.into(),
new_tags.into(),
)
.await?)
}

async fn update_record_value(
&self,
name: &str,
category: &str,
new_value: &str,
) -> VcxCoreResult<()> {
Ok(Locator::instance()
.non_secret_controller
.update_record_value(
self.wallet_handle,
category.into(),
name.into(),
new_value.into(),
)
.await?)
}

async fn delete_record(&self, name: &str, category: &str) -> VcxCoreResult<()> {
Ok(Locator::instance()
.non_secret_controller
.delete_record(self.wallet_handle, category.into(), name.into())
.await?)
}

async fn search_record(
&self,
category: &str,
search_filter: Option<SearchFilter>,
) -> VcxCoreResult<Vec<Record>> {
let json_filter = search_filter
.map(|filter| match filter {
SearchFilter::JsonFilter(inner) => Ok::<String, AriesVcxCoreError>(inner),
})
.transpose()?;

let query_json = json_filter.unwrap_or("{}".into());

let search_handle = Locator::instance()
.non_secret_controller
.open_search(
self.wallet_handle,
category.into(),
query_json,
SEARCH_OPTIONS.into(),
)
.await?;

let next = || async {
let record = Locator::instance()
.non_secret_controller
.fetch_search_next_records(self.wallet_handle, search_handle, 1)
.await?;

let indy_res: Value = serde_json::from_str(&record)?;

indy_res
.get("records")
.and_then(|v| v.as_array())
.and_then(|arr| arr.first())
.map(|item| IndyRecord::deserialize(item).map_err(AriesVcxCoreError::from))
.transpose()
};

let mut records = Vec::new();
while let Some(record) = next().await? {
records.push(record.into());
}

Ok(records)
}
}
Loading
Loading