draft: Demo for aries vcx #952

Patrik-Stas · 2023-08-22T15:33:11Z

Partially addressing #870

Added Alice & Faber demo to establish connection
Modified simple_message_relay to enable its consumption also as library, rather than executable only
Extended simple_message_relay with mpsc channels which forward any incoming message to mpsc receiver. This is nice because in the demo, we don't need to poll HTTP API for new messages - but rather instantly get notified in-memory.

Having to interact with aries-vcx from scratch from new application, some improvements I've identified:

Note

This can be thought of as temporary solution to demonstrate the APIs and usage. And while far from ideal yet (todos left, issues created), it's a lot easier to understand than pointing someone to integration tests today. Main issue of integration tests is that they are all couploed with vcxagency-node mediator, but also other issues.
After integration tests are in better shape after:

Declutter issuance&presentation protocols #945
Remove msg-sending IO for issuer and holder #946
and subsequent removal of IO, and therefore coupling with vcxagency-node mediator
and some refactoring of "Alice" "Faber" devsetup agents

I believe that integration tests could be in condition to demonstrate the usage (and surface outstanding API issues) equally good as this demo.
But for the time being, this demo is a lot simpler than the existing integration tests.

Demo

Running the demo produces following logs

codecov-commenter · 2023-08-22T16:09:27Z

Codecov Report

Merging #952 (ab545cc) into main (4e65aec) will increase coverage by 30.32%.
The diff coverage is 0.00%.

❗ Current head ab545cc differs from pull request most recent head 58f03a7. Consider uploading reports for the commit 58f03a7 to get more accurate results

@@            Coverage Diff             @@
##            main     #952       +/-   ##
==========================================
+ Coverage   0.05%   30.38%   +30.32%     
==========================================
  Files        384      422       +38     
  Lines      21249    26230     +4981     
  Branches    3835     5084     +1249     
==========================================
+ Hits          12     7970     +7958     
+ Misses     21236    16041     -5195     
- Partials       1     2219     +2218

Flag	Coverage Δ
unittests-aries-vcx	`30.38% <0.00%> (+30.32%)`	⬆️

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

Files	Coverage Δ
aries_vcx/src/protocols/connection/generic/mod.rs	`9.16% <ø> (+9.16%)`	⬆️
aries_vcx/src/protocols/connection/invitee/mod.rs	`78.94% <ø> (+78.94%)`	⬆️
aries_vcx/src/protocols/connection/mod.rs	`7.89% <ø> (+7.89%)`	⬆️
aries_vcx/src/handlers/out_of_band/sender.rs	`39.74% <0.00%> (+39.74%)`	⬆️

... and 349 files with indirect coverage changes

Patrik-Stas · 2023-08-23T16:39:54Z

@gmulhearn as a creator of original issue, do you agree this being in a single file? I think your vision to have 2 files, 1 for alice, one for faber.
I am fine with that too. Just started in one file to make this simple for me, but actually might be a lot more readable for someone if we split the roles.

mirgee · 2023-08-24T13:03:26Z

After integration tests are in better shape after:

Declutter issuance&presentation protocols #945

Remove msg-sending IO for issuer and holder #946

and subsequent removal of IO, and therefore coupling with vcxagency-node mediator

and some refactoring of "Alice" "Faber" devsetup agents

I believe that integration tests could be in condition to demonstrate the usage (and surface outstanding API issues) equally good as this demo.
But for the time being, this demo is a lot simpler than the existing integration tests.

Will we remove the demo after the work mentioned is completed, i.e. integration tests are in such a condition that they fulfill the current purpose of the demo, i.e. to demonstrate aries-vcx usage, as discussed in speaking?

Patrik-Stas · 2023-08-24T16:47:07Z

After integration tests are in better shape after:

Declutter issuance&presentation protocols #945

IO for issuer and holder as opt-in #946

and subsequent removal of IO, and therefore coupling with vcxagency-node mediator

and some refactoring of "Alice" "Faber" devsetup agents

I believe that integration tests could be in condition to demonstrate the usage (and surface outstanding API issues) equally good as this demo.
But for the time being, this demo is a lot simpler than the existing integration tests.

Will we remove the demo after the work mentioned is completed, i.e. integration tests are in such a condition that they fulfill the current purpose of the demo, i.e. to demonstrate aries-vcx usage, as discussed in speaking?

I'd be in favor, there surely will be a lot of duplication between this and integration tests after refactoring them

tools/simple_message_relay/src/main.rs

demo/src/http_client.rs

demo/src/main.rs

gmulhearn · 2023-09-06T10:17:38Z

nit; log colors are printing a little funky for me, is this expected?

(ran with RUST_LOG="info" cargo run)

gmulhearn · 2023-09-06T10:20:06Z

demo/src/demo_agent.rs

+    }
+
+    pub async fn prepare_invitation(&self) -> (Invitation, PairwiseInfo) {
+        let (faber_pw_did, faber_invite_key) = self.wallet.create_and_store_my_did(None, None).await.unwrap();


nit; not sure if you were looking for it , but you could also call PairwiseInfo::create(wallet) to avoid construction of PairwiseInfo at the end

I would like us to actually get rid of the whole PairwiseInfo thing, which why I am not using it, rather creating it as necessity at the point where it's needed.
The "did" portion of PairwiseInfo is generally useless

gmulhearn

This is really awesome, nice work. preemptive approval from me; just some comments to consider. (assuming others are happy too ocourse)

gmulhearn · 2023-09-06T10:25:53Z

send_message_2 bugs me a bit too.. maybe something like send_message_as_inviter send_message_as_invitee? not sure

demo/src/demo_agent.rs

bobozaur

Mostly left remarks.

I think the message sending could be improved, but that would require quite a bit of a refactor. The duplicate methods should be easy to remove though, which is why I'd request some changes.

demo/src/demo_agent.rs

tools/simple_message_relay/src/mpsc_registry.rs

bobozaur · 2023-10-03T20:56:19Z

demo/src/mpsc_registry.rs

+#[derive(Clone)]
+pub struct MpscRegistry<T: Debug> {
+    sending: Arc<RwLock<HashMap<String, mpsc::Sender<T>>>>,
+    receiving: Arc<RwLock<HashMap<String, mpsc::Receiver<T>>>>,
+}


Wasn't the previous implementation similar to this in terms of communication? Maybe I'm missing something but I don't get why the shared ownership and interior mutability is needed.

Each agent should be in charge of its own connections (senders to other agents and receivers from other agents) and practically own them exclusively. Or is there some reason why this can't be achieved?

Signed-off-by: Patrik Stas <[email protected]>

Patrik-Stas · 2024-03-17T22:29:42Z

Archiving this. Instead, a new demo will be created soon on top of aries-vcx-agent crate

Patrik-Stas mentioned this pull request Aug 22, 2023

Create Alice Faber CLI Demo for aries_vcx (Rust) #870

Open

Patrik-Stas changed the base branch from main to connection/ioless August 22, 2023 15:38

Patrik-Stas force-pushed the demo/aries-vcx branch from f37e957 to 0551ab2 Compare August 22, 2023 15:40

Patrik-Stas force-pushed the demo/aries-vcx branch from 7f6fee1 to ec8b9f3 Compare August 22, 2023 17:19

Patrik-Stas linked an issue Aug 22, 2023 that may be closed by this pull request

Create Alice Faber CLI Demo for aries_vcx (Rust) #870

Open

Patrik-Stas force-pushed the demo/aries-vcx branch from 164de4b to a2ab312 Compare August 22, 2023 18:40

Patrik-Stas force-pushed the demo/aries-vcx branch from d101979 to 8568afd Compare August 24, 2023 10:06

Patrik-Stas mentioned this pull request Aug 26, 2023

Remove agency_client as dependency of integration tests #959

Closed

Patrik-Stas force-pushed the connection/ioless branch from 25f1e1e to 5355c1e Compare August 29, 2023 21:59

Patrik-Stas force-pushed the demo/aries-vcx branch 2 times, most recently from 5dde0a7 to afcb250 Compare August 29, 2023 22:29

Patrik-Stas force-pushed the connection/ioless branch from 5355c1e to 11cbec1 Compare August 30, 2023 06:23

Base automatically changed from connection/ioless to main August 31, 2023 15:35

Patrik-Stas force-pushed the demo/aries-vcx branch 2 times, most recently from f1a5366 to 7933912 Compare August 31, 2023 16:49

Patrik-Stas marked this pull request as ready for review September 4, 2023 12:13

Patrik-Stas requested review from gmulhearn, mirgee and bobozaur September 4, 2023 12:13