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

ibc/02-client: import export GenesisState #6073

Merged
merged 12 commits into from
Apr 27, 2020

Conversation

fedekunze
Copy link
Collaborator

@fedekunze fedekunze commented Apr 24, 2020

ref: #5948

Description


For contributor use:

  • Targeted PR against correct branch (see CONTRIBUTING.md)
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
  • Code follows the module structure standards.
  • Wrote unit and integration tests
  • Updated relevant documentation (docs/) or specification (x/<module>/spec/)
  • Added relevant godoc comments.
  • Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
  • Re-reviewed Files changed in the Github PR explorer

For admin use:

  • Added appropriate labels to PR (ex. WIP, R4R, docs, etc)
  • Reviewers assigned
  • Squashed all commits, uses message "Merge pull request #XYZ: [title]" (coding standards)

@fedekunze fedekunze added x/ibc C:genesis relating to chain genesis labels Apr 24, 2020
@fedekunze fedekunze self-assigned this Apr 24, 2020
Comment on lines 14 to 19
for _, cs := range gs.ClientsConsensus {
for _, consState := range cs.ConsensusStates {
// TODO: double-check if the consState will be invalidated on import at 0 height.
k.SetClientConsensusState(ctx, cs.ClientID, consState.GetHeight(), consState)
}
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@cwgoes @AdityaSripal what happens in the case of a zero height export? should I just set one consensus state (the one w/ latest height) at height 0?

Copy link
Contributor

Choose a reason for hiding this comment

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

This is a client of another chain, why would we change the logic on a zero-height export?

In any case, we should stop doing zero-height exports anyways once IBC is operational.

@codecov
Copy link

codecov bot commented Apr 24, 2020

Codecov Report

Merging #6073 into master will increase coverage by 0.16%.
The diff coverage is 73.78%.

@@            Coverage Diff             @@
##           master    #6073      +/-   ##
==========================================
+ Coverage   54.43%   54.60%   +0.16%     
==========================================
  Files         431      435       +4     
  Lines       26156    26284     +128     
==========================================
+ Hits        14239    14352     +113     
- Misses      10930    10940      +10     
- Partials      987      992       +5     

x/ibc/02-client/types/genesis_test.go Outdated Show resolved Hide resolved
x/ibc/02-client/alias.go Show resolved Hide resolved
x/ibc/genesis_test.go Outdated Show resolved Hide resolved
@fedekunze fedekunze marked this pull request as ready for review April 27, 2020 16:00
@fedekunze fedekunze added R4R and removed WIP labels Apr 27, 2020
@@ -33,10 +33,10 @@ func defaultIdentifierValidator(id string, min, max int) error {
}

// DefaultClientIdentifierValidator is the default validator function for Client identifiers
// A valid Identifier must be between 10-20 characters and only contain lowercase
// A valid Identifier must be between 9-20 characters and only contain lowercase
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I had to change this because "localhost" has len 9

Copy link
Contributor

@alexanderbez alexanderbez left a comment

Choose a reason for hiding this comment

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

utACK

x/ibc/02-client/keeper/keeper.go Show resolved Hide resolved
…cosmos-sdk into fedekunze/import-export-ibc-pt-2
Copy link
Contributor

@cwgoes cwgoes left a comment

Choose a reason for hiding this comment

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

utACK

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C:genesis relating to chain genesis
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants