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

State metadata serialization for the stardust DID method #947

Merged
merged 14 commits into from
Jul 19, 2022

Conversation

PhilippGackstatter
Copy link
Contributor

Description of change

Adds a StateMetadataDocument that represents the serialized DID document in an alias output for the stardust DID method.

The AliasId, which corresponds to the DID, is not computable before the alias output itself has been created. So a placeholder is required in absence of the DID. In order to avoid having to publish twice when creating the alias output, so that the DID document contained in the alias output has the correct DID set, the approach is to always use the placeholder. During resolution a transformation step that takes the block or alias output, and the contained DID document as inputs, produces the actual StardustDocument that contains the correct DID. This means that our solution to this issue is consistent no matter if the alias output existed or is newly created.

Since we'd like to enable smart contracts to build on top of the DID document in the alias output, we might not be able to use an expensive serialization format or even compression. Thus the shortest valid CoreDID is chosen as the placeholder, which is did:0:0. All references to the DID of the document itself are replaced by this DID. When using JSON, a simple string replace suffices to replace this string with the actual DID during the transformation. However, it's likely that we will not end up using JSON, due to determinism considerations and perhaps because other serialization formats offer tighter packing, so the lack of compression doesn't weigh in as much. This PR introduces a dedicated StateMetadataDocument that handles conversion to and from StardustDocument. StateMetadataDocument can eventually be used to implement the serialization format of choice. The trade-off is that the conversion requires quite a few lines of code.

In general, always using a small placeholder DID has significant effects on the size of the document. For example, a StateMetadataDocument with 10 verification methods is about half the size of the corresponding StardustDocument, when converted to a vector of uncompressed json bytes. That in turn saves byte costs.

Links to any relevant issues

part of #942 and #908.

Type of change

Add an x to the boxes that are relevant to your changes.

  • Bug fix (a non-breaking change which fixes an issue)
  • Enhancement (a non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation Fix

How the change has been tested

A transformation roundtrip (StateMetadataDocument <-> StardustDocument) test was added.

Change checklist

Add an x to the boxes that are relevant to your changes.

  • I have followed the contribution guidelines for this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

@PhilippGackstatter PhilippGackstatter added Added A new feature that requires a minor release. Part of "Added" section in changelog Rust Related to the core Rust code. Becomes part of the Rust changelog. labels Jul 16, 2022
@PhilippGackstatter PhilippGackstatter added this to the v0.7 Features milestone Jul 16, 2022
Copy link
Contributor

@cycraig cycraig left a comment

Choose a reason for hiding this comment

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

The code can be simplified but I agree with the overall approach and reasoning (i.e. why string replacement won't work with alternative serializations).

Can we avoid DIDOrPlaceholder somehow or is it necessary for performance?

There are also a few other missing pieces:

  • We still need something similar to DIDMessage with a version flag, and maybe a format flag and magic bytes "DID" prefix.
  • The controllers and all other fields like id that (should be) inferred from the AliasOutput should be removed from the serialization. Maybe. We might need to keep the id field if that's difficult to remove and to keep the network information.

Otherwise looking good, let's keep iterating.

identity_core/src/common/ordered_set.rs Outdated Show resolved Hide resolved
identity_stardust/examples/create_did.rs Outdated Show resolved Hide resolved
identity_stardust/src/did_or_placeholder.rs Outdated Show resolved Hide resolved
identity_stardust/src/did_or_placeholder.rs Outdated Show resolved Hide resolved
identity_stardust/src/stardust_document.rs Show resolved Hide resolved
identity_stardust/src/state_metadata_document.rs Outdated Show resolved Hide resolved
identity_stardust/src/state_metadata_document.rs Outdated Show resolved Hide resolved
identity_stardust/src/state_metadata_document.rs Outdated Show resolved Hide resolved
identity_stardust/src/did_or_placeholder.rs Outdated Show resolved Hide resolved
@PhilippGackstatter
Copy link
Contributor Author

Thanks for the simplifications! Not sure how I ended up making it so unnecessarily complicated.

How would it be possible to remove the controllers from the serialization? Only in the case where the only controller is the id itself, as far as I can see. Is that what you mean? Otherwise we lose information if controller != id.

Agreed, we have to keep the network information in the serialization. Since it's unclear what serialization format we'll end up using, I'd leave this for the PR that eventually closes 942. Implementing removal of id now would require some custom serialization implementation and that may or may not be wasted work.

@cycraig
Copy link
Contributor

cycraig commented Jul 18, 2022

How would it be possible to remove the controllers from the serialization? Only in the case where the only controller is the id itself, as far as I can see. Is that what you mean? Otherwise we lose information if controller != id.

The controller will be the state controller of the AliasOutput, so it's determined by the AliasOutput unlock conditions.

Edit: where the controller is another AliasOutput or Ed25519 address, not sure what to do when it's something else like an NFT yet.

identity_stardust/src/stardust_document.rs Outdated Show resolved Hide resolved
identity_stardust/src/error.rs Outdated Show resolved Hide resolved
identity_stardust/src/state_metadata/document.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@olivereanderson olivereanderson left a comment

Choose a reason for hiding this comment

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

LGTM

@PhilippGackstatter PhilippGackstatter merged commit 7e95e25 into dev Jul 19, 2022
@PhilippGackstatter PhilippGackstatter deleted the feat/state-metadata-serialization branch July 19, 2022 06:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Added A new feature that requires a minor release. Part of "Added" section in changelog Rust Related to the core Rust code. Becomes part of the Rust changelog.
Projects
Development

Successfully merging this pull request may close these issues.

3 participants