abci: Change hashes' type from Bytes to tendermint::Hash or `tend…

…ermint::AppHash` (#1232)

* Use AppHash in a field of OfferSnapshot

* tendermint: Implement Default for AppHash

* Change last_block_app_hash field of Info to Hash

* Remove AppHash::value, add AppHash::as_bytes

The less cloney data accessor is better.

* Use AppHash in a field of response::InitChain

* Less allocatey deserializers for Hash

* Change app_hash field type in Genesis to AppHash

* Changelog entry for #1232

.changelog/unreleased/breaking-changes/1095-abci-change-bytes-to-hash.md

@@ -0,0 +1,13 @@
+- `[tendermint]` Change hash fields' type from `Bytes`
+  ([#1095](https://github.com/informalsystems/tendermint-rs/issues/1095)):
+
+  | Struct                         | Field                 | Type      |
+  | ------------------------------ | --------------------- | --------- |
+  | `abci::request::OfferSnapshot` | `app_hash`            | `AppHash` |
+  | `abci::response::Info`         | `last_block_app_hash` | `AppHash` |
+  | `abci::response::InitChain`    | `app_hash`            | `AppHash` |
+  | `Genesis`                      | `app_hash`            | `AppHash` |
+
+- `[tendermint]` Remove method `AppHash::value`,
+  replaced with non-allocating `AppHash::as_bytes`
+  [#1232](https://github.com/informalsystems/tendermint-rs/pull/1232).

rpc/tests/kvstore_fixtures.rs

@@ -322,7 +322,10 @@ fn incoming_fixtures() {
                 let result = endpoint::abci_info::Response::from_string(content).unwrap();
                 assert_eq!(result.response.app_version, 1);
                 assert_eq!(result.response.data, "{\"size\":0}");
-                assert_eq!(result.response.last_block_app_hash, b"AAAAAAAAAAA="[..]);
+                assert_eq!(
+                    result.response.last_block_app_hash.as_bytes(),
+                    b"AAAAAAAAAAA="
+                );
                 assert_eq!(result.response.version, "0.17.0");
             },
             "abci_query_with_existing_key" => {
@@ -370,7 +373,7 @@ fn incoming_fixtures() {
                 let result = endpoint::block::Response::from_string(content).unwrap();
                 assert!(result.block.data.get(0).is_none());
                 assert!(result.block.evidence.iter().next().is_none());
-                assert!(result.block.header.app_hash.value().is_empty());
+                assert!(result.block.header.app_hash.as_bytes().is_empty());
                 assert_eq!(result.block.header.chain_id.as_str(), CHAIN_ID);
                 assert!(!result.block.header.consensus_hash.is_empty());
                 assert_eq!(result.block.header.data_hash, empty_merkle_root_hash);
@@ -411,7 +414,7 @@ fn incoming_fixtures() {
                 let result = endpoint::block::Response::from_string(content).unwrap();
                 assert!(result.block.data.get(0).is_none());
                 assert!(result.block.evidence.iter().next().is_none());
-                assert_eq!(result.block.header.app_hash.value(), [0u8; 8]);
+                assert_eq!(result.block.header.app_hash.as_bytes(), &[0u8; 8]);
                 assert_eq!(result.block.header.chain_id.as_str(), CHAIN_ID);
                 assert!(!result.block.header.consensus_hash.is_empty());
                 assert_eq!(result.block.header.data_hash, empty_merkle_root_hash);
@@ -590,7 +593,7 @@ fn incoming_fixtures() {
                 assert!(result.signed_header.commit.signatures[0]
                     .validator_address()
                     .is_some());
-                assert_eq!(result.signed_header.header.app_hash.value(), [0u8; 8]);
+                assert_eq!(result.signed_header.header.app_hash.as_bytes(), [0u8; 8]);
                 assert_eq!(result.signed_header.header.chain_id.as_str(), CHAIN_ID);
                 assert!(!result.signed_header.header.consensus_hash.is_empty());
                 assert_eq!(
@@ -653,7 +656,7 @@ fn incoming_fixtures() {
                 let result =
                     endpoint::genesis::Response::<Option<serde_json::Value>>::from_string(content)
                         .unwrap();
-                assert!(result.genesis.app_hash.is_empty());
+                assert!(result.genesis.app_hash.as_bytes().is_empty());
                 assert_eq!(result.genesis.chain_id.as_str(), CHAIN_ID);
                 assert_eq!(result.genesis.consensus_params.block.max_bytes, 22020096);
                 assert_eq!(result.genesis.consensus_params.block.max_gas, -1);
@@ -740,7 +743,7 @@ fn incoming_fixtures() {
                 assert_eq!(result.node_info.version.to_string(), "0.34.21");
                 assert!(!result.sync_info.catching_up);
                 assert_eq!(
-                    result.sync_info.latest_app_hash.value(),
+                    result.sync_info.latest_app_hash.as_bytes(),
                     [6, 0, 0, 0, 0, 0, 0, 0]
                 );
                 assert!(!result.sync_info.latest_block_hash.is_empty());
@@ -765,14 +768,14 @@ fn incoming_fixtures() {
                     assert_eq!(block_meta.block_id.part_set_header.total, 1);
                     assert!(block_meta.block_size > 0);
                     if block_meta.header.height.value() == 1 {
-                        assert!(block_meta.header.app_hash.value().is_empty());
+                        assert!(block_meta.header.app_hash.as_bytes().is_empty());
                         assert_eq!(block_meta.header.data_hash, empty_merkle_root_hash);
                         assert_eq!(block_meta.header.evidence_hash, empty_merkle_root_hash);
                         assert!(block_meta.header.last_block_id.is_none());
                         assert_eq!(block_meta.header.last_commit_hash, empty_merkle_root_hash);
                         assert_eq!(block_meta.header.last_results_hash, empty_merkle_root_hash);
                     } else {
-                        assert!(!block_meta.header.app_hash.value().is_empty());
+                        assert!(!block_meta.header.app_hash.as_bytes().is_empty());
                         assert!(block_meta.header.data_hash.is_some());
                         assert!(block_meta.header.evidence_hash.is_some());
                         assert!(block_meta.header.last_block_id.is_some());
@@ -836,7 +839,7 @@ fn incoming_fixtures() {
                     let b = block.unwrap();
                     assert!(b.data.get(0).is_none());
                     assert!(b.evidence.iter().next().is_none());
-                    assert!(!b.header.app_hash.value().is_empty());
+                    assert!(!b.header.app_hash.as_bytes().is_empty());
                     assert_eq!(b.header.chain_id.as_str(), CHAIN_ID);
                     assert!(!b.header.consensus_hash.is_empty());
                     assert_eq!(b.header.data_hash, empty_merkle_root_hash);
@@ -891,7 +894,7 @@ fn incoming_fixtures() {
                     let b = block.unwrap();
                     assert!(b.data.get(0).is_none());
                     assert!(b.evidence.iter().next().is_none());
-                    assert!(!b.header.app_hash.value().is_empty());
+                    assert!(!b.header.app_hash.as_bytes().is_empty());
                     assert_eq!(b.header.chain_id.as_str(), CHAIN_ID);
                     assert!(!b.header.consensus_hash.is_empty());
                     assert_eq!(b.header.data_hash, empty_merkle_root_hash);
@@ -968,7 +971,7 @@ fn incoming_fixtures() {
                     let b = block.unwrap();
                     assert!(b.data.get(0).is_none());
                     assert!(b.evidence.iter().next().is_none());
-                    assert!(!b.header.app_hash.value().is_empty());
+                    assert!(!b.header.app_hash.as_bytes().is_empty());
                     assert_eq!(b.header.chain_id.as_str(), CHAIN_ID);
                     assert!(!b.header.consensus_hash.is_empty());
                     assert_eq!(b.header.data_hash, empty_merkle_root_hash);
@@ -1023,7 +1026,7 @@ fn incoming_fixtures() {
                     let b = block.unwrap();
                     assert!(b.data.get(0).is_none());
                     assert!(b.evidence.iter().next().is_none());
-                    assert!(!b.header.app_hash.value().is_empty());
+                    assert!(!b.header.app_hash.as_bytes().is_empty());
                     assert_eq!(b.header.chain_id.as_str(), CHAIN_ID);
                     assert!(!b.header.consensus_hash.is_empty());
                     assert_eq!(b.header.data_hash, empty_merkle_root_hash);
@@ -1078,7 +1081,7 @@ fn incoming_fixtures() {
                     let b = block.unwrap();
                     assert!(b.data.get(0).is_none());
                     assert!(b.evidence.iter().next().is_none());
-                    assert!(!b.header.app_hash.value().is_empty());
+                    assert!(!b.header.app_hash.as_bytes().is_empty());
                     assert_eq!(b.header.chain_id.as_str(), CHAIN_ID);
                     assert!(!b.header.consensus_hash.is_empty());
                     assert_eq!(b.header.data_hash, empty_merkle_root_hash);

tendermint/src/abci/request/offer_snapshot.rs

@@ -1,19 +1,16 @@
-use bytes::Bytes;
-
 use super::super::types::Snapshot;
 // bring into scope for doc links
 #[allow(unused)]
 use super::ApplySnapshotChunk;
-use crate::prelude::*;
+use crate::{prelude::*, AppHash};
 
 #[doc = include_str!("../doc/request-offersnapshot.md")]
 #[derive(Clone, PartialEq, Eq, Debug)]
 pub struct OfferSnapshot {
     /// The snapshot offered for restoration.
     pub snapshot: Snapshot,
     /// The light client verified app hash for this height.
-    // XXX(hdevalence): replace with apphash
-    pub app_hash: Bytes,
+    pub app_hash: AppHash,
 }
 
 // =============================================================================
@@ -28,7 +25,7 @@ impl From<OfferSnapshot> for pb::RequestOfferSnapshot {
     fn from(offer_snapshot: OfferSnapshot) -> Self {
         Self {
             snapshot: Some(offer_snapshot.snapshot.into()),
-            app_hash: offer_snapshot.app_hash,
+            app_hash: offer_snapshot.app_hash.into(),
         }
     }
 }
@@ -42,7 +39,7 @@ impl TryFrom<pb::RequestOfferSnapshot> for OfferSnapshot {
                 .snapshot
                 .ok_or_else(crate::Error::missing_data)?
                 .try_into()?,
-            app_hash: offer_snapshot.app_hash,
+            app_hash: offer_snapshot.app_hash.try_into()?,
         })
     }
 }

tendermint/src/abci/response/info.rs

@@ -1,9 +1,8 @@
-use crate::{block, prelude::*, Error};
+use crate::{block, prelude::*, AppHash, Error};
 use core::convert::TryFrom;
 use tendermint_proto::abci as pb;
 use tendermint_proto::Protobuf;
 
-use bytes::Bytes;
 use serde::{Deserialize, Serialize};
 
 #[doc = include_str!("../doc/response-info.md")]
@@ -19,8 +18,7 @@ pub struct Info {
     /// The latest block for which the app has called [`Commit`](super::super::Request::Commit).
     pub last_block_height: block::Height,
     /// The latest result of [`Commit`](super::super::Request::Commit).
-    // XXX(hdevalence): fix this, should be apphash?
-    pub last_block_app_hash: Bytes,
+    pub last_block_app_hash: AppHash,
 }
 
 // =============================================================================
@@ -34,7 +32,7 @@ impl From<Info> for pb::ResponseInfo {
             version: info.version,
             app_version: info.app_version,
             last_block_height: info.last_block_height.into(),
-            last_block_app_hash: info.last_block_app_hash,
+            last_block_app_hash: info.last_block_app_hash.into(),
         }
     }
 }
@@ -48,7 +46,7 @@ impl TryFrom<pb::ResponseInfo> for Info {
             version: info.version,
             app_version: info.app_version,
             last_block_height: info.last_block_height.try_into()?,
-            last_block_app_hash: info.last_block_app_hash,
+            last_block_app_hash: info.last_block_app_hash.try_into()?,
         })
     }
 }

tendermint/src/abci/response/init_chain.rs

@@ -1,4 +1,4 @@
-use bytes::Bytes;
+use crate::AppHash;
 
 use crate::{consensus, prelude::*, validator};
 
@@ -17,7 +17,7 @@ pub struct InitChain {
     /// [`request::InitChain::validators`](super::super::request::InitChain::validators).
     pub validators: Vec<validator::Update>,
     /// Initial application hash.
-    pub app_hash: Bytes,
+    pub app_hash: AppHash,
 }
 
 // =============================================================================
@@ -33,7 +33,7 @@ impl From<InitChain> for pb::ResponseInitChain {
         Self {
             consensus_params: init_chain.consensus_params.map(Into::into),
             validators: init_chain.validators.into_iter().map(Into::into).collect(),
-            app_hash: init_chain.app_hash,
+            app_hash: init_chain.app_hash.into(),
         }
     }
 }
@@ -52,7 +52,7 @@ impl TryFrom<pb::ResponseInitChain> for InitChain {
                 .into_iter()
                 .map(TryInto::try_into)
                 .collect::<Result<_, _>>()?,
-            app_hash: init_chain.app_hash,
+            app_hash: init_chain.app_hash.try_into()?,
         })
     }
 }

tendermint/src/genesis.rs

@@ -2,7 +2,7 @@
 
 use serde::{Deserialize, Serialize};
 
-use crate::{chain, consensus, prelude::*, serializers, validator, Time};
+use crate::{chain, consensus, prelude::*, serializers, validator, AppHash, Time};
 
 /// Genesis data
 #[derive(Clone, Debug, Serialize, Deserialize)]
@@ -25,8 +25,8 @@ pub struct Genesis<AppState = serde_json::Value> {
     pub validators: Vec<validator::Info>,
 
     /// App hash
-    #[serde(with = "serializers::bytes::hexstring")]
-    pub app_hash: Vec<u8>,
+    #[serde(with = "serializers::apphash")]
+    pub app_hash: AppHash,
 
     /// App state
     pub app_state: AppState,

tendermint/src/hash.rs

@@ -172,12 +172,12 @@ impl FromStr for Hash {
 // Serialization is used in light-client config
 impl<'de> Deserialize<'de> for Hash {
     fn deserialize<D: Deserializer<'de>>(deserializer: D) -> Result<Self, D::Error> {
-        let hex = String::deserialize(deserializer)?;
+        let hex = <&str>::deserialize(deserializer)?;
 
         if hex.is_empty() {
             Err(D::Error::custom("empty hash"))
         } else {
-            Ok(Self::from_str(&hex).map_err(|e| D::Error::custom(format!("{}", e)))?)
+            Ok(Self::from_str(hex).map_err(|e| D::Error::custom(format!("{}", e)))?)
         }
     }
 }
@@ -206,13 +206,13 @@ pub mod allow_empty {
     where
         D: Deserializer<'de>,
     {
-        let hex = String::deserialize(deserializer)?;
-        Hash::from_str(&hex).map_err(serde::de::Error::custom)
+        let hex = <&str>::deserialize(deserializer)?;
+        Hash::from_str(hex).map_err(serde::de::Error::custom)
     }
 }
 
 /// AppHash is usually a SHA256 hash, but in reality it can be any kind of data
-#[derive(Clone, PartialEq, Eq)]
+#[derive(Clone, PartialEq, Eq, Default)]
 pub struct AppHash(Vec<u8>);
 
 impl Protobuf<Vec<u8>> for AppHash {}
@@ -230,10 +230,23 @@ impl From<AppHash> for Vec<u8> {
     }
 }
 
+impl TryFrom<Bytes> for AppHash {
+    type Error = Error;
+
+    fn try_from(value: Bytes) -> Result<Self, Self::Error> {
+        Ok(AppHash(value.into()))
+    }
+}
+impl From<AppHash> for Bytes {
+    fn from(value: AppHash) -> Self {
+        value.0.into()
+    }
+}
+
 impl AppHash {
-    /// Return AppHash value as `Vec<u8>`
-    pub fn value(&self) -> Vec<u8> {
-        self.0.clone()
+    /// Return the hash bytes as a byte slice.
+    pub fn as_bytes(&self) -> &[u8] {
+        &self.0
     }
 
     /// Decode a `Hash` from upper-case hexadecimal