Skip to content

Commit

Permalink
use ibc_proto::protobuf::Protobuf to replace tendermint_proto::Protob…
Browse files Browse the repository at this point in the history
…uf. (#754)

* use ibc_proto::protobuf::Protobuf to replace tendermint_proto::Protobuf.

* fix tests

* Create 747-fix-747-Protobuf-out-of-memory.md

* Fix issue 747 by replacing tendermint_proto::Protobuf with ibc_proto::protobuf::Protobuf (#754)
  • Loading branch information
DaviRain-Su committed Jul 11, 2023
1 parent e125999 commit bd2c1e6
Show file tree
Hide file tree
Showing 10 changed files with 14 additions and 26 deletions.
2 changes: 2 additions & 0 deletions .changelog/unreleased/breaking-changes/754-fix-issue-747.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
- use ibc_proto::protobuf::Protobuf to replace tendermint_proto::Protobuf
([#754](https://github.com/cosmos/ibc-rs/pull/754))
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
- Tendermint ConsensusState -> Any can crash if out of memory
([#747](https://github.com/cosmos/ibc-rs/issues/747))
2 changes: 1 addition & 1 deletion crates/ibc-derive/src/consensus_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ pub fn consensus_state_derive_impl(ast: DeriveInput) -> TokenStream {
}
}

fn encode_vec(&self) -> core::result::Result<Vec<u8>, tendermint_proto::Error> {
fn encode_vec(&self) -> Vec<u8> {
match self {
#(#encode_vec_impl),*
}
Expand Down
8 changes: 3 additions & 5 deletions crates/ibc/src/clients/ics07_tendermint/consensus_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,9 @@ use crate::prelude::*;

use ibc_proto::google::protobuf::Any;
use ibc_proto::ibc::lightclients::tendermint::v1::ConsensusState as RawConsensusState;
use ibc_proto::protobuf::Protobuf;
use tendermint::{hash::Algorithm, time::Time, Hash};
use tendermint_proto::google::protobuf as tpb;
use tendermint_proto::Error as TmProtoError;
use tendermint_proto::Protobuf;

use crate::clients::ics07_tendermint::error::Error;
use crate::clients::ics07_tendermint::header::Header;
Expand Down Expand Up @@ -125,8 +124,7 @@ impl From<ConsensusState> for Any {
fn from(consensus_state: ConsensusState) -> Self {
Any {
type_url: TENDERMINT_CONSENSUS_STATE_TYPE_URL.to_string(),
value: Protobuf::<RawConsensusState>::encode_vec(&consensus_state)
.expect("Out of memory"),
value: Protobuf::<RawConsensusState>::encode_vec(&consensus_state),
}
}
}
Expand Down Expand Up @@ -156,7 +154,7 @@ impl ConsensusStateTrait for ConsensusState {
self.timestamp.into()
}

fn encode_vec(&self) -> Result<Vec<u8>, TmProtoError> {
fn encode_vec(&self) -> Vec<u8> {
<Self as Protobuf<Any>>::encode_vec(self)
}
}
Expand Down
5 changes: 1 addition & 4 deletions crates/ibc/src/core/ics02_client/consensus_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,5 @@ pub trait ConsensusState: Send + Sync {
/// Serializes the `ConsensusState`. This is expected to be implemented as
/// first converting to the raw type (i.e. the protobuf definition), and then
/// serializing that.
///
/// Note that the `Protobuf` trait in `tendermint-proto` provides convenience methods
/// to do this automatically.
fn encode_vec(&self) -> Result<Vec<u8>, tendermint_proto::Error>;
fn encode_vec(&self) -> Vec<u8>;
}
3 changes: 0 additions & 3 deletions crates/ibc/src/core/ics03_connection/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,13 @@ use crate::Height;
use alloc::string::String;
use displaydoc::Display;
use ibc_proto::protobuf::Error as ProtoError;
use tendermint_proto::Error as TmProtoError;

#[derive(Debug, Display)]
pub enum ConnectionError {
/// client error: `{0}`
Client(client_error::ClientError),
/// invalid connection state: expected `{expected}`, actual `{actual}`
InvalidState { expected: String, actual: String },
/// failed to encode consensus state: `{0}`
ConsensusStateEncodeFailure(TmProtoError),
/// invalid connection end error: `{0}`
InvalidConnectionEnd(ProtoError),
/// consensus height claimed by the client on the other party is too advanced: `{target_height}` (host chain current height: `{current_height}`)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,9 +117,7 @@ where
&msg.proof_consensus_state_of_a_on_b,
consensus_state_of_b_on_a.root(),
Path::ClientConsensusState(client_cons_state_path_on_b),
expected_consensus_state_of_a_on_b
.encode_vec()
.map_err(ConnectionError::ConsensusStateEncodeFailure)?,
expected_consensus_state_of_a_on_b.encode_vec(),
)
.map_err(|e| ConnectionError::ConsensusStateVerificationFailure {
height: msg.proofs_height_on_b,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,9 +114,7 @@ where
&msg.proof_consensus_state_of_b_on_a,
consensus_state_of_a_on_b.root(),
Path::ClientConsensusState(client_cons_state_path_on_a),
expected_consensus_state_of_b_on_a
.encode_vec()
.map_err(ConnectionError::ConsensusStateEncodeFailure)?,
expected_consensus_state_of_b_on_a.encode_vec(),
)
.map_err(|e| ConnectionError::ConsensusStateVerificationFailure {
height: msg.proofs_height_on_a,
Expand Down
8 changes: 2 additions & 6 deletions crates/ibc/src/mock/consensus_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@ use ibc_proto::google::protobuf::Any;
use ibc_proto::ibc::mock::ConsensusState as RawMockConsensusState;
use ibc_proto::protobuf::Protobuf;

use tendermint_proto::{Error, Protobuf as TmProtobuf};

use crate::core::ics02_client::consensus_state::ConsensusState;
use crate::core::ics02_client::error::ClientError;
use crate::core::ics23_commitment::commitment::CommitmentRoot;
Expand Down Expand Up @@ -96,8 +94,6 @@ impl From<MockConsensusState> for Any {
}
}

impl TmProtobuf<Any> for MockConsensusState {}

impl ConsensusState for MockConsensusState {
fn root(&self) -> &CommitmentRoot {
&self.root
Expand All @@ -107,7 +103,7 @@ impl ConsensusState for MockConsensusState {
self.header.timestamp
}

fn encode_vec(&self) -> Result<Vec<u8>, Error> {
<Self as TmProtobuf<Any>>::encode_vec(self)
fn encode_vec(&self) -> Vec<u8> {
<Self as Protobuf<Any>>::encode_vec(self)
}
}
2 changes: 1 addition & 1 deletion crates/ibc/src/mock/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ use core::fmt::Debug;
use core::ops::{Add, Sub};
use core::time::Duration;
use derive_more::{From, TryInto};
use ibc_proto::protobuf::Protobuf;
use parking_lot::Mutex;
use tendermint_proto::Protobuf;

use ibc_proto::google::protobuf::Any;
use tracing::debug;
Expand Down

0 comments on commit bd2c1e6

Please sign in to comment.