From a4ad003069b86911225baa26cf279af0f3a630cb Mon Sep 17 00:00:00 2001 From: jamshale <31809382+jamshale@users.noreply.github.com> Date: Fri, 21 Jun 2024 11:03:16 -0700 Subject: [PATCH] Handle failed tails server issuance [Anoncreds] (#3049) * Fix anoncreds legacy indy revocation rotate without tails server / Refactor Signed-off-by: jamshale * Refactor / More descriptive variable name Signed-off-by: jamshale * Update some class docs / Remove un-needed TODO's Signed-off-by: jamshale * Reformat with black Signed-off-by: jamshale * Remove failed state Signed-off-by: jamshale --------- Signed-off-by: jamshale --- aries_cloudagent/anoncreds/base.py | 11 +- .../anoncreds/default/did_indy/registry.py | 8 +- .../anoncreds/default/did_web/registry.py | 2 +- .../anoncreds/default/legacy_indy/registry.py | 8 +- aries_cloudagent/anoncreds/events.py | 9 - aries_cloudagent/anoncreds/issuer.py | 2 +- .../anoncreds/models/anoncreds_cred_def.py | 60 ++--- .../anoncreds/models/anoncreds_revocation.py | 27 -- .../anoncreds/models/anoncreds_schema.py | 14 +- aries_cloudagent/anoncreds/revocation.py | 247 ++++++++++-------- .../anoncreds/revocation_setup.py | 13 +- .../anoncreds/tests/test_issuer.py | 2 +- .../anoncreds/tests/test_revocation.py | 42 ++- aries_cloudagent/ledger/indy_vdr.py | 19 +- .../endorse_transaction/v1_0/manager.py | 6 +- demo/runners/support/agent.py | 4 +- 16 files changed, 221 insertions(+), 253 deletions(-) diff --git a/aries_cloudagent/anoncreds/base.py b/aries_cloudagent/anoncreds/base.py index 319e8d1baa..2e0fc87c34 100644 --- a/aries_cloudagent/anoncreds/base.py +++ b/aries_cloudagent/anoncreds/base.py @@ -6,11 +6,7 @@ from ..config.injection_context import InjectionContext from ..core.error import BaseError from ..core.profile import Profile -from .models.anoncreds_cred_def import ( - CredDef, - CredDefResult, - GetCredDefResult, -) +from .models.anoncreds_cred_def import CredDef, CredDefResult, GetCredDefResult from .models.anoncreds_revocation import ( GetRevListResult, GetRevRegDefResult, @@ -59,10 +55,7 @@ def __init__( Args: message: Message obj_id: Object ID - obj: Object - - TODO: update this docstring - Anoncreds-break. - + obj: Generic Object """ super().__init__(message, obj_id, obj, *args, **kwargs) self._message = message diff --git a/aries_cloudagent/anoncreds/default/did_indy/registry.py b/aries_cloudagent/anoncreds/default/did_indy/registry.py index 9c29af1c48..6099ed574b 100644 --- a/aries_cloudagent/anoncreds/default/did_indy/registry.py +++ b/aries_cloudagent/anoncreds/default/did_indy/registry.py @@ -7,11 +7,7 @@ from ....config.injection_context import InjectionContext from ....core.profile import Profile from ...base import BaseAnonCredsRegistrar, BaseAnonCredsResolver -from ...models.anoncreds_cred_def import ( - CredDef, - CredDefResult, - GetCredDefResult, -) +from ...models.anoncreds_cred_def import CredDef, CredDefResult, GetCredDefResult from ...models.anoncreds_revocation import ( GetRevListResult, GetRevRegDefResult, @@ -32,7 +28,7 @@ def __init__(self): """Initialize an instance. Args: - TODO: update this docstring - Anoncreds-break. + None """ self._supported_identifiers_regex = re.compile(r"^did:indy:.*$") diff --git a/aries_cloudagent/anoncreds/default/did_web/registry.py b/aries_cloudagent/anoncreds/default/did_web/registry.py index 84a9494f45..63382f3606 100644 --- a/aries_cloudagent/anoncreds/default/did_web/registry.py +++ b/aries_cloudagent/anoncreds/default/did_web/registry.py @@ -25,7 +25,7 @@ def __init__(self): """Initialize an instance. Args: - TODO: update this docstring - Anoncreds-break. + None """ self._supported_identifiers_regex = re.compile( diff --git a/aries_cloudagent/anoncreds/default/legacy_indy/registry.py b/aries_cloudagent/anoncreds/default/legacy_indy/registry.py index 21f199454e..ba9e2f3cc1 100644 --- a/aries_cloudagent/anoncreds/default/legacy_indy/registry.py +++ b/aries_cloudagent/anoncreds/default/legacy_indy/registry.py @@ -111,7 +111,7 @@ def __init__(self): """Initialize an instance. Args: - TODO: update this docstring - Anoncreds-break. + None """ B58 = alphabet if isinstance(alphabet, str) else alphabet.decode("ascii") @@ -621,7 +621,9 @@ async def register_revocation_registry_definition( endorser_did=endorser_did, ) except LedgerError as err: - raise AnonCredsRegistrationError(err.roll_up) from err + LOGGER.error( + f"Error registering revocation registry definition {rev_reg_def_id}: {err.roll_up}" # noqa: E501 + ) # Didn't need endorsement if write_ledger: @@ -1108,7 +1110,6 @@ def _wallet_accumalator_matches_ledger_list( ) async with profile.session() as session: - LOGGER.debug(f"revocation_list = {rev_list.revocation_list}") LOGGER.debug(f"rev_reg_delta = {rev_reg_delta.get('value')}") @@ -1117,7 +1118,6 @@ def _wallet_accumalator_matches_ledger_list( ) if not _wallet_accumalator_matches_ledger_list(rev_list, rev_reg_delta): - recovery_txn = await generate_ledger_rrrecovery_txn( genesis_transactions, rev_list ) diff --git a/aries_cloudagent/anoncreds/events.py b/aries_cloudagent/anoncreds/events.py index 7be3bc8603..4511b546f7 100644 --- a/aries_cloudagent/anoncreds/events.py +++ b/aries_cloudagent/anoncreds/events.py @@ -37,9 +37,6 @@ def __init__( Args: payload: CredDefFinishedPayload - - TODO: update this docstring - Anoncreds-break. - """ self._topic = CRED_DEF_FINISHED_EVENT self._payload = payload @@ -82,9 +79,6 @@ def __init__(self, payload: RevRegDefFinishedPayload): Args: payload: RevRegDefFinishedPayload - - TODO: update this docstring - Anoncreds-break. - """ self._topic = REV_REG_DEF_FINISHED_EVENT self._payload = payload @@ -122,9 +116,6 @@ def __init__(self, payload: RevListFinishedPayload): Args: payload: RevListFinishedPayload - - TODO: update this docstring - Anoncreds-break. - """ self._topic = REV_LIST_FINISHED_EVENT self._payload = payload diff --git a/aries_cloudagent/anoncreds/issuer.py b/aries_cloudagent/anoncreds/issuer.py index 45c018de2b..78bc65f0d2 100644 --- a/aries_cloudagent/anoncreds/issuer.py +++ b/aries_cloudagent/anoncreds/issuer.py @@ -310,7 +310,7 @@ async def create_and_register_credential_definition( if not isinstance(support_revocation, bool): raise ValueError("support_revocation must be a boolean") - max_cred_num = options.get("max_cred_num", DEFAULT_MAX_CRED_NUM) + max_cred_num = options.get("revocation_registry_size", DEFAULT_MAX_CRED_NUM) if not isinstance(max_cred_num, int): raise ValueError("max_cred_num must be an integer") diff --git a/aries_cloudagent/anoncreds/models/anoncreds_cred_def.py b/aries_cloudagent/anoncreds/models/anoncreds_cred_def.py index 2315ade427..a194973e01 100644 --- a/aries_cloudagent/anoncreds/models/anoncreds_cred_def.py +++ b/aries_cloudagent/anoncreds/models/anoncreds_cred_def.py @@ -34,14 +34,15 @@ def __init__(self, n: str, s: str, r: dict, rctxt: str, z: str, **kwargs): """Initialize an instance. Args: - n: n - s: s - r: r - rctxt: rctxt - z: z - - TODO: update this docstring - Anoncreds-break. - + n: is a safe RSA-2048 number. + s: is a randomly selected quadratic residue of n. + r: is an object that defines a CL-RSA public key fragment + for each attribute in the credential. + rctxt: is equal to s^(xrctxt), where xrctxt is a randomly selected integer + between 2 and p'q'-1. + z: is equal to s^(xz), where xz is a randomly selected integer between 2 + and p'q'-1. This makes up part of the CL-RSA public key, independent of + the message blocks being signed. """ super().__init__(**kwargs) self.n = n @@ -92,20 +93,20 @@ def __init__( """Initialize an instance. Args: - g: g - g_dash: g_dash - h: h - h0: h0 - h1: h1 - h2: h2 - htilde: htilde - h_cap: h_cap - u: u - pk: pk - y: y - - TODO: update this docstring - Anoncreds-break. - + g: is a generator for the elliptic curve group G1. + g_dash: is a generator for the elliptic curve group G2. + h: is an elliptic curve point selected uniformly at random from G1. + h0: is an elliptic curve point selected uniformly at random from G1. + h1: is an elliptic curve point selected uniformly at random from G1. + h2: is an elliptic curve point selected uniformly at random from G1. + htilde: is an elliptic curve point selected uniformly at random from G1. + h_cap: is an elliptic curve point selected uniformly at random from G2. + u: is an elliptic curve point selected uniformly at random from G2. + pk: is the public key in G1 for the issuer with respect to this accumulator, + computed as g^sk (in multiplicative notation), where sk is from + r_key above. + y: is the an elliptic curve point in G2. computed as h_cap^x + (in multiplicative notation), where x is from r_key above """ self.g = g self.g_dash = g_dash @@ -171,9 +172,6 @@ def __init__( Args: primary: Cred Def value primary revocation: Cred Def value revocation - - TODO: update this docstring - Anoncreds-break. - """ super().__init__(**kwargs) self.primary = primary @@ -225,9 +223,6 @@ def __init__( type: Type tag: Tag value: Cred Def value - - TODO: update this docstring - Anoncreds-break. - """ super().__init__(**kwargs) self.issuer_id = issuer_id @@ -307,9 +302,6 @@ def __init__( state: State credential_definition_id: Cred Def ID credential_definition: Cred Def - - TODO: update this docstring - Anoncreds-break. - """ self.state = state self.credential_definition_id = credential_definition_id @@ -370,9 +362,6 @@ def __init__( credential_definition_state: Cred Def state registration_metadata: Registration metadata credential_definition_metadata: Cred Def metadata - - TODO: update this docstring - Anoncreds-break. - """ super().__init__(**kwargs) self.job_id = job_id @@ -420,9 +409,6 @@ def __init__( credential_definition: Cred Def resolution_metadata: Resolution metadata credential_definition_metadata: Cred Def metadata - - TODO: update this docstring - Anoncreds-break. - """ super().__init__(**kwargs) self.credential_definition_id = credential_definition_id diff --git a/aries_cloudagent/anoncreds/models/anoncreds_revocation.py b/aries_cloudagent/anoncreds/models/anoncreds_revocation.py index bdcf152f5c..4ee1be85a6 100644 --- a/aries_cloudagent/anoncreds/models/anoncreds_revocation.py +++ b/aries_cloudagent/anoncreds/models/anoncreds_revocation.py @@ -41,9 +41,6 @@ def __init__( max_cred_num: Max. number of Creds tails_location: Tails file location tails_hash: Tails file hash - - TODO: update this docstring - Anoncreds-break. - """ super().__init__(**kwargs) self.public_keys = public_keys @@ -102,9 +99,6 @@ def __init__( cred_def_id: Cred Def ID tag: Tag value: Rev Reg Def Value - - TODO: update this docstring - Anoncreds-break. - """ super().__init__(**kwargs) self.issuer_id = issuer_id @@ -183,9 +177,6 @@ def __init__( state: State revocation_registry_definition_id: Rev Reg Definition ID revocation_registry_definition: Rev Reg Definition - - TODO: update this docstring - Anoncreds-break. - """ self.state = state self.revocation_registry_definition_id = revocation_registry_definition_id @@ -247,9 +238,6 @@ def __init__( revocation_registry_definition_state: Rev Reg Def state registration_metadata: Registration metadata revocation_registry_definition_metadata: Rev Reg Def metadata - - TODO: update this docstring - Anoncreds-break. - """ super().__init__(**kwargs) self.job_id = job_id @@ -311,9 +299,6 @@ def __init__( revocation_registry_id: Revocation Registry ID resolution_metadata: Resolution metadata revocation_registry_metadata: Revocation Registry metadata - - TODO: update this docstring - Anoncreds-break. - """ super().__init__(**kwargs) self.revocation_registry = revocation_registry @@ -362,9 +347,6 @@ def __init__( revocation_list: Revocation list current_accumulator: Current accumulator timestamp: Timestamp - - TODO: update this docstring - Anoncreds-break. - """ super().__init__(**kwargs) self.issuer_id = issuer_id @@ -453,9 +435,6 @@ def __init__( Args: state: State revocation_list: Revocation list - - TODO: update this docstring - Anoncreds-break. - """ self.state = state self.revocation_list = revocation_list @@ -508,9 +487,6 @@ def __init__( revocation_list_state: Revocation list state registration_metadata: Registration metadata revocation_list_metadata: Revocation list metadata - - TODO: update this docstring - Anoncreds-break. - """ super().__init__(**kwargs) self.job_id = job_id @@ -561,9 +537,6 @@ def __init__( revocation_list: Revocation list resolution_metadata: Resolution metadata revocation_registry_metadata: Rev Reg metadata - - TODO: update this docstring - Anoncreds-break. - """ super().__init__(**kwargs) self.revocation_list = revocation_list diff --git a/aries_cloudagent/anoncreds/models/anoncreds_schema.py b/aries_cloudagent/anoncreds/models/anoncreds_schema.py index c9190239fa..bc2a6884e7 100644 --- a/aries_cloudagent/anoncreds/models/anoncreds_schema.py +++ b/aries_cloudagent/anoncreds/models/anoncreds_schema.py @@ -7,10 +7,7 @@ from marshmallow.validate import OneOf from ...messaging.models.base import BaseModel, BaseModelSchema -from ...messaging.valid import ( - INDY_OR_KEY_DID_EXAMPLE, - INDY_SCHEMA_ID_EXAMPLE, -) +from ...messaging.valid import INDY_OR_KEY_DID_EXAMPLE, INDY_SCHEMA_ID_EXAMPLE class AnonCredsSchema(BaseModel): @@ -31,9 +28,6 @@ def __init__( attr_names: Schema Attribute Name list name: Schema name version: Schema version - - TODO: update this docstring - Anoncreds-break. - """ super().__init__(**kwargs) self.issuer_id = issuer_id @@ -106,9 +100,6 @@ def __init__( schema_id: Schema ID resolution_metadata: Resolution Metdata schema_metadata: Schema Metadata - - TODO: update this docstring - Anoncreds-break. - """ super().__init__(**kwargs) self.schema_value = schema @@ -221,9 +212,6 @@ def __init__( schema_state: Schema state registration_metadata: Registration Metdata schema_metadata: Schema Metadata - - TODO: update this docstring - Anoncreds-break. - """ super().__init__(**kwargs) self.job_id = job_id diff --git a/aries_cloudagent/anoncreds/revocation.py b/aries_cloudagent/anoncreds/revocation.py index a5d49e0dbb..29c4c3c283 100644 --- a/aries_cloudagent/anoncreds/revocation.py +++ b/aries_cloudagent/anoncreds/revocation.py @@ -8,7 +8,7 @@ import os import time from pathlib import Path -from typing import List, NamedTuple, Optional, Sequence, Tuple, Union +from typing import List, Mapping, NamedTuple, Optional, Sequence, Tuple, Union from urllib.parse import urlparse import base58 @@ -21,6 +21,7 @@ RevocationStatusList, W3cCredential, ) +from aries_askar import Entry from aries_askar.error import AskarError from requests import RequestException, Session from uuid_utils import uuid4 @@ -43,6 +44,7 @@ from .models.anoncreds_revocation import ( RevList, RevListResult, + RevListState, RevRegDef, RevRegDefResult, RevRegDefState, @@ -226,8 +228,6 @@ async def store_revocation_registry_definition( "Revocation registry definition id or job id not found" ) - # TODO Handle `failed` state - rev_reg_def = ( result.revocation_registry_definition_state.revocation_registry_definition ) @@ -459,7 +459,9 @@ async def create_and_register_revocation_list( self.profile, rev_reg_def, RevList.from_native(rev_list), options ) - # TODO Handle `failed` state + if options.get("failed_to_upload", False): + result.revocation_list_state.state = RevListState.STATE_FAILED + await self.store_revocation_registry_list(result) return result @@ -481,9 +483,10 @@ async def store_revocation_registry_list(self, result: RevListResult): identifier, value_json={ "rev_list": rev_list.serialize(), - "pending": None, - # TODO THIS IS A HACK; this fixes ACA-Py expecting 1-based indexes # noqa: E501 + # Anoncreds uses the 0 index internally + # and can't be used for a credential "next_index": 1, + "pending": None, }, tags={ "state": result.revocation_list_state.state, @@ -577,7 +580,6 @@ async def update_revocation_list( self.profile, rev_reg_def, prev, curr, revoked, options ) - # TODO Handle `failed` state try: async with self.profile.session() as session: rev_list_entry_upd = await session.handle.fetch( @@ -800,9 +802,9 @@ async def handle_full_registry(self, rev_reg_def_id: str): tag=str(uuid4()), max_cred_num=active_rev_reg_def.value_json["value"]["maxCredNum"], ) - LOGGER.info(f"previous rev_reg_def_id = {rev_reg_def_id}") - LOGGER.info(f"current rev_reg_def_id = {backup_rev_reg_def_id}") - LOGGER.info(f"backup reg = {backup_reg}") + LOGGER.info(f"Previous rev_reg_def_id = {rev_reg_def_id}") + LOGGER.info(f"Current rev_reg_def_id = {backup_rev_reg_def_id}") + LOGGER.info(f"Backup reg = {backup_reg.rev_reg_def_id}") async def decommission_registry(self, cred_def_id: str): """Decommission post-init registries and start the next registry generation.""" @@ -856,9 +858,9 @@ async def decommission_registry(self, cred_def_id: str): max_cred_num=active_reg.rev_reg_def.value.max_cred_num, ) - LOGGER.info(f"new reg = {new_reg}") - LOGGER.info(f"backup reg = {backup_reg}") - LOGGER.info(f"decommissioned regs = {recs}") + LOGGER.info(f"New registry = {new_reg}") + LOGGER.info(f"Backup registry = {backup_reg}") + LOGGER.debug(f"Decommissioned registries = {recs}") return recs async def get_or_create_active_registry(self, cred_def_id: str) -> RevRegDefResult: @@ -874,7 +876,6 @@ async def get_or_create_active_registry(self, cred_def_id: str) -> RevRegDefResu ) if not rev_reg_defs: - # TODO Create a registry if none available raise AnonCredsRevocationError("No active registry") entry = rev_reg_defs[0] @@ -922,6 +923,45 @@ async def create_credential_w3c( retries=retries, ) + async def _get_cred_def_objects( + self, credential_definition_id: str + ) -> tuple[Entry, Entry]: + try: + async with self.profile.session() as session: + cred_def = await session.handle.fetch( + CATEGORY_CRED_DEF, credential_definition_id + ) + cred_def_private = await session.handle.fetch( + CATEGORY_CRED_DEF_PRIVATE, credential_definition_id + ) + except AskarError as err: + raise AnonCredsRevocationError( + "Error retrieving credential definition" + ) from err + if not cred_def or not cred_def_private: + raise AnonCredsRevocationError( + "Credential definition not found for credential issuance" + ) + return cred_def, cred_def_private + + def _check_and_get_attribute_raw_values( + self, schema_attributes: List[str], credential_values: dict + ) -> Mapping[str, str]: + raw_values = {} + for attribute in schema_attributes: + # Ensure every attribute present in schema to be set. + # Extraneous attribute names are ignored. + try: + credential_value = credential_values[attribute] + except KeyError: + raise AnonCredsRevocationError( + "Provided credential values are missing a value " + f"for the schema attribute '{attribute}'" + ) + + raw_values[attribute] = str(credential_value) + return raw_values + async def _create_credential( self, credential_definition_id: str, @@ -949,93 +989,79 @@ async def _create_credential( A tuple of created credential and revocation ID """ - try: - async with self.profile.session() as session: - cred_def = await session.handle.fetch( - CATEGORY_CRED_DEF, credential_definition_id + + def _handle_missing_entries( + rev_list: Entry, rev_reg_def: Entry, rev_key: Entry + ): + if not rev_list: + raise AnonCredsRevocationError("Revocation registry list not found") + if not rev_reg_def: + raise AnonCredsRevocationError( + "Revocation registry definition not found" ) - cred_def_private = await session.handle.fetch( - CATEGORY_CRED_DEF_PRIVATE, credential_definition_id + if not rev_key: + raise AnonCredsRevocationError( + "Revocation registry definition private data not found" ) - except AskarError as err: - raise AnonCredsRevocationError( - "Error retrieving credential definition" - ) from err - if not cred_def or not cred_def_private: - raise AnonCredsRevocationError( - "Credential definition not found for credential issuance" - ) - raw_values = {} - for attribute in schema_attributes: - # Ensure every attribute present in schema to be set. - # Extraneous attribute names are ignored. - try: - credential_value = credential_values[attribute] - except KeyError: - raise AnonCredsRevocationError( - "Provided credential values are missing a value " - f"for the schema attribute '{attribute}'" + def _has_required_id_and_tails_path(): + return rev_reg_def_id and tails_file_path + + revoc = None + credential_revocation_id = None + rev_list = None + + if _has_required_id_and_tails_path(): + async with self.profile.session() as session: + rev_reg_def = await session.handle.fetch( + CATEGORY_REV_REG_DEF, rev_reg_def_id + ) + rev_list = await session.handle.fetch(CATEGORY_REV_LIST, rev_reg_def_id) + rev_key = await session.handle.fetch( + CATEGORY_REV_REG_DEF_PRIVATE, rev_reg_def_id ) - raw_values[attribute] = str(credential_value) + _handle_missing_entries(rev_list, rev_reg_def, rev_key) + + rev_list_value_json = rev_list.value_json + rev_list_tags = rev_list.tags - if rev_reg_def_id and tails_file_path: + # If the rev_list state is failed then the tails file was never uploaded, + # try to upload it now and finish the revocation list + if rev_list_tags.get("state") == RevListState.STATE_FAILED: + await self.upload_tails_file( + RevRegDef.deserialize(rev_reg_def.value_json) + ) + rev_list_tags["state"] = RevListState.STATE_FINISHED + + rev_reg_index = rev_list_value_json["next_index"] try: - async with self.profile.transaction() as txn: - rev_list = await txn.handle.fetch(CATEGORY_REV_LIST, rev_reg_def_id) - rev_reg_def = await txn.handle.fetch( - CATEGORY_REV_REG_DEF, rev_reg_def_id - ) - rev_key = await txn.handle.fetch( - CATEGORY_REV_REG_DEF_PRIVATE, rev_reg_def_id - ) - if not rev_list: - raise AnonCredsRevocationError("Revocation registry not found") - if not rev_reg_def: - raise AnonCredsRevocationError( - "Revocation registry definition not found" - ) - if not rev_key: - raise AnonCredsRevocationError( - "Revocation registry definition private data not found" - ) - # NOTE: we increment the index ahead of time to keep the - # transaction short. The revocation registry itself will NOT - # be updated because we always use ISSUANCE_BY_DEFAULT. - # If something goes wrong later, the index will be skipped. - # FIXME - double check issuance type in case of upgraded wallet? - rev_info = rev_list.value_json - rev_info_tags = rev_list.tags - rev_reg_index = rev_info["next_index"] - try: - rev_reg_def = RevocationRegistryDefinition.load( - rev_reg_def.raw_value - ) - rev_list = RevocationStatusList.load(rev_info["rev_list"]) - except AnoncredsError as err: - raise AnonCredsRevocationError( - "Error loading revocation registry definition" - ) from err - if rev_reg_index > rev_reg_def.max_cred_num: - raise AnonCredsRevocationRegistryFullError( - "Revocation registry is full" - ) - rev_info["next_index"] = rev_reg_index + 1 - await txn.handle.replace( - CATEGORY_REV_LIST, - rev_reg_def_id, - value_json=rev_info, - tags=rev_info_tags, - ) - await txn.commit() - except AskarError as err: + rev_reg_def = RevocationRegistryDefinition.load(rev_reg_def.raw_value) + rev_list = RevocationStatusList.load(rev_list_value_json["rev_list"]) + except AnoncredsError as err: raise AnonCredsRevocationError( - "Error updating revocation registry index" + "Error loading revocation registry" ) from err - # rev_info["next_index"] is 1 based but getting from - # rev_list is zero based... + # NOTE: we increment the index ahead of time to keep the + # transaction short. The revocation registry itself will NOT + # be updated because we always use ISSUANCE_BY_DEFAULT. + # If something goes wrong later, the index will be skipped. + # FIXME - double check issuance type in case of upgraded wallet? + if rev_reg_index > rev_reg_def.max_cred_num: + raise AnonCredsRevocationRegistryFullError( + "Revocation registry is full" + ) + rev_list_value_json["next_index"] = rev_reg_index + 1 + async with self.profile.transaction() as txn: + await txn.handle.replace( + CATEGORY_REV_LIST, + rev_reg_def_id, + value_json=rev_list_value_json, + tags=rev_list_tags, + ) + await txn.commit() + revoc = CredentialRevocationConfig( rev_reg_def, rev_key.raw_value, @@ -1043,10 +1069,10 @@ async def _create_credential( rev_reg_index, ) credential_revocation_id = str(rev_reg_index) - else: - revoc = None - credential_revocation_id = None - rev_list = None + + cred_def, cred_def_private = await self._get_cred_def_objects( + credential_definition_id + ) try: credential = await asyncio.get_event_loop().run_in_executor( @@ -1056,7 +1082,9 @@ async def _create_credential( cred_def_private=cred_def_private.raw_value, cred_offer=credential_offer, cred_request=credential_request, - attr_raw_values=raw_values, + attr_raw_values=self._check_and_get_attribute_raw_values( + schema_attributes, credential_values + ), revocation_config=revoc, ), ) @@ -1161,24 +1189,28 @@ async def _create_credential_helper( rev_reg_def_id, tails_file_path, ) - except AnonCredsRevocationRegistryFullError: - # unlucky, another instance filled the registry first + except AnonCredsRevocationError as err: + LOGGER.warning(f"Failed to create credential: {err.message}, retrying") continue - # cred rev id is zero based - # max cred num is one based - # however, if we wait until max cred num is reached, we are too late. - if rev_reg_def_result: - if ( + def _is_full_registry( + rev_reg_def_result: RevRegDefResult, cred_rev_id: str + ) -> bool: + # if we wait until max cred num is reached, we are too late. + return ( rev_reg_def_result.rev_reg_def.value.max_cred_num <= int(cred_rev_id) + 1 - ): - await self.handle_full_registry(rev_reg_def_id) + ) + + if rev_reg_def_result and _is_full_registry( + rev_reg_def_result, cred_rev_id + ): + await self.handle_full_registry(rev_reg_def_id) return cred_json, cred_rev_id, rev_reg_def_id raise AnonCredsRevocationError( - f"Cred def '{cred_def_id}' has no active revocation registry" + f"Cred def '{cred_def_id}' revocation registry or list is in a bad state" ) async def revoke_pending_credentials( @@ -1342,8 +1374,7 @@ async def revoke_pending_credentials( ) if not rev_info_upd: LOGGER.warning( - "Revocation registry missing, skipping update: {}", - revoc_reg_id, + f"Revocation registry missing, skipping update: {revoc_reg_id}" # noqa: E501 ) updated_list = None break diff --git a/aries_cloudagent/anoncreds/revocation_setup.py b/aries_cloudagent/anoncreds/revocation_setup.py index c12b9060a8..f24e59bad0 100644 --- a/aries_cloudagent/anoncreds/revocation_setup.py +++ b/aries_cloudagent/anoncreds/revocation_setup.py @@ -5,7 +5,7 @@ from aries_cloudagent.protocols.endorse_transaction.v1_0.util import is_author_role -from ..anoncreds.revocation import AnonCredsRevocation +from ..anoncreds.revocation import AnonCredsRevocation, AnonCredsRevocationError from ..core.event_bus import EventBus from ..core.profile import Profile from ..revocation.util import notify_revocation_published_event @@ -95,7 +95,16 @@ async def on_rev_reg_def(self, profile: Profile, event: RevRegDefFinishedEvent): if auto_create_revocation: revoc = AnonCredsRevocation(profile) - await revoc.upload_tails_file(payload.rev_reg_def) + failed_to_upload_tails = False + try: + await revoc.upload_tails_file(payload.rev_reg_def) + except AnonCredsRevocationError as err: + LOGGER.warning(f"Failed to upload tails file: {err}") + failed_to_upload_tails = True + + if failed_to_upload_tails: + payload.options["failed_to_upload"] = True + await revoc.create_and_register_revocation_list( payload.rev_reg_def_id, payload.options ) diff --git a/aries_cloudagent/anoncreds/tests/test_issuer.py b/aries_cloudagent/anoncreds/tests/test_issuer.py index b1ba1517a3..673dfe462e 100644 --- a/aries_cloudagent/anoncreds/tests/test_issuer.py +++ b/aries_cloudagent/anoncreds/tests/test_issuer.py @@ -471,7 +471,7 @@ async def test_create_and_register_credential_definition_invalid_options_raises_ issuer_id="issuer-id", schema_id="schema-id", signature_type="CL", - options={"max_cred_num": "100"}, # requires integer + options={"support_revocation": "100"}, # requires integer ) @mock.patch.object(test_module.AnonCredsIssuer, "notify") diff --git a/aries_cloudagent/anoncreds/tests/test_revocation.py b/aries_cloudagent/anoncreds/tests/test_revocation.py index f4a3a12ad6..7f98764348 100644 --- a/aries_cloudagent/anoncreds/tests/test_revocation.py +++ b/aries_cloudagent/anoncreds/tests/test_revocation.py @@ -11,9 +11,6 @@ RevocationRegistryDefinitionPrivate, RevocationStatusList, Schema, - # AnoncredsError, - # W3cCredential, - # CredentialRevocationConfig, ) from aries_askar import AskarError, AskarErrorCode from requests import RequestException, Session @@ -34,13 +31,9 @@ GetSchemaResult, ) from aries_cloudagent.anoncreds.registry import AnonCredsRegistry -from aries_cloudagent.anoncreds.tests.mock_objects import ( - MOCK_REV_REG_DEF, -) +from aries_cloudagent.anoncreds.tests.mock_objects import MOCK_REV_REG_DEF from aries_cloudagent.anoncreds.tests.test_issuer import MockCredDefEntry -from aries_cloudagent.askar.profile_anon import ( - AskarAnoncredsProfile, -) +from aries_cloudagent.askar.profile_anon import AskarAnoncredsProfile from aries_cloudagent.core.event_bus import Event, EventBus, MockEventBus from aries_cloudagent.core.in_memory.profile import ( InMemoryProfile, @@ -78,6 +71,8 @@ class MockRevRegDefEntry: def __init__(self, name="name"): self.name = name + rev_reg_def_id = "test-rev-reg-def-id" + tags = { "state": RevRegDefState.STATE_ACTION, } @@ -890,7 +885,7 @@ async def test_upload_tails_file(self): @mock.patch.object( test_module.AnonCredsRevocation, "create_and_register_revocation_registry_definition", - return_value="backup", + return_value=MockRevRegDefEntry(), ) async def test_handle_full_registry( self, mock_create_and_register, mock_set_active_registry, mock_handle @@ -1067,14 +1062,14 @@ async def test_create_credential_private_no_rev_reg_or_tails( credential_type=Credential, ) - @mock.patch.object(InMemoryProfileSession, "handle") @mock.patch.object( RevocationRegistryDefinition, "load", return_value=rev_reg_def.value ) @mock.patch("aries_cloudagent.anoncreds.revocation.CredentialRevocationConfig") + @mock.patch.object(InMemoryProfileSession, "handle") @mock.patch.object(Credential, "create", return_value=mock.MagicMock()) async def test_create_credential_private_with_rev_reg_and_tails( - self, mock_create, mock_config, mock_load_rev_reg_def, mock_handle + self, mock_create, mock_handle, *_ ): async def call_test_func(): await self.revocation._create_credential( @@ -1096,21 +1091,21 @@ async def call_test_func(): credential_type=Credential, ) - # missing rev list + # missing rev def mock_handle.fetch = mock.CoroutineMock( - side_effect=[MockEntry(), MockEntry(), None, MockEntry(), MockEntry()] + side_effect=[None, MockEntry(), MockEntry()] ) with self.assertRaises(test_module.AnonCredsRevocationError): await call_test_func() - # missing rev def + # missing rev list mock_handle.fetch = mock.CoroutineMock( - side_effect=[MockEntry(), MockEntry(), MockEntry(), None, MockEntry()] + side_effect=[MockEntry(), None, MockEntry()] ) with self.assertRaises(test_module.AnonCredsRevocationError): await call_test_func() # missing rev key mock_handle.fetch = mock.CoroutineMock( - side_effect=[MockEntry(), MockEntry(), MockEntry(), MockEntry(), None] + side_effect=[MockEntry(), MockEntry(), None] ) with self.assertRaises(test_module.AnonCredsRevocationError): await call_test_func() @@ -1119,35 +1114,34 @@ async def call_test_func(): mock_handle.replace = mock.CoroutineMock(return_value=None) mock_handle.fetch = mock.CoroutineMock( side_effect=[ - MockEntry(), - MockEntry(), + MockEntry(raw_value=rev_reg_def.serialize()), MockEntry( value_json={ "rev_list": rev_list.serialize(), "next_index": 0, } ), - MockEntry(raw_value=rev_reg_def.serialize()), + MockEntry(), + MockEntry(), MockEntry(), ] ) await call_test_func() assert mock_create.called assert mock_handle.replace.called - assert mock_handle.fetch.call_count == 5 # revocation registry is full mock_handle.fetch = mock.CoroutineMock( side_effect=[ - MockEntry(), - MockEntry(), + MockEntry(raw_value=rev_reg_def.serialize()), MockEntry( value_json={ "rev_list": rev_list.serialize(), "next_index": 101, } ), - MockEntry(raw_value=rev_reg_def.serialize()), + MockEntry(), + MockEntry(), MockEntry(), ] ) diff --git a/aries_cloudagent/ledger/indy_vdr.py b/aries_cloudagent/ledger/indy_vdr.py index 3e8c2dbb2b..fb3aa537f8 100644 --- a/aries_cloudagent/ledger/indy_vdr.py +++ b/aries_cloudagent/ledger/indy_vdr.py @@ -1169,13 +1169,18 @@ async def send_revoc_reg_def( rev_reg_def_req.set_endorser(endorser_did) legacy_indy_registry = LegacyIndyRegistry() - resp = await legacy_indy_registry.txn_submit( - self, - rev_reg_def_req, - sign=True, - sign_did=did_info, - write_ledger=write_ledger, - ) + try: + resp = await legacy_indy_registry.txn_submit( + self, + rev_reg_def_req, + sign=True, + sign_did=did_info, + write_ledger=write_ledger, + ) + except LedgerError as err: + raise LedgerError( + "Exception when sending revocation registry definition" + ) from err if not write_ledger: return revoc_reg_def["id"], {"signed_txn": resp} diff --git a/aries_cloudagent/protocols/endorse_transaction/v1_0/manager.py b/aries_cloudagent/protocols/endorse_transaction/v1_0/manager.py index 9fca6aec4b..167fd842cc 100644 --- a/aries_cloudagent/protocols/endorse_transaction/v1_0/manager.py +++ b/aries_cloudagent/protocols/endorse_transaction/v1_0/manager.py @@ -425,8 +425,10 @@ async def complete_transaction( ) legacy_indy_registry = LegacyIndyRegistry() - ledger_response_json = await legacy_indy_registry.txn_submit( - ledger, ledger_transaction, sign=False, taa_accept=False + ledger_response_json = await shield( + legacy_indy_registry.txn_submit( + ledger, ledger_transaction, sign=False, taa_accept=False + ) ) else: async with ledger: diff --git a/demo/runners/support/agent.py b/demo/runners/support/agent.py index ae03cde81d..606d87d0f7 100644 --- a/demo/runners/support/agent.py +++ b/demo/runners/support/agent.py @@ -7,8 +7,8 @@ import subprocess import sys from concurrent.futures import ThreadPoolExecutor -from timeit import default_timer from secrets import token_hex +from timeit import default_timer import asyncpg import yaml @@ -464,7 +464,7 @@ async def register_schema_and_creddef_anoncreds( }, "options": { "support_revocation": support_revocation, - "max_cred_num": max_cred_num, + "revocation_registry_size": max_cred_num, }, } credential_definition_response = await self.admin_POST(