Rebase ABCI domain types onto main (#1203)

* split A.4.1, proto tooling changes * split A.2, changelog abci * split A.1, tendermint and abci changes * split A.3, abci-in-rpc * clean up imports in abci crate * Return to 0.34 ABCI encoding * Cherry-pick chrono changes Replace chrono with time 0.3 (#1030) * pbt-gen: Converted from chrono to time 0.3 chrono has soundness issues (see RUSTSEC-2020-0159) and does not seem to be maintained. * Replace dependency on chrono with time 0.3 Change Time implementation to crate time: chrono has soundness issues (see RUSTSEC-2020-0159) and does not seem to be actively maintained. * Add Time methods checked_add and checked_sub These should be used instead of the overloaded operators that broke the operator convention by returning a Result. * proto: Don't use formatting methods of time Drop the "formatting" feature of time, as this brings in std. * pbt-gen: Add arb_datetime_for_rfc3339 With crate time, the range of supported dates stretches to the ancient past beyond the common era, which is not representable in RFC 3339. Add a generator to produce `OffsetDateTime` values that are within the RFC 3339 spec. * Ensure Time can only have values good for RFC 3339 As the time values are meant for human-readable representation in the RFC 3339 format, make it not possible to construct Time with values that fall outside this standard representation. Conversion from time::OffsetDateTime is made fallible with a TryFrom impl replacing the From impl. Conversion from Unix timestamps is also affected. * rpc: Replaced chrono with time 0.3 * testgen: Replaced chrono with time 0.3 * Less allocatey ways of formatting date-times Provide and use another helper in proto mod serializers::timestamp, one that formats into a provided fmt::Write object rather than allocating a string. * light-client: port from chrono to time * Revert to Add/Sub returning Result * light-client: changed the MBTs from chrono to time * tendermint: Remove a comment referencing chrono We use ErrorDetail::DurationOutOfRange without the source error from the time library because that is uninformative in the context. Also move the DateOutOfRange close with DurationOutOfRange since they can occur in the same operations and are generally similar. * light-client: add std feature for time The clock needs OffsetDateTime::now_utc(). * tendermint: Simplify Time::duration_since * testgen: minor code cleanup * Restrict valid Time years to 1-9999 Exclude year 0 because the spec on Google protobuf Timestamp forbids it. Add more helpers in pbt-gen to produce protobuf-safe values in both OffsetDateTime and RFC 3339 strings. Restrict the property tests to only use the protobuf-safe values where expected. * Test Time::checked_add and Time::checked_sub * Changelog entries for #1030 * Improve documentation of tendermint::Time * proto: remove the chrono type conversions The From impls are panicky and do not enforce compliance with protobuf message value restrictions. * tendermint: remove direct uses of chrono types Replace with tendermint::Time. * Harden Timestamp conversions and serde Require the timestamp to be in the validity range (years 1-9999 in UTC) and the nanosecond member value to not exceed 999_999_999. Rename ErrorDetail::TimestampOverflow to TimestampNanosOutOfRange, because the old variant was not very informative (the chained TryFromIntError did not help) and we also use it for the above-range case now which is not an overflow. * Changelog entry about changed error variants * Restore nanosecond range check in Time::from_unix_timestamp Add a unit test to exercise the check. * proto: Improve timestamp::fmt_as_rfc3339_nanos - More ergonomic signature - A non-allocating implementation * Fix component name in changelog for 1030-remove-chrono Co-authored-by: Thane Thomson <thane@informal.systems> * time: Use Self instead of the type name in methods Co-authored-by: Thane Thomson <thane@informal.systems> * Comment on the inner representation of `Time` * Don't alias crate time in testgen * Document the Time::from_utc helper Co-authored-by: Thane Thomson <thane@informal.systems> * fixup! split A.3, abci-in-rpc * split A.4.1.1, check_event_attrs * split A.4.1.2, some slice change? * split A.4.1.3, b.data.get(0).is_none() * split A.4.1.4, remove test attrs * split A.4.1.5, someting optiony * clippy fix * Remove unnessarily generated files * Fix deliver_tx.events KV tests * Remove Option wart in net_info::Response * rpc: Revert Event member events to a KV map The event list change did not make it into tendermint 0.37. * Revert tests to checking decoded KVs * rpc: Restore base64 serialization of event KV tags * Post-merge fixes for rpc tests * rpc: Use the re-exported serializers mod * Remove duplicate changelog entries * Fix kvstore-test build * Changelog entry for switching to Bytes * Remove a commented out line * abci: Restore Client::set_option as deprecated * Update abci/src/client.rs Co-authored-by: Henry de Valence <hdevalence@penumbra.zone> Co-authored-by: Thane Thomson <thane@informal.systems>
informalsystems · Sep 28, 2022 · 00401ae · 00401ae
1 parent 17ddb50
commit 00401ae
Show file tree

Hide file tree

Showing 126 changed files with 3,423 additions and 573 deletions.
diff --git a/.changelog/unreleased/breaking-changes/1203-bytes-for-abci-proto.md b/.changelog/unreleased/breaking-changes/1203-bytes-for-abci-proto.md
@@ -0,0 +1,2 @@
+- `[tendermint-proto]` Use `Bytes` for byte array fields of ABCI protobuf types.
+  ([#1203](https://github.com/informalsystems/tendermint-rs/pull/1203))
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/enhancements/1203-deprecate-abci-client-set-option.md b/.changelog/unreleased/enhancements/1203-deprecate-abci-client-set-option.md
@@ -0,0 +1,2 @@
+- `[tendermint-abci]` Deprecate `Client::set_option`.
+  ([#1203](https://github.com/informalsystems/tendermint-rs/pull/1203))
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
@@ -12,10 +12,7 @@ use tendermint_proto::abci::{
 };
 use tracing::{debug, info};
 
-use crate::{
-    codec::{encode_varint, MAX_VARINT_LENGTH},
-    Application, Error,
-};
+use crate::{codec::MAX_VARINT_LENGTH, Application, Error};
 
 /// In-memory, hashmap-backed key/value store ABCI application.
 ///
@@ -50,15 +47,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 +126,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 +155,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 +168,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 +180,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 +199,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 +225,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 +282,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
@@ -114,6 +114,10 @@ impl Client {
     }
 
     /// Request that the application set an option to a particular value.
+    ///
+    /// This request lacks specification and should not be used.
+    /// It will be removed in Tendermint Core v0.37.
+    #[deprecated(note = "The set_option ABCI method will be removed in Tendermint Core v0.37")]
     pub fn set_option(&mut self, req: RequestSetOption) -> Result<ResponseSetOption, Error> {
         perform!(self, SetOption, req)
     }

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"] }

diff --git a/proto/src/prelude.rs b/proto/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},