Skip to content
This repository was archived by the owner on Oct 10, 2022. It is now read-only.

feat: support bounded structs #14

Merged
wischli merged 6 commits intomasterfrom
wf-bounded-vec
Aug 12, 2021
Merged

feat: support bounded structs #14
wischli merged 6 commits intomasterfrom
wf-bounded-vec

Conversation

@wischli
Copy link
Contributor

@wischli wischli commented Aug 11, 2021

Mashnet node companion

  • Fixes build issue of feat: add types19 #13 which did not catch my attention (see CI)
  • Bumps TS and Polkadot types versions

@wischli wischli mentioned this pull request Aug 11, 2021
4 tasks
@rflechtner
Copy link
Contributor

I may be wrong, but I'd expect this to result in a type for the DidUpdate where you cannot actually set any keys or services when selecting Change, but instead you are faced with a value-less enum.
What should definitely work is the following, but that's not pretty:

  // We need to specify the two variants of the generic DidFragmentUpdateAction separately
  DidFragmentUpdateAction_DidVerificationKey: {
    _enum: {
      Ignore: "Null",
      Change: "DidVerificationKey",
      Delete: "Null",
    },
  },
  DidFragmentUpdateAction_ServiceEndPoints: {
    _enum: {
      Ignore: "Null",
      Change: "ServiceEndPoints",
      Delete: "Null",
    },
  },

  ...
 
  DidUpdateDetails: {
    newAuthenticationKey: "Option<DidVerificationKey>",
    newKeyAgreementKeys: "BTreeSet<DidEncryptionKey>",
    attestationKeyUpdate: "DidFragmentUpdateAction_DidVerificationKey",
    delegationKeyUpdate: "DidFragmentUpdateAction_DidVerificationKey",
    publicKeysToRemove: "BTreeSet<KeyIdOf>",
    serviceEndpointsUpdate: "DidFragmentUpdateAction_ServiceEndpoints",
  },

@wischli
Copy link
Contributor Author

wischli commented Aug 11, 2021

I may be wrong, but I'd expect this to result in a type for the DidUpdate where you cannot actually set any keys or services when selecting Change, but instead you are faced with a value-less enum.

I suppose you are right.

Screenshot 2021-08-11 at 12 07 17

@rflechtner
Copy link
Contributor

This is what we would want:
image

@wischli
Copy link
Contributor Author

wischli commented Aug 11, 2021

I may be wrong, but I'd expect this to result in a type for the DidUpdate where you cannot actually set any keys or services when selecting Change, but instead you are faced with a value-less enum.
What should definitely work is the following, but that's not pretty:

  // We need to specify the two variants of the generic DidFragmentUpdateAction separately
  DidFragmentUpdateAction_DidVerificationKey: {
    _enum: {
      Ignore: "Null",
      Change: "DidVerificationKey",
      Delete: "Null",
    },
  },
  DidFragmentUpdateAction_ServiceEndPoints: {
    _enum: {
      Ignore: "Null",
      Change: "ServiceEndPoints",
      Delete: "Null",
    },
  },

  ...
 
  DidUpdateDetails: {
    newAuthenticationKey: "Option<DidVerificationKey>",
    newKeyAgreementKeys: "BTreeSet<DidEncryptionKey>",
    attestationKeyUpdate: "DidFragmentUpdateAction_DidVerificationKey",
    delegationKeyUpdate: "DidFragmentUpdateAction_DidVerificationKey",
    publicKeysToRemove: "BTreeSet<KeyIdOf>",
    serviceEndpointsUpdate: "DidFragmentUpdateAction_ServiceEndpoints",
  },

I tried to find a solution with generics but nothing worked. I pushed your suggestion for now.

Could you do a review @rflechtner? I would like to do a runtime upgrade on Peregrine Stg and Prod tomorrow but need the types to be published before.

@wischli wischli requested a review from rflechtner August 11, 2021 16:03
Copy link
Contributor

@rflechtner rflechtner left a comment

Choose a reason for hiding this comment

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

There is a typo, and I noticed we are missing a type for DelegationStorageVersion.

src/index.ts Outdated
contentType: "ContentType",
},
DidFragmentUpdateAction: {
DidFragmentUpdateAction_ServicEndpoints: {
Copy link
Contributor

Choose a reason for hiding this comment

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

TYPO ALAAARM 🚨⏰🚨🔔🚨 😱

Copy link
Contributor Author

@wischli wischli Aug 12, 2021

Choose a reason for hiding this comment

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

Good catch. StakingStorageVersion was also missing 🥲 Fixed in fa98fac.

@wischli wischli requested a review from rflechtner August 12, 2021 11:34
@wischli wischli merged commit fd4564b into master Aug 12, 2021
@wischli wischli deleted the wf-bounded-vec branch August 12, 2021 13:40
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants