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 specification #5426

Merged
merged 8 commits into from
Feb 18, 2020
Merged

IBC SDK specification #5426

merged 8 commits into from
Feb 18, 2020

Conversation

fedekunze
Copy link
Collaborator

@fedekunze fedekunze commented Dec 18, 2019

Description

This PR introduces the spec for the x/ibc module within the SDK.


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)

@cwgoes cwgoes requested review from cwgoes and mossid December 20, 2019 02:09
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.

Looks like a work-in-progress still - great start, I think this will be useful.

@@ -1,3 +0,0 @@
# Cosmos Inter-Blockchain Communication (IBC) Protocol

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it might be worth leaving this - we have linked to this folder before.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

all specs live in the corresponding modules now

Copy link
Contributor

Choose a reason for hiding this comment

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

I realise - I mean externally (on some blog posts). Should we link to the new module specs then?

# State

The paths for the values stored in state can be found [here](https://github.com/cosmos/ics/blob/master/spec/ics-024-host-requirements/README.md#path-space). Additionally, the SDK adds
a prefix to the path to be able to aggregate the values for querying purposes.
Copy link
Contributor

Choose a reason for hiding this comment

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

This will no longer be true once the new store layout is implemented, correct?

The message validates the header and updates the consensus state with the new
height, commitment root and validator sets, which are then stored.

## ICS 03 - Connection
Copy link
Contributor

Choose a reason for hiding this comment

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

Documentation to be added?


### MsgChannelOpenInit

A channel handshake is initiated by a chain A using the `MsgChannelOpenInit`
Copy link
Contributor

Choose a reason for hiding this comment

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

Why specifically a "chain A"?

}
```

This message is expected to fail if:
Copy link
Contributor

Choose a reason for hiding this comment

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

When will it fail?

@melekes melekes closed this Feb 7, 2020
@fedekunze fedekunze reopened this Feb 7, 2020
@fedekunze fedekunze marked this pull request as ready for review February 18, 2020 14:06
@fedekunze
Copy link
Collaborator Author

I'll merge this since I think it provides value and I will complete this at a later stage once the implementation is stable enough

@fedekunze fedekunze merged commit d25f3d3 into ibc-alpha Feb 18, 2020
@fedekunze fedekunze deleted the ibc-sdk-spec branch February 18, 2020 14:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants