Merge branch 'tiago/consistent-datetime-serialization' (#3389)

* tiago/consistent-datetime-serialization:
  Changelog for #3389
  Improve tx salting
  Fix from tm time impl for `DateTimeUtc`
  Misc fixes
  Increase gas limit in `FinalizeBlock` tests
  Rebuild tx fixtures
  Resign localnet genesis txs
  Rebuild wasms for tests
  gen_localnet.py: Fix genesis time string
  Add datetime encoding tests
  Enforce fixed RFC3339 format

.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

@@ -3504,12 +3504,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/core/src/time.rs

@@ -154,6 +154,8 @@ impl Display for DateTimeUtc {
 }
 
 impl DateTimeUtc {
+    const FORMAT: &'static str = "%Y-%m-%dT%H:%M:%SZ";
+
     /// 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)
     }
 }
 
@@ -250,7 +265,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)
     }
 }
@@ -259,9 +274,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))
     }
 }
 
@@ -336,30 +350,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 = ();
 
-    fn try_from(t: crate::tendermint::time::Time) -> Result<Self, Self::Error> {
-        Rfc3339String(t.to_rfc3339()).try_into()
+    fn try_from(t: crate::tendermint::time::Time) -> Result<Self, ()> {
+        DateTimeUtc::from_unix_timestamp(t.unix_timestamp()).ok_or(())
     }
 }
 
@@ -374,3 +387,34 @@ impl From<DurationNanos> for crate::tendermint::Timeout {
         Self::from(std::time::Duration::from(val))
     }
 }
+
+#[cfg(test)]
+mod core_time_tests {
+    use super::*;
+
+    // TODO: if someone wants to take this on, convert this test
+    // into a proptest
+    #[test]
+    fn test_valid_reverse_datetime_utc_encoding_roundtrip() {
+        const TIMESTAMP: &str = "1966-03-03T00:06:56Z";
+
+        let datetime = DateTimeUtc::from_rfc3339(TIMESTAMP).unwrap();
+        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.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());
+    }
+}

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

@@ -353,12 +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 namada_sdk::time::DateTimeUtc;
-
+    use namada_sdk::time::{DateTimeUtc, DurationSecs};
     crate::storage::Header {
         hash: crate::hash::Hash([0; 32]),
-        #[allow(clippy::disallowed_methods)]
-        time: DateTimeUtc::now(),
+        #[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/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 = "signam1qpl79ztm2sge2uetemw3u9nyhe5v3khmd8ju34u66vspnxp8p276sy3v73anf83mv7twtnkd3seq7r70m2lkvd9t5cfq6m4jqzwhexsqt0w2n5"

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 = "signam1qzrz8vj7m9hyc95vdn93p5tqz0ny4redwj4rx7fehme8xykh4eslrud7utlde7evxgcxvr8727c48p8at44ngd6f4cavhwe6ed6vlkqww2e7k5"
 
 [[bond]]
 source = "tnam1q9vhfdur7gadtwx4r223agpal0fvlqhywylf2mzx"
 validator = "tnam1q9vhfdur7gadtwx4r223agpal0fvlqhywylf2mzx"
 amount = "100000"
 
 [bond.signatures]
-tpknam1qpg2tsrplvhu3fd7z7tq5ztc2ne3s7e2ahjl2a2cddufrzdyr752g666ytj = "signam1qqyn5ljq7z090ad7p8p2uxqelhhywt6y0kf4snqp3kt5rsqz48xl405vjuj40fsjprkwngy4q99hjrydp78wrathf45r9xdwx56k45sqe3990l"
+tpknam1qpg2tsrplvhu3fd7z7tq5ztc2ne3s7e2ahjl2a2cddufrzdyr752g666ytj = "signam1qrq5e0qtekw7tf496mn9ncuwm4s0xjkyn5n5ulvj526n2xfm8nm5htakxmn0rza252qdf9ahdfzs8m75hgz9z29hymweyh4q9r3gdqcfwhw0zq"

genesis/localnet/transactions.toml

@@ -41,15 +41,15 @@ authorization = "signam1qy9r5vrja246ajayx5hhm68elzm3rms993gmyhjw7lm8lpt5kufmkeq3
 email = "[email protected]"
 
 [validator_account.signatures]
-tpknam1qpg2tsrplvhu3fd7z7tq5ztc2ne3s7e2ahjl2a2cddufrzdyr752g666ytj = "signam1qzp6jjp57ahlerqnygjvans5h6jqaguzd5cu7h6exs5m4fpw4fnp07dnfrvjevnmvkf9tyv2azgygkhpdxtalvfqk5jez6vp9rsey8gq3386xd"
+tpknam1qpg2tsrplvhu3fd7z7tq5ztc2ne3s7e2ahjl2a2cddufrzdyr752g666ytj = "signam1qzrz8vj7m9hyc95vdn93p5tqz0ny4redwj4rx7fehme8xykh4eslrud7utlde7evxgcxvr8727c48p8at44ngd6f4cavhwe6ed6vlkqww2e7k5"
 
 [[bond]]
 source = "tnam1q9vhfdur7gadtwx4r223agpal0fvlqhywylf2mzx"
 validator = "tnam1q9vhfdur7gadtwx4r223agpal0fvlqhywylf2mzx"
 amount = "100000"
 
 [bond.signatures]
-tpknam1qpg2tsrplvhu3fd7z7tq5ztc2ne3s7e2ahjl2a2cddufrzdyr752g666ytj = "signam1qqyn5ljq7z090ad7p8p2uxqelhhywt6y0kf4snqp3kt5rsqz48xl405vjuj40fsjprkwngy4q99hjrydp78wrathf45r9xdwx56k45sqe3990l"
+tpknam1qpg2tsrplvhu3fd7z7tq5ztc2ne3s7e2ahjl2a2cddufrzdyr752g666ytj = "signam1qrq5e0qtekw7tf496mn9ncuwm4s0xjkyn5n5ulvj526n2xfm8nm5htakxmn0rza252qdf9ahdfzs8m75hgz9z29hymweyh4q9r3gdqcfwhw0zq"
 
 # 2.
 
@@ -74,4 +74,4 @@ validator = "tnam1q9vhfdur7gadtwx4r223agpal0fvlqhywylf2mzx"
 amount = "20000"
 
 [bond.signatures]
-tpknam1qrnw8mxyqlj60mykgevnldcj5mg2fya7fs5a8xqdkd2gwtxhef0zy8a2wha = "signam1qrax95lz0j46388xlpxmhlamx7taemskp39feal9jf8s5kculdqaam3tzzjsktd6hw55hewy7nn2etgeupkse4a2trls0uk65yravdcx8z8mp5"
+tpknam1qrnw8mxyqlj60mykgevnldcj5mg2fya7fs5a8xqdkd2gwtxhef0zy8a2wha = "signam1qpl79ztm2sge2uetemw3u9nyhe5v3khmd8ju34u66vspnxp8p276sy3v73anf83mv7twtnkd3seq7r70m2lkvd9t5cfq6m4jqzwhexsqt0w2n5"

scripts/gen_localnet.py

@@ -88,7 +88,9 @@ def init_network(
         die(f"Cannot find wasm directory that is not empty at {wasm_path}")
 
     chain_prefix = "local"
-    genesis_time = datetime.datetime.now(datetime.timezone.utc).isoformat()
+    genesis_time = datetime.datetime.now(datetime.timezone.utc).strftime(
+        "%Y-%m-%dT%H:%M:%SZ"
+    )
 
     templates = setup_templates(working_directory, args)