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

Statuslist: Merge issuer and verifier #2851

Merged
merged 7 commits into from
Mar 6, 2024

Conversation

gerardsn
Copy link
Member

@gerardsn gerardsn commented Mar 1, 2024

closes #2850

This PR

  • merges all issuer and verifier logic in 1 object
  • rewrite verifiers
  • adds storage to the verifier
  • moves issuing of StatusList2021Credential from the issuer to the joint package
  • new db table

Files will be renamed to the following:
store.go -> issuer.go
credential_status.go -> verifier.go
Will have to wait until after this PR since Github saw the name changes as deleting/adding files.

Copy link
Member

@woutslakhorst woutslakhorst left a comment

Choose a reason for hiding this comment

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

credential != credential in comments. Propose to use StatusList2021Credential where applicable.

some minor things to fix in godoc.

Optionally: I find it a bit hard to read, what do you think about:

  • naming the tables: status_list, status_list_credential and status_list_entry (or _revocation)?
  • name the package revocation and the service struct StatusList2021?

vcr/statuslist2021/types.go Outdated Show resolved Hide resolved
vcr/statuslist2021/types.go Outdated Show resolved Hide resolved
@gerardsn
Copy link
Member Author

gerardsn commented Mar 5, 2024

I've added comments, and replaced credential with StatusList2021Credential where applicable.

Tables are renamed as proposed. (I think, status_list_credential and status_list are hard to keep apart)

I like revocation.StatusList2021 more than statuslist2021.CredentialStatus, but renaming the package also requires prefixing many of the exposed variables with StatusList2021. see ca7a80b. I'm not sure renaming the package is an improvement overall, but it would allow adding BitstringStatusList here. What do you think? (A better name for CredentialStatus would be good.)

Yay or nay or removing the interfaces? They are not used...

create table status_list_credential
(
-- id: VC.credentialSubject.ID; URL where this status list credential can be downloaded.
subject_id varchar(500) not null primary key,
-- the credential served at this URL may change over time, so we only keep the latest
Copy link
Member

Choose a reason for hiding this comment

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

incorrect docs

@gerardsn gerardsn merged commit 480867e into master Mar 6, 2024
8 of 9 checks passed
@gerardsn gerardsn deleted the merge-statuslist2021-issuer-and-verifier branch March 6, 2024 14:21
rolandgroen added a commit that referenced this pull request Mar 8, 2024
* master: (40 commits)
  Bump github.com/go-jose/go-jose/v3 from 3.0.1 to 3.0.3 (#2889)
  StatusList2021: add e2e test (#2881)
  remove default storage backend (#2885)
  Bump github.com/lestrrat-go/jwx/v2 from 2.0.20 to 2.0.21 (#2887)
  SQL: Rename table vdr_didweb to did (#2882)
  VDR: Replace v2 API panics with errors (#2872)
  Network: don't enable TLS when not configured (#2877)
  Statuslist: Merge issuer and verifier (#2851)
  revert go-version to stable (#2879)
  set column length for did and id (#2878)
  Bump golang from 1.22.0-alpine to 1.22.1-alpine (#2876)
  Bump google.golang.org/protobuf from 1.32.0 to 1.33.0 (#2875)
  Docker: drop 'v' prefix from versions (#2855)
  Upgrade to go 1.22 (#2862)
  Bump google.golang.org/grpc from 1.62.0 to 1.62.1 (#2874)
  Bump golang.org/x/crypto from 0.20.0 to 0.21.0 (#2859)
  HTTP: correct status code logging for errors (#2848)
  IAM: Handle ErrNotFound for unknown tokens when introspecting (#2847)
  Bump github.com/stretchr/testify from 1.8.4 to 1.9.0 (#2849)
  allow for empty VPs (#2840)
  ...
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.

Statuslist2021: add joint storage for verifier/issuer
3 participants