Uh oh!
There was an error while loading. Please reload this page.
This repository was archived by the owner on Aug 10, 2026. It is now read-only.
[EDX-156] Replace the undefined Params type in IDL - #1464
Merged
tbedford merged 2 commits intoJun 20, 2022
Conversation
lawrence-forooghianforce-pushed
the
EDX-156-clarify-getDefaultParams-arguments-in-IDL
branch
from
June 15, 2022 17:08
786deee to
4d88decComparelawrence-forooghianforce-pushed
the
EDX-156-clarify-getDefaultParams-arguments-in-IDL
branch
from
June 15, 2022 17:10
4d88dec to
2f17ea9Comparelawrence-forooghian
marked this pull request as ready for review
June 15, 2022 17:12
lawrence-forooghian
requested review from
Morganamilo, Peter-Maguire, QuintinWillison, Rosalita, ikbalkaya, maratal, mohyour, owenpearson, stmoreau and tomkirbygreenJune 15, 2022 17:12
QuintinWillison
suggested changes
Jun 17, 2022
QuintinWillison
left a comment
Contributor
There was a problem hiding this comment.
It would be nice to see the CO2 sub-points have some explanation and probably example values (e.g. AES, CBC and 128-bit; being typical).
Uh oh!
There was an error while loading. Please reload this page.
I’ve taken the opportunity to turn this into a concrete CipherParamOptions type in the spec, because the current language of “any subset of @CipherParams@ fields…” is not correct since the types of the `key` value do not agree — in CipherParams it can only be a binary, but in CipherParamsOptions it can be a binary or a string. I’ve taken the type name from ably-js@48a66d6. The descriptions of the properties come from the corresponding descriptions of CipherParams in TZ2.
lawrence-forooghianforce-pushed
the
EDX-156-clarify-getDefaultParams-arguments-in-IDL
branch
from
June 19, 2022 18:48
2f17ea9 to
ae3a2f0Comparelawrence-forooghian
commented
Jun 19, 2022
ContributorAuthor
Done. I’ve adapted the descriptions from the existing ones for |
QuintinWillison
approved these changes
Jun 20, 2022
Peter-Maguire
approved these changes
Jun 20, 2022
7 tasks
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
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.
6 participants
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.
Description
This addresses the
Paramstype that’s mentioned in the IDL as the value forChannelOptions.cipherorCrypto.getDefaultParams()’s argument. This type is not actually defined anywhere.I’ve taken the opportunity to turn this into a concrete
CipherParamOptionstype in the spec, because the current language of “any subset of @CipherParams@ fields…” is not correct since the types of thekeyvalue do not agree — inCipherParamsit can only be a binary, but inCipherParamsOptionsit can be a binary or a string.I’ve taken the type name from ably/ably-js@48a66d6.