Keep the naming of OneBlock consistent

[zhouhuitian]

As topic.

This change may also be a breakthrough for vc-sdk.

[linear]

P-176 Naming inconsistency in oneblock assertion type

[kziemianek]

I think it breaks CLI and ts api package. - maybe we should have more detailed labels to signal which components have breaking changes.

error: unrecognized subcommand 'oneblock'

  note: subcommand 'one-block' exists
  note: to pass 'oneblock' as a value, use 'litentry-cli trusted request-vc-direct -- oneblock'

While we have such documentation for requestin OneBlock vc:

// oneblock VC:
// ./bin/litentry-cli trusted -d request-vc-direct \
//   did:litentry:substrate:0x8eaf04151687736326c9fea17e25fc5287613693c912909cb226aa4794f26a48 oneblock completion

Please update example usage.

[Kailai-Wang]

IMO this is a good example to show the advantage of using enum encoding to identify the assertion type from client side - related: #2364 (comment)

We changed Oneblock to OneBlock - but VCs were issued with old names already. I'm not sure if we are using case-sensitive string comparison, but either way, what if we want to change it to TwoBlock? We still want to be able to tell it's the same VC as the old one - or in another way, VCs with old names are still valid and can be mapped to TwoBlock.

If we had used enum encoding, then we would:

1. get the same encoding as long as it's not repositioned (if it's meant to be two different VCs, we should change the index)
2. in case we want to check it further - be "enforced" to call the correct API when decoding it, e.g. isOneblock vs isOneBlock or asOneBlock vs asOneblock => it will make us aware when compiling SDKs already and think about the handling.

cc @jonalvarezz @grumpygreenguy

[kziemianek]

@zhouhuitian it seems it still doesn't work :(

kziemianek@kziemianek:~/projects/litentry/litentry-parachain/tee-worker/bin$ ./litentry-cli trusted -d request-vc-direct did:litentry:substrate:0x8eaf04151687736326c9fea17e25fc5287613693c912909cb226aa4794f26a48 OneBlock completion
error: unrecognized subcommand 'OneBlock'

  note: subcommand 'one-block' exists
  note: to pass 'OneBlock' as a value, use 'litentry-cli trusted request-vc-direct -- OneBlock'

Usage: litentry-cli trusted request-vc-direct <DID> <COMMAND>

For more information, try '--help'.

It expects one-block.

[jonalvarezz]

This may be answered by Kai's reply but here goes my feedback:

Can we keep Oneblock working as well? i.e., if clients sends Oneblock we internally map it to OneBlock.

I'm aiming to create layers that reduce the number of breaking changes we ship.

And in any case, could you please create the additional (sub)ticket to properly update vc-sdk too?

[jonalvarezz]

We changed Oneblock to OneBlock - but VCs were issued with old names already. I'm not sure if we are using case-sensitive string comparison, but either way, what if we want to change it to TwoBlock? We still want to be able to tell it's the same VC as the old one - or in another way, VCs with old names are still valid and can be mapped to TwoBlock.

I get your point, but issued VCs don't include the assertion name. That improvement is coming through the new unreleased parachain version.

But it could be an issue for the on-chain events which in fact (used to) hold the Assertion struct, right?

[zhouhuitian]

This may be answered by Kai's reply but here goes my feedback:

Can we keep Oneblock working as well? i.e., if clients sends Oneblock we internally map it to OneBlock.

I'm aiming to create layers that reduce the number of breaking changes we ship.

And in any case, could you please create the additional (sub)ticket to properly update vc-sdk too?

If we still keep Oneblock, then this PR does not need to exist I don't think it's possible to do internal mapping for this.

and What issue do I need to create? I still don't quite understand.

any ideas? @BillyWooo @Kailai-Wang

[jonalvarezz]

This may be answered by Kai's reply but here goes my feedback:
Can we keep Oneblock working as well? i.e., if clients sends Oneblock we internally map it to OneBlock.
I'm aiming to create layers that reduce the number of breaking changes we ship.
And in any case, could you please create the additional (sub)ticket to properly update vc-sdk too?

If we still keep Oneblock, then this PR does not need to exist I don't think it's possible to do internal mapping for this.

and What issue do I need to create? I still don't quite understand.

any ideas? @BillyWooo @Kailai-Wang

If doing it backwards compatible is not feasible, then feel free to proceed and merge.

About the follow-up vc-sdk task, I just created it: https://linear.app/litentry/issue/P-506/client-sdk-update-oneblock-assertion-to-support-oneblock – this is for us not to forget to update the client-sdk before releasing this change.

[BillyWooo]

As discussed, we will merge this PR now. @jonalvarezz Please update sdk accordingly. Thank you.