From 9da4f76f127cf5e4cf5607850c1bb64095ebdc54 Mon Sep 17 00:00:00 2001 From: sklump Date: Fri, 21 Aug 2020 15:10:00 +0000 Subject: [PATCH 1/3] streamline cred-def peri-creation activity; parallel schema logic Signed-off-by: sklump --- aries_cloudagent/ledger/indy.py | 18 +--- aries_cloudagent/ledger/tests/test_indy.py | 103 +++++++++++++++++++-- 2 files changed, 98 insertions(+), 23 deletions(-) diff --git a/aries_cloudagent/ledger/indy.py b/aries_cloudagent/ledger/indy.py index 7077fa6215..9e5c874c20 100644 --- a/aries_cloudagent/ledger/indy.py +++ b/aries_cloudagent/ledger/indy.py @@ -36,7 +36,6 @@ ) from .util import TAA_ACCEPTED_RECORD_TYPE, EndpointType - GENESIS_TRANSACTION_PATH = tempfile.gettempdir() GENESIS_TRANSACTION_PATH = path.join( GENESIS_TRANSACTION_PATH, "indy_genesis_transactions.txt" @@ -568,7 +567,7 @@ async def create_and_send_credential_definition( if not schema: raise LedgerError(f"Ledger {self.pool_name} has no schema {schema_id}") - # check if cred def is on ledger already + # check if cred def is on ledger alread for test_tag in [tag] if tag else ["tag", DEFAULT_CRED_DEF_TAG]: credential_definition_id = issuer.make_credential_definition_id( public_info.did, schema, signature_type, test_tag @@ -624,7 +623,6 @@ async def create_and_send_credential_definition( "Error cannot write cred def when ledger is in read only mode" ) - wallet_cred_def = json.loads(credential_definition_json) with IndyErrorHandler( "Exception when building cred def request", LedgerError ): @@ -632,19 +630,9 @@ async def create_and_send_credential_definition( public_info.did, credential_definition_json ) await self._submit(request_json, True, sign_did=public_info) - ledger_cred_def = await self.fetch_credential_definition( - credential_definition_id - ) - assert wallet_cred_def["value"] == ledger_cred_def["value"] - # Add non-secrets records if not yet present - storage = self.get_indy_storage() - found = await storage.search_records( - type_filter=CRED_DEF_SENT_RECORD_TYPE, - tag_query={"cred_def_id": credential_definition_id}, - ).fetch_all() - - if not found: + # Add non-secrets record + storage = self.get_indy_storage() schema_id_parts = schema_id.split(":") cred_def_tags = { "schema_id": schema_id, diff --git a/aries_cloudagent/ledger/tests/test_indy.py b/aries_cloudagent/ledger/tests/test_indy.py index 028f7956c7..61e7b2eae1 100644 --- a/aries_cloudagent/ledger/tests/test_indy.py +++ b/aries_cloudagent/ledger/tests/test_indy.py @@ -519,7 +519,10 @@ async def test_send_schema_already_exists( mock_wallet.get_public_did = async_mock.CoroutineMock() mock_wallet.get_public_did.return_value.did = "abc" - fetch_schema_id = f"{mock_wallet.get_public_did.return_value.did}:{2}:schema_name:schema_version" + fetch_schema_id = ( + f"{mock_wallet.get_public_did.return_value.did}:2:" + "schema_name:schema_version" + ) mock_check_existing.return_value = (fetch_schema_id, {}) issuer = async_mock.MagicMock(BaseIssuer) @@ -556,7 +559,10 @@ async def test_send_schema_ledger_transaction_error_already_exists( mock_wallet.get_public_did = async_mock.CoroutineMock() mock_wallet.get_public_did.return_value.did = "abc" - fetch_schema_id = f"{mock_wallet.get_public_did.return_value.did}:{2}:schema_name:schema_version" + fetch_schema_id = ( + f"{mock_wallet.get_public_did.return_value.did}:2:" + "schema_name:schema_version" + ) mock_check_existing.side_effect = [None, (fetch_schema_id, "{}")] issuer = async_mock.MagicMock(BaseIssuer) @@ -592,7 +598,7 @@ async def test_send_schema_ledger_read_only( mock_wallet.get_public_did.return_value.did = "abc" fetch_schema_id = ( - f"{mock_wallet.get_public_did.return_value.did}:{2}:" + f"{mock_wallet.get_public_did.return_value.did}:2:" "schema_name:schema_version" ) mock_check_existing.side_effect = [None, fetch_schema_id] @@ -628,7 +634,7 @@ async def test_send_schema_issuer_error( mock_wallet.get_public_did.return_value.did = "abc" fetch_schema_id = ( - f"{mock_wallet.get_public_did.return_value.did}:{2}:" + f"{mock_wallet.get_public_did.return_value.did}:2:" "schema_name:schema_version" ) mock_check_existing.side_effect = [None, fetch_schema_id] @@ -670,7 +676,7 @@ async def test_send_schema_ledger_transaction_error( mock_wallet.get_public_did.return_value.did = "abc" fetch_schema_id = ( - f"{mock_wallet.get_public_did.return_value.did}:{2}:" + f"{mock_wallet.get_public_did.return_value.did}:2:" "schema_name:schema_version" ) mock_check_existing.side_effect = [None, fetch_schema_id] @@ -1019,6 +1025,90 @@ async def test_send_credential_definition( mock_build_cred_def.assert_called_once_with(mock_did.did, cred_def_json) + @async_mock.patch("aries_cloudagent.ledger.indy.IndyLedger.get_schema") + @async_mock.patch("aries_cloudagent.ledger.indy.IndyLedger._context_open") + @async_mock.patch("aries_cloudagent.ledger.indy.IndyLedger._context_close") + @async_mock.patch( + "aries_cloudagent.ledger.indy.IndyLedger.fetch_credential_definition" + ) + @async_mock.patch("aries_cloudagent.ledger.indy.IndyLedger._submit") + @async_mock.patch("aries_cloudagent.storage.indy.IndyStorage.search_records") + @async_mock.patch("aries_cloudagent.storage.indy.IndyStorage.add_record") + @async_mock.patch("indy.ledger.build_cred_def_request") + async def test_send_credential_definition_exists_in_ledger_and_wallet( + self, + mock_build_cred_def, + mock_add_record, + mock_search_records, + mock_submit, + mock_fetch_cred_def, + mock_close, + mock_open, + mock_get_schema, + ): + mock_wallet = async_mock.MagicMock() + mock_wallet.type = "indy" + + mock_search_records.return_value.fetch_all = async_mock.CoroutineMock( + return_value=[] + ) + + mock_get_schema.return_value = {"seqNo": 999} + cred_def_id = f"{self.test_did}:3:CL:999:default" + cred_def_value = { + "primary": {"n": "...", "s": "...", "r": "...", "revocation": None} + } + cred_def = { + "ver": "1.0", + "id": cred_def_id, + "schemaId": "999", + "type": "CL", + "tag": "default", + "value": cred_def_value, + } + cred_def_json = json.dumps(cred_def) + + mock_fetch_cred_def.return_value = {"mock": "cred-def"} + + issuer = async_mock.MagicMock(BaseIssuer) + issuer.make_credential_definition_id.return_value = cred_def_id + issuer.create_and_store_credential_definition.return_value = ( + cred_def_id, + cred_def_json, + ) + issuer.credential_definition_in_wallet.return_value = True + ledger = IndyLedger("name", mock_wallet) + + schema_id = "schema_issuer_did:name:1.0" + tag = "default" + + with async_mock.patch.object( + ledger, "get_indy_storage", async_mock.MagicMock() + ) as mock_get_storage: + mock_get_storage.return_value = async_mock.MagicMock( + add_record=async_mock.CoroutineMock() + ) + + async with ledger: + mock_wallet.get_public_did = async_mock.CoroutineMock() + mock_wallet.get_public_did.return_value = DIDInfo( + self.test_did, self.test_verkey, None + ) + mock_did = mock_wallet.get_public_did.return_value + + (result_id, result_def) = ( + await ledger.create_and_send_credential_definition( + issuer, schema_id, None, tag + ) + ) + assert result_id == cred_def_id + + mock_wallet.get_public_did.assert_called_once_with() + mock_get_schema.assert_called_once_with(schema_id) + + mock_build_cred_def.assert_not_called() + mock_get_storage.assert_not_called() + @async_mock.patch("aries_cloudagent.ledger.indy.IndyLedger.get_schema") @async_mock.patch("aries_cloudagent.ledger.indy.IndyLedger._context_open") @async_mock.patch("aries_cloudagent.ledger.indy.IndyLedger._context_close") @@ -1457,9 +1547,6 @@ async def test_send_credential_definition_create_cred_def_exception( issuer.create_and_store_credential_definition.side_effect = IssuerError( "invalid structure" ) - # issuer.credential_definition_in_wallet.side_effect = IndyError( - # error_code=ErrorCode.CommonInvalidStructure - # ) ledger = IndyLedger("name", mock_wallet) schema_id = "schema_issuer_did:name:1.0" From 7fe5f4ab234fd7563559850b380102ffdd070def Mon Sep 17 00:00:00 2001 From: sklump Date: Fri, 21 Aug 2020 15:11:00 +0000 Subject: [PATCH 2/3] black tweak Signed-off-by: sklump --- aries_cloudagent/ledger/tests/test_indy.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/aries_cloudagent/ledger/tests/test_indy.py b/aries_cloudagent/ledger/tests/test_indy.py index 61e7b2eae1..8edb248f3b 100644 --- a/aries_cloudagent/ledger/tests/test_indy.py +++ b/aries_cloudagent/ledger/tests/test_indy.py @@ -1096,10 +1096,11 @@ async def test_send_credential_definition_exists_in_ledger_and_wallet( ) mock_did = mock_wallet.get_public_did.return_value - (result_id, result_def) = ( - await ledger.create_and_send_credential_definition( - issuer, schema_id, None, tag - ) + ( + result_id, + result_def, + ) = await ledger.create_and_send_credential_definition( + issuer, schema_id, None, tag ) assert result_id == cred_def_id From acc73834a1383cb8a7ce1e09410b4583c4fb0cb1 Mon Sep 17 00:00:00 2001 From: sklump Date: Fri, 21 Aug 2020 17:00:28 +0000 Subject: [PATCH 3/3] only create rev reg, tails file for novel cred defs Signed-off-by: sklump --- aries_cloudagent/ledger/base.py | 5 +++- aries_cloudagent/ledger/indy.py | 12 +++++++--- aries_cloudagent/ledger/tests/test_indy.py | 15 ++++++++++-- .../credential_definitions/routes.py | 24 ++++++++----------- .../tests/test_routes.py | 2 +- 5 files changed, 37 insertions(+), 21 deletions(-) diff --git a/aries_cloudagent/ledger/base.py b/aries_cloudagent/ledger/base.py index 4b0ab0b8f3..fcdcc94dc3 100644 --- a/aries_cloudagent/ledger/base.py +++ b/aries_cloudagent/ledger/base.py @@ -160,7 +160,7 @@ async def create_and_send_credential_definition( signature_type: str = None, tag: str = None, support_revocation: bool = False, - ) -> Tuple[str, dict]: + ) -> Tuple[str, dict, bool]: """ Send credential definition to ledger and store relevant key matter in wallet. @@ -171,6 +171,9 @@ async def create_and_send_credential_definition( tag: Optional tag to distinguish multiple credential definitions support_revocation: Optional flag to enable revocation for this cred def + Returns: + Tuple with cred def id, cred def structure, and whether it's novel + """ @abstractmethod diff --git a/aries_cloudagent/ledger/indy.py b/aries_cloudagent/ledger/indy.py index 9e5c874c20..8095942664 100644 --- a/aries_cloudagent/ledger/indy.py +++ b/aries_cloudagent/ledger/indy.py @@ -545,7 +545,7 @@ async def create_and_send_credential_definition( signature_type: str = None, tag: str = None, support_revocation: bool = False, - ) -> Tuple[str, dict]: + ) -> Tuple[str, dict, bool]: """ Send credential definition to ledger and store relevant key matter in wallet. @@ -556,6 +556,9 @@ async def create_and_send_credential_definition( tag: Optional tag to distinguish multiple credential definitions support_revocation: Optional flag to enable revocation for this cred def + Returns: + Tuple with cred def id, cred def structure, and whether it's novel + """ public_info = await self.wallet.get_public_did() if not public_info: @@ -567,7 +570,9 @@ async def create_and_send_credential_definition( if not schema: raise LedgerError(f"Ledger {self.pool_name} has no schema {schema_id}") - # check if cred def is on ledger alread + novel = False + + # check if cred def is on ledger already for test_tag in [tag] if tag else ["tag", DEFAULT_CRED_DEF_TAG]: credential_definition_id = issuer.make_credential_definition_id( public_info.did, schema, signature_type, test_tag @@ -608,6 +613,7 @@ async def create_and_send_credential_definition( raise LedgerError(err.message) from err # Cred def is neither on ledger nor in wallet: create and send it + novel = True try: ( credential_definition_id, @@ -648,7 +654,7 @@ async def create_and_send_credential_definition( ) await storage.add_record(record) - return credential_definition_id, json.loads(credential_definition_json) + return (credential_definition_id, json.loads(credential_definition_json), novel) async def get_credential_definition(self, credential_definition_id: str) -> dict: """ diff --git a/aries_cloudagent/ledger/tests/test_indy.py b/aries_cloudagent/ledger/tests/test_indy.py index 8edb248f3b..3c864dac07 100644 --- a/aries_cloudagent/ledger/tests/test_indy.py +++ b/aries_cloudagent/ledger/tests/test_indy.py @@ -1015,10 +1015,15 @@ async def test_send_credential_definition( ) mock_did = mock_wallet.get_public_did.return_value - result_id, result_def = await ledger.create_and_send_credential_definition( + ( + result_id, + result_def, + novel, + ) = await ledger.create_and_send_credential_definition( issuer, schema_id, None, tag ) assert result_id == cred_def_id + assert novel mock_wallet.get_public_did.assert_called_once_with() mock_get_schema.assert_called_once_with(schema_id) @@ -1099,10 +1104,12 @@ async def test_send_credential_definition_exists_in_ledger_and_wallet( ( result_id, result_def, + novel, ) = await ledger.create_and_send_credential_definition( issuer, schema_id, None, tag ) assert result_id == cred_def_id + assert not novel mock_wallet.get_public_did.assert_called_once_with() mock_get_schema.assert_called_once_with(schema_id) @@ -1489,7 +1496,11 @@ async def test_send_credential_definition_on_ledger_in_wallet( ) mock_did = mock_wallet.get_public_did.return_value - result_id, result_def = await ledger.create_and_send_credential_definition( + ( + result_id, + result_def, + novel, + ) = await ledger.create_and_send_credential_definition( issuer, schema_id, None, tag ) assert result_id == cred_def_id diff --git a/aries_cloudagent/messaging/credential_definitions/routes.py b/aries_cloudagent/messaging/credential_definitions/routes.py index d0dab8491e..76691cd710 100644 --- a/aries_cloudagent/messaging/credential_definitions/routes.py +++ b/aries_cloudagent/messaging/credential_definitions/routes.py @@ -139,9 +139,9 @@ async def credential_definitions_send_credential_definition(request: web.BaseReq raise web.HTTPForbidden(reason=reason) issuer: BaseIssuer = await context.inject(BaseIssuer) - try: + try: # even if in wallet, send it and raise if erroneously so async with ledger: - credential_definition_id, credential_definition = await shield( + (cred_def_id, cred_def, novel) = await shield( ledger.create_and_send_credential_definition( issuer, schema_id, @@ -153,19 +153,17 @@ async def credential_definitions_send_credential_definition(request: web.BaseReq except LedgerError as e: raise web.HTTPBadRequest(reason=e.message) from e - # If revocation is requested, create revocation registry - if support_revocation: + # If revocation is requested and cred def is novel, create revocation registry + if support_revocation and novel: tails_base_url = context.settings.get("tails_server_base_url") if not tails_base_url: raise web.HTTPBadRequest(reason="tails_server_base_url not configured") try: # Create registry - issuer_did = credential_definition_id.split(":")[0] + issuer_did = cred_def_id.split(":")[0] revoc = IndyRevocation(context) registry_record = await revoc.init_issuer_registry( - credential_definition_id, - issuer_did, - max_cred_num=revocation_registry_size, + cred_def_id, issuer_did, max_cred_num=revocation_registry_size, ) except RevocationNotSupportedError as e: @@ -199,7 +197,7 @@ async def credential_definitions_send_credential_definition(request: web.BaseReq except RevocationError as e: raise web.HTTPBadRequest(reason=e.message) from e - return web.json_response({"credential_definition_id": credential_definition_id}) + return web.json_response({"credential_definition_id": cred_def_id}) @docs( @@ -253,7 +251,7 @@ async def credential_definitions_get_credential_definition(request: web.BaseRequ """ context = request.app["request_context"] - credential_definition_id = request.match_info["cred_def_id"] + cred_def_id = request.match_info["cred_def_id"] ledger: BaseLedger = await context.inject(BaseLedger, required=False) if not ledger: @@ -263,11 +261,9 @@ async def credential_definitions_get_credential_definition(request: web.BaseRequ raise web.HTTPForbidden(reason=reason) async with ledger: - credential_definition = await ledger.get_credential_definition( - credential_definition_id - ) + cred_def = await ledger.get_credential_definition(cred_def_id) - return web.json_response({"credential_definition": credential_definition}) + return web.json_response({"credential_definition": cred_def}) async def register(app: web.Application): diff --git a/aries_cloudagent/messaging/credential_definitions/tests/test_routes.py b/aries_cloudagent/messaging/credential_definitions/tests/test_routes.py index 4330b83ca8..6d2baba131 100644 --- a/aries_cloudagent/messaging/credential_definitions/tests/test_routes.py +++ b/aries_cloudagent/messaging/credential_definitions/tests/test_routes.py @@ -24,7 +24,7 @@ def setUp(self): self.ledger = async_mock.create_autospec(BaseLedger) self.ledger.__aenter__ = async_mock.CoroutineMock(return_value=self.ledger) self.ledger.create_and_send_credential_definition = async_mock.CoroutineMock( - return_value=(CRED_DEF_ID, {"cred": "def"}) + return_value=(CRED_DEF_ID, {"cred": "def"}, True) ) self.ledger.get_credential_definition = async_mock.CoroutineMock( return_value={"cred": "def"}