feat: Trigger revocation recovery with endorsement / Move indy code t…#198
Closed
jamshale wants to merge 6 commits into
Closed
feat: Trigger revocation recovery with endorsement / Move indy code t…#198jamshale wants to merge 6 commits into
jamshale wants to merge 6 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR wires up revocation-list recovery to be triggered when an endorsed revocation-list update fails, and relocates Indy-specific recovery logic into the anoncreds/default/legacy_indy plugin area.
Changes:
- Emit a new anoncreds event on endorsed revocation-list update failure (instead of using the Indy revocation failure notifier).
- Add legacy Indy event subscription + recovery workflow entrypoint to retry/fix accumulator issues after endorsement failures.
- Remove the prior
anoncreds.revocation.recovermodule and stop re-exporting its helpers from the anoncreds revocation packages.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| acapy_agent/revocation_anoncreds/init.py | Removes re-exports of recovery helpers while keeping backward-compat module wrapper. |
| acapy_agent/protocols/endorse_transaction/v1_0/handlers/endorsed_transaction_response_handler.py | Emits anoncreds-specific failure event + broadens exception handling for anoncreds registration errors. |
| acapy_agent/anoncreds/revocation/recover.py | Deletes the anoncreds revocation recovery module (moved to legacy_indy). |
| acapy_agent/anoncreds/revocation/init.py | Removes recovery imports but still advertises recovery symbols in __all__. |
| acapy_agent/anoncreds/events.py | Adds REV_LIST_ENDORSED_UPDATE_FAILED_EVENT topic constant. |
| acapy_agent/anoncreds/default/legacy_indy/routes.py | Subscribes to the new event and triggers recovery logic. |
| acapy_agent/anoncreds/default/legacy_indy/registry.py | Routes invalid accumulator recovery through the new legacy_indy fix_ledger_entry. |
| acapy_agent/anoncreds/default/legacy_indy/recover.py | Introduces/extends legacy Indy recovery + endorsement retry workflow and exposes fix_ledger_entry. |
Comments suppressed due to low confidence (1)
acapy_agent/anoncreds/default/legacy_indy/recover.py:140
set_revoked = set(rev_list.revocation_list)is using the bit-array values (0/1), producing{0, 1}instead of the revoked indexes. This makesmismatch/updatesincorrect and will generate a bad recovery transaction. Build the set from indexes where the bit-array value is 1 (as the prior logic did).
async def generate_ledger_rrrecovery_txn(genesis_txns: str, rev_list: RevList):
"""Generate a new ledger accum entry, based on wallet vs ledger revocation state."""
new_delta = None
ledger_data = await fetch_txns(
genesis_txns, rev_list.rev_reg_def_id, rev_list.issuer_id
)
if not ledger_data:
return new_delta
registry_from_ledger, prev_revoked = ledger_data
set_revoked = set(rev_list.revocation_list)
mismatch = prev_revoked - set_revoked
if mismatch:
LOGGER.warning(
"Credential index(es) revoked on the ledger, but not in wallet: %s",
mismatch,
)
updates = set_revoked - prev_revoked
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
jamshale
force-pushed
the
feat/4086-anoncreds-indy-accum-fix-with-endorsement
branch
2 times, most recently
from
5289abf to
ae3c351
Compare
…o legacy_indy plugin Signed-off-by: jamshale <jamiehalebc@gmail.com>
jamshale
force-pushed
the
feat/4086-anoncreds-indy-accum-fix-with-endorsement
branch
from
ae3c351 to
2682863
Compare
There was a problem hiding this comment.
Pull request overview
This PR refactors anoncreds revocation recovery by moving legacy Indy–specific recovery logic into the anoncreds/default/legacy_indy implementation, and adds an anoncreds-specific event path to trigger recovery when an endorsed revocation list update fails.
Changes:
- Emit a new anoncreds event when endorsed revocation list updates fail, and add a legacy-indy event subscriber to trigger recovery/retry logic.
- Move revocation recovery implementation code into
anoncreds/default/legacy_indy/recover.py, leaving only shared exception(s) inanoncreds/revocation/recover.py. - Update tests and registry code to use the new recovery entry points.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| acapy_agent/revocation_anoncreds/init.py | Removes legacy recovery re-exports from the deprecated compatibility module. |
| acapy_agent/protocols/issue_credential/v2_0/tests/test_routes.py | Updates event test construction for the cred-revoked listener. |
| acapy_agent/protocols/endorse_transaction/v1_0/handlers/endorsed_transaction_response_handler.py | Emits anoncreds-specific failure event on endorsed transaction handling errors. |
| acapy_agent/anoncreds/revocation/recover.py | Strips implementation details, leaving shared exception(s)/helpers. |
| acapy_agent/anoncreds/revocation/init.py | Adjusts exports for anoncreds revocation package. |
| acapy_agent/anoncreds/events.py | Adds REV_LIST_ENDORSED_UPDATE_FAILED_EVENT. |
| acapy_agent/anoncreds/default/legacy_indy/tests/test_registry.py | Refactors tests to call new fix_ledger_entry helper. |
| acapy_agent/anoncreds/default/legacy_indy/tests/test_recover.py | Minor test comment correction. |
| acapy_agent/anoncreds/default/legacy_indy/routes.py | Adds event subscription to trigger recovery on endorsement failure. |
| acapy_agent/anoncreds/default/legacy_indy/registry.py | Uses extracted recovery helper in invalid-accumulator retry path. |
| acapy_agent/anoncreds/default/legacy_indy/recover.py | Introduces legacy-indy recovery implementation including endorsement retry workflow. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: jamshale <jamiehalebc@gmail.com>
There was a problem hiding this comment.
Pull request overview
This PR refactors AnonCreds revocation registry recovery by moving Indy-specific recovery logic into the anoncreds.default.legacy_indy plugin and wiring an event-driven retry path for failures during endorsed revocation-list updates.
Changes:
- Move revocation registry recovery helpers into
anoncreds/default/legacy_indy/recover.pyand trimanoncreds/revocation/recover.pydown to shared exceptions only. - Emit a new AnonCreds event on endorsed-transaction failure and subscribe to it in the legacy Indy plugin to trigger recovery/retry behavior.
- Update legacy Indy registry usage and adjust/trim related tests and exports.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| acapy_agent/revocation_anoncreds/init.py | Removes re-exports of Indy-specific recovery helpers from the backward-compat module. |
| acapy_agent/protocols/endorse_transaction/v1_0/handlers/endorsed_transaction_response_handler.py | Emits a new AnonCreds failure event (instead of the Indy revocation failure notification) for anoncreds wallets. |
| acapy_agent/anoncreds/revocation/recover.py | Removes Indy-specific recovery implementation, leaving only a shared exception. |
| acapy_agent/anoncreds/revocation/init.py | Stops exporting removed recovery helpers. |
| acapy_agent/anoncreds/events.py | Adds REV_LIST_ENDORSED_UPDATE_FAILED_EVENT constant. |
| acapy_agent/anoncreds/default/legacy_indy/tests/test_registry.py | Adjusts tests to use the moved recovery entrypoint and removes sync test coverage. |
| acapy_agent/anoncreds/default/legacy_indy/tests/test_recover.py | Minor comment fix and continued coverage for moved recovery helper. |
| acapy_agent/anoncreds/default/legacy_indy/routes.py | Subscribes to the new event and triggers recovery flow. |
| acapy_agent/anoncreds/default/legacy_indy/registry.py | Switches to calling the new standalone fix_ledger_entry helper and removes the class method implementation. |
| acapy_agent/anoncreds/default/legacy_indy/recover.py | New/expanded legacy Indy recovery implementation plus event-driven fix-and-retry logic. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: jamshale <jamiehalebc@gmail.com>
Signed-off-by: jamshale <jamiehalebc@gmail.com>
There was a problem hiding this comment.
Pull request overview
This PR shifts anoncreds revocation registry recovery logic into the anoncreds/default/legacy_indy plugin and introduces an event-driven path to trigger recovery when an endorsed revocation-list update fails.
Changes:
- Removes indy-specific recovery helpers from
acapy_agent/anoncreds/revocation/*exports, leaving only shared exception types. - Emits a new anoncreds event on endorsed transaction failures and adds a legacy-indy event subscriber that attempts recovery + resends an endorsement transaction.
- Moves/rewires legacy indy recovery and related tests to use the new module layout.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| acapy_agent/revocation_anoncreds/init.py | Stops re-exporting indy-specific recovery helpers from this package. |
| acapy_agent/protocols/endorse_transaction/v1_0/handlers/endorsed_transaction_response_handler.py | Emits anoncreds-specific failure event (instead of legacy revocation failure notification) for anoncreds wallets; broadens caught exceptions. |
| acapy_agent/anoncreds/revocation/recover.py | Reduces to shared recovery exception + module description. |
| acapy_agent/anoncreds/revocation/manager.py | Removes legacy indy recovery delegation from anoncreds revocation manager. |
| acapy_agent/anoncreds/revocation/init.py | Stops re-exporting indy-specific recovery helpers. |
| acapy_agent/anoncreds/events.py | Adds REV_LIST_ENDORSED_UPDATE_FAILED_EVENT. |
| acapy_agent/anoncreds/default/legacy_indy/tests/test_registry.py | Updates tests to call the new recovery helper location/function. |
| acapy_agent/anoncreds/default/legacy_indy/tests/test_recover.py | Minor test comment fix; continues to cover legacy-indy recovery helpers. |
| acapy_agent/anoncreds/default/legacy_indy/routes.py | Registers an event subscriber to trigger recovery after endorsement failures. |
| acapy_agent/anoncreds/default/legacy_indy/registry.py | Switches from an instance method to the new fix_ledger_entry helper. |
| acapy_agent/anoncreds/default/legacy_indy/recover.py | Adds legacy-indy recovery/repair utilities and endorsement-resend flow. |
Comments suppressed due to low confidence (1)
acapy_agent/anoncreds/revocation/manager.py:206
update_rev_reg_revoked_statenow always raisesRevocationManagerErrorafter confirming the revocation list exists. The admin routeanoncreds/routes/revocation/registry/routes.py:update_rev_reg_revoked_statestill calls this method, so this change makes that endpoint unusable. Either restore delegation to the legacy indy implementation (e.g., call into theLegacyIndyRegistry/legacy_indy.recover.fix_ledger_entryflow) or update/remove the route so it no longer depends on this method.
revoc = AnonCredsRevocation(self._profile)
rev_list = await revoc.get_created_revocation_list(rev_reg_def_id)
if not rev_list:
raise RevocationManagerError(
f"No revocation list found for revocation registry id {rev_reg_def_id}"
)
raise RevocationManagerError(
"Indy registry does not support revocation registry "
f"identified by {rev_reg_def_id}"
)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+355
to
+373
| async with profile.session() as session: | ||
| # get rev reg records from wallet (revocations and status) | ||
| recs = await IssuerCredRevRecord.query_by_ids( | ||
| session, rev_reg_id=rev_list.rev_reg_def_id | ||
| ) | ||
|
|
||
| revoked_ids, rec_count = _get_revoked_discrepancies(recs, rev_reg_delta) | ||
|
|
||
| LOGGER.debug(f"Fixed entry recs count = {rec_count}") | ||
| LOGGER.debug(f"Fixed entry recs revoked ids = {revoked_ids}") | ||
|
|
||
| # No update required if no discrepancies | ||
| if rec_count == 0: | ||
| return (rev_reg_delta, {}, {}) | ||
|
|
||
| # We have revocation discrepancies, generate the recovery txn | ||
| recovery_txn = await generate_ledger_rrrecovery_txn( | ||
| genesis_transactions, rev_list | ||
| ) |
Comment on lines
+186
to
+203
| if cache is None: | ||
| LOGGER.warning( | ||
| "No cache backend configured; skipping retry tracking for %s", | ||
| accum, | ||
| ) | ||
| return | ||
| retry_value = await cache.get(accum) | ||
| if not retry_value: | ||
| await cache.set(accum, 5) | ||
| else: | ||
| if retry_value > 0: | ||
| await cache.set(accum, retry_value - 1) | ||
| else: | ||
| LOGGER.error( | ||
| "Revocation registry entry transaction failed for %s", | ||
| accum, | ||
| ) | ||
|
|
Comment on lines
+279
to
+280
| # If the accum from the ledger matches the error message, fix it | ||
| # if accum and accum in err_msg: |
Comment on lines
1
to
6
| """Recover a revocation registry.""" | ||
|
|
||
| import hashlib | ||
| import importlib | ||
| import logging | ||
| import tempfile | ||
| import time | ||
|
|
||
| import aiohttp | ||
| import base58 | ||
|
|
||
| LOGGER = logging.getLogger(__name__) | ||
|
|
||
|
|
||
| """ | ||
| This module calculates a new ledger accumulator, based on the revocation status | ||
| on the ledger vs revocations recorded in the wallet. | ||
| The calculated transaction can be written to the ledger to get the ledger back | ||
| in sync with the wallet. | ||
| This function can be used if there were previous revocation errors (i.e. the | ||
| credential revocation was successfully written to the wallet but the ledger write | ||
| failed.) | ||
| This module contains general exceptions or helper functions related to revocation | ||
| registry recovery that are not specific to any one implementation. | ||
| """ |
Comment on lines
+179
to
+181
| async def fix_and_publish_from_invalid_accum_err(profile: Profile, err_msg: str): | ||
| """Fix and publish revocation registry entries from invalid accumulator error.""" | ||
| cache = profile.inject_or(BaseCache) |
Comment on lines
+394
to
+409
| applied_txn = ledger_response["result"] | ||
|
|
||
| # Update the local wallets rev reg entry with the new accumulator value | ||
| rev_list_value_json = rev_list.value_json | ||
| rev_list_value_json["rev_list"]["currentAccumulator"] = applied_txn["txn"][ | ||
| "data" | ||
| ]["value"]["accum"] | ||
| rev_list.current_accumulator = applied_txn["txn"]["data"]["value"]["accum"] | ||
| await session.handle.replace( | ||
| CATEGORY_REV_LIST, | ||
| rev_list.rev_reg_def_id, | ||
| rev_list_value_json, | ||
| rev_list.tags, | ||
| ) | ||
|
|
||
| return (rev_reg_delta, recovery_txn, applied_txn) |
Comment on lines
+394
to
+407
| applied_txn = ledger_response["result"] | ||
|
|
||
| # Update the local wallets rev reg entry with the new accumulator value | ||
| rev_list_value_json = rev_list.value_json | ||
| rev_list_value_json["rev_list"]["currentAccumulator"] = applied_txn["txn"][ | ||
| "data" | ||
| ]["value"]["accum"] | ||
| rev_list.current_accumulator = applied_txn["txn"]["data"]["value"]["accum"] | ||
| await session.handle.replace( | ||
| CATEGORY_REV_LIST, | ||
| rev_list.rev_reg_def_id, | ||
| rev_list_value_json, | ||
| rev_list.tags, | ||
| ) |
Signed-off-by: jamshale <jamiehalebc@gmail.com>
Signed-off-by: jamshale <jamiehalebc@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
…o legacy_indy plugin