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: Multiple Ledger and Endorser Selection + additional UI Controls #790

Merged
merged 14 commits into from
Sep 18, 2023

Conversation

shaangill025
Copy link
Contributor

@shaangill025
Copy link
Contributor Author

Re the bug discovered that is preventing transaction from being completed when switching ledger/endorser after initial selection. I believe that no further changes would be needed in Traction and only ACA-Py bug to be fixed.

ACA-Py issue: hyperledger/aries-cloudagent-python#2473

@shaangill025 shaangill025 marked this pull request as ready for review September 6, 2023 19:49
@shaangill025 shaangill025 temporarily deployed to development September 6, 2023 19:53 — with GitHub Actions Inactive
@shaangill025 shaangill025 temporarily deployed to development September 6, 2023 21:23 — with GitHub Actions Inactive
@loneil
Copy link
Collaborator

loneil commented Sep 7, 2023

@shaangill025 @esune wondering if we'd need to wait merging this before the ACA-Py issue Shaanjot has found gets completed? Or would this be independant?

Copy link
Collaborator

@loneil loneil left a comment

Choose a reason for hiding this comment

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

Regression tested out existing behaviour with BCovrin-test here on this PR and it's working.
Could not connect to -dev at this point but I think we discussed that.

Minor UI, don't think we need the search and filter for the ledgers here (won't be pages of them), so we can slim things down a bit?
If so, added some comments on places things could be removed. Think I got them all but not sure.

Also had merged a Traction 0.10.1 update earlier so you might find some merge conflicts FYI around the same things for that.

@esune
Copy link
Member

esune commented Sep 8, 2023

@shaangill025 other than @loneil 's comments, there's merge conflicts that require attention

…ase related changes

Signed-off-by: Shaanjot Gill <gill.shaanjots@gmail.com>
Signed-off-by: Shaanjot Gill <gill.shaanjots@gmail.com>
Signed-off-by: Shaanjot Gill <gill.shaanjots@gmail.com>
Signed-off-by: Shaanjot Gill <gill.shaanjots@gmail.com>
Signed-off-by: Shaanjot Gill <gill.shaanjots@gmail.com>
@esune
Copy link
Member

esune commented Sep 14, 2023

@shaangill025 there are conflicts that need resolving.
@loneil leving this PR with you since you're likely going to take on the next steps after the endorser approval investigation you're completing.

@loneil
Copy link
Collaborator

loneil commented Sep 14, 2023

Sure, will test out under current bcovrin-test state after any conflicts resolved and next update.
If it makes issuer all good that can merge and go to environments, but yeah as mentioned, the other Endorser paradigms where there need to be manual intervention steps (accept connection, accept transactions for register DID steps) will alter how the UI needs to be done. I can work through those as part of #757

Signed-off-by: Shaanjot Gill <gill.shaanjots@gmail.com>
Signed-off-by: Shaanjot Gill <gill.shaanjots@gmail.com>
@shaangill025 shaangill025 temporarily deployed to development September 14, 2023 19:41 — with GitHub Actions Inactive
Signed-off-by: Shaanjot Gill <gill.shaanjots@gmail.com>
@shaangill025 shaangill025 temporarily deployed to development September 14, 2023 20:02 — with GitHub Actions Inactive
loneil

This comment was marked as resolved.

@loneil

This comment was marked as resolved.

@esune
Copy link
Member

esune commented Sep 15, 2023

Think the make issuer mechanism is looking good for the current state of allowing to connect to the "auto-everything" style endorser we are currently running all Traction envs against.

However testing out against BCovrin-Test I'm getting abandoned states when trying to issue revokable credentials to my BC Wallet

image

With a rev-reg error

"error_msg": 
"Cred def id 6hChBFAutS7HvAqSHkENTL:3:CL:70263:test has no active revocation registry."

I can successfully issue a non-revocable cred.

Can dig more tomorrow.

2023-09-15 16:17:36,597 aries_cloudagent.messaging.base_handler ERROR Error responding to credential request
Traceback (most recent call last):
File "/home/aries/.venv/lib/python3.9/site-packages/aries_cloudagent/protocols/issue_credential/v1_0/handlers/credential_request_handler.py", line 79, in handle
) = await credential_manager.issue_credential(
File "/home/aries/.venv/lib/python3.9/site-packages/aries_cloudagent/protocols/issue_credential/v1_0/manager.py", line 682, in issue_credential
raise CredentialManagerError(
aries_cloudagent.protocols.issue_credential.v1_0.manager.CredentialManagerError: Cred def id 6hChBFAutS7HvAqSHkENTL:3:CL:70263:test has no active revocation registry

This issue is unrelated to the changes @shaangill025 has made, we're seen this with other tenants in our test instance - c.c.: @WadeBarnes
Was this on a newly created credential definition? Or on an old, existing one? Any hints at how this might be reproduced?

Either way, I logged an issue to track this specific problem: bcgov/DITP#62

@loneil
Copy link
Collaborator

loneil commented Sep 15, 2023

It's reproducible from what I can see. New tenant, create new schema and def, get tails error, can't accept.

@loneil
Copy link
Collaborator

loneil commented Sep 15, 2023

Needs fix from hyperledger/aries-cloudagent-python#2482 included in 0.10.2 (not related to other 0.10.1 issue affecting some single tenant aca-py instances) until this can be merged.

@esune
Copy link
Member

esune commented Sep 15, 2023

Needs fix from hyperledger/aries-cloudagent-python#2482 included in 0.10.2 (not related to other 0.10.1 issue affecting some single tenant aca-py instances) until this can be merged.

We could use the latest nightly to test out without waiting.

@shaangill025
Copy link
Contributor Author

We could use the latest nightly to test out without waiting.

I will update to use 0.10.2-rc0 for that.

@shaangill025 shaangill025 temporarily deployed to development September 15, 2023 21:17 — with GitHub Actions Inactive
@shaangill025 shaangill025 temporarily deployed to development September 15, 2023 21:29 — with GitHub Actions Inactive
Copy link
Collaborator

@loneil loneil left a comment

Choose a reason for hiding this comment

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

Can issue and revoke revocable creds now

image

On the Python lib update, is updating pyproject.toml sufficient, or should the poetry.lock file be done as well?
When I did other ACA-Py updates I would run poetry update in the dev contianer setup Sherman made and that would update the locks as well.
(I'm not super sure about Poetry stuff myself so not sure if it makes a difference?)

Signed-off-by: Lucas ONeil <lucasoneil@gmail.com>
@loneil
Copy link
Collaborator

loneil commented Sep 15, 2023

Probably next question for @esune if we'd want to merge this since it's looking like it's working with the rc0. Or if 0.10.2 is imminent just wait for that?

@loneil loneil temporarily deployed to development September 15, 2023 22:53 — with GitHub Actions Inactive
Copy link
Collaborator

@loneil loneil left a comment

Choose a reason for hiding this comment

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

Works on current bcovrin-test case. As discussed, future changes coming down for other Endorser flows.

Just pending question about whether to use rc0 here or need next full aca-py release.

@esune esune temporarily deployed to development September 18, 2023 17:04 — with GitHub Actions Inactive
@esune esune temporarily deployed to development September 18, 2023 17:08 — with GitHub Actions Inactive
@loneil loneil merged commit d3f3631 into main Sep 18, 2023
11 checks passed
@loneil loneil temporarily deployed to development September 18, 2023 17:46 — with GitHub Actions Inactive
@loneil loneil deleted the issue_681 branch May 3, 2024 18:00
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.

Select which Ledger/Endorser to connect to - Tenant UI
3 participants