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 SDK Integration #5124

Closed

Conversation

fedekunze
Copy link
Collaborator

@fedekunze fedekunze commented Oct 1, 2019

Replaces: #5057

Depends on: #4515 #4516 #4517 #4548


IMPORTANT: Please do not merge in the downstream PRs until they have been merged and this PR has been rebased to master.


  • 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.

  • Wrote tests

  • Updated relevant documentation (docs/)

  • 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, ready-for-review, docs)
  • Reviewers Assigned
  • Squashed all commits, uses message "Merge pull request #XYZ: [title]" (coding standards)

@mossid mossid mentioned this pull request Oct 1, 2019
5 tasks
@fedekunze fedekunze changed the title IBC sdk interface IBC SDK Integration Oct 1, 2019
x/ibc/ante.go Outdated

return ctx, res, false
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

File is not goimports-ed (from goimports)

Suggested change
}
}

x/ibc/keeper.go Outdated

func (k Keeper) Port(id string) channel.Port {
return k.channel.Port(id)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

File is not goimports-ed (from goimports)

Suggested change
}
}

x/ibc/module.go Outdated

func (am AppModule) EndBlock(ctx sdk.Context, req abci.RequestEndBlock) []abci.ValidatorUpdate {
return []abci.ValidatorUpdate{}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

File is not goimports-ed (from goimports)

Suggested change
}
}

x/ibc/mock/send/keeper.go Outdated Show resolved Hide resolved
@alexanderbez alexanderbez mentioned this pull request Oct 1, 2019
30 tasks
@alexanderbez alexanderbez added this to the IBC Implementation & Integration milestone Oct 1, 2019
@codecov
Copy link

codecov bot commented Oct 2, 2019

Codecov Report

Merging #5124 into joon/ics-04-implementation will decrease coverage by 0.07%.
The diff coverage is 13.09%.

@@                      Coverage Diff                       @@
##           joon/ics-04-implementation    #5124      +/-   ##
==============================================================
- Coverage                       55.84%   55.77%   -0.08%     
==============================================================
  Files                             313      314       +1     
  Lines                           18980    19029      +49     
==============================================================
+ Hits                            10600    10613      +13     
- Misses                           7625     7660      +35     
- Partials                          755      756       +1

* Cherry pick from bez/5116-multi-ctx-verifier

* fix test
fedekunze added a commit to cosmos/gaia that referenced this pull request Oct 3, 2019
chainID := viper.GetString(flags.FlagChainID)
home := viper.GetString(flags.FlagHome)
nodeURI := viper.GetString(flags.FlagNode)
// NewCLIContextIBC takes additional arguements
Copy link
Contributor

Choose a reason for hiding this comment

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

arguements is a misspelling of arguments (from misspell)

Suggested change
// NewCLIContextIBC takes additional arguements

x/ibc/04-channel/client/cli/tx.go Show resolved Hide resolved
x/ibc/04-channel/client/cli/tx.go Outdated Show resolved Hide resolved
x/ibc/04-channel/client/cli/tx.go Outdated Show resolved Hide resolved
x/ibc/04-channel/client/cli/tx.go Outdated Show resolved Hide resolved
x/ibc/04-channel/client/cli/tx.go Outdated Show resolved Hide resolved
x/ibc/04-channel/client/cli/tx.go Outdated Show resolved Hide resolved
x/ibc/04-channel/client/cli/tx.go Outdated Show resolved Hide resolved
x/ibc/04-channel/manager.go Show resolved Hide resolved
x/ibc/mock/send/keeper.go Show resolved Hide resolved
@fedekunze fedekunze added the S:blocked Status: Blocked label Oct 10, 2019
x/ibc/02-client/msgs.go Outdated Show resolved Hide resolved
x/ibc/02-client/client/cli/tx.go Show resolved Hide resolved
@@ -15,6 +15,16 @@ type MsgOpenInit struct {
Signer sdk.AccAddress `json:"signer"`
}

func NewMsgOpenInit(connectionID string, connection Connection, counterpartyClient string, nextTimeout uint64, signer sdk.AccAddress) MsgOpenInit {
return MsgOpenInit{
ConnectionID: connectionID,
Copy link
Contributor

Choose a reason for hiding this comment

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

File is not goimports-ed (from goimports)

fmt.Fprintf(os.Stderr, "failed to unmarshall input into struct, checking for file...")
contents, err := ioutil.ReadFile(args[1])
if err != nil {
return fmt.Errorf("error opening state file: %v\n", err)
Copy link
Contributor

Choose a reason for hiding this comment

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

error strings should not be capitalized or end with punctuation or a newline (from golint)

return fmt.Errorf("error opening state file: %v\n", err)
}
if err := cdc.UnmarshalJSON(contents, &state); err != nil {
return fmt.Errorf("error unmarshalling state file: %v\n", err)
Copy link
Contributor

Choose a reason for hiding this comment

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

error strings should not be capitalized or end with punctuation or a newline (from golint)

fmt.Fprintf(os.Stderr, "failed to unmarshall input into struct, checking for file...")
contents, err := ioutil.ReadFile(args[1])
if err != nil {
return fmt.Errorf("error opening header file: %v\n", err)
Copy link
Contributor

Choose a reason for hiding this comment

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

error strings should not be capitalized or end with punctuation or a newline (from golint)

jackzampolin and others added 2 commits October 11, 2019 08:32
Co-Authored-By: Bot from GolangCI <42910462+golangcibot@users.noreply.github.com>
Co-Authored-By: Bot from GolangCI <42910462+golangcibot@users.noreply.github.com>
return MsgCreateClient{
ClientID: clientID,
ConsensusState: consState,
Signer: signer,
Copy link
Contributor

Choose a reason for hiding this comment

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

File is not goimports-ed (from goimports)

Suggested change
Signer: signer,
Signer: signer,

fmt.Fprintf(os.Stderr, "failed to unmarshall input into struct, checking for file...")
contents, err := ioutil.ReadFile(args[1])
if err != nil {
return fmt.Errorf("error opening state file: %v\n", err)
Copy link
Contributor

Choose a reason for hiding this comment

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

error strings should not be capitalized or end with punctuation or a newline (from golint)

return fmt.Errorf("error opening state file: %v\n", err)
}
if err := cdc.UnmarshalJSON(contents, &state); err != nil {
return fmt.Errorf("error unmarshalling state file: %v\n", err)
Copy link
Contributor

Choose a reason for hiding this comment

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

error strings should not be capitalized or end with punctuation or a newline (from golint)

fmt.Fprintf(os.Stderr, "failed to unmarshall input into struct, checking for file...")
contents, err := ioutil.ReadFile(args[1])
if err != nil {
return fmt.Errorf("error opening header file: %v\n", err)
Copy link
Contributor

Choose a reason for hiding this comment

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

error strings should not be capitalized or end with punctuation or a newline (from golint)

@@ -14,6 +14,14 @@ type Connection struct {
Path commitment.Prefix `json:"path"`
}

func NewConnection(client string, counterParty string, path commitment.Prefix) Connection {
return Connection{
Client: client,
Copy link
Contributor

Choose a reason for hiding this comment

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

File is not goimports-ed (from goimports)

Suggested change
Client: client,
Client: client,

fmt.Fprintf(os.Stderr, "failed to unmarshall input into struct, checking for file...")
contents, err := ioutil.ReadFile(arg)
if err != nil {
return path, fmt.Errorf("error opening path file: %v\n", err)
Copy link
Contributor

Choose a reason for hiding this comment

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

error strings should not be capitalized or end with punctuation or a newline (from golint)

return path, fmt.Errorf("error opening path file: %v\n", err)
}
if err := cdc.UnmarshalJSON(contents, &path); err != nil {
return path, fmt.Errorf("error unmarshalling path file: %v\n", err)
Copy link
Contributor

Choose a reason for hiding this comment

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

error strings should not be capitalized or end with punctuation or a newline (from golint)

@@ -2,22 +2,20 @@ package cli

import (
"io/ioutil"
// "os"
"fmt"
"io/ioutil"
Copy link
Contributor

Choose a reason for hiding this comment

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

ioutil redeclared in this block (from typecheck)

@jackzampolin jackzampolin mentioned this pull request Oct 14, 2019
5 tasks
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.

There seems to be some duplication between this PR and various ICS PRs.

Either ICS implementations should exist in internal modules and all exports be pulled in at the top level (e.g. message handling, querying) or they should be split up, but here there is duplication, which we should avoid.

My preference would be the former, but it's up to the SDK team.

@fedekunze
Copy link
Collaborator Author

replaced by #5277

@fedekunze fedekunze closed this Nov 5, 2019
@fedekunze fedekunze deleted the fedekunze/ibc-sdk-interface branch December 24, 2019 16:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S:blocked Status: Blocked
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants