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

🐛 Fix registering issuer did #1219

Merged
merged 39 commits into from
Dec 11, 2024
Merged

🐛 Fix registering issuer did #1219

merged 39 commits into from
Dec 11, 2024

Conversation

ff137
Copy link
Collaborator

@ff137 ff137 commented Dec 5, 2024

During onboarding, registration of the issuer's DID should be called by the issuer controller, with create_transaction_for_endorser=True. Previously this was being called by the endorser controller, and automatically written to ledger.

🔧 New env var to configure timeout waiting for DID endorsement: ISSUER_DID_ENDORSE_TIMEOUT

Extras in this PR:
✅ Includes full unit test coverage for endorser module (while I was at it)
⚡ The unit test for onboarding issuer no longer requires the 4.5s sleeps


I merged the old, dangling PR #987 into this branch, since the increased load on the endorser now requires increased default timeouts. So, we also add:
🔧 new env var: PUBLISH_REVOCATIONS_TIMEOUT

And while we're at it, made this configurable too:
🔧 CRED_DEF_ACK_TIMEOUT to configure timeout waiting for endorsement of cred defs


Edit:
🚧 Getting some wonky behaviour ...
DID registration and issuer onboarding succeeds, but then trying to create a did-exchange request to that public did fails ... due to not resolving the DIDComm services....

HTTP post `/v1/connections/did-exchange/create-request` failed. Status code: 400. Response: `{"detail":"Failed to resolve DIDComm services from did:sov:Ur2xba9cVQutag8P8moCpZ: Cannot connect via DID that has no associated services. Cannot connect via DID that has no associated services."}`.

despite the raw data on the ledger looking pretty much identical with the new way vs the old way of writing it. 🤷‍♂️


Edit2: Fixed the above... We just need to wait for 2 transactions to be endorsed now, instead of just the 1!
i.e. await did written to ledger before setting as public did, and then await attrib data set before proceeding

@cl0ete cl0ete marked this pull request as ready for review December 6, 2024 09:33
@cl0ete cl0ete force-pushed the fix-issuer-did-registration branch from 797132f to fbcc927 Compare December 6, 2024 09:33
@ff137 ff137 marked this pull request as draft December 6, 2024 20:01
@ff137 ff137 self-assigned this Dec 10, 2024
@ff137 ff137 added bug Something isn't working improvement improving on something that is already existing (not a new feature as such) labels Dec 10, 2024
@ff137 ff137 requested a review from cl0ete December 10, 2024 18:41
Copy link

Coverage

K8s Test Coverage
FileStmtsMissCoverMissing
app
   main.py78297%119–120
app/models
   wallet.py36489%100–105
app/routes
   jsonld.py604722%23–93, 104–137
   messaging.py241154%43–52, 85–95
   oob.py462839%58–99, 131–144, 170–181
   trust_registry.py62297%52–53
   verifier.py1393376%88–90, 95, 152–154, 159, 231, 239–241, 246, 288–293, 299–301, 362–364, 369, 406–408, 413, 449–451, 505–507
app/routes/wallet
   credentials.py805334%57–70, 96–108, 129–140, 166–180, 216–232, 266–283, 309–321, 342–353
app/services
   acapy_ledger.py611280%110–111, 124–140, 196–197
   acapy_wallet.py40490%60–61, 98–99
   revocation_registry.py1663579%118–160, 201–205, 360–364, 403–408, 441–445, 473–478, 527–528
app/services/issuer
   acapy_issuer_v2.py1011288%63, 97–106, 110, 126–127, 174, 228–229, 256
app/services/onboarding
   tenants.py634922%30–103, 113–143
   verifier.py37392%67, 83–87
app/services/onboarding/util
   register_issuer_did.py97595%103–105, 279–284
   set_endorser_metadata.py724044%32–34, 58–60, 85–87, 118–137, 149–160, 172–185, 198–212
app/services/trust_registry
   actors.py107992%95–100, 110, 209–210, 212–217, 282–285
   schemas.py54983%57–63, 89–99
app/services/trust_registry/util
   actor.py31294%26–27
   issuer.py23483%39–40, 48–49
app/tests/e2e
   conftest.py23578%47–55
   test_connections.py141299%276–277
   test_did_exchange.py732171%124–187
   test_jsonld.py591673%134–169
   test_proof_request_models.py24196%101
   test_tenants.py4211596%203, 247–248, 287, 789, 833–834, 843–844, 853–854, 866–867, 876–877
   test_wallet_credentials.py46198%57
   test_wallet_dids.py701086%101–116
   test_wallet_jws.py40198%56
   test_wallet_sd_jws.py40198%56
app/tests/e2e/issuer
   test_get_records_paginated.py63297%65–66
   test_save_exchange_record.py98199%262
app/tests/e2e/verifier
   test_get_records_paginated.py62297%59–60
   test_many_revocations.py634233%31–107, 117–130, 141–216
   test_proof_revoked_credential.py451273%128–200
   test_verifier.py2211295%654–724
app/tests/exception_handling
   test_handle_model_with_validation.py23196%22
app/tests/fixtures
   credentials.py1413774%174, 269–349, 362–433
   definitions.py662661%29–50, 64–65, 94–124, 143–144
   member_acapy_clients.py36489%47–50, 73–74
   member_connections.py40880%79–90, 116–127
   member_wallets.py611575%29–34, 49–54, 69–74, 89–94, 111–118
app/tests/routes/issuer
   test_create_offer.py39197%145
   test_send_credential.py36197%125
app/tests/routes/wallet/dids
   test_get_public_did.py29197%40
app/tests/services
   test_revocation_registry.py141299%74, 105
app/tests/services/issuer
   test_issuer.py112298%282–286
app/tests/util
   connections.py1083568%121–148, 158–186, 213–223, 278–301
   ledger.py501080%35, 43, 55, 63–67, 77, 83
   regression_testing.py351751%33, 41–68, 74–81
   sse_listener.py28486%51–56
   trust_registry.py29390%24, 27–28
   webhooks.py531572%19, 65–81, 88–90, 95, 119
app/util
   acapy_verifier_utils.py123596%81–84, 164–168, 245
   assert_public_did.py18856%17–33
   credentials.py14286%13, 18
   retry_method.py361558%20–35, 76–77, 91–103
   save_exchange_record.py13192%29
   tenants.py451762%24–29, 44–45, 52–59, 90–91, 97, 124
   transaction_acked.py14286%34–35
endorser/tests
   test_util_endorsement.py182597%290, 295, 354–356
trustregistry
   main.py82495%46–48, 110
trustregistry/tests
   test_main.py137199%90
waypoint
   main.py48198%64
waypoint/services
   nats_service.py109397%69–71
waypoint/tests/routers
   test_waypoint_sse.py83594%61–62, 120, 138, 174
waypoint/tests/services
   test_nats_service.py177995%47, 161, 188, 221, 263, 303, 366, 370, 373
TOTAL1311576894% 

Tests Skipped Failures Errors Time
887 6 💤 0 ❌ 0 🔥 3m 38s ⏱️

Copy link

Coverage

K8s Regression Test Coverage
FileStmtsMissCoverMissing
app
   main.py78297%119–120
app/models
   wallet.py36489%100–105
app/routes
   jsonld.py604722%23–93, 104–137
   messaging.py241154%43–52, 85–95
   oob.py462839%58–99, 131–144, 170–181
   trust_registry.py62297%52–53
   verifier.py1393376%88–90, 95, 152–154, 159, 231, 239–241, 246, 288–293, 299–301, 362–364, 369, 406–408, 413, 449–451, 505–507
app/routes/wallet
   credentials.py805334%57–70, 96–108, 129–140, 166–180, 216–232, 266–283, 309–321, 342–353
app/services
   acapy_ledger.py611969%43–49, 79, 110–111, 124–140, 146–149, 196–197
   acapy_wallet.py40490%60–61, 98–99
   revocation_registry.py1664672%118–160, 201–205, 360–364, 403–408, 441–445, 463–478, 484–493, 527–528
app/services/issuer
   acapy_issuer_v2.py1011288%63, 97–106, 110, 126–127, 174, 228–229, 256
app/services/onboarding
   tenants.py634922%30–103, 113–143
   verifier.py37392%67, 83–87
app/services/onboarding/util
   register_issuer_did.py97595%103–105, 279–284
   set_endorser_metadata.py724044%32–34, 58–60, 85–87, 118–137, 149–160, 172–185, 198–212
app/services/trust_registry
   actors.py107992%95–100, 110, 209–210, 212–217, 282–285
   schemas.py54983%57–63, 89–99
app/services/trust_registry/util
   actor.py31294%26–27
   issuer.py23483%39–40, 48–49
app/tests/e2e
   conftest.py23578%47–55
   test_auth.py664236%30–41, 51–94, 104–152
   test_connections.py141299%276–277
   test_definitions.py795135%33–56, 68–80, 92–106, 118–146, 164–209
   test_did_exchange.py732171%124–187
   test_jsonld.py591673%134–169
   test_proof_request_models.py24196%101
   test_revocation.py1187636%27–77, 90–107, 118–144, 156–181, 193–207, 219–256, 267–293, 305–335, 346–349, 370–376
   test_tenants.py42136414%29–73, 84–117, 128–172, 185–254, 265–316, 329–459, 468–520, 529–565, 574–623, 632–681, 692–734, 746–795, 806–877, 886–993
   test_wallet_credentials.py461078%79–106
   test_wallet_dids.py701579%101–116, 128–139
   test_wallet_jws.py40198%56
   test_wallet_sd_jws.py40198%56
app/tests/e2e/issuer
   test_get_records_paginated.py635217%25–161
   test_save_exchange_record.py98199%262
app/tests/e2e/verifier
   test_get_credentials_by_proof_id.py251252%27–81
   test_get_records_paginated.py624921%27–155
   test_many_revocations.py634233%31–107, 117–130, 141–216
   test_proof_revoked_credential.py451176%38–114
   test_verifier_oob.py682268%134–237
app/tests/exception_handling
   test_handle_model_with_validation.py23196%22
app/tests/fixtures
   credentials.py1417348%131–205, 214–222, 232–254, 288–347, 382–431, 448–510
   definitions.py661085%41–48, 57–63, 114–121, 134–139
   member_acapy_clients.py36489%47–50, 73–74
   member_async_clients.py50296%32–33
   member_connections.py40490%72, 90, 109, 127
   member_wallets.py611575%23–27, 43–47, 63–67, 83–87, 103–109
app/tests/routes/issuer
   test_create_offer.py39197%145
   test_send_credential.py36197%125
app/tests/routes/wallet/dids
   test_get_public_did.py29197%40
app/tests/services
   test_revocation_registry.py141299%74, 105
app/tests/services/issuer
   test_issuer.py112298%282–286
app/tests/util
   connections.py1083369%55, 178–186, 201–208, 223, 233–265, 296–301, 315–347
   ledger.py502648%34–59, 63–67, 73–86
   regression_testing.py35974%33, 60–68, 74–81
   sse_listener.py28486%51–56
   tenants.py241250%10, 16–17, 21–26, 30–35, 39–44, 48–53, 57
   trust_registry.py29776%20–28
   webhooks.py531572%19, 65–81, 88–90, 95, 119
app/util
   acapy_verifier_utils.py123596%81–84, 164–168, 245
   assert_public_did.py181233%14–34
   check_endorser_connection.py10460%14–24
   credentials.py14286%13, 18
   retry_method.py363017%15–36, 68–114
   save_exchange_record.py13192%29
   string.py15473%15–16, 20, 24
   tenants.py451762%24–29, 44–45, 52–59, 90–91, 97, 124
   transaction_acked.py14750%21–39
endorser/tests
   test_util_endorsement.py182597%290, 295, 354–356
trustregistry
   main.py82495%46–48, 110
trustregistry/tests
   test_main.py137199%90
TOTAL12582149088% 

Tests Skipped Failures Errors Time
887 46 💤 0 ❌ 0 🔥 5m 34s ⏱️

Copy link

Coverage

K8s Test Coverage
FileStmtsMissCoverMissing
app
   main.py78297%119–120
app/models
   wallet.py36489%100–105
app/routes
   jsonld.py604722%23–93, 104–137
   messaging.py241154%43–52, 85–95
   oob.py462839%58–99, 131–144, 170–181
   trust_registry.py62297%52–53
   verifier.py1393376%88–90, 95, 152–154, 159, 231, 239–241, 246, 288–293, 299–301, 362–364, 369, 406–408, 413, 449–451, 505–507
app/routes/wallet
   credentials.py805334%57–70, 96–108, 129–140, 166–180, 216–232, 266–283, 309–321, 342–353
app/services
   acapy_ledger.py611280%110–111, 124–140, 196–197
   acapy_wallet.py40490%60–61, 98–99
   revocation_registry.py1663579%118–160, 201–205, 360–364, 403–408, 441–445, 473–478, 527–528
app/services/issuer
   acapy_issuer_v2.py1011288%63, 97–106, 110, 126–127, 174, 228–229, 256
app/services/onboarding
   tenants.py634922%30–103, 113–143
   verifier.py37392%67, 83–87
app/services/onboarding/util
   register_issuer_did.py95595%100–102, 276–281
   set_endorser_metadata.py724044%32–34, 58–60, 85–87, 118–137, 149–160, 172–185, 198–212
app/services/trust_registry
   actors.py107992%95–100, 110, 209–210, 212–217, 282–285
   schemas.py54983%57–63, 89–99
app/services/trust_registry/util
   actor.py31294%26–27
   issuer.py23483%39–40, 48–49
app/tests/e2e
   conftest.py23578%47–55
   test_connections.py141299%276–277
   test_did_exchange.py732171%124–187
   test_jsonld.py591673%134–169
   test_proof_request_models.py24196%101
   test_tenants.py4211596%203, 247–248, 287, 789, 833–834, 843–844, 853–854, 866–867, 876–877
   test_wallet_credentials.py46198%57
   test_wallet_dids.py701086%101–116
   test_wallet_jws.py40198%56
   test_wallet_sd_jws.py40198%56
app/tests/e2e/issuer
   test_get_records_paginated.py63297%65–66
   test_save_exchange_record.py98199%262
app/tests/e2e/verifier
   test_get_records_paginated.py62297%59–60
   test_many_revocations.py634233%31–107, 117–130, 141–216
   test_proof_revoked_credential.py451273%128–200
   test_verifier.py2211295%654–724
app/tests/exception_handling
   test_handle_model_with_validation.py23196%22
app/tests/fixtures
   credentials.py1413774%174, 269–349, 362–433
   definitions.py662661%29–50, 64–65, 94–124, 143–144
   member_acapy_clients.py36489%47–50, 73–74
   member_connections.py40880%79–90, 116–127
   member_wallets.py611575%29–34, 49–54, 69–74, 89–94, 111–118
app/tests/routes/issuer
   test_create_offer.py39197%145
   test_send_credential.py36197%125
app/tests/routes/wallet/dids
   test_get_public_did.py29197%40
app/tests/services
   test_revocation_registry.py141299%74, 105
app/tests/services/issuer
   test_issuer.py112298%282–286
app/tests/util
   connections.py1083568%121–148, 158–186, 213–223, 278–301
   ledger.py501080%35, 43, 55, 63–67, 77, 83
   regression_testing.py351751%33, 41–68, 74–81
   sse_listener.py28486%51–56
   trust_registry.py29390%24, 27–28
   webhooks.py531572%19, 65–81, 88–90, 95, 119
app/util
   acapy_verifier_utils.py123596%81–84, 164–168, 245
   assert_public_did.py18856%17–33
   credentials.py14286%13, 18
   retry_method.py361558%20–35, 76–77, 91–103
   save_exchange_record.py13192%29
   tenants.py451762%24–29, 44–45, 52–59, 90–91, 97, 124
   transaction_acked.py14286%34–35
endorser/tests
   test_util_endorsement.py182597%290, 295, 354–356
trustregistry
   main.py82495%46–48, 110
trustregistry/tests
   test_main.py137199%90
waypoint
   main.py48198%64
waypoint/services
   nats_service.py109397%69–71
waypoint/tests/routers
   test_waypoint_sse.py83594%61–62, 120, 138, 174
waypoint/tests/services
   test_nats_service.py177995%47, 161, 188, 221, 263, 303, 366, 370, 373
TOTAL1311176894% 

Tests Skipped Failures Errors Time
887 6 💤 0 ❌ 0 🔥 3m 40s ⏱️

Copy link

Coverage

K8s Regression Test Coverage
FileStmtsMissCoverMissing
app
   main.py78297%119–120
app/models
   wallet.py36489%100–105
app/routes
   jsonld.py604722%23–93, 104–137
   messaging.py241154%43–52, 85–95
   oob.py462839%58–99, 131–144, 170–181
   trust_registry.py62297%52–53
   verifier.py1393376%88–90, 95, 152–154, 159, 231, 239–241, 246, 288–293, 299–301, 362–364, 369, 406–408, 413, 449–451, 505–507
app/routes/wallet
   credentials.py805334%57–70, 96–108, 129–140, 166–180, 216–232, 266–283, 309–321, 342–353
app/services
   acapy_ledger.py611969%43–49, 79, 110–111, 124–140, 146–149, 196–197
   acapy_wallet.py40490%60–61, 98–99
   revocation_registry.py1664672%118–160, 201–205, 360–364, 403–408, 441–445, 463–478, 484–493, 527–528
app/services/issuer
   acapy_issuer_v2.py1011288%63, 97–106, 110, 126–127, 174, 228–229, 256
app/services/onboarding
   tenants.py634922%30–103, 113–143
   verifier.py37392%67, 83–87
app/services/onboarding/util
   register_issuer_did.py95595%100–102, 276–281
   set_endorser_metadata.py724044%32–34, 58–60, 85–87, 118–137, 149–160, 172–185, 198–212
app/services/trust_registry
   actors.py107992%95–100, 110, 209–210, 212–217, 282–285
   schemas.py54983%57–63, 89–99
app/services/trust_registry/util
   actor.py31294%26–27
   issuer.py23483%39–40, 48–49
app/tests/e2e
   conftest.py23578%47–55
   test_auth.py664236%30–41, 51–94, 104–152
   test_connections.py141299%276–277
   test_definitions.py795135%33–56, 68–80, 92–106, 118–146, 164–209
   test_did_exchange.py732171%124–187
   test_jsonld.py591673%134–169
   test_proof_request_models.py24196%101
   test_revocation.py1187636%27–77, 90–107, 118–144, 156–181, 193–207, 219–256, 267–293, 305–335, 346–349, 370–376
   test_tenants.py42136414%29–73, 84–117, 128–172, 185–254, 265–316, 329–459, 468–520, 529–565, 574–623, 632–681, 692–734, 746–795, 806–877, 886–993
   test_wallet_credentials.py461078%79–106
   test_wallet_dids.py701579%101–116, 128–139
   test_wallet_jws.py40198%56
   test_wallet_sd_jws.py40198%56
app/tests/e2e/issuer
   test_get_records_paginated.py635217%25–161
   test_save_exchange_record.py98199%262
app/tests/e2e/verifier
   test_get_credentials_by_proof_id.py251252%27–81
   test_get_records_paginated.py624921%27–155
   test_many_revocations.py634233%31–107, 117–130, 141–216
   test_proof_revoked_credential.py451176%38–114
   test_verifier_oob.py682268%134–237
app/tests/exception_handling
   test_handle_model_with_validation.py23196%22
app/tests/fixtures
   credentials.py1417348%131–205, 214–222, 232–254, 288–347, 382–431, 448–510
   definitions.py661085%41–48, 57–63, 114–121, 134–139
   member_acapy_clients.py36489%47–50, 73–74
   member_async_clients.py50296%32–33
   member_connections.py40490%72, 90, 109, 127
   member_wallets.py611575%23–27, 43–47, 63–67, 83–87, 103–109
app/tests/routes/issuer
   test_create_offer.py39197%145
   test_send_credential.py36197%125
app/tests/routes/wallet/dids
   test_get_public_did.py29197%40
app/tests/services
   test_revocation_registry.py141299%74, 105
app/tests/services/issuer
   test_issuer.py112298%282–286
app/tests/util
   connections.py1083369%55, 178–186, 201–208, 223, 233–265, 296–301, 315–347
   ledger.py502648%34–59, 63–67, 73–86
   regression_testing.py35974%33, 60–68, 74–81
   sse_listener.py28486%51–56
   tenants.py241250%10, 16–17, 21–26, 30–35, 39–44, 48–53, 57
   trust_registry.py29776%20–28
   webhooks.py531572%19, 65–81, 88–90, 95, 119
app/util
   acapy_verifier_utils.py123596%81–84, 164–168, 245
   assert_public_did.py181233%14–34
   check_endorser_connection.py10460%14–24
   credentials.py14286%13, 18
   retry_method.py363017%15–36, 68–114
   save_exchange_record.py13192%29
   string.py15473%15–16, 20, 24
   tenants.py451762%24–29, 44–45, 52–59, 90–91, 97, 124
   transaction_acked.py14750%21–39
endorser/tests
   test_util_endorsement.py182597%290, 295, 354–356
trustregistry
   main.py82495%46–48, 110
trustregistry/tests
   test_main.py137199%90
TOTAL12578149088% 

Tests Skipped Failures Errors Time
887 46 💤 0 ❌ 0 🔥 5m 41s ⏱️

@ff137 ff137 merged commit c649e76 into master Dec 11, 2024
51 checks passed
@ff137 ff137 deleted the fix-issuer-did-registration branch December 11, 2024 18:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working improvement improving on something that is already existing (not a new feature as such)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants