Fix abci_query RPC endpoint

tendermint/Cargo.toml

@@ -24,6 +24,9 @@ authors = [
     "Thane Thomson <thane@interchain.io>"
 ]
 
+[package.metadata.docs.rs]
+all-features = true
+
 [badges]
 circle-ci = { repository = "interchainio/tendermint-rs" }
 
@@ -49,6 +52,3 @@ zeroize = { version = "1.0", features = ["zeroize_derive"] }
 
 [dev-dependencies]
 serde_json = "1"
-
-[package.metadata.docs.rs]
-all-features = true

tendermint/src/amino_types/proposal.rs

@@ -1,3 +1,4 @@
+use std::convert::TryFrom;
 use super::{
     block_id::{BlockId, CanonicalBlockId, CanonicalPartSetHeader},
     remote_error::RemoteError,
@@ -35,7 +36,7 @@ pub struct Proposal {
 // TODO(tony): custom derive proc macro for this e.g. `derive(ParseBlockHeight)`
 impl block::ParseHeight for Proposal {
     fn parse_block_height(&self) -> Result<block::Height, Error> {
-        block::Height::try_from_i64(self.height)
+        block::Height::try_from(self.height)
     }
 }
 
@@ -74,7 +75,7 @@ impl chain::ParseId for CanonicalProposal {
 
 impl block::ParseHeight for CanonicalProposal {
     fn parse_block_height(&self) -> Result<block::Height, Error> {
-        block::Height::try_from_i64(self.height)
+        block::Height::try_from(self.height)
     }
 }
 
@@ -136,7 +137,7 @@ impl SignableMsg for SignProposalRequest {
     fn consensus_state(&self) -> Option<consensus::State> {
         match self.proposal {
             Some(ref p) => Some(consensus::State {
-                height: match block::Height::try_from_i64(p.height) {
+                height: match block::Height::try_from(p.height) {
                     Ok(h) => h,
                     Err(_err) => return None, // TODO(tarcieri): return an error?
                 },

tendermint/src/amino_types/vote.rs

@@ -1,3 +1,4 @@
+use std::convert::TryFrom;
 use super::{
     block_id::{BlockId, CanonicalBlockId, CanonicalPartSetHeader},
     remote_error::RemoteError,
@@ -51,7 +52,7 @@ impl Vote {
 
 impl block::ParseHeight for Vote {
     fn parse_block_height(&self) -> Result<block::Height, Error> {
-        block::Height::try_from_i64(self.height)
+        block::Height::try_from(self.height)
     }
 }
 
@@ -97,7 +98,7 @@ impl chain::ParseId for CanonicalVote {
 
 impl block::ParseHeight for CanonicalVote {
     fn parse_block_height(&self) -> Result<block::Height, Error> {
-        block::Height::try_from_i64(self.height)
+        block::Height::try_from(self.height)
     }
 }
 
@@ -162,7 +163,7 @@ impl SignableMsg for SignVoteRequest {
     fn consensus_state(&self) -> Option<consensus::State> {
         match self.vote {
             Some(ref v) => Some(consensus::State {
-                height: match block::Height::try_from_i64(v.height) {
+                height: match block::Height::try_from(v.height) {
                     Ok(h) => h,
                     Err(_err) => return None, // TODO(tarcieri): return an error?
                 },

tendermint/src/block/height.rs

@@ -1,33 +1,19 @@
 use crate::error::{Error, ErrorKind};
 use serde::{de::Error as _, Deserialize, Deserializer, Serialize, Serializer};
 use std::{
+    convert::TryFrom,
     fmt::{self, Debug, Display},
     str::FromStr,
 };
 
 /// Block height for a particular chain (i.e. number of blocks created since
 /// the chain began)
+/// 
+/// A height of 0 represents a chain which has not yet produced a block.
 #[derive(Copy, Clone, Eq, Hash, PartialEq, PartialOrd, Ord)]
-pub struct Height(u64);
+pub struct Height(pub u64);
 
 impl Height {
-    /// Convert `u64` to block height.
-    ///
-    /// Note that 0 is not a valid block height.
-    pub fn try_from_u64(n: u64) -> Result<Self, Error> {
-        // Minimum height is 1
-        if n > 0 {
-            Ok(Height(n))
-        } else {
-            Err(ErrorKind::OutOfRange.into())
-        }
-    }
-
-    /// Convert `i64` (used in e.g. Amino messages) to block height.
-    pub fn try_from_i64(n: i64) -> Result<Self, Error> {
-        Self::try_from_u64(n as u64)
-    }
-
     /// Get inner integer value. Alternative to `.0` or `.into()`
     pub fn value(self) -> u64 {
         self.0
@@ -57,15 +43,21 @@ impl Display for Height {
     }
 }
 
-impl From<i64> for Height {
-    fn from(n: i64) -> Height {
-        Self::try_from_i64(n).unwrap()
+impl TryFrom<i64> for Height {
+    type Error = Error;
+
+    fn try_from(n: i64) -> Result<Height, Error> {
+        if n >= 0 {
+            Ok(Height(n as u64))
+        } else {
+            Err(ErrorKind::OutOfRange.into())
+        }
     }
 }
 
 impl From<u64> for Height {
     fn from(n: u64) -> Height {
-        Self::try_from_u64(n).unwrap()
+        Height(n)
     }
 }
 
@@ -85,7 +77,7 @@ impl FromStr for Height {
     type Err = Error;
 
     fn from_str(s: &str) -> Result<Self, Error> {
-        Self::try_from_u64(s.parse::<u64>().map_err(|_| ErrorKind::Parse)?)
+        Ok(s.parse::<u64>().map_err(|_| ErrorKind::Parse)?.into())
     }
 }
 

tendermint/src/rpc/endpoint/abci_query.rs

@@ -13,6 +13,10 @@ pub struct Request {
     path: Option<Path>,
 
     /// Data to query
+    #[serde(
+        serialize_with = "serializers::serialize_hex",
+        deserialize_with = "serializers::parse_hex"
+    )]
     data: Vec<u8>,
 
     /// Block height
@@ -74,12 +78,20 @@ pub struct AbciQuery {
     pub index: i64,
 
     /// Key
-    // TODO(tarcieri): parse to Vec<u8>?
-    pub key: String,
+    #[serde(
+        default,
+        serialize_with = "serializers::serialize_option_base64",
+        deserialize_with = "serializers::parse_option_base64"
+    )]
+    pub key: Option<Vec<u8>>,
 
     /// Value
-    // TODO(tarcieri): parse to Vec<u8>?
-    pub value: String,
+    #[serde(
+        default,
+        serialize_with = "serializers::serialize_option_base64",
+        deserialize_with = "serializers::parse_option_base64"
+    )]
+    pub value: Option<Vec<u8>>,
 
     /// Proof (if requested)
     pub proof: Option<Proof>,

tendermint/src/serializers.rs

@@ -2,6 +2,7 @@
 
 use serde::{de::Error as _, Deserialize, Deserializer, Serialize, Serializer};
 use std::time::Duration;
+use subtle_encoding::{base64, hex};
 
 /// Parse `i64` from a JSON string
 pub(crate) fn parse_i64<'de, D>(deserializer: D) -> Result<i64, D::Error>
@@ -61,3 +62,67 @@ where
 {
     format!("{}", duration.as_nanos()).serialize(serializer)
 }
+
+pub(crate) fn serialize_hex<S, T>(bytes: T, serializer: S) -> Result<S::Ok, S::Error>
+    where
+        S: Serializer,
+        T: AsRef<[u8]>
+{
+    use serde::ser::Error;
+    let hex_bytes = hex::encode(bytes.as_ref());
+    let hex_string = String::from_utf8(hex_bytes)
+        .map_err(Error::custom)?;
+    serializer.serialize_str(&hex_string)
+}
+
+pub(crate) fn parse_hex<'de, D>(deserializer: D) -> Result<Vec<u8>, D::Error>
+    where D: Deserializer<'de>
+{
+    use serde::de::Error;
+    let string = String::deserialize(deserializer)?;
+    hex::decode(&string)
+        .map_err(Error::custom)
+}
+
+pub(crate) fn serialize_base64<S, T>(bytes: T, serializer: S) -> Result<S::Ok, S::Error>
+    where
+        S: Serializer,
+        T: AsRef<[u8]>
+{
+    use serde::ser::Error;
+    let base64_bytes = base64::encode(bytes.as_ref());
+    let base64_string = String::from_utf8(base64_bytes)
+        .map_err(Error::custom)?;
+    serializer.serialize_str(&base64_string)
+}
+
+pub(crate) fn parse_base64<'de, D>(deserializer: D) -> Result<Vec<u8>, D::Error>
+    where D: Deserializer<'de>
+{
+    use serde::de::Error;
+    let string = String::deserialize(deserializer)?;
+    base64::decode(&string)
+        .map_err(Error::custom)
+}
+
+pub(crate) fn serialize_option_base64<S>(maybe_bytes: &Option<Vec<u8>>, serializer: S) -> Result<S::Ok, S::Error>
+    where S: Serializer
+{
+    #[derive(Serialize)]
+    struct Wrapper<'a>(#[serde(serialize_with = "serialize_base64")] &'a Vec<u8>);
+
+    match maybe_bytes {
+        Some(bytes) => Wrapper(bytes).serialize(serializer),
+        None => maybe_bytes.serialize(serializer)
+    }
+}
+
+pub(crate) fn parse_option_base64<'de, D>(deserializer: D) -> Result<Option<Vec<u8>>, D::Error>
+    where D: Deserializer<'de>
+{
+    #[derive(Deserialize)]
+    struct Wrapper(#[serde(deserialize_with = "parse_base64")] Vec<u8>);
+
+    let v = Option::deserialize(deserializer)?;
+    Ok(v.map(|Wrapper(a)| a))
+}

tendermint/tests/integration.rs

@@ -2,8 +2,8 @@
 
 /// RPC integration tests.
 ///
-/// These are all ignored by default, since they test against running `gaiad`.
-/// They can be run using:
+/// These are all ignored by default, since they test against running
+/// `tendermint node --proxy_app=kvstore`. They can be run using:
 ///
 /// ```
 /// cargo test -- --ignored
@@ -28,7 +28,12 @@ mod rpc {
     #[test]
     #[ignore]
     fn abci_query() {
-        // TODO(tarcieri): write integration test for this endpoint
+        let key = "unpopulated_key".parse().unwrap();
+        let abci_query = localhost_rpc_client()
+            .abci_query(Some(key), vec![], None, false)
+            .unwrap();
+        assert_eq!(abci_query.key.as_ref().unwrap(), &Vec::<u8>::new());
+        assert_eq!(abci_query.value.as_ref(), None);
     }
 
     /// `/block` endpoint
@@ -94,8 +99,7 @@ mod rpc {
         // For lack of better things to test
         assert_eq!(
             status.validator_info.voting_power.value(),
-            0,
-            "don't integration test against a validator"
+            10
         );
     }
 }