feat: support aries-askar #1063

xprazak2 · 2023-11-16T14:10:25Z

No description provided.

xprazak2 · 2023-11-16T14:13:43Z

aries_vcx_core/src/wallet/askar/askar_wallet.rs

+    pub async fn create_key(
+        &self,
+        name: &str,
+        alg: KeyAlg,


We will need our own type for alg should this method become a part of public interface.

xprazak2 · 2023-11-16T14:13:44Z

aries_vcx_core/src/wallet/askar/askar_wallet.rs

+
+        let local_key = key_entry.load_local_key()?;
+
+        let did_bytes = &local_key.to_public_bytes()?[0..16];


create_and_store_my_did from BaseWallet always derives did from verkey, but it might be nice to allow did to be passed in here as an argument.

xprazak2 · 2023-11-16T14:14:15Z

aries_vcx_core/src/wallet/askar/askar_wallet.rs

+    use super::*;
+
+    #[tokio::test]
+    #[cfg_attr(not(feature = "askar_tests"), ignore)]


Do not run tests yet since they will require additional setup.

xprazak2 · 2023-11-16T14:14:59Z

aries_vcx_core/src/wallet/askar/askar_wallet.rs

+    async fn test_should_open_askar_wallet() {
+        let uri = "postgres://postgres:postgres@localhost/askar";
+
+        AskarWallet::open(uri, Some(StoreKeyMethod::Unprotected), None.into(), None)


We need to make sure the wallet is created first for this test. And for the following one as well.

codecov-commenter · 2023-11-16T14:18:07Z

Codecov Report

Attention: 21 lines in your changes are missing coverage. Please review.

Comparison is base (37019db) 0.05% compared to head (8ad85b8) 0.05%.
Report is 2 commits behind head on main.

❗ Current head 8ad85b8 differs from pull request most recent head ce14d17. Consider uploading reports for the commit ce14d17 to get more accurate results

Files	Patch %	Lines
aries/aries_vcx_core/src/errors/mapping_askar.rs	0.00%	12 Missing ⚠️
aries/aries_vcx_core/src/wallet2/mod.rs	0.00%	7 Missing ⚠️
aries/aries_vcx/src/errors/mapping_others.rs	0.00%	2 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff            @@
##            main   #1063      +/-   ##
========================================
- Coverage   0.05%   0.05%   -0.01%     
========================================
  Files        471     473       +2     
  Lines      24009   24032      +23     
  Branches    4306    4303       -3     
========================================
  Hits          13      13              
- Misses     23995   24018      +23     
  Partials       1       1

Flag	Coverage Δ
unittests-aries-vcx	`0.05% <0.00%> (-0.01%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Patrik-Stas · 2023-11-16T14:44:43Z

aries_vcx_core/src/errors/error.rs

Ultimately the fact that a particular wallet implementation is being used should not be visible to the rest of the codebase.
We should rather reuse existing wallet errors - those might not be covering all of our needs, so we could tweak them, rename them, extend them as need - but the general approach should be to for example use WalletCreate error type regardless underlying wallet implementation.

Some of the current types seem to be quite tied to the Indy wallet implementation, so it seems like they will need changes.

Patrik-Stas · 2023-11-16T14:48:10Z

aries_vcx_core/Cargo.toml

@@ -11,9 +11,12 @@ credx = ["dep:indy-credx"]
 vdr_proxy_ledger = ["credx", "dep:indy-vdr-proxy-client"]
 # Feature flag to allow legacy proof verification
 legacy_proof = []
+askar_tests = []


Not only tests, but anything Askar related in the codebase should be behind a feature flag. Given the implementation must stay hidden behind the BaseWallet trait, the rest of the codebase should not know of Askar's existence.
You might be aware of this, but keeping that constraint will "reviewlessly" assure these desired properties.

For example having it feature flagged from get go would probably naturally lead you to not create Askar* specific error kinds. It might be somewhat restrictive while POCing, but ultimately will guide you toward the optimal design/integration with the rest of the codebase

Patrik-Stas · 2023-11-16T14:52:15Z

aries_vcx_core/src/wallet/askar/askar_wallet.rs

+        &self,
+        xtype: &str,
+        id: &str,
+        value: &str,
+        tags: Option<HashMap<String, String>>,


For completeness sake just remind of what we mentioned on the community call earlier.

The BaseWallet method signatures were catered for the particular vdrtools based wallet implementation. We didn't try to over-generalize for the future at the time we designed BaseWallet.
So probably you'll find it desirable to redesign BaseWallet trait - you can draw inspiration from this POC #1042 which didn't meant to add support for Askar in particular, but I guess even more ambitiously tried to enable arbitrary wallet implementation in the future. That might or might not been too long shot.

xprazak2 · 2023-11-27T14:39:33Z

I added an implementation of Wallet trait from #1071. It is incomplete and very rough but it illustrates the trait usage.

xprazak2 · 2023-12-06T15:12:41Z

Closing in favor of #1085

xprazak2 commented Nov 16, 2023

View reviewed changes

xprazak2 force-pushed the askar-wallet branch from c5fcd9d to 9d77e41 Compare November 16, 2023 14:18

xprazak2 force-pushed the askar-wallet branch from 9d77e41 to d95603b Compare November 16, 2023 14:18

Patrik-Stas reviewed Nov 16, 2023

View reviewed changes

Add new Wallet trait

f7dee38

xprazak2 force-pushed the askar-wallet branch 4 times, most recently from 8ad85b8 to 0d05eee Compare November 27, 2023 14:32

xprazak2 force-pushed the askar-wallet branch from 0d05eee to c3f4169 Compare November 27, 2023 14:42

Implement Wallet trait for askar

ce14d17

xprazak2 force-pushed the askar-wallet branch from c3f4169 to ce14d17 Compare November 28, 2023 07:15

xprazak2 mentioned this pull request Nov 30, 2023

feat: implement RecordWallet for askar #1078

Closed

xprazak2 closed this Dec 6, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: support aries-askar #1063

feat: support aries-askar #1063

xprazak2 commented Nov 16, 2023

xprazak2 Nov 16, 2023

xprazak2 Nov 16, 2023

xprazak2 Nov 16, 2023

xprazak2 Nov 16, 2023 •

edited

Loading

codecov-commenter commented Nov 16, 2023 •

edited

Loading

Patrik-Stas Nov 16, 2023

xprazak2 Nov 27, 2023

Patrik-Stas Nov 16, 2023

Patrik-Stas Nov 16, 2023

Patrik-Stas Nov 16, 2023 •

edited

Loading

xprazak2 commented Nov 27, 2023 •

edited

Loading

xprazak2 commented Dec 6, 2023


		let local_key = key_entry.load_local_key()?;

		let did_bytes = &local_key.to_public_bytes()?[0..16];

feat: support aries-askar #1063

feat: support aries-askar #1063

Conversation

xprazak2 commented Nov 16, 2023

xprazak2 Nov 16, 2023

Choose a reason for hiding this comment

xprazak2 Nov 16, 2023

Choose a reason for hiding this comment

xprazak2 Nov 16, 2023

Choose a reason for hiding this comment

xprazak2 Nov 16, 2023 • edited Loading

Choose a reason for hiding this comment

codecov-commenter commented Nov 16, 2023 • edited Loading

Codecov Report

Patrik-Stas Nov 16, 2023

Choose a reason for hiding this comment

xprazak2 Nov 27, 2023

Choose a reason for hiding this comment

Patrik-Stas Nov 16, 2023

Choose a reason for hiding this comment

Patrik-Stas Nov 16, 2023

Choose a reason for hiding this comment

Patrik-Stas Nov 16, 2023 • edited Loading

Choose a reason for hiding this comment

xprazak2 commented Nov 27, 2023 • edited Loading

xprazak2 commented Dec 6, 2023

xprazak2 Nov 16, 2023 •

edited

Loading

codecov-commenter commented Nov 16, 2023 •

edited

Loading

Patrik-Stas Nov 16, 2023 •

edited

Loading

xprazak2 commented Nov 27, 2023 •

edited

Loading