Skip to content
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

feat: single signature DID calls/creation #551

Merged
merged 35 commits into from
Sep 18, 2023
Merged

Conversation

weichweich
Copy link
Contributor

@weichweich weichweich commented Aug 21, 2023

fix https://github.com/KILTprotocol/ticket/issues/2849

This PR enables blockchain accounts to manage DIDs on the KILT chain.

Sending DID calls and creating DIDs will be possible with any substrate enabled wallet (polkadotJS, Nova, ...).

Creating a DID will also be possible but still requires two signatures. By supporting wrapped payloads as a special signature type, wallets that support signRaw or similar will be able to create DIDs. With this wrapped type the default way of sending DIDCalls would also work, but still require a second signature.

Metadata Diff to Develop Branch

Peregrine Diff
!!! THE SUBWASM REDUCED DIFFER IS EXPERIMENTAL, DOUBLE CHECK THE RESULTS !!!
[≠] pallet 64: Did -> 5 change(s)
  - calls changes:
    [≠]  1: set_authentication_key ( new_key: DidVerificationKey, )  )
        [Signature(SignatureChange { args: [Changed(0, [Ty(StringChange("DidVerificationKey", "DidVerificationKey<AccountIdOf<T>>"))])] })]
    [≠]  2: set_delegation_key ( new_key: DidVerificationKey, )  )
        [Signature(SignatureChange { args: [Changed(0, [Ty(StringChange("DidVerificationKey", "DidVerificationKey<AccountIdOf<T>>"))])] })]
    [≠]  4: set_attestation_key ( new_key: DidVerificationKey, )  )
        [Signature(SignatureChange { args: [Changed(0, [Ty(StringChange("DidVerificationKey", "DidVerificationKey<AccountIdOf<T>>"))])] })]
    [+] CallDesc { index: 15, name: "dispatch_as", signature: SignatureDesc { args: [ArgDesc { name: "did_identifier", ty: "DidIdentifierOf<T>" }, ArgDesc { name: "call", ty: "Box<DidCallableOf<T>>" }] } }
    [+] CallDesc { index: 16, name: "create_from_account", signature: SignatureDesc { args: [ArgDesc { name: "authentication_key", ty: "DidVerificationKey<AccountIdOf<T>>" }] } }

SUMMARY:
- Compatible.......................: false
- Require transaction_version bump.: false

!!! THE SUBWASM REDUCED DIFFER IS EXPERIMENTAL, DOUBLE CHECK THE RESULTS !!!
Spiritnet Diff
!!! THE SUBWASM REDUCED DIFFER IS EXPERIMENTAL, DOUBLE CHECK THE RESULTS !!!
[≠] pallet 64: Did -> 5 change(s)
  - calls changes:
    [≠]  1: set_authentication_key ( new_key: DidVerificationKey, )  )
        [Signature(SignatureChange { args: [Changed(0, [Ty(StringChange("DidVerificationKey", "DidVerificationKey<AccountIdOf<T>>"))])] })]
    [≠]  2: set_delegation_key ( new_key: DidVerificationKey, )  )
        [Signature(SignatureChange { args: [Changed(0, [Ty(StringChange("DidVerificationKey", "DidVerificationKey<AccountIdOf<T>>"))])] })]
    [≠]  4: set_attestation_key ( new_key: DidVerificationKey, )  )
        [Signature(SignatureChange { args: [Changed(0, [Ty(StringChange("DidVerificationKey", "DidVerificationKey<AccountIdOf<T>>"))])] })]
    [+] CallDesc { index: 15, name: "dispatch_as", signature: SignatureDesc { args: [ArgDesc { name: "did_identifier", ty: "DidIdentifierOf<T>" }, ArgDesc { name: "call", ty: "Box<DidCallableOf<T>>" }] } }
    [+] CallDesc { index: 16, name: "create_from_account", signature: SignatureDesc { args: [ArgDesc { name: "authentication_key", ty: "DidVerificationKey<AccountIdOf<T>>" }] } }

SUMMARY:
- Compatible.......................: false
- Require transaction_version bump.: false

!!! THE SUBWASM REDUCED DIFFER IS EXPERIMENTAL, DOUBLE CHECK THE RESULTS !!!

Checklist:

  • I have verified that the code works
    • No panics! (checked arithmetic ops, no indexing array[3] use get(3), ...)
  • I have verified that the code is easy to understand
    • If not, I have left a well-balanced amount of inline comments
  • I have left the code in a better state
  • I have documented the changes (where applicable)
    • Either PR or Ticket to update the Docs
    • Link the PR/Ticket here

pallets/did/src/lib.rs Outdated Show resolved Hide resolved
pallets/did/src/lib.rs Outdated Show resolved Hide resolved
pallets/did/src/did_details.rs Outdated Show resolved Hide resolved
@@ -79,6 +79,7 @@

#![cfg_attr(not(feature = "std"), no_std)]
#![allow(clippy::unused_unit)]
#![recursion_limit = "256"]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was necessary because of the new benchmarks. We probably have too many benchmarks. 😁

@@ -28,6 +28,33 @@ use crate::{
mock_utils::*,
};

#[test]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i pulled the "successful-case" test to the top

// Measured: `353`
// Estimated: `5777`
// Minimum execution time: 186_836_000 picoseconds.
Weight::from_parts(189_377_000, 0)
Copy link
Contributor Author

@weichweich weichweich Sep 7, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

weights will be calculated on our benchmark server. They were done with debug builds

@weichweich weichweich changed the title feat: do_did_call feat: single signature DID calls/creation Sep 7, 2023
@weichweich weichweich requested a review from ntn-x2 September 7, 2023 10:58
@weichweich weichweich marked this pull request as ready for review September 7, 2023 10:58
@weichweich weichweich requested a review from Ad96el September 7, 2023 10:58
Copy link
Member

@Ad96el Ad96el left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3 minor comments. I like the modular test setup. 🥳

pallets/did/src/lib.rs Outdated Show resolved Hide resolved
pallets/did/src/did_details.rs Outdated Show resolved Hide resolved
pallets/did/src/did_details.rs Show resolved Hide resolved
pallets/did/src/lib.rs Show resolved Hide resolved
pallets/did/src/lib.rs Outdated Show resolved Hide resolved
Copy link
Member

@ntn-x2 ntn-x2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love it! I just don't understand why the widespread use of AsRef<[u8]> for DID keys?

pallets/did/src/lib.rs Show resolved Hide resolved
pallets/did/src/lib.rs Show resolved Hide resolved
pallets/did/src/lib.rs Outdated Show resolved Hide resolved
pallets/did/src/did_details.rs Outdated Show resolved Hide resolved
pallets/did/src/did_details.rs Outdated Show resolved Hide resolved
pallets/did/src/did_details.rs Outdated Show resolved Hide resolved
pallets/did/src/lib.rs Show resolved Hide resolved
pallets/did/src/lib.rs Outdated Show resolved Hide resolved
This enables individual tests for each benchmark.
@weichweich weichweich enabled auto-merge (squash) September 18, 2023 13:31
@weichweich weichweich merged commit e30d652 into develop Sep 18, 2023
@weichweich weichweich deleted the aw-did-origins branch September 18, 2023 13:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants