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

feat: change DI VM purpose resolution #3630

Conversation

mishasizov-SK
Copy link
Contributor

@mishasizov-SK mishasizov-SK commented Sep 1, 2023

Using ResolveSigningVMWithRelationship function causes a bug in VM relationship verification for Data integrity.

  1. For instance, let's take issuer's DID, that after resolving has the following structure:
{
  "verificationMethod": [
   ID: "abc",
   .....
  ],
  "authentication": [
   ID: "abc",
   .....
  ]
}
  1. Function docRes.DIDDocument.VerificationMethods() with empty arguments returns a map with 2 elements: key VerificationRelationshipGeneral and Authentication.
    Values for those keys are verification methods with the same ID field.
  2. Because func docRes.DIDDocument.VerificationMethods() returns a map, the order of elements in for ... range loop is not predictable. It might be a case, when first element will be VerificationRelationshipGeneral.
  3. In this case function verificationRelationshipName() returns ""
  4. Then, condition
if rel == "" {
			rel = "assertionMethod"
		}

takes place and declares hardcoded relationship.

@mishasizov-SK mishasizov-SK force-pushed the feat_change_DI_VM_purpose_resolution branch 2 times, most recently from ca15b9f to b23bca7 Compare September 1, 2023 09:08
Signed-off-by: Mykhailo Sizov <mykhailo.sizov@securekey.com>
@mishasizov-SK mishasizov-SK force-pushed the feat_change_DI_VM_purpose_resolution branch from b23bca7 to 82c6456 Compare September 1, 2023 09:15
@codecov
Copy link

codecov bot commented Sep 1, 2023

Codecov Report

Merging #3630 (82c6456) into main (717faac) will decrease coverage by 0.01%.
The diff coverage is 82.19%.

@@            Coverage Diff             @@
##             main    #3630      +/-   ##
==========================================
- Coverage   86.93%   86.92%   -0.01%     
==========================================
  Files         367      367              
  Lines       50046    50098      +52     
==========================================
+ Hits        43507    43549      +42     
- Misses       4954     4961       +7     
- Partials     1585     1588       +3     
Files Changed Coverage Δ
component/models/dataintegrity/signer.go 83.57% <81.15%> (+0.23%) ⬆️
.../models/dataintegrity/suite/ecdsa2019/ecdsa2019.go 93.18% <100.00%> (-0.37%) ⬇️

@fqutishat fqutishat merged commit e17eddd into hyperledger-archives:main Sep 1, 2023
12 of 14 checks passed
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.

None yet

2 participants