-
Notifications
You must be signed in to change notification settings - Fork 326
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
Generic query, channel query and reduce code #152
Changes from 5 commits
86fb408
0882154
1e32302
2a88624
53d8f1a
85a9439
980d039
53a4d50
c71709e
882902c
b584b88
7e8dc89
35ee937
843191f
1676a52
621717c
8ed3f61
988960d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,6 +7,7 @@ use serde_derive::{Deserialize, Serialize}; | |
// Import proto declarations. | ||
use ibc_proto::connection::ConnectionEnd as ProtoConnectionEnd; | ||
use ibc_proto::connection::Counterparty as ProtoCounterparty; | ||
use std::convert::TryFrom; | ||
|
||
#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] | ||
pub struct ConnectionEnd { | ||
|
@@ -16,43 +17,31 @@ pub struct ConnectionEnd { | |
versions: Vec<String>, | ||
} | ||
|
||
impl ConnectionEnd { | ||
pub fn new( | ||
client_id: ClientId, | ||
counterparty: Counterparty, | ||
versions: Vec<String>, | ||
) -> Result<Self, Error> { | ||
Ok(Self { | ||
state: State::Uninitialized, | ||
client_id, | ||
counterparty, | ||
versions: validate_versions(versions).map_err(|e| Kind::InvalidVersion.context(e))?, | ||
}) | ||
} | ||
impl TryFrom<ProtoConnectionEnd> for ConnectionEnd { | ||
type Error = anomaly::Error<Kind>; | ||
|
||
pub fn set_state(&mut self, new_state: State) { | ||
self.state = new_state; | ||
} | ||
|
||
pub fn from_proto_connection_end(pc: ProtoConnectionEnd) -> Result<Self, Error> { | ||
if pc.id == "" { | ||
fn try_from(value: ProtoConnectionEnd) -> Result<Self, Self::Error> { | ||
// Todo: Is validation complete here? (Code was moved from `from_proto_connection_end`.) | ||
if value.id == "" { | ||
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. Following the recent changes in the code, this I think we can now delete this 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. I made a commit to capture concretely my suggestion from the comment above. Full commit details here. Relevant code: @greg-szabo if this makes sense with what you have in mind, then we can delete the 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. Nice, thanks! I submitted a followup that fixes the Clippy warning. I'm not sure how much we want to manually check different results of the response before we return, but this case seems straightforward. There's a TODO in the type validation which I think should go into it's own issue. There's a lot to improve on that piece of code. 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. Awesome! |
||
return Err(Kind::ConnectionNotFound.into()); | ||
} | ||
|
||
// The Counterparty field is an Option, may be missing. | ||
match pc.counterparty { | ||
match value.counterparty { | ||
Some(cp) => { | ||
let mut conn = ConnectionEnd::new( | ||
pc.client_id | ||
value | ||
.client_id | ||
.parse() | ||
.map_err(|e| Kind::IdentifierError.context(e))?, | ||
Counterparty::from_proto_counterparty(cp)?, | ||
validate_versions(pc.versions).map_err(|e| Kind::InvalidVersion.context(e))?, | ||
Counterparty::try_from(cp)?, | ||
validate_versions(value.versions) | ||
.map_err(|e| Kind::InvalidVersion.context(e))?, | ||
) | ||
.unwrap(); | ||
|
||
// Set the state. | ||
conn.set_state(State::from_i32(pc.state)); | ||
conn.set_state(State::from_i32(value.state)); | ||
Ok(conn) | ||
} | ||
|
||
|
@@ -62,6 +51,25 @@ impl ConnectionEnd { | |
} | ||
} | ||
|
||
impl ConnectionEnd { | ||
pub fn new( | ||
client_id: ClientId, | ||
counterparty: Counterparty, | ||
versions: Vec<String>, | ||
) -> Result<Self, Error> { | ||
Ok(Self { | ||
state: State::Uninitialized, | ||
client_id, | ||
counterparty, | ||
versions: validate_versions(versions).map_err(|e| Kind::InvalidVersion.context(e))?, | ||
}) | ||
} | ||
|
||
pub fn set_state(&mut self, new_state: State) { | ||
self.state = new_state; | ||
} | ||
} | ||
|
||
impl Connection for ConnectionEnd { | ||
type ValidationError = Error; | ||
|
||
|
@@ -95,6 +103,22 @@ pub struct Counterparty { | |
prefix: CommitmentPrefix, | ||
} | ||
|
||
impl TryFrom<ProtoCounterparty> for Counterparty { | ||
type Error = anomaly::Error<Kind>; | ||
|
||
fn try_from(value: ProtoCounterparty) -> Result<Self, Self::Error> { | ||
// Todo: Is validation complete here? (code was moved from `from_proto_counterparty`) | ||
match value.prefix { | ||
Some(prefix) => Counterparty::new( | ||
value.client_id, | ||
value.connection_id, | ||
CommitmentPrefix::new(prefix.key_prefix), | ||
), | ||
None => Err(Kind::MissingCounterpartyPrefix.into()), | ||
} | ||
} | ||
} | ||
|
||
impl Counterparty { | ||
pub fn new( | ||
client_id: String, | ||
|
@@ -111,17 +135,6 @@ impl Counterparty { | |
prefix, | ||
}) | ||
} | ||
|
||
pub fn from_proto_counterparty(pc: ProtoCounterparty) -> Result<Self, Error> { | ||
match pc.prefix { | ||
Some(prefix) => Counterparty::new( | ||
pc.client_id, | ||
pc.connection_id, | ||
CommitmentPrefix::new(prefix.key_prefix), | ||
), | ||
None => Err(Kind::MissingCounterpartyPrefix.into()), | ||
} | ||
} | ||
} | ||
|
||
impl ConnectionCounterparty for Counterparty { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,4 +5,3 @@ pub mod error; | |
pub mod events; | ||
pub mod exported; | ||
pub mod msgs; | ||
pub mod query; |
This file was deleted.
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 wonder if we should consider renaming the way we import proto data structures to be
as RawConnectionEnd;
instead ofas ProtoConnectionEnd;
. The same would go forRawCounterparty
and others. The reason to do this renaming is purely aesthetic and in the interest of consistency with thetendermint-rs
codebase (I am referring to theRawCommitSig
struct, though I am not sure that the Raw types are analogous). WDYT?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.
Good idea to keep the same naming schema. Proto was used here for the underlying implementation which you never know. Raw makes more sense. Raw, as in coming straight from the wire.
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.
Submitted a commit for it.