feat: Remove issuance v1 protocols #3923
Conversation
jamshale
force-pushed
the
remove-issue-credential-v1
branch
from
7ed3a9d to
d30db20
Compare
jamshale
force-pushed
the
remove-issue-credential-v1
branch
from
a5a7559 to
dec3ab4
Compare
jamshale
force-pushed
the
remove-issue-credential-v1
branch
from
22b5cc4 to
92401d9
Compare
There was a problem hiding this comment.
Pull request overview
This pull request removes the v1 issuance protocols from ACA-Py core in favor of a plugin-based approach. The removal is part of a larger effort to modularize the codebase and allow v1 protocols to be optionally added via plugins when needed.
Key Changes:
- Complete removal of v1.0 issue credential protocol implementation
- Updates to tests and examples to use v2.0 protocols instead
- Addition of event bus handling in v2.0 routes for credential revocation
- Simplification of revocation manager by removing v1-specific logic
Reviewed changes
Copilot reviewed 56 out of 63 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| scenarios/examples/connectionless/example.py | Removed v1 credential issuance example function (icv1), keeping only v2 |
| scenarios/examples/connectionless/docker-compose.yml | Changed log level from debug to info for alice and bob agents |
| acapy_agent/utils/tests/test_tracing.py | Updated imports and tests to use V20CredExRecord instead of V10CredentialExchange |
| acapy_agent/revocation/tests/test_manager.py | Updated tests to use v2 models, removed v1-specific test cases |
| acapy_agent/revocation/manager.py | Removed v1/v2 credential exchange state updates from set_cred_revoked_state method |
| acapy_agent/protocols/present_proof/v1_0/tests/test_manager.py | Updated import to use v2 credential record for state constant |
| acapy_agent/protocols/out_of_band/v1_0/tests/test_manager.py | Removed v1 credential offer tests and imports |
| acapy_agent/protocols/out_of_band/v1_0/manager.py | Removed v1 credential exchange lookup, now only supports v2 |
| acapy_agent/protocols/issue_credential/v2_0/tests/test_routes.py | Added comprehensive tests for new event bus integration |
| acapy_agent/protocols/issue_credential/v2_0/routes.py | Added register_events and cred_revoked functions for event handling |
| acapy_agent/protocols/issue_credential/v1_0/* | Deleted entire v1.0 protocol directory including routes, handlers, managers, models, messages, and all tests |
| acapy_agent/core/tests/test_goal_code_registry.py | Updated import to reference v2 CONTROLLERS instead of v1 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
jamshale
force-pushed
the
remove-issue-credential-v1
branch
from
0d91e96 to
dbea310
Compare
|
The interop tests are expected to fail without the plugin installed. Here is a branch using this PR and installing the plugin. openwallet-foundation/owl-agent-test-harness#919 The tests are passing with this branch and the plugin installed. |
jamshale
force-pushed
the
remove-issue-credential-v1
branch
from
b6cbeb5 to
a8cbd82
Compare
| CRED_20_OFFER, | ||
| PRESENTATION_REQUEST, | ||
| PRES_20_REQUEST, | ||
| "issue-credential/1.0/offer-credential", |
There was a problem hiding this comment.
This should be the only reference for issue-credential/1.0. I wasn't able to think of a way to inject it into this class without maybe an ugly monkey patch of the function in the plugin.
| except StorageNotFoundError: | ||
| continue | ||
|
|
||
| async with self._profile.transaction() as txn: |
There was a problem hiding this comment.
This is a main change. The revocation is done via event handler in the routes file now.
| bus.subscribe(re.compile(r"^acapy::cred-revoked$"), cred_revoked) | ||
|
|
||
|
|
||
| async def cred_revoked(profile: Profile, event: EventWithMetadata): |
There was a problem hiding this comment.
This is where the revocation is handled now for the cred_ex record. The plugin has the same thing for V1 IssuerCredRevRecords's.
|
I'd like to get approval on the plugin PR before merging this. openwallet-foundation/acapy-plugins#2288. |
On it, going through it currently |
|
@jamshale don't we want to remove the associated bdd tests and have those live in the plugin repo instead? |
You're right. I forgot to remove the BDD tests in the repo. I'll do that. They only run on nightly and release runs. We have a basic test for coverage in the plugin PR and a few tests in the OATH project which we can keep. No need to remove those. |
Signed-off-by: jamshale <[email protected]>
Signed-off-by: jamshale <[email protected]>
Signed-off-by: jamshale <[email protected]>
Signed-off-by: jamshale <[email protected]>
Signed-off-by: jamshale <[email protected]>
Signed-off-by: jamshale <[email protected]>
Signed-off-by: jamshale <[email protected]>
Signed-off-by: jamshale <[email protected]>
Signed-off-by: jamshale <[email protected]>
Signed-off-by: jamshale <[email protected]>
jamshale
force-pushed
the
remove-issue-credential-v1
branch
from
73f41f8 to
79f96c4
Compare
|
|
So I'm pretty sure all the v1 protocol bdd tests were already removed from the acapy repo. I think this is good to go. |
3 participants
This removes the v1 issuance protocols in favor of a plugin. See #3252 for original work.
The plugin PR openwallet-foundation/acapy-plugins#2288 uses this to re-enable v1 issuance support.
When this is finished and successful the same method will be used for the presentation v1 protocols.