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: support x/collection migration on old chains #1106

Merged
merged 3 commits into from
Sep 4, 2023

Conversation

0Tech
Copy link
Collaborator

@0Tech 0Tech commented Sep 1, 2023

Description

This patch would support old chains in which the default class ids start from 0.

related: #1105

Motivation and context

How has this been tested?

Screenshots (if appropriate):

Checklist:

  • I followed the contributing guidelines and code of conduct.
  • I have added a relevant changelog to CHANGELOG.md
  • I have added tests to cover my changes.
  • I have updated the documentation accordingly.
  • I have updated API documentation client/docs/swagger-ui/swagger.yaml

@0Tech 0Tech added A: bug Something isn't working C:x/collection A: State Machine Breaking Any changes that result in a different AppState given same genesisState and txList. labels Sep 1, 2023
@0Tech 0Tech added this to the v0.48.0 milestone Sep 1, 2023
@0Tech 0Tech self-assigned this Sep 1, 2023
@codecov
Copy link

codecov bot commented Sep 1, 2023

Codecov Report

Merging #1106 (a7790c8) into rc/v0.48.0-rcx (0a27aef) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

@@                Coverage Diff                 @@
##           rc/v0.48.0-rcx    #1106      +/-   ##
==================================================
+ Coverage           69.75%   69.76%   +0.01%     
==================================================
  Files                 645      645              
  Lines               67468    67469       +1     
==================================================
+ Hits                47060    47068       +8     
+ Misses              18220    18213       -7     
  Partials             2188     2188              
Files Changed Coverage Δ
x/collection/keeper/migrations/v2/store.go 88.29% <100.00%> (+6.57%) ⬆️

... and 1 file with indirect coverage changes

@0Tech 0Tech marked this pull request as ready for review September 4, 2023 00:51
Copy link
Member

@zemyblue zemyblue left a comment

Choose a reason for hiding this comment

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

Could you add any unittest for this change?

@0Tech
Copy link
Collaborator Author

0Tech commented Sep 4, 2023

Could you add any unittest for this change?

Instead of adding a unit test, I've added a comment explaining the reason of the change.

@0Tech 0Tech requested a review from zemyblue September 4, 2023 08:39
@0Tech 0Tech merged commit 4f7b891 into Finschia:rc/v0.48.0-rcx Sep 4, 2023
36 checks passed
@0Tech 0Tech deleted the additional-fix branch September 4, 2023 11:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A: bug Something isn't working A: State Machine Breaking Any changes that result in a different AppState given same genesisState and txList. C:x/collection
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants