-
Notifications
You must be signed in to change notification settings - Fork 89
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
Add parity-scale-codec and borsh for IbcEvent #321
Add parity-scale-codec and borsh for IbcEvent #321
Conversation
Codecov ReportBase: 63.80% // Head: 62.44% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #321 +/- ##
==========================================
- Coverage 63.80% 62.44% -1.37%
==========================================
Files 124 124
Lines 13771 14072 +301
==========================================
Hits 8787 8787
- Misses 4984 5285 +301
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
Cargo.toml
Outdated
@@ -9,3 +9,6 @@ members = [ | |||
exclude = [ | |||
"ci/no-std-check", | |||
] | |||
|
|||
[patch.crates-io] | |||
ibc-proto = { git = "https://github.com/octopus-network/ibc-proto-rs.git", branch = "add-codec-borsh-for-any" } |
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.
How about we leave this PR in draft until this patch is removed?
blocked on cosmos/ibc-proto-rs#51 |
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.
Looks good, only minor changes needed
ci/no-std-check/Cargo.toml
Outdated
@@ -6,7 +6,7 @@ resolver = "2" | |||
|
|||
[dependencies] | |||
ibc = { path = "../../crates/ibc", default-features = false } | |||
ibc-proto = { version = "0.24.0", default-features = false } | |||
ibc-proto = { version = "0.24.0", default-features = false, features = ["parity-scale-codec", "borsh"]} |
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.
0.24.1
is now released with the required change; can you update it here and remove the [patch.crates-io]
section?
@@ -10,6 +10,7 @@ use crate::{ | |||
timestamp::Timestamp, | |||
}; | |||
use derive_more::From; | |||
use serde_derive::{Deserialize, Serialize}; |
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.
Can you remove all new serde_derive
imports? This will cause conflicts with #331
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 added a comment at all the other ones so that we don't forget any
@@ -3,6 +3,7 @@ use crate::prelude::*; | |||
use derive_more::{From, Into}; | |||
use ibc_proto::ibc::core::channel::v1::MsgAcknowledgement as RawMsgAcknowledgement; | |||
use ibc_proto::protobuf::Protobuf; | |||
use serde_derive::{Deserialize, Serialize}; |
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.
here's one
@@ -10,6 +10,7 @@ use crate::core::ics02_client::height::Height; | |||
use crate::core::ics24_host::identifier::ClientId; | |||
use crate::events::IbcEventType; | |||
use crate::prelude::*; | |||
use serde_derive::{Deserialize, Serialize}; |
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.
here's one
@@ -3,6 +3,7 @@ | |||
mod channel_attributes; | |||
mod packet_attributes; | |||
|
|||
use serde_derive::{Deserialize, Serialize}; |
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.
here too
@@ -1,6 +1,7 @@ | |||
///! This module holds all the abci event attributes for IBC events emitted | |||
///! during the channel handshake. | |||
use derive_more::From; | |||
use serde_derive::{Deserialize, Serialize}; |
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.
here too
Signed-off-by: Davirain <davirain.yin@gmail.com>
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.
🙏
* add parity-scale-codec and borsh for IbcEvent * Create 320-add-borsh-and-scale-codec-for-ibcevent.md * Update no-std-check ibc-proto dep * Add patch ibc-proto for no-std-check * Update ibc-proto patch * Update Cargo.toml * Update ibc-proto version and Add serde feature Signed-off-by: Davirain <davirain.yin@gmail.com>
Closes: #320
Description
You need to merge this PR( cosmos/ibc-proto-rs#48) first before merging the PR.
PR author checklist:
unclog
.docs/
).Reviewer checklist:
Files changed
in the GitHub PR explorer.