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

Add did:indy transaction version 2 support #3253

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

jamshale
Copy link
Contributor

@jamshale jamshale commented Sep 24, 2024

This adds the ability to create a did:indy with transaction version 2 algorithm. https://hyperledger.github.io/indy-did-method/#nym-transaction-version.

  • It's created in a new /did/indy/create endpoint. If the controller tries to create it in the /wallet/did/create endpoint it will get a indy method not supported (same as before) and tell them to use the new endpoint.
  • Adds seed support with allow-insecure-seed configuration
  • When starting up an agent wither a did:indy or did:sov seed can be used.
  • Does a refactor on wallet startup function to reduce complexity.

@jamshale jamshale changed the title [WIP] Add did:indy transaction version 2 support Add did:indy transaction version 2 support Sep 25, 2024
@jamshale jamshale marked this pull request as ready for review September 25, 2024 23:45
@jamshale
Copy link
Contributor Author

The security alerts are nothing. http used in the scenario tests. Not sure how to ignore it yet.

@dbluhm
Copy link
Member

dbluhm commented Sep 26, 2024

I like what you're doing in this PR -- really appreciate the wallet startup cleanup as well. Question: how do we get the DID onto the ledger? Are we saying this is handled out of band?

@jamshale
Copy link
Contributor Author

jamshale commented Sep 26, 2024

It's the same way as a did:sov. You use the /did/indy/create and then post it to /wallet/did/public. There's no way to start up a fresh agent with a seed and a did:indy currently. Doing that with a seed still creates a did:sov.

Edit: oh, to get it on the ledger you just post the did:indy:12345 and the verkey. So, yes I think that would be out of band.

Copy link

sonarcloud bot commented Sep 30, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
2 Security Hotspots

See analysis details on SonarCloud

Copy link
Member

@dbluhm dbluhm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couple of quick comments but otherwise looks good!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're doing as the Romans do here but I think this logic ought to be captured directly in DID registration/creation endpoints. This "centralized" logic made sense when there was a single endpoint for creating DIDs but, "decentralizing" the logic, as we've talked about doing and as you've done here for did:indy, I don't think it's necessary for this to all be in one place.

Comment on lines +585 to +588
if did_info.method != SOV and did_info.method != INDY:
raise WalletError(
"Setting DID endpoint is only allowed for did:sov or did:indy DIDs"
)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Setting DID Endpoint in this way will get picked up by the legacy resolution support in Indy VDR but doesn't get picked up by the universal resolver, just as an aside. Ideally, we set this by updating the nym with diddocContent rather than with an attrib. We can address that in the future, though, of course.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants