Skip to content

Commit

Permalink
Merge branch 'tiago/consistent-datetime-serialization' (#3389)
Browse files Browse the repository at this point in the history
* origin/tiago/consistent-datetime-serialization:
  Changelog for #3389
  Regen tx fixtures
  Resign localnet genesis txs
  Rebuild wasms for tests
  gen_localnet.py: Fix genesis time string
  Increase precision of timestamps to 9 nanos
  Fix genesis time in tests
  Keep nanoseconds during CometBFT time conversions
  Switch to fixed offset format in UTC
  Increase robustness of datetime test
  Improve tx salting
  Fix from tm time impl for `DateTimeUtc`
  Misc fixes
  Increase gas limit in `FinalizeBlock` tests
  Add datetime encoding tests
  Enforce fixed RFC3339 format
  • Loading branch information
brentstone committed Jul 3, 2024
2 parents dae8c35 + 7f90393 commit e96c3c8
Show file tree
Hide file tree
Showing 37 changed files with 148 additions and 56 deletions.
Original file line number Diff line number Diff line change
@@ -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))
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 4 additions & 6 deletions crates/apps_lib/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 3 additions & 2 deletions crates/apps_lib/src/config/genesis/chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -843,6 +843,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
Expand All @@ -861,8 +863,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();
Expand Down
108 changes: 96 additions & 12 deletions crates/core/src/time.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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
Expand All @@ -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)
}
}

Expand Down Expand Up @@ -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)
}
}
Expand All @@ -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))
}
}

Expand Down Expand Up @@ -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 = 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()
}
}

Expand All @@ -374,3 +387,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());
}
}
7 changes: 3 additions & 4 deletions crates/namada/src/ledger/native_vp/ibc/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]),
}
}
Expand Down
2 changes: 1 addition & 1 deletion crates/node/src/shell/finalize_block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion crates/tests/fixtures/txs.json

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion crates/tests/src/e2e/setup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
];
Expand Down
6 changes: 4 additions & 2 deletions crates/tx/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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

Expand Down
36 changes: 18 additions & 18 deletions crates/tx/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
}
Expand Down Expand Up @@ -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,
}
Expand All @@ -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,
}
Expand Down
2 changes: 1 addition & 1 deletion crates/tx_prelude/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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" }

Expand Down
2 changes: 1 addition & 1 deletion crates/vp_prelude/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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" }

Expand Down
2 changes: 1 addition & 1 deletion genesis/localnet/src/pre-genesis/signed-transactions.toml
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,4 @@ validator = "tnam1q9vhfdur7gadtwx4r223agpal0fvlqhywylf2mzx"
amount = "20000"

[bond.signatures]
tpknam1qrnw8mxyqlj60mykgevnldcj5mg2fya7fs5a8xqdkd2gwtxhef0zy8a2wha = "signam1qrax95lz0j46388xlpxmhlamx7taemskp39feal9jf8s5kculdqaam3tzzjsktd6hw55hewy7nn2etgeupkse4a2trls0uk65yravdcx8z8mp5"
tpknam1qrnw8mxyqlj60mykgevnldcj5mg2fya7fs5a8xqdkd2gwtxhef0zy8a2wha = "signam1qqp5p6k7g384x6gu65hzv9dve8fwftv02h4p3sgtwvz8vzgwjkr6cflxdv2kx7nrtxqha0mmpcmjeclrjwnlt7fphj4ugxsludlmt8qfs3k9q0"
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Loading

0 comments on commit e96c3c8

Please sign in to comment.