Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Bump to latest scale-encode/decode/value and fix test running #1103

Merged
merged 8 commits into from
Aug 2, 2023

Conversation

jsdw
Copy link
Collaborator

@jsdw jsdw commented Aug 2, 2023

Running tests locally failed because we were still explicitly passing incorrect paths, so I fixed that. I also bumped versions of scale-encode, scale-decode and scale-value and fixed anything that broke as a result, and changed the CI to download to substrate-node to mirror the changed binary name.

One change that was spotted here in a failing test: previously, scale_value::Value's would each come with a type ID of the type they decoded from, and this could be used to re-encode the value in the same way. This is no longer always possible, because we now "skip" over Compact types when decoding. It's a trade-off:

  • Previously, any compact encoded thing (value, struct, whatever) was decoded to a single number. This lost information about the structs etc on the way there, and decoding into matching structs would fail because of this.
  • Now, we preserve all of the struct (composite) types in the decoding, but there's no longer a sensible place for the "compact" type ID to live (each decoded struct etc has its own type ID).

I think this is fair enough; the Type IDs that are attached to values are meant to help with diagnostics and such more than anything, and values should be encoded, as other types, by providing a target type ID to aim them at.

@jsdw jsdw requested review from a team as code owners August 2, 2023 10:45
@jsdw jsdw changed the title Bump to latest scale-encode,decode,value and fix test running Bump to latest scale-encode/decode/value and fix test running Aug 2, 2023
@jsdw jsdw merged commit 2176ec9 into master Aug 2, 2023
@jsdw jsdw deleted the jsdw-bump-deps branch August 2, 2023 12:55
@jsdw jsdw mentioned this pull request Aug 2, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants