-
Notifications
You must be signed in to change notification settings - Fork 41
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
Use script.LocalValue
for arguments and this
#477
Conversation
e2cac52
to
7fc65e4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving, but please wait for review from Mozilla just to double check it's correct.
7fc65e4
to
ada8256
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is OK. The main change seems to be that we allow clients to send maps with channels or remote values as arguments.
The description of script.LocalValue
needs to be updated to match the change.
I think adding all these types to local-cddl
is wrong; I don't think clients ever have to parse these values.
ada8256
to
6eda744
Compare
Sure, but CDDL complains about unknown types if you don't include it because LocalValue references it and LocalValue exists in |
According to the steps, `script.LocalValue` must include `script.RemoteReference` and `script.ChannelValue`. A priori, there is no purpose in `script.ArgumentValue`
6eda744
to
829f709
Compare
As discussed offline, the solution is to just make |
According to the steps,
script.LocalValue
must includescript.RemoteReference
andscript.ChannelValue
. A priori, there is no purpose inscript.ArgumentValue
Preview | Diff