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

Validate construction and deserialization of validator::Set #1350

Merged
merged 13 commits into from
Sep 14, 2023
Merged
Show file tree
Hide file tree
Changes from all 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
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
- `[tendermint]` Integer overflows are prevented when calculating the total
voting power value in `validator::Set`
([\#1348](https://github.com/informalsystems/tendermint-rs/issues/1348)).
4 changes: 4 additions & 0 deletions .changelog/unreleased/bug-fixes/1350-pubkey-serde-fixes.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
- `[tendermint-proto]` `Serialize` and `Deserialize` impls for
`v*::crypto::PublicKey` are corrected to match the JSON schema used by
other implementations
([\#1350](https://github.com/informalsystems/tendermint-rs/pull/1350)).
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
- `[tendermint]` Improve and validate deserialization of `validator::Set`
([\#1348](https://github.com/informalsystems/tendermint-rs/issues/1348)).
The `total_voting_power` field no longer has to be present in the format
processed by `Deserialize`. If it is present, it is validated against the
sum of the `voting_power` values of the listed validators. The sum value
is also checked against the protocol-defined maximum.
- `[tendermint-proto]` In the `Deserialize` impls derived for
`v*::types::ValidatorSet`, the `total_voting_power` field value is retrieved
when present.
- `[tendermint-proto]` Add serialziation helper module
`serializers::from_str_allow_null`. Use it to allow the `proposed_priority`
field value of null in the deserialization of `v*::types::Validator`.
1 change: 0 additions & 1 deletion proto/src/prost/v0_34/tendermint.crypto.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@ pub struct ProofOps {
pub ops: ::prost::alloc::vec::Vec<ProofOp>,
}
/// PublicKey defines the keys available for use with Validators
#[derive(::serde::Deserialize, ::serde::Serialize)]
#[allow(clippy::derive_partial_eq_without_eq)]
#[derive(Clone, PartialEq, ::prost::Message)]
pub struct PublicKey {
Expand Down
6 changes: 4 additions & 2 deletions proto/src/prost/v0_34/tendermint.types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ pub struct ValidatorSet {
#[prost(message, optional, tag = "2")]
pub proposer: ::core::option::Option<Validator>,
#[prost(int64, tag = "3")]
#[serde(skip)]
#[serde(with = "crate::serializers::from_str", default)]
#[serde(skip_serializing)]
pub total_voting_power: i64,
}
#[derive(::serde::Deserialize, ::serde::Serialize)]
Expand All @@ -23,7 +24,8 @@ pub struct Validator {
#[serde(alias = "power", with = "crate::serializers::from_str")]
pub voting_power: i64,
#[prost(int64, tag = "4")]
#[serde(with = "crate::serializers::from_str", default)]
#[serde(with = "crate::serializers::from_str_allow_null")]
#[serde(default)]
pub proposer_priority: i64,
}
#[allow(clippy::derive_partial_eq_without_eq)]
Expand Down
1 change: 0 additions & 1 deletion proto/src/prost/v0_37/tendermint.crypto.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@ pub struct ProofOps {
pub ops: ::prost::alloc::vec::Vec<ProofOp>,
}
/// PublicKey defines the keys available for use with Validators
#[derive(::serde::Deserialize, ::serde::Serialize)]
#[allow(clippy::derive_partial_eq_without_eq)]
#[derive(Clone, PartialEq, ::prost::Message)]
pub struct PublicKey {
Expand Down
6 changes: 4 additions & 2 deletions proto/src/prost/v0_37/tendermint.types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ pub struct ValidatorSet {
#[prost(message, optional, tag = "2")]
pub proposer: ::core::option::Option<Validator>,
#[prost(int64, tag = "3")]
#[serde(skip)]
#[serde(with = "crate::serializers::from_str", default)]
#[serde(skip_serializing)]
pub total_voting_power: i64,
}
#[derive(::serde::Deserialize, ::serde::Serialize)]
Expand All @@ -23,7 +24,8 @@ pub struct Validator {
#[serde(alias = "power", with = "crate::serializers::from_str")]
pub voting_power: i64,
#[prost(int64, tag = "4")]
#[serde(with = "crate::serializers::from_str", default)]
#[serde(with = "crate::serializers::from_str_allow_null")]
#[serde(default)]
pub proposer_priority: i64,
}
#[allow(clippy::derive_partial_eq_without_eq)]
Expand Down
1 change: 0 additions & 1 deletion proto/src/prost/v0_38/tendermint.crypto.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@ pub struct ProofOps {
pub ops: ::prost::alloc::vec::Vec<ProofOp>,
}
/// PublicKey defines the keys available for use with Validators
#[derive(::serde::Deserialize, ::serde::Serialize)]
#[allow(clippy::derive_partial_eq_without_eq)]
#[derive(Clone, PartialEq, ::prost::Message)]
pub struct PublicKey {
Expand Down
6 changes: 4 additions & 2 deletions proto/src/prost/v0_38/tendermint.types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,8 @@ pub struct ValidatorSet {
#[prost(message, optional, tag = "2")]
pub proposer: ::core::option::Option<Validator>,
#[prost(int64, tag = "3")]
#[serde(skip)]
#[serde(with = "crate::serializers::from_str", default)]
#[serde(skip_serializing)]
pub total_voting_power: i64,
}
#[derive(::serde::Deserialize, ::serde::Serialize)]
Expand All @@ -120,7 +121,8 @@ pub struct Validator {
#[serde(alias = "power", with = "crate::serializers::from_str")]
pub voting_power: i64,
#[prost(int64, tag = "4")]
#[serde(with = "crate::serializers::from_str", default)]
#[serde(with = "crate::serializers::from_str_allow_null")]
#[serde(default)]
pub proposer_priority: i64,
}
#[allow(clippy::derive_partial_eq_without_eq)]
Expand Down
3 changes: 3 additions & 0 deletions proto/src/serializers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,13 @@ pub mod allow_null;
pub mod bytes;
pub mod evidence;
pub mod from_str;
pub mod from_str_allow_null;
pub mod nullable;
pub mod optional;
pub mod optional_from_str;
pub mod part_set_header_total;
pub mod time_duration;
pub mod timestamp;
pub mod txs;

mod public_key;
42 changes: 42 additions & 0 deletions proto/src/serializers/from_str_allow_null.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
//! Combines [`from_str`] and [`allow_null`].
//!
//! Use this module to serialize and deserialize any `T` where `T` implements
//! [`FromStr`] and [`Display`] to convert from or into a string.
//! The serialized form is that of `Option<String>`,
//! and a nil is deserialized to the `Default` value. For JSON, this means both
//! quoted string values and `null` are accepted. A value is always serialized
//! as `Some<String>`.
//! Note that this can be used for all primitive data types.
//!
//! [`from_str`]: super::from_str
//! [`allow_null`]: super::allow_null

use alloc::borrow::Cow;
use core::fmt::Display;
use core::str::FromStr;

use serde::{de::Error as _, Deserialize, Deserializer, Serializer};

use crate::prelude::*;

/// Deserialize a nullable string into T
pub fn deserialize<'de, D, T>(deserializer: D) -> Result<T, D::Error>
where
D: Deserializer<'de>,
T: FromStr + Default,
<T as FromStr>::Err: Display,
{
match <Option<Cow<'_, str>>>::deserialize(deserializer)? {
Some(s) => s.parse::<T>().map_err(D::Error::custom),
None => Ok(T::default()),
}
}

/// Serialize from T into string
pub fn serialize<S, T>(value: &T, serializer: S) -> Result<S::Ok, S::Error>
where
S: Serializer,
T: Display,
{
serializer.serialize_some(&value.to_string())
}
71 changes: 71 additions & 0 deletions proto/src/serializers/public_key.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
mod v0_34 {
use crate::v0_34::crypto::{public_key, PublicKey};
use serde::{Deserialize, Deserializer, Serialize, Serializer};

impl<'de> Deserialize<'de> for PublicKey {
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
where
D: Deserializer<'de>,
{
let sum = Option::<public_key::Sum>::deserialize(deserializer)?;
Ok(Self { sum })
}
}

impl Serialize for PublicKey {
fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
where
S: Serializer,
{
self.sum.serialize(serializer)
}
}
}

mod v0_37 {
use crate::v0_37::crypto::{public_key, PublicKey};
use serde::{Deserialize, Deserializer, Serialize, Serializer};

impl<'de> Deserialize<'de> for PublicKey {
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
where
D: Deserializer<'de>,
{
let sum = Option::<public_key::Sum>::deserialize(deserializer)?;
Ok(Self { sum })
}
}

impl Serialize for PublicKey {
fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
where
S: Serializer,
{
self.sum.serialize(serializer)
}
}
}

mod v0_38 {
use crate::v0_38::crypto::{public_key, PublicKey};
use serde::{Deserialize, Deserializer, Serialize, Serializer};

impl<'de> Deserialize<'de> for PublicKey {
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
where
D: Deserializer<'de>,
{
let sum = Option::<public_key::Sum>::deserialize(deserializer)?;
Ok(Self { sum })
}
}

impl Serialize for PublicKey {
fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
where
S: Serializer,
{
self.sum.serialize(serializer)
}
}
}
8 changes: 7 additions & 1 deletion tendermint/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ define_error! {

InvalidKey
{ detail: String }
|e| { format_args!("invalid key: {e}") },
|e| { format_args!("invalid key: {}", e.detail) },

Length
|_| { format_args!("length error") },
Expand Down Expand Up @@ -229,6 +229,12 @@ define_error! {
NegativeProofIndex
[ DisplayOnly<TryFromIntError> ]
|_| { "negative item index in proof" },

TotalVotingPowerMismatch
|_| { "total voting power in validator set does not match the sum of participants' powers" },

TotalVotingPowerOverflow
|_| { "total voting power in validator set exceeds the allowed maximum" },
}
}

Expand Down
Loading