feat: use DidOrigin for DID update and deletion #231
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* feat: initial commit * feat: pallet compiling * wip: fixing test cases * wip: refactoring test cases * test: unit tests passing again * benchmark tests passing * test: attestation unit tests passing * chore: clippy fixes * chore: fmt fixes * bench: re-execute benchmarks for delegation and attestation + temporarily remove benchmarks from peregrine runtime * fix: adjust pre-condition for an attestation unit test * wip: add initial support for delegation storage migrations * delegation storage migrator compiling * wip: storage migrator working, now writing unit tests for it * test: unit tests passing * chore: clippy hints * chore: further clippy fixes * chore: fmt * wip: adjusting last things before possible to review * chore: clippy + fmt * fix: address part of PR review comments * fix: more comments from PR review * wip: change vector of versions to enum * test: unit tests refactor for the new storage migrator version * chore: refactor * test: unit tests passing again * chore: move deprecated type to deprecated.rs * chore: refactor import/export * doc: refine documentation * chore: fmt + clippy * test: add TODO to improve post_migration tests * feat: remove test feature from traits * test: add checks during migration in try-runtime * bench: add spiritnet benchmarks for delegation and attestation * feat: additional logs for count of migrated nodes * chore: fmt * chore: removed useless file * chore: bump runtime version * fix: PR comments * fix: re-introduce root_id in delegation creation hash * chore: capitalize storage version enum * chore: move migrations in their own module * chore: move migrations.rs out of the migrations folder * fix: add check for parent revocation status when creating a delegation * chore: fmt * feat: remove revoke_hierarchy extrinsic * bench: add peregrine benchmarks * bench: re-run delegation and attestation default benchmarks * chore: fmt
ntn-x2
requested review from
weichweich and
wischli
and removed request for
wischli
wischli
approved these changes
Aug 4, 2021
Contributor
wischli
left a comment
wischli
left a comment
There was a problem hiding this comment.
LGTM! Just found a minor typo and I have a question.
pallets/did/src/lib.rs
Outdated
| //! - `create` - Register a new DID on the KILT blockchain under the given DID | ||
| //! identifier. | ||
| //! - `update` - Update any keys or the endpoint URL of an existing DID. | ||
| //! - `dekete` - Delete the specified DID and all related keys from the KILT |
Contributor
There was a problem hiding this comment.
Suggested change
| //! - `dekete` - Delete the specified DID and all related keys from the KILT | |
| //! - `delete` - Delete the specified DID and all related keys from the KILT |
| Call::Attestation(_) => Some(did::DidVerificationKeyRelationship::AssertionMethod), | ||
| Call::Ctype(_) => Some(did::DidVerificationKeyRelationship::AssertionMethod), | ||
| Call::Delegation(_) => Some(did::DidVerificationKeyRelationship::CapabilityDelegation), | ||
| // DID creation is not allowed through the DID proxy. |
Contributor
There was a problem hiding this comment.
Just wondering what can happen if we allowed this?
Contributor
Author
There was a problem hiding this comment.
The verification prior to dispatch would always fail with a DidNotPresent error as there's no DID in the storage. I think it would be most clear to fail with a BadOrigin error, as is the case now, rather than with a DidNotPresent. What do you think?
weichweich
suggested changes
Aug 4, 2021
Contributor
weichweich
left a comment
weichweich
left a comment
There was a problem hiding this comment.
few minor things that i would like to talk about.
Do we need the generic origin? Can we not require a did origin?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fixes KILTProtocol/ticket#1490
Please include a summary of the changes provided with this pull request and which issue has been fixed.
Please also provide some context if necessary.
Checklist:
array[3]useget(3), ...)