Skip to content

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

Merged
tac0turtle merged 3 commits intocosmos:masterfrom
yihuang:initial-version
Jan 18, 2023
Merged

test: add a unit test to demonstrate a un-intuitive behavior#660
tac0turtle merged 3 commits intocosmos:masterfrom
yihuang:initial-version

Conversation

@yihuang
Copy link

@yihuang yihuang commented Jan 17, 2023

No description provided.

@yihuang yihuang requested a review from a team January 17, 2023 06:41
yihuang
yihuang commented Jan 17, 2023
View reviewed changes
yihuang
yihuang commented Jan 17, 2023
View reviewed changes
@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
Contributor

cool-develope commented Jan 17, 2023

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

@yihuang
Copy link
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
Contributor

tac0turtle commented Jan 18, 2023

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
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
Contributor

cool-develope commented Jan 18, 2023

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
@yihuang yihuang mentioned this pull request Oct 4, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@tac0turtle tac0turtle tac0turtle approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@cool-develope cool-develope

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@yihuang @cool-develope @tac0turtle