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

Use core and alloc crates for no_std compatibility #980

Closed
wants to merge 18 commits into from
Closed
Show file tree
Hide file tree
Changes from 14 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion abci/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ bytes = "1.0"
prost = "0.7"
tendermint-proto = { version = "0.22.0", path = "../proto" }
tracing = "0.1"
flex-error = { version = "0.4.1", default-features = false }
flex-error = { version = "0.4.3", default-features = false }

structopt = { version = "0.3", optional = true }
tracing-subscriber = { version = "0.2", optional = true }
5 changes: 3 additions & 2 deletions light-client/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@ secp256k1 = ["tendermint/secp256k1", "tendermint-rpc/secp256k1"]
lightstore-sled = ["sled"]
unstable = []
std = [
"flex-error/std"
"flex-error/std",
"tendermint/std",
]
# Enable to execute long-running model-based tests
mbt = []
Expand All @@ -54,7 +55,7 @@ serde_derive = "1.0.106"
sled = { version = "0.34.3", optional = true }
static_assertions = "1.1.0"
tokio = { version = "1.0", features = ["rt"], optional = true }
flex-error = { version = "0.4.1", default-features = false }
flex-error = { version = "0.4.3", default-features = false }

[dev-dependencies]
tendermint-testgen = { path = "../testgen" }
Expand Down
10 changes: 4 additions & 6 deletions light-client/src/components/verifier.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,14 +128,12 @@ where
///
/// - Ensure the latest trusted header hasn't expired
/// - Ensure the header validator hashes match the given validators
/// - Ensure the header next validator hashes match the given next
/// validators
/// - Ensure the header next validator hashes match the given next validators
/// - Additional implementation specific validation via `commit_validator`
/// - Check that the untrusted block is more recent than the trusted state
/// - If the untrusted block is the very next block after the trusted block,
/// check that their (next) validator sets hashes match.
/// - Otherwise, ensure that the untrusted block has a greater height than
/// the trusted block.
/// - If the untrusted block is the very next block after the trusted block, check that their
/// (next) validator sets hashes match.
/// - Otherwise, ensure that the untrusted block has a greater height than the trusted block.
///
/// **NOTE**: If the untrusted state's `next_validators` field is `None`,
/// this will not (and will not be able to) check whether the untrusted
Expand Down
4 changes: 2 additions & 2 deletions light-client/tests/model_based.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,8 +125,8 @@ mod mbt {
impl SingleStepTestFuzzer for HeaderVersionFuzzer {
// TODO: rehash the header and re-compute commit with it
// TODO: Unlike in tendermint-go, we don't assert for a particular version in rust
// TODO: Either add this check in verification or remove this test because otherwise there's no
// point of it
// TODO: Either add this check in verification or remove this test because otherwise there's
// no point of it
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It seems like there are some changes the Rust nightly that cause comments to be formatted slightly differently by cargo fmt.

fn fuzz_input(input: &mut BlockVerdict) -> (String, LiteVerdict) {
let mut rng = rand::thread_rng();

Expand Down
2 changes: 1 addition & 1 deletion p2p/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ x25519-dalek = "1.1"
zeroize = "1"
signature = "1.3.0"
aead = "0.4.1"
flex-error = { version = "0.4.1", default-features = false }
flex-error = { version = "0.4.3", default-features = false }

# path dependencies
tendermint = { path = "../tendermint", version = "0.22.0" }
Expand Down
2 changes: 1 addition & 1 deletion pbt-gen/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,5 +20,5 @@ default = ["time"]
time = ["chrono"]

[dependencies]
chrono = { version = "0.4", features = ["serde"], optional = true}
chrono = { version = "0.4", default-features = false, features = ["serde"], optional = true}
proptest = "0.10.1"
20 changes: 10 additions & 10 deletions proto/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,16 @@ description = """
all-features = true

[dependencies]
prost = "0.7"
prost-types = "0.7"
bytes = "1.0"
serde = { version = "1.0", features = ["derive"] }
subtle-encoding = "0.5"
serde_bytes = "0.11"
num-traits = "0.2"
num-derive = "0.3"
chrono = { version = "0.4", features = ["serde"] }
flex-error = { version = "0.4.1", default-features = false }
prost = { version = "0.7", default-features = false }
prost-types = { version = "0.7", default-features = false }
bytes = { version = "1.0", default-features = false }
serde = { version = "1.0", default-features = false, features = ["derive"] }
serde_bytes = { version = "0.11", default-features = false, features = ["alloc"] }
subtle-encoding = { version = "0.5", default-features = false, features = ["hex", "base64", "std"] }
num-traits = { version = "0.2", default-features = false }
num-derive = { version = "0.3", default-features = false }
chrono = { version = "0.4", default-features = false, features = ["serde", "clock"] }
flex-error = { version = "0.4.3", default-features = false }

[dev-dependencies]
serde_json = "1.0"
Expand Down
7 changes: 4 additions & 3 deletions proto/src/error.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
//! This module defines the various errors that be raised during Protobuf conversions.

use crate::prelude::*;
use core::convert::TryFrom;
use core::fmt::Display;
use core::num::TryFromIntError;
use flex_error::{define_error, DisplayOnly};
use prost::{DecodeError, EncodeError};
use std::convert::TryFrom;
use std::fmt::Display;
use std::num::TryFromIntError;

define_error! {
Error {
Expand Down
42 changes: 31 additions & 11 deletions proto/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,18 @@
//! tendermint-proto library gives the developer access to the Tendermint proto-defined structs.

#![no_std]
#![deny(warnings, trivial_casts, trivial_numeric_casts, unused_import_braces)]
#![allow(clippy::large_enum_variant)]
#![forbid(unsafe_code)]
#![doc(html_root_url = "https://docs.rs/tendermint-proto/0.22.0")]

extern crate alloc;

#[cfg(feature = "std")]
extern crate std;

mod prelude;

/// Built-in prost_types with slight customization to enable JSON-encoding
#[allow(warnings)]
pub mod google {
Expand All @@ -23,13 +31,15 @@ pub use error::Error;
pub use tendermint::*;

use bytes::{Buf, BufMut};
use core::convert::{TryFrom, TryInto};
use core::fmt::Display;
use prost::encoding::encoded_len_varint;
use prost::Message;
use std::convert::{TryFrom, TryInto};
use std::fmt::Display;

pub mod serializers;

use prelude::*;

/// Allows for easy Google Protocol Buffers encoding and decoding of domain
/// types with validation.
///
Expand All @@ -38,7 +48,7 @@ pub mod serializers;
/// ```rust
/// use bytes::BufMut;
/// use prost::Message;
/// use std::convert::TryFrom;
/// use core::convert::TryFrom;
/// use tendermint_proto::Protobuf;
///
/// // This struct would ordinarily be automatically generated by prost.
Expand Down Expand Up @@ -107,9 +117,15 @@ pub mod serializers;
/// // We expect a validation error here
/// assert!(MyDomainType::decode(invalid_raw_bytes.as_ref()).is_err());
/// ```
pub trait Protobuf<T: Message + From<Self> + Default>
pub trait Protobuf<T>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have modified the Protobuf trait such that it allows the message type to implement TryFrom instead of From. This allows safe parsing of the TimeStamp type, to prevent undefined behavior caused by integer overflow in DOS attack.

Copy link
Contributor

Choose a reason for hiding this comment

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

Originally the idea here was that we wanted conversions from SerializationType -> DomainType to be fallible to allow for validation, and then from DomainType -> SerializationType to be infallible because DomainType should be a subset of SerializationType.

Changing this would imply that we could possibly have DomainTypes that possibly do not have valid serialized representations, which implies to me that those DomainTypes are incorrectly modeled, does it not?

So then the practical consequences of this architectural change would be:

  1. All From<DomainType> for SerializationType impls now need to become TryFrom<DomainType> for SerializationType, touching all domain types
  2. We need custom serde Serialize trait impls, as per https://github.com/informalsystems/tendermint-rs/pull/980/files#r717530415

This seems like a lot of work/code just to be able to cater for the DomainTimestamp -> SerializedTimestamp conversion, does it not? Is there no way to just implement a better DomainTimestamp whose conversion to SerializedTimestamp is infallible (as it should actually be)?

where
Self: Sized + Clone + TryFrom<T>,
T: Message,
T: Default,
Self: Sized,
Self: Clone,
T: TryFrom<Self>,
Self: TryFrom<T>,
<T as TryFrom<Self>>::Error: Display,
<Self as TryFrom<T>>::Error: Display,
{
/// Encode into a buffer in Protobuf format.
Expand All @@ -119,7 +135,8 @@ where
///
/// [`prost::Message::encode`]: https://docs.rs/prost/*/prost/trait.Message.html#method.encode
fn encode<B: BufMut>(&self, buf: &mut B) -> Result<(), Error> {
T::from(self.clone())
T::try_from(self.clone())
.map_err(Error::try_from::<Self, T, _>)?
.encode(buf)
.map_err(Error::encode_message)
}
Expand All @@ -133,7 +150,8 @@ where
///
/// [`prost::Message::encode_length_delimited`]: https://docs.rs/prost/*/prost/trait.Message.html#method.encode_length_delimited
fn encode_length_delimited<B: BufMut>(&self, buf: &mut B) -> Result<(), Error> {
T::from(self.clone())
T::try_from(self.clone())
.map_err(Error::try_from::<Self, T, _>)?
.encode_length_delimited(buf)
.map_err(Error::encode_message)
}
Expand Down Expand Up @@ -173,13 +191,15 @@ where
/// counterpart Protobuf data structure.
///
/// [`prost::Message::encoded_len`]: https://docs.rs/prost/*/prost/trait.Message.html#method.encoded_len
fn encoded_len(&self) -> usize {
T::from(self.clone()).encoded_len()
fn encoded_len(&self) -> Result<usize, Error> {
Ok(T::try_from(self.clone())
.map_err(Error::try_from::<Self, T, _>)?
.encoded_len())
}

/// Encodes into a Protobuf-encoded `Vec<u8>`.
fn encode_vec(&self) -> Result<Vec<u8>, Error> {
let mut wire = Vec::with_capacity(self.encoded_len());
let mut wire = Vec::with_capacity(self.encoded_len()?);
self.encode(&mut wire).map(|_| wire)
}

Expand All @@ -191,7 +211,7 @@ where

/// Encode with a length-delimiter to a `Vec<u8>` Protobuf-encoded message.
fn encode_length_delimited_vec(&self) -> Result<Vec<u8>, Error> {
let len = self.encoded_len();
let len = self.encoded_len()?;
let lenu64 = len.try_into().map_err(Error::parse_length)?;
let mut wire = Vec::with_capacity(len + encoded_len_varint(lenu64));
self.encode_length_delimited(&mut wire).map(|_| wire)
Expand Down
11 changes: 11 additions & 0 deletions proto/src/prelude.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
pub use core::prelude::v1::*;

// Re-export according to alloc::prelude::v1 because it is not yet stabilized
// https://doc.rust-lang.org/src/alloc/prelude/v1.rs.html
pub use alloc::borrow::ToOwned;
pub use alloc::boxed::Box;
pub use alloc::string::{String, ToString};
pub use alloc::vec::Vec;

pub use alloc::format;
pub use alloc::vec;
2 changes: 1 addition & 1 deletion proto/src/serializers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
//! Available serializers:
//! i64 <-> string: #[serde(with="serializers::from_str")]
//! u64 <-> string: #[serde(with="serializers::from_str")]
//! std::time::Duration <-> nanoseconds as string #[serde(with="serializers::time_duration")]
//! core::time::Duration <-> nanoseconds as string #[serde(with="serializers::time_duration")]
//! Vec<u8> <-> HexString: #[serde(with="serializers::bytes::hexstring")]
//! Vec<u8> <-> Base64String: #[serde(with="serializers::bytes::base64string")]
//! Vec<u8> <-> String: #[serde(with="serializers::bytes::string")]
Expand Down
5 changes: 5 additions & 0 deletions proto/src/serializers/bytes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

/// Serialize into hexstring, deserialize from hexstring
pub mod hexstring {
use crate::prelude::*;
use serde::{Deserialize, Deserializer, Serializer};
use subtle_encoding::hex;

Expand Down Expand Up @@ -30,6 +31,7 @@ pub mod hexstring {

/// Serialize into base64string, deserialize from base64string
pub mod base64string {
use crate::prelude::*;
use serde::{Deserialize, Deserializer, Serializer};
use subtle_encoding::base64;

Expand Down Expand Up @@ -66,6 +68,7 @@ pub mod base64string {

/// Serialize into Vec<base64string>, deserialize from Vec<base64string>
pub mod vec_base64string {
use crate::prelude::*;
use serde::{Deserialize, Deserializer, Serializer};
use subtle_encoding::base64;

Expand Down Expand Up @@ -99,6 +102,7 @@ pub mod vec_base64string {

/// Serialize into Option<base64string>, deserialize from Option<base64string>
pub mod option_base64string {
use crate::prelude::*;
use serde::{Deserialize, Deserializer, Serializer};
use subtle_encoding::base64;

Expand All @@ -125,6 +129,7 @@ pub mod option_base64string {

/// Serialize into string, deserialize from string
pub mod string {
use crate::prelude::*;
use serde::{Deserialize, Deserializer, Serializer};

/// Deserialize string into Vec<u8>
Expand Down
11 changes: 6 additions & 5 deletions proto/src/serializers/from_str.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
//! Serialize and deserialize any `T` that implements [[std::str::FromStr]]
//! and [[std::fmt::Display]] from or into string. Note this can be used for
//! Serialize and deserialize any `T` that implements [[core::str::FromStr]]
//! and [[core::fmt::Display]] from or into string. Note this can be used for
//! all primitive data types.
use crate::prelude::*;
use serde::{de::Error as _, Deserialize, Deserializer, Serialize, Serializer};

/// Deserialize string into T
pub fn deserialize<'de, D, T>(deserializer: D) -> Result<T, D::Error>
where
D: Deserializer<'de>,
T: std::str::FromStr,
<T as std::str::FromStr>::Err: std::fmt::Display,
T: core::str::FromStr,
<T as core::str::FromStr>::Err: core::fmt::Display,
{
String::deserialize(deserializer)?
.parse::<T>()
Expand All @@ -19,7 +20,7 @@ where
pub fn serialize<S, T>(value: &T, serializer: S) -> Result<S::Ok, S::Error>
where
S: Serializer,
T: std::fmt::Display,
T: core::fmt::Display,
{
format!("{}", value).serialize(serializer)
}
8 changes: 5 additions & 3 deletions proto/src/serializers/optional_from_str.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
//! De/serialize an optional type that must be converted from/to a string.

use crate::prelude::*;
use core::fmt::Display;
use core::str::FromStr;
use serde::de::Error;
use serde::{Deserialize, Deserializer, Serializer};
use std::str::FromStr;

pub fn serialize<S, T>(value: &Option<T>, serializer: S) -> Result<S::Ok, S::Error>
where
Expand All @@ -19,13 +21,13 @@ pub fn deserialize<'de, D, T>(deserializer: D) -> Result<Option<T>, D::Error>
where
D: Deserializer<'de>,
T: FromStr,
T::Err: std::error::Error,
T::Err: Display,
{
let s = match Option::<String>::deserialize(deserializer)? {
Some(s) => s,
None => return Ok(None),
};
Ok(Some(s.parse().map_err(|e: <T as FromStr>::Err| {
D::Error::custom(e.to_string())
D::Error::custom(format!("{}", e))
})?))
}
7 changes: 4 additions & 3 deletions proto/src/serializers/part_set_header_total.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,10 @@
//! string-quoted integer value into an integer value without quotes in Tendermint Core v0.34.0.
//! This deserializer allows backwards-compatibility by deserializing both ways.
//! See also: <https://github.com/informalsystems/tendermint-rs/issues/679>
use crate::prelude::*;
use core::convert::TryFrom;
use core::fmt::Formatter;
use serde::{de::Error, de::Visitor, Deserializer, Serialize, Serializer};
use std::convert::TryFrom;
use std::fmt::Formatter;

struct PartSetHeaderTotalStringOrU32;

Expand All @@ -30,7 +31,7 @@ where
impl<'de> Visitor<'de> for PartSetHeaderTotalStringOrU32 {
type Value = u32;

fn expecting(&self, formatter: &mut Formatter<'_>) -> std::fmt::Result {
fn expecting(&self, formatter: &mut Formatter<'_>) -> core::fmt::Result {
formatter.write_str("an u32 integer or string between 0 and 2^32")
}

Expand Down
5 changes: 3 additions & 2 deletions proto/src/serializers/time_duration.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
//! Serialize/deserialize std::time::Duration type from and into string:
//! Serialize/deserialize core::time::Duration type from and into string:
use crate::prelude::*;
use serde::{de::Error as _, Deserialize, Deserializer, Serialize, Serializer};

use std::time::Duration;
use core::time::Duration;

/// Deserialize string into Duration
pub fn deserialize<'de, D>(deserializer: D) -> Result<Duration, D::Error>
Expand Down
1 change: 1 addition & 0 deletions proto/src/serializers/timestamp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
use serde::{de::Error as _, Deserialize, Deserializer, Serialize, Serializer};

use crate::google::protobuf::Timestamp;
use crate::prelude::*;
use chrono::{DateTime, LocalResult, TimeZone, Utc};
use serde::ser::Error;

Expand Down
1 change: 1 addition & 0 deletions proto/src/serializers/txs.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
//! Serialize/deserialize Vec<Vec<u8>> type from and into transactions (Base64String array).
use crate::prelude::*;
use serde::{Deserialize, Deserializer, Serialize, Serializer};
use subtle_encoding::base64;

Expand Down
Loading