Skip to content

Commit

Permalink
feat(sns): Use human readable date-time in AdvanceSnsTargetVersion pr…
Browse files Browse the repository at this point in the history
…oposal rendering (#2896)

This PR improves the rendering of AdvanceSnsTargetVersion proposals by
using a human-readable date-time format rather than raw Unix timestamps.
Also, a potentially confusing remark is dropped from the disclaimer.

---------

Co-authored-by: Yusef Habib <yusef.fernandez@dfinity.org>
Co-authored-by: IDX GitHub Automation <infra+github-automation@dfinity.org>
  • Loading branch information
3 people authored Nov 29, 2024
1 parent 7a20299 commit 25c1bb0
Show file tree
Hide file tree
Showing 5 changed files with 53 additions and 6 deletions.
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.

1 change: 1 addition & 0 deletions rs/sns/governance/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ DEPENDENCIES = [
"@crate_index//:serde",
"@crate_index//:serde_bytes",
"@crate_index//:strum",
"@crate_index//:time",
]

MACRO_DEPENDENCIES = [
Expand Down
2 changes: 1 addition & 1 deletion rs/sns/governance/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ path = "tests/proposal.rs"

[dependencies]
build-info = { workspace = true }

async-trait = { workspace = true }
base64 = { workspace = true }
candid = { workspace = true }
Expand Down Expand Up @@ -81,6 +80,7 @@ serde = { workspace = true }
serde_bytes = { workspace = true }
strum = { workspace = true }
strum_macros = { workspace = true }
time = { workspace = true }

[target.'cfg(not(target_arch = "wasm32"))'.dev-dependencies]
ic-types = { path = "../../types/types" }
Expand Down
30 changes: 26 additions & 4 deletions rs/sns/governance/src/proposal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ use std::{
convert::TryFrom,
fmt::Write,
};
use time;

/// The maximum number of bytes in an SNS proposal's title.
pub const PROPOSAL_TITLE_BYTES_MAX: usize = 256;
Expand Down Expand Up @@ -1716,6 +1717,21 @@ fn validate_and_render_manage_dapp_canister_settings(
}
}

/// Attempts to format `` as a human-readable string.
///
/// For example:
/// ```
/// assert_eq!(format_timestamp(1732896850), Some("2024-11-29 16:14:10 UTC".to_string()));
/// ```
fn format_timestamp(timestamp_seconds: u64) -> Option<String> {
let timestamp_seconds = i64::try_from(timestamp_seconds).ok()?;
let dt_offset = time::OffsetDateTime::from_unix_timestamp(timestamp_seconds).ok()?;
let format =
time::format_description::parse("[year]-[month]-[day] [hour]:[minute]:[second] UTC")
.ok()?;
dt_offset.format(&format).ok()
}

/// Attempts to validate an `AdvanceSnsTargetVersion` action and render its human-readable text.
/// Invalidates the action in the following cases:
/// - There are no pending upgrades.
Expand All @@ -1737,7 +1753,14 @@ fn validate_and_render_advance_sns_target_version_proposal(
let (upgrade_steps, target_version) = governance_proto
.validate_new_target_version(advance_sns_target_version.new_target.clone())?;

let valid_timestamp_seconds = upgrade_steps.approximate_time_of_validity_timestamp_seconds();
let time_of_validity = {
let timestamp_seconds = upgrade_steps.approximate_time_of_validity_timestamp_seconds();
// This fallback should not occur unless `timestamp_seconds` is outside of the range
// from +1970-01-01 00:00:00 UTC (0)
// till +9999-12-31 23:59:59 UTC (253402300799).
format_timestamp(timestamp_seconds)
.unwrap_or_else(|| format!("timestamp {} seconds", timestamp_seconds))
};

let current_target_versions_render =
render_two_versions_as_markdown_table(upgrade_steps.current(), &target_version);
Expand All @@ -1753,9 +1776,8 @@ fn validate_and_render_advance_sns_target_version_proposal(
### Upgrade steps\n\n\
{upgrade_steps}\n\n\
### Monitoring the upgrade process\n\n\
Please note: the upgrade steps above (valid around timestamp {valid_timestamp_seconds} \
seconds) might change during this proposal's voting period. Such changes are unlikely and \
are subject to NNS community's approval.\n\n\
Please note: the upgrade steps mentioned above (valid around {time_of_validity}) \
might change during this proposal's voting period.\n\n\
The **upgrade journal** provides up-to-date information on this SNS's upgrade process:\n\n\
{upgrade_journal_url_render}"
);
Expand Down
25 changes: 24 additions & 1 deletion rs/sns/governance/src/proposal/advance_sns_target_version.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use crate::pb::v1::Governance as GovernancePb;
use crate::types::test_helpers::NativeEnvironment;
use futures::FutureExt;
use ic_test_utilities_types::ids::canister_test_id;
use pretty_assertions::assert_eq;

fn sns_version_for_tests() -> Version {
Version {
Expand Down Expand Up @@ -65,6 +66,28 @@ fn standard_governance_proto_for_tests(deployed_version: Option<Version>) -> Gov
}
}

#[test]
fn test_format_timestamp() {
for (expected, timestamp_seconds) in [
(Some("1970-01-01 00:00:00 UTC"), 0),
(Some("2024-11-29 16:14:10 UTC"), 1732896850),
(Some("2038-01-19 03:14:07 UTC"), i32::MAX as u64),
(Some("4571-09-24 08:52:47 UTC"), 82102668767),
(Some("9999-12-31 23:59:59 UTC"), 253402300799),
(None, 253402300800),
(None, i64::MAX as u64),
(None, u64::MAX),
] {
let observed = format_timestamp(timestamp_seconds);
assert_eq!(
observed,
expected.map(|s| s.to_string()),
"unexpected result from format_timestamp({})",
timestamp_seconds,
);
}
}

#[test]
fn test_validate_and_render_advance_target_version_action() {
// Prepare the world.
Expand Down Expand Up @@ -176,7 +199,7 @@ fn test_validate_and_render_advance_target_version_action() {
### Monitoring the upgrade process
Please note: the upgrade steps above (valid around timestamp 0 seconds) might change during this proposal's voting period. Such changes are unlikely and are subject to NNS community's approval.
Please note: the upgrade steps mentioned above (valid around 1970-01-01 00:00:00 UTC) might change during this proposal's voting period.
The **upgrade journal** provides up-to-date information on this SNS's upgrade process:
Expand Down

0 comments on commit 25c1bb0

Please sign in to comment.