#3967: get credential status from local store for issued VCs (did:web) #3969
Conversation
reinkrul
requested review from
confiks,
gerardsn,
stevenvegt and
woutslakhorst
as code owners
2 new issues
|
|
Coverage Impact ⬇️ Merging this pull request will decrease total coverage on Modified Files with Diff Coverage (3)
🤖 Increase coverage with AI coding...🚦 See full report on Qlty Cloud » 🛟 Help
|
| StatusList(ctx context.Context, issuer did.DID, page int) (*vc.VerifiableCredential, error) | ||
| // GetRevocation returns a revocation for a credential ID. | ||
| // Returns a types.ErrNotFound when the revocation is not in the store | ||
| // Returns a types.ErrMultipleFound when there are multiple revocations for this credential ID in the store |
There was a problem hiding this comment.
Not sure why this could be possible, but even if there are more, can we not just return the most recent? It is still revoked. If there are more, this specific credential will be broken since there probably is no way of fixing this error.
There was a problem hiding this comment.
I did it this way to have it consistent with the did:nuts storage, but I think the same way. revoked=revoked, doesn't really matter if it's revoked multiple times.
I'll change the implementations.
| }) | ||
| } | ||
|
|
||
| func Test_issuer_GetRevocation(t *testing.T) { |
There was a problem hiding this comment.
can you test the multiple revocations here as well?
| Select("status_list_credential.status_purpose, status_list_entry.created_at"). | ||
| Joins("JOIN status_list_credential ON status_list_entry.status_list_credential = status_list_credential.subject_id"). | ||
| Where("status_list_entry.credential_id = ?", credentialID.String()). | ||
| First(&result). |
There was a problem hiding this comment.
can we somehow create a unique index on the table so that is would be impossible to have multiple revocations per VC?
stevenvegt
left a comment
stevenvegt
left a comment
There was a problem hiding this comment.
I'm not sure how the ErrMultipleFound can be caused and fixed. But I prefer a setup in which this error should not be possible.
Fixes #3967