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.

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

test: add a unit test to demonstrate a un-intuitive behavior #660

Merged
merged 3 commits into from
Jan 18, 2023
Merged

test: add a unit test to demonstrate a un-intuitive behavior #660

merged 3 commits into from
Jan 18, 2023

Conversation

yihuang
Copy link
Collaborator

@yihuang yihuang commented Jan 17, 2023

No description provided.

@yihuang yihuang requested a review from a team as a code owner January 17, 2023 06:41
yihuang
yihuang commented Jan 17, 2023
View reviewed changes
mutable_tree_test.go Outdated Show resolved Hide resolved
yihuang
yihuang commented Jan 17, 2023
View reviewed changes
mutable_tree_test.go Outdated Show resolved Hide resolved
@yihuang yihuang changed the title add a unit test to demonstrate a un-intuitive behavior test: add a unit test to demonstrate a un-intuitive behavior Jan 17, 2023
@cool-develope
Copy link
Collaborator

It is a kind of should be fixed, isn't it?

@yihuang
Copy link
Collaborator Author

yihuang commented Jan 18, 2023

It is a kind of should be fixed, isn't it?

I think it's hard to fix because of legacy data, maybe we can fix in the 2.0

@tac0turtle
Copy link
Member

merging but agree we should fix, want to open an issue?

@tac0turtle tac0turtle merged commit e9ee47e into cosmos:master Jan 18, 2023
@yihuang
Copy link
Collaborator Author

yihuang commented Jan 18, 2023 •
edited
Loading

merging but agree we should fix, want to open an issue?

My concern of updating this rule is this is a root hash breaking change.
So far I'm able to replay our change sets since genesis to reproduce same root hash with the same algorithm, I find it surprisingly lucky, it means we haven't done any breaking change at this level since our genesis, this'll be the first one.

@cool-develope
Copy link
Collaborator

merging but agree we should fix, want to open an issue?

My concern of updating this rule is this is a root hash breaking change. So far I'm able to replay our change sets since genesis to reproduce same root hash with the same algorithm, I find it surprisingly lucky, it means we haven't done any breaking change at this level since our genesis, this'll be the first one.

yeah, you are right, the version participate in the hash and proof calc

@yihuang yihuang deleted the initial-version branch January 18, 2023 14:57
@cool-develope cool-develope mentioned this pull request Jan 25, 2023
Closed
@drklee3 drklee3 mentioned this pull request May 30, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tac0turtle tac0turtle tac0turtle approved these changes

Assignees

@cool-develope cool-develope

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@yihuang @cool-develope @tac0turtle