Rebase ABCI Domain Types onto v0.34.20 branch.

There are two issues to fix: * The original ABCI domain types work used `chrono`; this temporarily re-adds `chrono` as a dependency, so that the changes that eliminated it can be replayed on top of this rebasing. * The original ABCI domain types work was, for some reason, mixed in with changes to the RPC test harness for 0.35 compatibility, and that may break this code. I didn't attempt to fix this, because I didn't touch the RPC tests when writing the ABCI domain types. Original concatenation of commit messages follows: Add domain types (updated) and fix integration tests for Tendermint v0.35.0 (informalsystems#1022) * tendermint: add From<Infallible> for Error * Use hex for AppHash Debug formatting * Regenerate protobuf types using tools/proto-compiler. * Regenerate protos against tendermint v0.35.0-rc3. * Remove SetOption-related code in tendermint-abci. * Update imports to reflect protobuf movements. The BlockParams and ConsensusParams structs moved out of the ABCI protos. * Update tendermint-abci to reflect upstream proto changes. * p2p: treat all non-Ed25519 keys as unsupported This fixes a compile error introduced by upstream proto changes that add an Sr25519 variant. * Improve ABCI response code modeling with NonZeroU32 The previous data modeling allowed a user to construct an `Err(0)` value that would be serialized and deserialized as `Ok`. * Use the Bytes type in ABCI protos. * Add conversions between protobuf and chrono types. * tendermint: define Serde for Block in terms of RawBlock This changes the Serialize/Deserialize implementations for Block to convert to/from the proto-generated `RawBlock`, and use the derived serialization for that type. This is much cleaner and more maintainable than keeping Serde annotations for each sub-member of the data structure, because all of the serialization code is kept in one place, and there's only one validation path (the TryFrom conversion from RawBlock to Block) that applies to both kinds of serialization. * tendermint: simpler transaction modeling in Block This changes the Block type to hold the transactions as a plain `Vec<Vec<u8>>`, rather than as a custom `abci::transaction::Data` type (which is itself a wrapper for an `Option<Vec<Transaction>>`, the `Transaction` type being a wrapper for a `Vec<u8>`). This also updates the `Block` getter functions; it's not clear (to me) why they're there, since they access the public fields and aren't used anywhere else. * rpc: take over tendermint::abci The existing contents of the `tendermint::abci` module note that they're only for the purpose of implementing the RPC endpoints, not a general ABCI implementation. Moving that code from the `tendermint` crate to the `tendermint-rpc` crate decouples the RPC interface from improvements to the ABCI domain modeling. Eventually, it would be good to eliminate this code and align it with the new ABCI domain types. * tendermint: add ABCI domain types. These types mirror the generated types in tendermint_proto, but have better naming. The documentation for each request type is adapted from the ABCI Methods and Types spec document. However, the same logical request may appear three times, as a struct with the request data, as a Request variant, and as a CategoryRequest variant. To avoid duplication, this PR uses the `#[doc = include_str!(...)]` functionality stabilized in Rust 1.54 to keep common definitions of the documentation. * tendermint: eliminate &'static str errors in ABCI domain types. * Merge `abci::params::ConsensusParams` with `consensus::Params`. The code in the `abci` module had more complete documentation from the ABCI docs, so I copied it onto the existing structure. * Add hex encoding Serde attribute to Sr25519 keys. * Replace integers with `block::Height`, `vote::Power` * Replace integer with block::Round * Fix tools build by using correct imports Signed-off-by: Thane Thomson <connect@thanethomson.com> * Fix clippy complaints in tools Signed-off-by: Thane Thomson <connect@thanethomson.com> * Fix clippy warning Signed-off-by: Thane Thomson <connect@thanethomson.com> * Fix clippy lints Signed-off-by: Thane Thomson <connect@thanethomson.com> * Fix more clippy lints Signed-off-by: Thane Thomson <connect@thanethomson.com> * Fix deprecation notices from ed25519 crate Signed-off-by: Thane Thomson <connect@thanethomson.com> * Regenerate protos for Tendermint v0.35.0 Signed-off-by: Thane Thomson <connect@thanethomson.com> * Fix raw bytes conversion in tests/docs Signed-off-by: Thane Thomson <connect@thanethomson.com> * Add Tendermint v0.35.0 Docker config Signed-off-by: Thane Thomson <connect@thanethomson.com> * Add Tendermint v0.35.0 Docker image for ABCI integration testing Signed-off-by: Thane Thomson <connect@thanethomson.com> * Remove RPC deserialization tests The support files for these tests were manually generated some time ago. We should rather favour testing with the `kvstore_fixtures` tests, whose fixtures are automatically generated by our rpc-probe tool. Signed-off-by: Thane Thomson <connect@thanethomson.com> * Reformat Docker folder readme Signed-off-by: Thane Thomson <connect@thanethomson.com> * Bump version of Tendermint used in ABCI integration test Signed-off-by: Thane Thomson <connect@thanethomson.com> * Bump version of Tendermint used in rpc probe Signed-off-by: Thane Thomson <connect@thanethomson.com> * Bump version of Tendermint used in kvstore integration test Signed-off-by: Thane Thomson <connect@thanethomson.com> * Bump version of Tendermint used in proto-compiler Signed-off-by: Thane Thomson <connect@thanethomson.com> * Regenerate kvstore fixtures with rpc-probe for Tendermint v0.35.0 Signed-off-by: Thane Thomson <connect@thanethomson.com> * Update kvstore integration test to accommodate newly generated fixtures Signed-off-by: Thane Thomson <connect@thanethomson.com> * Update RPC tests and data structures to accommodate Tendermint v0.35.0 changes Signed-off-by: Thane Thomson <connect@thanethomson.com> * Update ABCI encoding scheme to accommodate breaking change in tendermint/tendermint#5783 Signed-off-by: Thane Thomson <connect@thanethomson.com> * Bump Tendermint version in GitHub Actions kvstore integration test Signed-off-by: Thane Thomson <connect@thanethomson.com> * Add changelog entries to capture breaking changes Signed-off-by: Thane Thomson <connect@thanethomson.com> * Change tx hash encoding from base64 to hex and update tests Signed-off-by: Thane Thomson <connect@thanethomson.com> * Add changelog entry for /tx endpoint change Signed-off-by: Thane Thomson <connect@thanethomson.com> Co-authored-by: Henry de Valence <hdevalence@penumbra.zone> Co-authored-by: Henry de Valence <hdevalence@hdevalence.ca>
penumbra-zone · Sep 20, 2022 · 84367a6 · 84367a6
1 parent 2f2e868
commit 84367a6
Show file tree

Hide file tree

Showing 218 changed files with 5,094 additions and 2,607 deletions.
diff --git a/.changelog/unreleased/breaking-changes/862-035-tendermint.md b/.changelog/unreleased/breaking-changes/862-035-tendermint.md
@@ -0,0 +1,2 @@
+- Updated integration testing to test against Tendermint v0.35.0
+  ([#862](https://github.com/informalsystems/tendermint-rs/issues/862))
diff --git a/.changelog/unreleased/breaking-changes/862-abci-domain-types.md b/.changelog/unreleased/breaking-changes/862-abci-domain-types.md
@@ -0,0 +1,2 @@
+- `[tendermint]` Added domain types for ABCI
+  ([#862](https://github.com/informalsystems/tendermint-rs/issues/862))
diff --git a/.changelog/unreleased/breaking-changes/862-abci-wire-protocol.md b/.changelog/unreleased/breaking-changes/862-abci-wire-protocol.md
@@ -0,0 +1,3 @@
+- `[tendermint-abci]` Changed low-level wire encoding protocol to
+  accommodate <https://github.com/tendermint/tendermint/issues/5783>
+  ([#862](https://github.com/informalsystems/tendermint-rs/issues/862))
diff --git a/.changelog/unreleased/breaking-changes/862-rpc-event-events.md b/.changelog/unreleased/breaking-changes/862-rpc-event-events.md
@@ -0,0 +1,5 @@
+- `[tendermint-rpc]` The `event::Event::events` field is now represented as
+  `Option<Vec<crate::abci::Event>>` as opposed to `Option<HashMap<String,
+  Vec<String>>>` to accommodate breaking change in Tendermint v0.35.0
+  subscription interface ([#862](https://github.com/informalsystems/tendermint-
+  rs/issues/862))
diff --git a/.changelog/unreleased/breaking-changes/862-rpc-tx-hash-hex.md b/.changelog/unreleased/breaking-changes/862-rpc-tx-hash-hex.md
@@ -0,0 +1,3 @@
+- `[tendermint-rpc]` The `/tx` endpoint now encodes
+  the `hash` parameter as hexadecimal instead of base64
+  ([#862](https://github.com/informalsystems/tendermint-rs/issues/862))
diff --git a/abci/src/application.rs b/abci/src/application.rs
@@ -8,11 +8,11 @@ pub mod kvstore;
 use tendermint_proto::abci::{
     request::Value, response, Request, RequestApplySnapshotChunk, RequestBeginBlock,
     RequestCheckTx, RequestDeliverTx, RequestEcho, RequestEndBlock, RequestInfo, RequestInitChain,
-    RequestLoadSnapshotChunk, RequestOfferSnapshot, RequestQuery, RequestSetOption, Response,
+    RequestLoadSnapshotChunk, RequestOfferSnapshot, RequestQuery, Response,
     ResponseApplySnapshotChunk, ResponseBeginBlock, ResponseCheckTx, ResponseCommit,
     ResponseDeliverTx, ResponseEcho, ResponseEndBlock, ResponseFlush, ResponseInfo,
     ResponseInitChain, ResponseListSnapshots, ResponseLoadSnapshotChunk, ResponseOfferSnapshot,
-    ResponseQuery, ResponseSetOption,
+    ResponseQuery,
 };
 
 /// An ABCI application.
@@ -76,12 +76,6 @@ pub trait Application: Send + Clone + 'static {
         Default::default()
     }
 
-    /// Allows the Tendermint node to request that the application set an
-    /// option to a particular value.
-    fn set_option(&self, _request: RequestSetOption) -> ResponseSetOption {
-        Default::default()
-    }
-
     /// Used during state sync to discover available snapshots on peers.
     fn list_snapshots(&self) -> ResponseListSnapshots {
         Default::default()
@@ -123,7 +117,6 @@ impl<A: Application> RequestDispatcher for A {
                 Value::Echo(req) => response::Value::Echo(self.echo(req)),
                 Value::Flush(_) => response::Value::Flush(self.flush()),
                 Value::Info(req) => response::Value::Info(self.info(req)),
-                Value::SetOption(req) => response::Value::SetOption(self.set_option(req)),
                 Value::InitChain(req) => response::Value::InitChain(self.init_chain(req)),
                 Value::Query(req) => response::Value::Query(self.query(req)),
                 Value::BeginBlock(req) => response::Value::BeginBlock(self.begin_block(req)),
@@ -141,6 +134,7 @@ impl<A: Application> RequestDispatcher for A {
                 Value::ApplySnapshotChunk(req) => {
                     response::Value::ApplySnapshotChunk(self.apply_snapshot_chunk(req))
                 },
+                Value::SetOption(_) => response::Value::SetOption(Default::default()),
             }),
         }
     }

diff --git a/abci/src/application/kvstore.rs b/abci/src/application/kvstore.rs
@@ -50,15 +50,15 @@ use crate::{
 /// // Deliver a transaction and then commit the transaction
 /// client
 ///     .deliver_tx(RequestDeliverTx {
-///         tx: "test-key=test-value".as_bytes().to_owned(),
+///         tx: "test-key=test-value".into(),
 ///     })
 ///     .unwrap();
 /// client.commit().unwrap();
 ///
 /// // We should be able to query for the data we just delivered above
 /// let res = client
 ///     .query(RequestQuery {
-///         data: "test-key".as_bytes().to_owned(),
+///         data: "test-key".into(),
 ///         path: "".to_string(),
 ///         height: 0,
 ///         prove: false,
@@ -129,25 +129,25 @@ impl Application for KeyValueStoreApp {
             version: "0.1.0".to_string(),
             app_version: 1,
             last_block_height,
-            last_block_app_hash,
+            last_block_app_hash: last_block_app_hash.into(),
         }
     }
 
     fn query(&self, request: RequestQuery) -> ResponseQuery {
-        let key = match String::from_utf8(request.data.clone()) {
+        let key = match std::str::from_utf8(&request.data) {
             Ok(s) => s,
             Err(e) => panic!("Failed to intepret key as UTF-8: {}", e),
         };
         debug!("Attempting to get key: {}", key);
-        match self.get(key.clone()) {
+        match self.get(key) {
             Ok((height, value_opt)) => match value_opt {
                 Some(value) => ResponseQuery {
                     code: 0,
                     log: "exists".to_string(),
                     info: "".to_string(),
                     index: 0,
                     key: request.data,
-                    value: value.into_bytes(),
+                    value: value.into_bytes().into(),
                     proof_ops: None,
                     height,
                     codespace: "".to_string(),
@@ -158,7 +158,7 @@ impl Application for KeyValueStoreApp {
                     info: "".to_string(),
                     index: 0,
                     key: request.data,
-                    value: vec![],
+                    value: Default::default(),
                     proof_ops: None,
                     height,
                     codespace: "".to_string(),
@@ -171,7 +171,7 @@ impl Application for KeyValueStoreApp {
     fn check_tx(&self, _request: RequestCheckTx) -> ResponseCheckTx {
         ResponseCheckTx {
             code: 0,
-            data: vec![],
+            data: Default::default(),
             log: "".to_string(),
             info: "".to_string(),
             gas_wanted: 1,
@@ -183,17 +183,17 @@ impl Application for KeyValueStoreApp {
     }
 
     fn deliver_tx(&self, request: RequestDeliverTx) -> ResponseDeliverTx {
-        let tx = String::from_utf8(request.tx).unwrap();
+        let tx = std::str::from_utf8(&request.tx).unwrap();
         let tx_parts = tx.split('=').collect::<Vec<&str>>();
         let (key, value) = if tx_parts.len() == 2 {
             (tx_parts[0], tx_parts[1])
         } else {
-            (tx.as_ref(), tx.as_ref())
+            (tx, tx)
         };
         let _ = self.set(key, value).unwrap();
         ResponseDeliverTx {
             code: 0,
-            data: vec![],
+            data: Default::default(),
             log: "".to_string(),
             info: "".to_string(),
             gas_wanted: 0,
@@ -202,18 +202,18 @@ impl Application for KeyValueStoreApp {
                 r#type: "app".to_string(),
                 attributes: vec![
                     EventAttribute {
-                        key: "key".as_bytes().to_owned(),
-                        value: key.as_bytes().to_owned(),
+                        key: "key".to_string().into_bytes().into(),
+                        value: key.to_string().into_bytes().into(),
                         index: true,
                     },
                     EventAttribute {
-                        key: "index_key".as_bytes().to_owned(),
-                        value: "index is working".as_bytes().to_owned(),
+                        key: "index_key".to_string().into_bytes().into(),
+                        value: "index is working".to_string().into_bytes().into(),
                         index: true,
                     },
                     EventAttribute {
-                        key: "noindex_key".as_bytes().to_owned(),
-                        value: "index is working".as_bytes().to_owned(),
+                        key: "noindex_key".to_string().into_bytes().into(),
+                        value: "index is working".to_string().into_bytes().into(),
                         index: false,
                     },
                 ],
@@ -228,7 +228,7 @@ impl Application for KeyValueStoreApp {
         let (height, app_hash) = channel_recv(&result_rx).unwrap();
         info!("Committed height {}", height);
         ResponseCommit {
-            data: app_hash,
+            data: app_hash.into(),
             retain_height: height - 1,
         }
     }
@@ -285,7 +285,7 @@ impl KeyValueStoreDriver {
         // As in the Go-based key/value store, simply encode the number of
         // items as the "app hash"
         let mut app_hash = BytesMut::with_capacity(MAX_VARINT_LENGTH);
-        encode_varint(self.store.len() as u64, &mut app_hash);
+        prost::encoding::encode_varint(self.store.len() as u64, &mut app_hash);
         self.app_hash = app_hash.to_vec();
         self.height += 1;
         channel_send(&result_tx, (self.height, self.app_hash.clone()))

diff --git a/abci/src/client.rs b/abci/src/client.rs
@@ -113,11 +113,6 @@ impl Client {
         perform!(self, Commit, RequestCommit {})
     }
 
-    /// Request that the application set an option to a particular value.
-    pub fn set_option(&mut self, req: RequestSetOption) -> Result<ResponseSetOption, Error> {
-        perform!(self, SetOption, req)
-    }
-
     /// Used during state sync to discover available snapshots on peers.
     pub fn list_snapshots(&mut self) -> Result<ResponseListSnapshots, Error> {
         perform!(self, ListSnapshots, RequestListSnapshots {})

diff --git a/abci/src/codec.rs b/abci/src/codec.rs
@@ -130,7 +130,7 @@ where
     message.encode(&mut buf).map_err(Error::encode)?;
 
     let buf = buf.freeze();
-    encode_varint(buf.len() as u64, &mut dst);
+    prost::encoding::encode_varint(buf.len() as u64, &mut dst);
     dst.put(buf);
     Ok(())
 }
@@ -142,11 +142,11 @@ where
 {
     let src_len = src.len();
     let mut tmp = src.clone().freeze();
-    let encoded_len = match decode_varint(&mut tmp) {
+    let encoded_len = match prost::encoding::decode_varint(&mut tmp) {
         Ok(len) => len,
         // We've potentially only received a partial length delimiter
         Err(_) if src_len <= MAX_VARINT_LENGTH => return Ok(None),
-        Err(e) => return Err(e),
+        Err(e) => return Err(Error::decode(e)),
     };
     let remaining = tmp.remaining() as u64;
     if remaining < encoded_len {
@@ -164,14 +164,3 @@ where
         Ok(Some(res))
     }
 }
-
-// encode_varint and decode_varint will be removed once
-// https://github.com/tendermint/tendermint/issues/5783 lands in Tendermint.
-pub fn encode_varint<B: BufMut>(val: u64, mut buf: &mut B) {
-    prost::encoding::encode_varint(val << 1, &mut buf);
-}
-
-pub fn decode_varint<B: Buf>(mut buf: &mut B) -> Result<u64, Error> {
-    let len = prost::encoding::decode_varint(&mut buf).map_err(Error::decode)?;
-    Ok(len >> 1)
-}
diff --git a/abci/tests/kvstore_app.rs b/abci/tests/kvstore_app.rs
@@ -25,19 +25,19 @@ mod kvstore_app_integration {
 
         client
             .deliver_tx(RequestDeliverTx {
-                tx: "test-key=test-value".as_bytes().to_owned(),
+                tx: "test-key=test-value".into(),
             })
             .unwrap();
         client.commit().unwrap();
 
         let res = client
             .query(RequestQuery {
-                data: "test-key".as_bytes().to_owned(),
+                data: "test-key".into(),
                 path: "".to_string(),
                 height: 0,
                 prove: false,
             })
             .unwrap();
-        assert_eq!(res.value, "test-value".as_bytes().to_owned());
+        assert_eq!(res.value, "test-value".as_bytes());
     }
 }
diff --git a/config/src/prelude.rs b/config/src/prelude.rs
@@ -1,7 +1,7 @@
 // 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::{
+    borrow::ToOwned,
     boxed::Box,
     format,
     string::{String, ToString},

diff --git a/light-client-verifier/src/prelude.rs b/light-client-verifier/src/prelude.rs
@@ -1,7 +1,7 @@
 // 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::{
+    borrow::ToOwned,
     boxed::Box,
     format,
     string::{String, ToString},

diff --git a/light-client/src/evidence.rs b/light-client/src/evidence.rs
@@ -1,8 +1,8 @@
 //! Fork evidence data structures and interfaces.
 
 use contracts::contract_trait;
-use tendermint::abci::transaction::Hash;
 pub use tendermint::evidence::Evidence;
+use tendermint_rpc::abci::transaction::Hash;
 
 use crate::{components::io::IoError, verifier::types::PeerId};
 

diff --git a/light-client/src/tests.rs b/light-client/src/tests.rs
@@ -5,11 +5,11 @@ use std::{collections::HashMap, time::Duration};
 use contracts::contract_trait;
 use serde::{Deserialize, Serialize};
 use tendermint::{
-    abci::transaction::Hash,
     block::Height as HeightStr,
     evidence::{Duration as DurationStr, Evidence},
 };
 use tendermint_rpc as rpc;
+use tendermint_rpc::abci::transaction::Hash;
 
 use crate::{
     components::{

diff --git a/proto/Cargo.toml b/proto/Cargo.toml
@@ -19,7 +19,7 @@ all-features = true
 [dependencies]
 prost = { version = "0.11", default-features = false }
 prost-types = { version = "0.11", default-features = false }
-bytes = { version = "1.0", default-features = false }
+bytes = { version = "1.0", default-features = false, features = ["serde"]}
 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", "alloc"] }
@@ -28,6 +28,8 @@ num-derive = { version = "0.3", default-features = false }
 # TODO(thane): Remove restrictions once js-sys issue is resolved (causes the Substrate no_std check to fail)
 time = { version = ">=0.3, <0.3.12", default-features = false, features = ["macros", "parsing"] }
 flex-error = { version = "0.4.4", default-features = false }
+# TODO(hdevalence): re-added while rebasing the ABCI domain types PR again
+chrono = { version = "0.4", default-features = false, features = ["serde", "alloc"] }
 
 [dev-dependencies]
 serde_json = { version = "1.0", default-features = false, features = ["alloc"] }
diff --git a/proto/src/chrono.rs b/proto/src/chrono.rs
@@ -0,0 +1,53 @@
+use core::convert::TryInto;
+
+use chrono::{DateTime, Duration, TimeZone, Utc};
+
+use crate::google::protobuf as pb;
+
+impl From<DateTime<Utc>> for pb::Timestamp {
+    fn from(dt: DateTime<Utc>) -> pb::Timestamp {
+        pb::Timestamp {
+            seconds: dt.timestamp(),
+            // This can exceed 1_000_000_000 in the case of a leap second, but
+            // even with a leap second it should be under 2_147_483_647.
+            nanos: dt
+                .timestamp_subsec_nanos()
+                .try_into()
+                .expect("timestamp_subsec_nanos bigger than i32::MAX"),
+        }
+    }
+}
+
+impl From<pb::Timestamp> for DateTime<Utc> {
+    fn from(ts: pb::Timestamp) -> DateTime<Utc> {
+        Utc.timestamp(ts.seconds, ts.nanos as u32)
+    }
+}
+
+// Note: we convert a protobuf::Duration into a chrono::Duration, not a
+// std::time::Duration, because std::time::Durations are unsigned, but the
+// protobuf duration is signed.
+
+impl From<Duration> for pb::Duration {
+    fn from(d: Duration) -> pb::Duration {
+        // chrono's Duration stores the fractional part as `nanos: i32`
+        // internally but doesn't provide a way to access it, only a way to get
+        // the *total* number of nanoseconds. so we have to do this cool and fun
+        // hoop-jumping maneuver
+        let seconds = d.num_seconds();
+        let nanos = (d - Duration::seconds(seconds))
+            .num_nanoseconds()
+            .expect("we computed the fractional part, so there's no overflow")
+            .try_into()
+            .expect("the fractional part fits in i32");
+
+        pb::Duration { seconds, nanos }
+    }
+}
+
+impl From<pb::Duration> for Duration {
+    fn from(d: pb::Duration) -> Duration {
+        // there's no constructor that supplies both at once
+        Duration::seconds(d.seconds) + Duration::nanoseconds(d.nanos as i64)
+    }
+}
diff --git a/proto/src/lib.rs b/proto/src/lib.rs
@@ -19,6 +19,7 @@ pub mod google {
     }
 }
 
+mod chrono;
 mod error;
 #[allow(warnings)]
 mod tendermint;