-
Notifications
You must be signed in to change notification settings - Fork 332
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
WIP Connection queries for Relayer #136
Merged
Merged
Changes from 5 commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
7e6e76a
Initial implementation of proto files required for connection. Proto …
andynog 030c4d0
Removed reference to MerklePrefix
andynog ae792c0
Implemented logic to proto_unmarshall the connection using prost #130
andynog d332fd6
Trimmed the proto file, fixed commitments. Ready for review. (#130)
adizere 949ad61
Fixed a couple of nits
adizere 107cccb
Fix for unused_qualifications h/t @gregszabo @andynog.
adizere 3aae417
Fixing unit testing since CommitPrefix was changed, failed to cargo b…
andynog 9c2f528
Replaced prost compilation with dependency on ibc-proto crate.
adizere 7423376
Better error propagation in connection query.
adizere 4db544c
Fixed error handling when specifying height (-h) parameter in a query…
andynog a8ed4d5
Added validation for versions in ConnectionEnd unmarshalling
adizere f8c0721
Implemented fmt::Debug trait for CommitPrefix to properly display its…
andynog File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
extern crate prost_build; | ||
|
||
fn main() { | ||
prost_build::compile_protos(&["src/proto/connection.proto"], &["src/proto"]).unwrap(); | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -42,6 +42,15 @@ impl State { | |
Self::Open => "OPEN", | ||
} | ||
} | ||
|
||
pub fn from_i32(nr: i32) -> Self { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @ancazamfir was not sure this is accurate, please double-check. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks good. |
||
match nr { | ||
1 => Self::Init, | ||
2 => Self::TryOpen, | ||
3 => Self::Open, | ||
_ => Self::Uninitialized, | ||
} | ||
} | ||
} | ||
|
||
impl std::str::FromStr for State { | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,54 @@ | ||
syntax = "proto3"; | ||
|
||
package connection; | ||
|
||
// ICS03 - Connection Data Structures as defined in | ||
// https://github.com/cosmos/ics/tree/master/spec/ics-003-connection-semantics#data-structures | ||
|
||
// ConnectionEnd defines a stateful object on a chain connected to another separate | ||
// one. | ||
// NOTE: there must only be 2 defined ConnectionEnds to establish a connection | ||
// between two chains. | ||
message ConnectionEnd { | ||
// connection identifier. | ||
string id = 1; | ||
// client associated with this connection. | ||
string client_id = 2; | ||
// opaque string which can be utilised to determine encodings or protocols for | ||
// channels or packets utilising this connection | ||
repeated string versions = 3; | ||
// current state of the connection end. | ||
State state = 4; | ||
// counterparty chain associated with this connection. | ||
Counterparty counterparty = 5; | ||
} | ||
|
||
// State defines if a connection is in one of the following states: | ||
// INIT, TRYOPEN, OPEN or UNINITIALIZED. | ||
enum State { | ||
// Default State | ||
STATE_UNINITIALIZED_UNSPECIFIED = 0; | ||
// A connection end has just started the opening handshake. | ||
STATE_INIT = 1; | ||
// A connection end has acknowledged the handshake step on the counterparty chain. | ||
STATE_TRYOPEN = 2; | ||
// A connection end has completed the handshake. | ||
STATE_OPEN = 3; | ||
} | ||
|
||
// Counterparty defines the counterparty chain associated with a connection end. | ||
message Counterparty { | ||
// identifies the client on the counterparty chain associated with a given connection. | ||
string client_id = 1; | ||
// identifies the connection end on the counterparty chain associated with a given connection. | ||
string connection_id = 2; | ||
// commitment merkle prefix of the counterparty chain | ||
MerklePrefix prefix = 3; | ||
} | ||
|
||
|
||
// MerklePrefix is merkle path prefixed to the key. | ||
// The constructed key from the Path and the key will be append(Path.KeyPath, append(Path.KeyPrefix, key...)) | ||
message MerklePrefix { | ||
bytes key_prefix = 1; | ||
} |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we get rid of the
unwrap()
s? If the user issues a query command and gets back a few pages of traces it does not help. I had this comment earlier but I guess it got lost. Try to query with non-exsiting connection id.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd like to make this into a From trait, if possible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also I guess this should be verifying all the fields, not just 2? And should we really have an error per field or maybe start with a more general InvalidConnectionEnd error of some kind and include what's missing as a string? Not sure ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Either option could work, the question is if we want to keep validation inside the
from
or outside. The current code uses theFromStr
trait, with the signaturefn from_str(s: &str) -> Result<Self, Self::Err>
so we can do validation inside there and return error if validation fails. WithFrom
, the sig isfn from(T) -> Self
so we'd have to do validation after callingfrom
. Is it generally speaking, or specifically in this case, better to do validation outside offrom
?LE: Maybe this helps?
Agree this deserves a discussion to find a more principled approach.