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 vcdm 2.0 model and context #3436

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

Conversation

PatStLouis
Copy link
Contributor

Needs issuanceDate validation for vcdm 1.0 (add a current timestamp if absent)
Needs hooking up with didcomm

@PatStLouis PatStLouis requested review from dbluhm and jamshale January 10, 2025 19:43
Copy link
Contributor

@ff137 ff137 left a comment

Choose a reason for hiding this comment

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

Just some early review comments

acapy_agent/vc/vc_ld/models/credential.py Outdated Show resolved Hide resolved
acapy_agent/messaging/valid.py Outdated Show resolved Hide resolved
acapy_agent/messaging/valid.py Outdated Show resolved Hide resolved
acapy_agent/vc/vc_ld/manager.py Outdated Show resolved Hide resolved
@dbluhm
Copy link
Contributor

dbluhm commented Jan 14, 2025

@PatStLouis I think it might be wise to get this "basic" support merged and then worry about integration with DIDComm ICv2 and PPv2 in a future PR.

@PatStLouis
Copy link
Contributor Author

@dbluhm I agree, I want at the minimum for the models not to break, since the didcomm protocols use the vc models

PatStLouis and others added 20 commits January 20, 2025 19:33
…s-cloudagent-python into vcdm-2.0-data-model-support
Signed-off-by: PatStLouis <[email protected]>
Signed-off-by: PatStLouis <[email protected]>
Signed-off-by: PatStLouis <[email protected]>
Signed-off-by: PatStLouis <[email protected]>
Signed-off-by: PatStLouis <[email protected]>
…s-cloudagent-python into vcdm-2.0-data-model-support
Signed-off-by: PatStLouis <[email protected]>
Signed-off-by: PatStLouis <[email protected]>
Signed-off-by: PatStLouis <[email protected]>
Signed-off-by: PatStLouis <[email protected]>
Signed-off-by: PatStLouis <[email protected]>
@PatStLouis PatStLouis marked this pull request as ready for review January 27, 2025 21:55
@PatStLouis
Copy link
Contributor Author

@dbluhm ready for review

Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
5 Security Hotspots
63.2% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

@PatStLouis PatStLouis requested a review from ff137 January 27, 2025 22:58
Copy link
Contributor

@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.

Quick comment but otherwise looks good

holder = presentation["holder"]
did = holder if isinstance(holder, str) else holder["id"]
async with context.session() as session:
wallet: BaseWallet | None = session.inject_or(BaseWallet)
info = await wallet.get_local_did(did)
key_type = info.key_type.key_type

if key_type == "ed25519":
options["proofType"] = "Ed25519Signature2020"
elif key_type == "bls12381g2":
options["proofType"] = "BbsBlsSignature2020"
elif key_type == "p256":
options["proofType"] = "EcdsaSecp256r1Signature2019"
options["proofType"] = "Ed25519Signature2020"
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see why we removed this code; automatically determining the proof type by the key associated with the DID seems like a good idea.

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.

3 participants