-
Notifications
You must be signed in to change notification settings - Fork 15
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
Support the DID DHT API #76
base: main
Are you sure you want to change the base?
Conversation
} | ||
} | ||
|
||
if err = s.db.WriteDID(storage.GatewayRecord{ |
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.
Is there a possible race condition when publishing the same did? Perhaps some idempotency key is necessary.
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.
yeah I'm also worried about multiple DIDs updating the type index at the same time...
} | ||
|
||
var ( | ||
knownTypes = []TypeMapping{ |
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.
Side question: how are types registered? Is it a centralized registry?
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.
} | ||
|
||
// WriteDID writes a DID to the storage and adds it to the type index(es) it is associated with | ||
func (s *Storage) WriteDID(record GatewayRecord) error { |
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.
How do you intend to handle failures in the middle? For example, the write succeeds, but the update does not.
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.
ideally this should be a txn with rollbacks
for _, currType := range currTypes { | ||
if _, ok := newTypeMap[currType]; !ok { | ||
if err := s.RemoveDIDFromTypeIndex(id, currType); err != nil { | ||
return err | ||
} | ||
} | ||
} | ||
|
||
// add the DID to any type indexes it is now associated with | ||
for _, newType := range newTypes { | ||
if _, ok := currTypeMap[newType]; !ok { | ||
if err := s.AddDIDToTypeIndex(id, newType); err != nil { | ||
return err | ||
} | ||
} | ||
} |
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.
Removing and then adding add steps which may fail in the middle.
Instead, consider replacing the types with the new value. This might be more easily supported by the storage layer.
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.
good idea will update
Fix #43 #13 #24 #72