feat: BbsBlsSignature2020 LDP suite for VC #2261
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2261 +/- ##
==========================================
+ Coverage 89.86% 89.90% +0.04%
==========================================
Files 225 226 +1
Lines 15090 15103 +13
==========================================
+ Hits 13560 13579 +19
+ Misses 909 903 -6
Partials 621 621
Continue to review full report at Codecov.
|
troyronda
requested review from
sandrask,
fqutishat,
aholovko,
baha-ai,
rolsonquadras,
soluchok,
mikelodder7 and
pfeairheller
| github.com/kilic/bls12-381 => github.com/trustbloc/bls12-381 v0.0.0-20201008080608-ba2e87ef05ef | ||
| github.com/phoreproject/bls => github.com/trustbloc/bls v0.0.0-20201008085849-81064514c3cc |
There was a problem hiding this comment.
How come you're replacing these instead of just directly using those specific commits? Are there multiple versions of this dependency being used in different areas?
There was a problem hiding this comment.
I'm not sure if this answers the question, but we are needing to replace modules due to forking those projects. There is an open issue to remove dependencies on forks.
There was a problem hiding this comment.
Is there a reason why we're using replace instead of just directly importing from github.com/trustbloc/bls12-381?
There was a problem hiding this comment.
No, it's like an indication that this replacement is temporary and should be dropped in the future.
| // BBSG2SignatureVerifier is a signature verifier that verifies a BBS+ Signature | ||
| // taking Bls12381G2Key2020 public key bytes as input. | ||
| // The reference implementation https://github.com/mattrglobal/bls12381-key-pair supports public key bytes only, | ||
| // JWK is not supported. JWK is not supported as well here. |
There was a problem hiding this comment.
Should be JWK is not supported here as well instead of JWK is not supported as well here
JWK is not supported as well here actually accidentally implies that JWK is supported, just to a lesser degree than something else.
|
@kdimak |
cmd/aries-agent-rest/go.mod
Outdated
| replace ( | ||
| github.com/kilic/bls12-381 => github.com/trustbloc/bls12-381 v0.0.0-20201008080608-ba2e87ef05ef | ||
| github.com/phoreproject/bls => github.com/trustbloc/bls v0.0.0-20201008085849-81064514c3cc | ||
| ) |
There was a problem hiding this comment.
We can combine these replaces with the one in line 7.
| } | ||
| } | ||
| }, | ||
| "BbsBlsSignatureProof2020": { |
There was a problem hiding this comment.
Are we using BbsBlsSignatureProof2020 json-ld type ? Or is it just here as we copied the context file entirely from the source ?
There was a problem hiding this comment.
Yes, we start to support it. But in the tests, we do not want to rely on the network availability of the contexts, and that's why we cache BbsBlsSignatureProof2020 context definition (referenced by https://w3c-ccg.github.io/ldp-bbs2020/context/v1). If you check pkg/doc/verifiable/testdata/context/, you can see much more cached contexts there... The contexts caching also speed ups the tests - I recall there was a speedup of verifiable tests run from about 2 minutes to several seconds after we started to use the context cache in the tests.
There was a problem hiding this comment.
perhaps the name of this file should be prefixed (or suffixed) with cache to indicate this is caching for testdata
closes hyperledger-archives#2221 Signed-off-by: Dmitriy Kinoshenko <dkinoshenko@gmail.com>
yeah, I had to write two extra tests for this :) |
closes #2221
Signed-off-by: Dmitriy Kinoshenko dkinoshenko@gmail.com