Skip to content

fix: add backtracking to opt decoding #979

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.

Sign up for GitHub

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

Jump to bottom
Closed
crusso wants to merge 8 commits into from
Closed

fix: add backtracking to opt decoding #979

crusso wants to merge 8 commits into from

Conversation

@crusso
Copy link
Contributor

@crusso crusso commented Mar 17, 2025 •
edited
Loading

Description

Fixes decoding of variants under option types, allowing compatible tagged values from wider options, defaulting incompatible tagged values to null.

Brings decoding of options closer to spec, but see child PR #981 for the remaining edge cases.

This PR is marked do not merge until #981 is merged into this one.

How Has This Been Tested?

See PR for additional candid tests.

Checklist:

  • My changes follow the guidelines in CONTRIBUTING.md.
  • The title of this PR complies with Conventional Commits.
  • I have edited the CHANGELOG accordingly.
  • I have made corresponding changes to the documentation.

What CHANGELOG?

@github-actions
Copy link
Contributor

github-actions bot commented Mar 17, 2025 •
edited
Loading

size-limit report 📦

Path Size
@dfinity/agent 71.97 KB (-0.06% 🔽)
@dfinity/candid 11.75 KB (+0.41% 🔺)
@dfinity/principal 4.21 KB (0%)
@dfinity/auth-client 50.17 KB (+0.17% 🔺)
@dfinity/assets 67.9 KB (+0.09% 🔺)
@dfinity/identity 47.73 KB (+0.07% 🔺)
@dfinity/identity-secp256k1 106.62 KB (+0.03% 🔺)

@crusso crusso mentioned this pull request Mar 17, 2025
Merged
4 tasks
@crusso crusso changed the title experiment: add backtracking to opt decoding fix: add backtracking to opt decoding Mar 17, 2025
krpeacock
krpeacock reviewed Mar 17, 2025
View reviewed changes
krpeacock
krpeacock reviewed Mar 17, 2025
View reviewed changes
krpeacock
krpeacock reviewed Mar 17, 2025
View reviewed changes
@crusso crusso marked this pull request as ready for review March 18, 2025 01:24
@crusso crusso requested a review from a team as a code owner March 18, 2025 01:24
jasonz-dfinity
jasonz-dfinity reviewed Mar 18, 2025
View reviewed changes
@crusso
Copy link
Contributor Author

crusso commented Mar 19, 2025

Closing in favor of #981 (which incorporates these changes)

@krpeacock krpeacock closed this Mar 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@krpeacock krpeacock krpeacock left review comments

@jasonz-dfinity jasonz-dfinity jasonz-dfinity left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@crusso @krpeacock @jasonz-dfinity