Enforce consistent datetime serialization

.changelog/unreleased/bug-fixes/3389-consistent-datetime-serialization.md

@@ -0,0 +1,3 @@
+- Our `DateTimeUtc` type allowed a relaxed representation of RFC3339 strings.
+  We now enforce a string subset of this format, to guarantee deterministic
+  serialization. ([\#3389](https://github.com/anoma/namada/pull/3389))

crates/apps_lib/src/cli.rs

@@ -3475,12 +3475,10 @@ pub mod args {
 
         fn def(app: App) -> App {
             app.arg(NAMADA_START_TIME.def().help(wrap!(
-                "The start time of the ledger. Accepts a relaxed form of \
-                 RFC3339. A space or a 'T' are accepted as the separator \
-                 between the date and time components. Additional spaces are \
-                 allowed between each component.\nAll of these examples are \
-                 equivalent:\n2023-01-20T12:12:12Z\n2023-01-20 \
-                 12:12:12Z\n2023-  01-20T12:  12:12Z"
+                "The start time of the ledger. Accepts a strict subset of \
+                 RFC3339. A 'T' is accepted as the separator between the date \
+                 and time components.\nHere is a valid timestamp: \
+                 2023-01-20T12:12:12Z"
             )))
             .arg(
                 PATH_OPT

crates/apps_lib/src/config/genesis/chain.rs

@@ -850,6 +850,8 @@ pub struct Metadata<ID> {
 mod test {
     use std::path::PathBuf;
 
+    use namada::core::time::test_utils::GENESIS_TIME;
+
     use super::*;
 
     /// Test that the [`finalize`] returns deterministic output with the same
@@ -868,8 +870,7 @@ mod test {
         let chain_id_prefix: ChainIdPrefix =
             FromStr::from_str("test-prefix").unwrap();
 
-        let genesis_time =
-            DateTimeUtc::from_str("2021-12-31T00:00:00Z").unwrap();
+        let genesis_time = DateTimeUtc::from_str(GENESIS_TIME).unwrap();
 
         let consensus_timeout_commit =
             crate::facade::tendermint::Timeout::from_str("1s").unwrap();

crates/core/src/time.rs

@@ -154,6 +154,8 @@ impl Display for DateTimeUtc {
 }
 
 impl DateTimeUtc {
+    const FORMAT: &'static str = "%Y-%m-%dT%H:%M:%S%.9f+00:00";
+
     /// Returns a DateTimeUtc which corresponds to the current date.
     pub fn now() -> Self {
         Self(
@@ -183,7 +185,19 @@ impl DateTimeUtc {
 
     /// Returns an rfc3339 string or an error.
     pub fn to_rfc3339(&self) -> String {
-        chrono::DateTime::to_rfc3339(&self.0)
+        self.0.format(DateTimeUtc::FORMAT).to_string()
+    }
+
+    /// Parses a rfc3339 string, or returns an error.
+    pub fn from_rfc3339(s: &str) -> Result<Self, ParseError> {
+        use chrono::format;
+        use chrono::format::strftime::StrftimeItems;
+
+        let format = StrftimeItems::new(Self::FORMAT);
+        let mut parsed = format::Parsed::new();
+        format::parse(&mut parsed, s, format)?;
+
+        parsed.to_datetime_with_timezone(&chrono::Utc).map(Self)
     }
 
     /// Returns the DateTimeUtc corresponding to one second in the future
@@ -196,8 +210,9 @@ impl DateTimeUtc {
 impl FromStr for DateTimeUtc {
     type Err = ParseError;
 
+    #[inline]
     fn from_str(s: &str) -> Result<Self, Self::Err> {
-        Ok(Self(s.parse::<DateTime<Utc>>()?))
+        Self::from_rfc3339(s)
     }
 }
 
@@ -238,7 +253,7 @@ impl BorshSerialize for DateTimeUtc {
         &self,
         writer: &mut W,
     ) -> std::io::Result<()> {
-        let raw = self.0.to_rfc3339();
+        let raw = self.to_rfc3339();
         BorshSerialize::serialize(&raw, writer)
     }
 }
@@ -247,9 +262,8 @@ impl BorshDeserialize for DateTimeUtc {
     fn deserialize_reader<R: Read>(reader: &mut R) -> std::io::Result<Self> {
         use std::io::{Error, ErrorKind};
         let raw: String = BorshDeserialize::deserialize_reader(reader)?;
-        let actual = DateTime::parse_from_rfc3339(&raw)
-            .map_err(|err| Error::new(ErrorKind::InvalidData, err))?;
-        Ok(Self(actual.into()))
+        Self::from_rfc3339(&raw)
+            .map_err(|err| Error::new(ErrorKind::InvalidData, err))
     }
 }
 
@@ -324,30 +338,29 @@ impl TryFrom<Rfc3339String> for DateTimeUtc {
     type Error = chrono::ParseError;
 
     fn try_from(str: Rfc3339String) -> Result<Self, Self::Error> {
-        let utc = DateTime::parse_from_rfc3339(&str.0)?;
-        Ok(Self(utc.into()))
+        Self::from_rfc3339(&str.0)
     }
 }
 
 impl From<DateTimeUtc> for Rfc3339String {
     fn from(dt: DateTimeUtc) -> Self {
-        Self(DateTime::to_rfc3339(&dt.0))
+        Self(dt.to_rfc3339())
     }
 }
 
 impl TryFrom<DateTimeUtc> for crate::tendermint::time::Time {
     type Error = crate::tendermint::Error;
 
     fn try_from(dt: DateTimeUtc) -> Result<Self, Self::Error> {
-        Self::parse_from_rfc3339(&DateTime::to_rfc3339(&dt.0))
+        Self::parse_from_rfc3339(&dt.to_rfc3339())
     }
 }
 
 impl TryFrom<crate::tendermint::time::Time> for DateTimeUtc {
-    type Error = chrono::ParseError;
+    type Error = prost_types::TimestampError;
 
     fn try_from(t: crate::tendermint::time::Time) -> Result<Self, Self::Error> {
-        Rfc3339String(t.to_rfc3339()).try_into()
+        crate::tendermint_proto::google::protobuf::Timestamp::from(t).try_into()
     }
 }
 
@@ -362,3 +375,74 @@ impl From<DurationNanos> for crate::tendermint::Timeout {
         Self::from(std::time::Duration::from(val))
     }
 }
+
+#[cfg(any(test, feature = "testing"))]
+pub mod test_utils {
+    //! Time related test utilities.
+
+    /// Genesis time used during tests.
+    pub const GENESIS_TIME: &str = "2023-08-30T00:00:00.000000000+00:00";
+}
+
+#[cfg(test)]
+mod core_time_tests {
+    use proptest::prelude::*;
+
+    use super::*;
+
+    proptest! {
+        #[test]
+        fn test_valid_reverse_datetime_utc_encoding_roundtrip(
+            year in 1974..=3_000,
+            month in 1..=12,
+            day in 1..=28,
+            hour in 0..=23,
+            min in 0..=59,
+            sec in 0..=59,
+            nanos in 0..=999_999_999,
+        )
+        {
+            let timestamp = format!("{year:04}-{month:02}-{day:02}T{hour:02}:{min:02}:{sec:02}.{nanos:09}+00:00");
+            println!("Testing timestamp: {timestamp}");
+            test_valid_reverse_datetime_utc_encoding_roundtrip_inner(&timestamp);
+        }
+    }
+
+    fn test_valid_reverse_datetime_utc_encoding_roundtrip_inner(
+        timestamp: &str,
+    ) {
+        // we should be able to parse our custom datetime
+        let datetime = DateTimeUtc::from_rfc3339(timestamp).unwrap();
+
+        // the chrono datetime, which uses a superset of
+        // our datetime format should also be parsable
+        let datetime_inner = DateTime::parse_from_rfc3339(timestamp)
+            .unwrap()
+            .with_timezone(&Utc);
+        assert_eq!(datetime, DateTimeUtc(datetime_inner));
+
+        let encoded = datetime.to_rfc3339();
+
+        assert_eq!(encoded, timestamp);
+    }
+
+    #[test]
+    fn test_invalid_datetime_utc_encoding() {
+        // NB: this is a valid rfc3339 string, but we enforce
+        // a subset of the format to get deterministic encoding
+        // results
+        const TIMESTAMP: &str = "1966-03-03T00:06:56.520Z";
+        // const TIMESTAMP: &str = "1966-03-03T00:06:56.520+00:00";
+
+        // this is a valid rfc3339 string
+        assert!(DateTime::parse_from_rfc3339(TIMESTAMP).is_ok());
+
+        // but it cannot be parsed as a `DateTimeUtc`
+        assert!(DateTimeUtc::from_rfc3339(TIMESTAMP).is_err());
+    }
+
+    #[test]
+    fn test_valid_test_utils_genesis_time() {
+        assert!(DateTimeUtc::from_rfc3339(test_utils::GENESIS_TIME).is_ok());
+    }
+}

crates/namada/src/ledger/native_vp/ibc/mod.rs

@@ -353,10 +353,11 @@ fn match_value(
 /// A dummy header used for testing
 #[cfg(any(test, feature = "testing", feature = "benches"))]
 pub fn get_dummy_header() -> crate::storage::Header {
-    use crate::tendermint::time::Time as TmTime;
+    use crate::time::{DateTimeUtc, DurationSecs};
     crate::storage::Header {
         hash: crate::hash::Hash([0; 32]),
-        time: TmTime::now().try_into().unwrap(),
+        #[allow(clippy::disallowed_methods, clippy::arithmetic_side_effects)]
+        time: DateTimeUtc::now() + DurationSecs(5),
         next_validators_hash: crate::hash::Hash([0; 32]),
     }
 }

crates/node/src/shell/finalize_block.rs

@@ -1186,7 +1186,7 @@ mod test_finalize_block {
         FinalizeBlock, ProcessedTx,
     };
 
-    const WRAPPER_GAS_LIMIT: u64 = 10_000;
+    const WRAPPER_GAS_LIMIT: u64 = 11_000;
     const STORAGE_VALUE: &str = "test_value";
 
     /// Make a wrapper tx and a processed tx from the wrapped tx that can be

crates/tests/src/e2e/setup.rs

@@ -423,7 +423,7 @@ pub fn network(
         "--wasm-checksums-path",
         &checksums_path,
         "--genesis-time",
-        "2023-08-30T00:00:00Z",
+        namada_core::time::test_utils::GENESIS_TIME,
         "--archive-dir",
         &archive_dir,
     ];

crates/tx/Cargo.toml

@@ -13,8 +13,9 @@ repository.workspace = true
 version.workspace = true
 
 [features]
-default = []
+default = ["salt"]
 testing = ["proptest", "namada_core/testing"]
+salt = ["rand_core"]
 migrations = [
     "namada_migrations",
     "linkme",
@@ -39,8 +40,9 @@ num-traits.workspace = true
 proptest = { workspace = true, optional = true }
 prost-types.workspace = true
 prost.workspace = true
-serde_json.workspace = true
+rand_core = { workspace = true, optional = true, features = ["getrandom"] }
 serde.workspace = true
+serde_json.workspace = true
 sha2.workspace = true
 thiserror.workspace = true
 

crates/tx/src/types.rs

@@ -242,14 +242,14 @@ impl PartialEq for Data {
 impl Data {
     /// Make a new data section with the given bytes
     pub fn new(data: Vec<u8>) -> Self {
+        use rand_core::{OsRng, RngCore};
+
         Self {
             salt: {
-                #[allow(clippy::disallowed_methods)]
-                DateTimeUtc::now()
-            }
-            .0
-            .timestamp_millis()
-            .to_le_bytes(),
+                let mut buf = [0; 8];
+                OsRng.fill_bytes(&mut buf);
+                buf
+            },
             data,
         }
     }
@@ -359,14 +359,14 @@ impl PartialEq for Code {
 impl Code {
     /// Make a new code section with the given bytes
     pub fn new(code: Vec<u8>, tag: Option<String>) -> Self {
+        use rand_core::{OsRng, RngCore};
+
         Self {
             salt: {
-                #[allow(clippy::disallowed_methods)]
-                DateTimeUtc::now()
-            }
-            .0
-            .timestamp_millis()
-            .to_le_bytes(),
+                let mut buf = [0; 8];
+                OsRng.fill_bytes(&mut buf);
+                buf
+            },
             code: Commitment::Id(code),
             tag,
         }
@@ -377,14 +377,14 @@ impl Code {
         hash: namada_core::hash::Hash,
         tag: Option<String>,
     ) -> Self {
+        use rand_core::{OsRng, RngCore};
+
         Self {
             salt: {
-                #[allow(clippy::disallowed_methods)]
-                DateTimeUtc::now()
-            }
-            .0
-            .timestamp_millis()
-            .to_le_bytes(),
+                let mut buf = [0; 8];
+                OsRng.fill_bytes(&mut buf);
+                buf
+            },
             code: Commitment::Hash(hash),
             tag,
         }

crates/tx_prelude/Cargo.toml

@@ -27,7 +27,7 @@ namada_parameters = { path = "../parameters" }
 namada_proof_of_stake = { path = "../proof_of_stake" }
 namada_storage = { path = "../storage" }
 namada_token = { path = "../token" }
-namada_tx = { path = "../tx" }
+namada_tx = { path = "../tx", default-features = false }
 namada_tx_env = { path = "../tx_env" }
 namada_vm_env = { path = "../vm_env" }
 

crates/vp_prelude/Cargo.toml

@@ -26,7 +26,7 @@ namada_parameters = { path = "../parameters" }
 namada_proof_of_stake = { path = "../proof_of_stake" }
 namada_storage = { path = "../storage" }
 namada_token = { path = "../token" }
-namada_tx = { path = "../tx" }
+namada_tx = { path = "../tx", default-features = false }
 namada_vm_env = { path = "../vm_env" }
 namada_vp_env = { path = "../vp_env" }
 

genesis/localnet/src/pre-genesis/signed-transactions.toml

@@ -19,4 +19,4 @@ validator = "tnam1q9vhfdur7gadtwx4r223agpal0fvlqhywylf2mzx"
 amount = "20000"
 
 [bond.signatures]
-tpknam1qrnw8mxyqlj60mykgevnldcj5mg2fya7fs5a8xqdkd2gwtxhef0zy8a2wha = "signam1qrax95lz0j46388xlpxmhlamx7taemskp39feal9jf8s5kculdqaam3tzzjsktd6hw55hewy7nn2etgeupkse4a2trls0uk65yravdcx8z8mp5"
+tpknam1qrnw8mxyqlj60mykgevnldcj5mg2fya7fs5a8xqdkd2gwtxhef0zy8a2wha = "signam1qqp5p6k7g384x6gu65hzv9dve8fwftv02h4p3sgtwvz8vzgwjkr6cflxdv2kx7nrtxqha0mmpcmjeclrjwnlt7fphj4ugxsludlmt8qfs3k9q0"

genesis/localnet/src/pre-genesis/validator-0/signed-transactions.toml

@@ -34,12 +34,12 @@ authorization = "signam1qy9r5vrja246ajayx5hhm68elzm3rms993gmyhjw7lm8lpt5kufmkeq3
 email = "[email protected]"
 
 [validator_account.signatures]
-tpknam1qpg2tsrplvhu3fd7z7tq5ztc2ne3s7e2ahjl2a2cddufrzdyr752g666ytj = "signam1qzp6jjp57ahlerqnygjvans5h6jqaguzd5cu7h6exs5m4fpw4fnp07dnfrvjevnmvkf9tyv2azgygkhpdxtalvfqk5jez6vp9rsey8gq3386xd"
+tpknam1qpg2tsrplvhu3fd7z7tq5ztc2ne3s7e2ahjl2a2cddufrzdyr752g666ytj = "signam1qpz549e55sgwa8zxlq35sehr8csyl3ww394g22sh46r5u2z9tr0gxjyhkqa9554s6q2u58rx2yqz2lw746yprk6vmk4lmx5k2ff6apc9ralkc2"
 
 [[bond]]
 source = "tnam1q9vhfdur7gadtwx4r223agpal0fvlqhywylf2mzx"
 validator = "tnam1q9vhfdur7gadtwx4r223agpal0fvlqhywylf2mzx"
 amount = "100000"
 
 [bond.signatures]
-tpknam1qpg2tsrplvhu3fd7z7tq5ztc2ne3s7e2ahjl2a2cddufrzdyr752g666ytj = "signam1qqyn5ljq7z090ad7p8p2uxqelhhywt6y0kf4snqp3kt5rsqz48xl405vjuj40fsjprkwngy4q99hjrydp78wrathf45r9xdwx56k45sqe3990l"
+tpknam1qpg2tsrplvhu3fd7z7tq5ztc2ne3s7e2ahjl2a2cddufrzdyr752g666ytj = "signam1qpkzy7xs362mkycd58f3ve9jjyvszmxz2huulveyxyd29ru9n30ptj6uaflkxms96rmvvvntvj0k53t9rm2mg9f8rwx4d0y629xj7jc0upuhlw"