-
Notifications
You must be signed in to change notification settings - Fork 3
feat(#577): add draft implementation for listing certs. #658
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
base: main
Are you sure you want to change the base?
Conversation
Manual Testsℹ️ Remember to ask team members to perform manual tests and to assign |
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR implements draft functionality for listing and acquiring identity certificates in a wallet system. The implementation introduces data structures and conversion methods to support certificate management, including direct certificate acquisition and certificate listing with filtering capabilities.
Key Changes
- Added certificate acquisition logic with support for direct protocol
- Implemented certificate listing with filtering by certifiers and types
- Created type-safe wrapper types for certificate fields, keyrings, and verifiers
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
File | Description |
---|---|
pkg/wdk/table_certificate_field.go | Added helper function to convert certificate field maps to database models |
pkg/wdk/certificate.go | Introduced type aliases and conversion methods for certificate-related data structures |
pkg/wallet/wallet_list_certificates_test.go | Added integration test for certificate listing functionality |
pkg/wallet/wallet_acquire_certificate_test.go | Added integration test for certificate acquisition |
pkg/wallet/wallet.go | Implemented AcquireCertificate and ListCertificates methods |
pkg/wallet/internal/testabilities/certificates.go | Created test helper functions for certificate-related test data |
pkg/storage/mappings.go | Updated mapping to use new VerifierString type |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
func certModelToResult(model *entity.Certificate) *wdk.CertificateResult { | ||
return &wdk.CertificateResult{ | ||
Verifier: model.Verifier, | ||
Verifier: wdk.VerifierString(model.Verifier), | ||
Keyring: certificateModelFieldsToKeyringResult(model.CertificateFields), |
Copilot
AI
Oct 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The return type of certificateModelFieldsToKeyringResult
should be updated to wdk.KeyringMap
to match the new type definition, ensuring type consistency across the codebase.
Copilot uses AI. Check for mistakes.
// parseSignature converts a HexString into an EC signature. | ||
// The input `s` is expected to be a concatenation of the R and S values in hex format (64 characters each). | ||
// Returns a pointer to an ec.Signature containing the parsed R and S values. | ||
func parseSignature(s primitives.HexString) *ec.Signature { |
Copilot
AI
Oct 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing bounds check before slicing. If s
has length less than 128 characters, this will panic with an index out of range error.
func parseSignature(s primitives.HexString) *ec.Signature { | |
func parseSignature(s primitives.HexString) *ec.Signature { | |
if len(s) < 128 { | |
// Input is too short to contain both R and S values (64 hex chars each) | |
return nil | |
} |
Copilot uses AI. Check for mistakes.
rHex := fmt.Sprintf("%064x", args.Signature.R) | ||
sHex := fmt.Sprintf("%064x", args.Signature.S) |
Copilot
AI
Oct 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The format string %064x
expects an integer type, but args.Signature.R
and args.Signature.S
are *big.Int
. This will not format correctly. Use args.Signature.R.Text(16)
with padding or fmt.Sprintf(\"%064s\", args.Signature.R.Text(16))
instead.
rHex := fmt.Sprintf("%064x", args.Signature.R) | |
sHex := fmt.Sprintf("%064x", args.Signature.S) | |
rHex := fmt.Sprintf("%064s", args.Signature.R.Text(16)) | |
sHex := fmt.Sprintf("%064s", args.Signature.S.Text(16)) |
Copilot uses AI. Check for mistakes.
Description of Changes
Provide a brief description of the changes you've made.
Linked Issues / Tickets
Reference any related issues or tickets, e.g. "Closes #123".
Testing Procedure
Describe the tests you've added or any testing steps you've taken.
Checklist: