Skip to content

Checking keys during internal nodes deserialization #91

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
Merged
merged 2 commits into from
Nov 17, 2021
Merged

Checking keys during internal nodes deserialization #91

merged 2 commits into from
Nov 17, 2021

Conversation

kushti
Copy link
Member

@kushti kushti commented Nov 13, 2021

No description provided.

@kushti kushti requested a review from knizhnik November 13, 2021 20:52
knizhnik
knizhnik reviewed Nov 14, 2021
View reviewed changes
Copy link

@knizhnik knizhnik left a comment

Choose a reason for hiding this comment

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

I wonder why you are using BigInt to compare keys while in all other places in scypto (for example in BatchAVLProver.scala) ByteArray.compare is used?
It seems to be more logical to use everywhere the same comparison function.

@kushti
Copy link
Member Author

kushti commented Nov 15, 2021

I wonder why you are using BigInt to compare keys while in all other places in scypto (for example in BatchAVLProver.scala) ByteArray.compare is used? It seems to be more logical to use everywhere the same comparison function.

good point, fixed

@kushti kushti merged commit ffcdb03 into master Nov 17, 2021
@kushti kushti deleted the keys-check branch November 17, 2021 12:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
1 more reviewer

@knizhnik knizhnik knizhnik approved these changes

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.
2 participants
@kushti @knizhnik