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

Update aries-vcx backchannel - aries_vcx: 0.64 #833

Merged

Conversation

gmulhearn
Copy link

@gmulhearn gmulhearn commented Jun 11, 2024

Changes backchannel image approach. Previously the aries-vcx repo AND the aries-agent-test-harness was maintaining the same backchannel codebase. It made sense for the repo to maintain the backchannel, as then breaking code changes could be immediately fixed. But it's a bit of a burden to have to sync the code between there and here.

Therefore this new approach is to have the aries-vcx repo be responsible for creating the backchannel image, then the image here simply consumes that backchannel image. This seems to match the approach of some other backchannels in this repo, such as findy.

aries-vcx repo change to this approach: hyperledger/aries-vcx#1221

Closes #778

Signed-off-by: George Mulhearn <gmulhearn@anonyome.com>
Signed-off-by: George Mulhearn <gmulhearn@anonyome.com>
Signed-off-by: George Mulhearn <gmulhearn@anonyome.com>
@gmulhearn
Copy link
Author

gmulhearn commented Jun 11, 2024

result of ./manage runset ariesvcx-ariesvcx -b running locally:

1 feature passed, 2 failed, 12 skipped
20 scenarios passed, 2 failed, 138 skipped
173 steps passed, 2 failed, 1208 skipped, 0 undefined
Took 8m19.642s

compared to previous/main:

1 feature passed, 2 failed, 12 skipped
19 scenarios passed, 3 failed, 138 skipped
168 steps passed, 3 failed, 1212 skipped, 0 undefined
Took 7m51.920s

does not seem to have regressed

@gmulhearn
Copy link
Author

hey @swcurran @nodlesh , looking for preliminary feedback: is this approach acceptable?

not read for review yet, as i'll need the aries-vcx guys to review this + my changes in the aries-vcx repo

Signed-off-by: George Mulhearn <gmulhearn@anonyome.com>
@gmulhearn
Copy link
Author

update: new result of runsets ariesvcx-ariesvcx after enabling 0023:

Failing scenarios:
  features/0037-present-proof.feature:55  Present Proof of specific types and proof is acknowledged with a Drivers License credential type -- @1.1 
  features/0160-connection.feature:69  Inviter Sends invitation for one agent second agent tries during first share phase

2 features passed, 2 failed, 11 skipped
24 scenarios passed, 2 failed, 134 skipped
212 steps passed, 2 failed, 1169 skipped, 0 undefined
Took 8m38.015s

@gmulhearn gmulhearn marked this pull request as ready for review June 12, 2024 06:53
@swcurran
Copy link
Contributor

swcurran commented Jul 4, 2024

I'm happy to merge this. I ran both the old and the new running all the tests except those tagged wip (work in progress) and while there is a bit of regression, it's not much.

Looks like the base branch can be updated, and then we can merge it.

0a1,2
> 
> 
2,5d3
<   features/0023-did-exchange.feature:9  Establish a connection with DID Exchange between two agents with an explicit invitation
<   features/0023-did-exchange.feature:27  Establish a connection with DID Exchange between two agents with an explicit invitation with a public DID
<   features/0023-did-exchange.feature:45  Establish a connection with DID Exchange between two agents with an implicit invitation
<   features/0023-did-exchange.feature:65  Establish a connection with DID Exchange between two agents with attempt to continue after protocol is completed
13d10
<   features/0025-didcomm-transports.feature:65  Create 0160 connection between two agents with overlapping transports -- @1.1 0160 connection with both agents using HTTP for inbound and outbound transport
16,18d12
<   features/0025-didcomm-transports.feature:82  Create 0160 connection between two agents with overlapping transports -- @4.1 0160 connection with both agents using HTTP and WS for inbound and outbound transport
<   features/0025-didcomm-transports.feature:83  Create 0160 connection between two agents with overlapping transports -- @4.2 0160 connection with both agents using HTTP and WS for inbound and outbound transport
<   features/0025-didcomm-transports.feature:88  Create 0160 connection between two agents with overlapping transports -- @5.1 0160 connection with one agent using http for inbound and the other using ws and both agents supporting ws and http outbound
24,27d17
<   features/0044-mime-types.feature:23  Perform DID Exchange between two agents that have the same default envelope media profile -- @1.1 
<   features/0044-mime-types.feature:24  Perform DID Exchange between two agents that have the same default envelope media profile -- @1.2 
<   features/0044-mime-types.feature:45  Perform DID Exchange between two permissive agents that have different default envelope media profiles -- @1.1 
<   features/0044-mime-types.feature:46  Perform DID Exchange between two permissive agents that have different default envelope media profiles -- @1.2 
30,33d19
<   features/0044-mime-types.feature:83  Perform DID Exchange with OOB media type handshake, with one accept parameter -- @1.1 
<   features/0044-mime-types.feature:84  Perform DID Exchange with OOB media type handshake, with one accept parameter -- @1.2 
<   features/0044-mime-types.feature:105  Perform DID Exchange with OOB media type handshake, with two accept parameters -- @1.1 
<   features/0044-mime-types.feature:106  Perform DID Exchange with OOB media type handshake, with two accept parameters -- @1.2 
36a23
>   features/0183-revocation.feature:24  Credential revoked by Issuer and Holder attempts to prove with a prover that doesn't care if it was revoked -- @1.1 
37a25
>   features/0183-revocation.feature:60  Credential revoked by Issuer and Holder attempts to prove with a prover that doesn't care if it was revoked -- @1.1 
38a27
>   features/0183-revocation.feature:116  Proof in process while Issuer revokes credential before presentation and the verifier doesn't care about revocation status -- @1.1 
52d40
<   features/0183-revocation.feature:364  Revocable Credential, not revoked, and holder proves claims with the credential with timesstamp -- @1.1 
87d74
<   features/0793-peer-did.feature:29  Establish a connection with DID Exchange between two agents utilizing qualified did:peer DIDs -- @1.1 
99,102c86,89
< 0 features passed, 15 failed, 0 skipped
< 26 scenarios passed, 96 failed, 38 skipped
< 464 steps passed, 96 failed, 823 skipped, 0 undefined
< Took 20m46.943s
---
> 1 feature passed, 14 failed, 0 skipped
> 41 scenarios passed, 81 failed, 38 skipped
> 625 steps passed, 81 failed, 677 skipped, 0 undefined
> Took 20m31.056s

@gmulhearn
Copy link
Author

@swcurran thanks for taking the time to run it! to be honest, i'm a bit surprised there is regressions. which agents were you running there? vcx<->vcx?

I'm just making some final updates to use a proper image tag (main), and revise the runsets for vcx<->vcx, acapy<->vcx (and vice versa). I'll request review once complete. cheers

Signed-off-by: George Mulhearn <gmulhearn@anonyome.com>
@gmulhearn
Copy link
Author

gmulhearn commented Jul 5, 2024

Alright, @swcurran i'm done with these changes, and happy to merge (if you're happy with it). i tried to clean up some of the runsets we have against ACAPy. There is still some inconsistent results i'm dealing with... this is probably a VCX problem though, i'll handle it there: hyperledger/aries-vcx#1252

@gmulhearn
Copy link
Author

^ also the ariesvcx-acapy runset (not the acapy-ariesvcx runset) is in a bad state due to vcx reporting the wrong state to the backchannel hyperledger/aries-vcx#1253

Signed-off-by: George Mulhearn <gmulhearn@anonyome.com>
@swcurran
Copy link
Contributor

swcurran commented Jul 5, 2024

Very happy with it -- looking great. Ran another test and a pile more successes. Nice!

@swcurran swcurran merged commit 7d15138 into openwallet-foundation:main Jul 5, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants