Skip to content
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

fix: In order to ensure consensus passes, forex rates must be sorted. #247

Merged
merged 3 commits into from
Nov 28, 2023

Conversation

dfinity-ryancroote
Copy link
Collaborator

@dfinity-ryancroote dfinity-ryancroote commented Nov 28, 2023

As of writing this description, the XRC on production has only been able to collect rates from 2 forex sources due to consensus errors. This is due to encoding a hash map to candid. The hash map does not preserve any sense of order; therefore, a b-tree map should be used as it will preserve order of the values when encoded.

This PR also adds logging into the transform so we may better see what is actually occurring during that process.

@dfinity-ryancroote dfinity-ryancroote changed the title bug: In order to ensure consensus passes, forex rates must be sorted. fix: In order to ensure consensus passes, forex rates must be sorted. Nov 28, 2023
@dfinity-ryancroote dfinity-ryancroote marked this pull request as ready for review November 28, 2023 15:01
@dfinity-ryancroote dfinity-ryancroote requested a review from a team as a code owner November 28, 2023 15:01
Copy link
Contributor

@yotamhc yotamhc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Ryan!

@dfinity-ryancroote dfinity-ryancroote merged commit 9ee933f into main Nov 28, 2023
14 checks passed
@dfinity-ryancroote dfinity-ryancroote deleted the rc/fix-determinism branch November 28, 2023 15:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants