-
Notifications
You must be signed in to change notification settings - Fork 9
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
Conversation
This file contains 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
cl0ete
force-pushed
the
fix-issuer-did-registration
branch
from
December 6, 2024 09:33
797132f
to
fbcc927
Compare
Should be called as the issuer, with `create_transaction_for_endorser=True`. Previously was being called as endorser and automatically written to ledger
ff137
force-pushed
the
fix-issuer-did-registration
branch
from
December 10, 2024 17:38
fbcc927
to
10fd6a3
Compare
… unknown config warning)
…er before setting as public did
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
K8s Regression Test Coverage
|
Quality Gate passedIssues Measures |
K8s Test Coverage
|
K8s Regression Test Coverage
|
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)
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.
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 defsEdit:
🚧 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....
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