feat(core): ledger module registerPublicDid implementation #398
Conversation
Signed-off-by: Ariel Gentile <gentilester@gmail.com>
Codecov Report
@@ Coverage Diff @@
## main #398 +/- ##
==========================================
+ Coverage 86.52% 86.53% +0.01%
==========================================
Files 246 246
Lines 5010 5022 +12
Branches 788 791 +3
==========================================
+ Hits 4335 4346 +11
- Misses 675 676 +1
Continue to review full report at Codecov.
|
Signed-off-by: Ariel Gentile <gentilester@gmail.com>
Signed-off-by: Ariel Gentile <gentilester@gmail.com>
| response, | ||
| }) | ||
|
|
||
| return targetDid |
There was a problem hiding this comment.
One final nitpick: Maybe we shouldn't return the targetDid? It's already an input parameter, so it doesn't provide extra context to the method caller to return it. WDYT?
There was a problem hiding this comment.
Yes, you are right about that. Actually I left it for consistency with other methods from ledger service such as registerSchema and registerCredentialDefinition. Do you think it'd better to return void?
In that way, in the registerPublicDid test I can do a getPublicDid right after the DID registration (something I don't want to do because that that method is tested in a separate test, but probably should be the unique 'good' way to test it).
There was a problem hiding this comment.
I think it is fine for now. We can tackle this in a separate PR and overhaul all methods
Add missing implementation for registerPublicDid in ledger module.
Apart from ledger source files, there were some updates in ledger setup scripts in order to properly run the tests, as it is needed to have some permissions to write public DIDs.
Signed-off-by: Ariel Gentile gentilester@gmail.com