Skip to content
This repository has been archived by the owner on Mar 27, 2024. It is now read-only.

feat: BbsBlsSignatureProof2020 LDP suite for VC #2384

Merged
merged 1 commit into from
Dec 8, 2020

Conversation

kdimak
Copy link
Contributor

@kdimak kdimak commented Dec 7, 2020

Signed-off-by: Dmitriy Kinoshenko [email protected]

#2295

Copy link
Contributor

@llorllale llorllale left a comment

Choose a reason for hiding this comment

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

Thread-safety concerns due to the forced use of the CachingDocumentLoader


procOptions := prepareOpts(opts)

useDocumentLoader(options, procOptions.documentLoader, procOptions.documentLoaderCache)
Copy link
Contributor

Choose a reason for hiding this comment

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

@kdimak the CachingDocumentLoader is not thread-safe (see comments I left in #1833)

Copy link
Contributor Author

@kdimak kdimak Dec 8, 2020

Choose a reason for hiding this comment

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

@llorllale I absolutely agree that's not safe. But let's fix this all together for #1833

@codecov
Copy link

codecov bot commented Dec 7, 2020

Codecov Report

Merging #2384 (8301ac0) into master (f648795) will decrease coverage by 0.20%.
The diff coverage is 72.05%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2384      +/-   ##
==========================================
- Coverage   89.47%   89.26%   -0.21%     
==========================================
  Files         231      235       +4     
  Lines       16262    16459     +197     
==========================================
+ Hits        14550    14692     +142     
- Misses       1005     1042      +37     
- Partials      707      725      +18     
Impacted Files Coverage Δ
pkg/doc/signature/proof/data.go 90.24% <ø> (+4.87%) ⬆️
pkg/doc/verifiable/embedded_proof.go 90.00% <12.50%> (-10.00%) ⬇️
pkg/doc/verifiable/credential_bbs.go 51.72% <51.72%> (ø)
...signature/suite/bbsblssignatureproof2020/signer.go 75.19% <75.19%> (ø)
pkg/doc/signature/jsonld/processor.go 81.75% <76.47%> (-0.75%) ⬇️
pkg/doc/signature/proof/proof.go 91.76% <100.00%> (ø)
...te/bbsblssignatureproof2020/public_key_verifier.go 100.00% <100.00%> (ø)
.../signature/suite/bbsblssignatureproof2020/suite.go 100.00% <100.00%> (ø)
pkg/doc/signature/verifier/public_key_verifier.go 94.40% <100.00%> (+0.14%) ⬆️
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f648795...8301ac0. Read the comment docs.

@llorllale llorllale dismissed their stale review December 8, 2020 00:48

@dima as agreed to fix the CachingDocumentLoader thread-safety issue in a separate PR

@fqutishat fqutishat merged commit b1d78bb into hyperledger-archives:master Dec 8, 2020
@kdimak kdimak deleted the issue-2295-ldp branch December 8, 2020 00:58
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants