From fd5d494f1185c4f36f27ddc88b1675340cc51bf8 Mon Sep 17 00:00:00 2001 From: Ava Howell Date: Wed, 20 Mar 2024 18:56:05 -0700 Subject: [PATCH 1/6] pcli: round withdrawal timestamp --- crates/bin/pcli/src/command/tx.rs | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/crates/bin/pcli/src/command/tx.rs b/crates/bin/pcli/src/command/tx.rs index 4395b2510a..5ad5f86e68 100644 --- a/crates/bin/pcli/src/command/tx.rs +++ b/crates/bin/pcli/src/command/tx.rs @@ -1051,16 +1051,22 @@ impl TxCmd { } }; - // get the current time on the local machine - let current_time_u64_ms = SystemTime::now() + // get the current time on the local machine (rounded to the nearest even second) + let mut current_time_s = SystemTime::now() .duration_since(UNIX_EPOCH) .expect("Time went backwards") - .as_nanos() as u64; + .as_secs() as u64; + + if current_time_s % 2 != 0 { + current_time_s += 1 + } + + let current_time_ns = current_time_s * 1_000_000_000; let mut timeout_timestamp = *timeout_timestamp; if timeout_timestamp == 0u64 { // add 2 days to current time - timeout_timestamp = current_time_u64_ms + 1.728e14 as u64; + timeout_timestamp = current_time_ns + 1.728e14 as u64; } fn parse_denom_and_amount(value_str: &str) -> anyhow::Result<(Amount, Metadata)> { From 24c161da4a0acc84d631cbca5bca85d4c69f7c58 Mon Sep 17 00:00:00 2001 From: Ava Howell Date: Wed, 20 Mar 2024 19:04:38 -0700 Subject: [PATCH 2/6] add chain rule: enforce nearest-even-second timestamps for withdrawals --- crates/bin/pcli/src/command/tx.rs | 15 ++++++--------- .../shielded-pool/src/ics20_withdrawal.rs | 11 ++++++++++- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/crates/bin/pcli/src/command/tx.rs b/crates/bin/pcli/src/command/tx.rs index 5ad5f86e68..65e81e8965 100644 --- a/crates/bin/pcli/src/command/tx.rs +++ b/crates/bin/pcli/src/command/tx.rs @@ -1051,17 +1051,11 @@ impl TxCmd { } }; - // get the current time on the local machine (rounded to the nearest even second) - let mut current_time_s = SystemTime::now() + // get the current time on the local machine + let current_time_ns = SystemTime::now() .duration_since(UNIX_EPOCH) .expect("Time went backwards") - .as_secs() as u64; - - if current_time_s % 2 != 0 { - current_time_s += 1 - } - - let current_time_ns = current_time_s * 1_000_000_000; + .as_nanos() as u64; let mut timeout_timestamp = *timeout_timestamp; if timeout_timestamp == 0u64 { @@ -1069,6 +1063,9 @@ impl TxCmd { timeout_timestamp = current_time_ns + 1.728e14 as u64; } + // round to the nearest even second + timeout_timestamp += timeout_timestamp % 2_000_000_000; + fn parse_denom_and_amount(value_str: &str) -> anyhow::Result<(Amount, Metadata)> { let denom_re = Regex::new(r"^([0-9.]+)(.+)$").context("denom regex invalid")?; if let Some(captures) = denom_re.captures(value_str) { diff --git a/crates/core/component/shielded-pool/src/ics20_withdrawal.rs b/crates/core/component/shielded-pool/src/ics20_withdrawal.rs index e548b471fb..76bf3aea16 100644 --- a/crates/core/component/shielded-pool/src/ics20_withdrawal.rs +++ b/crates/core/component/shielded-pool/src/ics20_withdrawal.rs @@ -33,7 +33,7 @@ pub struct Ics20Withdrawal { // prevent relayer censorship attacks. The core IBC implementation does this // in its handling of validation of timeouts. pub timeout_height: IbcHeight, - // the timestamp at which this transfer expires. + // the timestamp at which this transfer expires, in nanoseconds after unix epoch. pub timeout_time: u64, // the source channel used for the withdrawal pub source_channel: ChannelId, @@ -77,6 +77,15 @@ impl Ics20Withdrawal { anyhow::bail!("timeout time must be non-zero"); } + // in order to prevent clients from inadvertantly identifying themselves by their clock + // skew, enforce that timeout time is rounded to the nearest even second + if self.timeout_time % 2_000_000_000 != 0 { + anyhow::bail!( + "withdrawal timeout timestamp {} is not an even number of seconds", + self.timeout_time + ); + } + // NOTE: we could validate the destination chain address as bech32 to prevent mistyped // addresses, but this would preclude sending to chains that don't use bech32 addresses. From abf66d95d55ad7b6d57d26e2ea9e21982e9f2c80 Mon Sep 17 00:00:00 2001 From: Ava Howell Date: Thu, 21 Mar 2024 13:11:19 -0700 Subject: [PATCH 3/6] round to nearest single second --- crates/bin/pcli/src/command/tx.rs | 4 ++-- crates/core/component/shielded-pool/src/ics20_withdrawal.rs | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/crates/bin/pcli/src/command/tx.rs b/crates/bin/pcli/src/command/tx.rs index 65e81e8965..18a4efb525 100644 --- a/crates/bin/pcli/src/command/tx.rs +++ b/crates/bin/pcli/src/command/tx.rs @@ -1063,8 +1063,8 @@ impl TxCmd { timeout_timestamp = current_time_ns + 1.728e14 as u64; } - // round to the nearest even second - timeout_timestamp += timeout_timestamp % 2_000_000_000; + // round to the nearest second + timeout_timestamp += timeout_timestamp % 1_000_000_000; fn parse_denom_and_amount(value_str: &str) -> anyhow::Result<(Amount, Metadata)> { let denom_re = Regex::new(r"^([0-9.]+)(.+)$").context("denom regex invalid")?; diff --git a/crates/core/component/shielded-pool/src/ics20_withdrawal.rs b/crates/core/component/shielded-pool/src/ics20_withdrawal.rs index 76bf3aea16..efff1418e6 100644 --- a/crates/core/component/shielded-pool/src/ics20_withdrawal.rs +++ b/crates/core/component/shielded-pool/src/ics20_withdrawal.rs @@ -78,10 +78,10 @@ impl Ics20Withdrawal { } // in order to prevent clients from inadvertantly identifying themselves by their clock - // skew, enforce that timeout time is rounded to the nearest even second - if self.timeout_time % 2_000_000_000 != 0 { + // skew, enforce that timeout time is rounded to the nearest second + if self.timeout_time % 1_000_000_000 != 0 { anyhow::bail!( - "withdrawal timeout timestamp {} is not an even number of seconds", + "withdrawal timeout timestamp {} is not rounded to one second", self.timeout_time ); } From fb6bf897432b60cbb9b2283ef1bc3d531ba8e567 Mon Sep 17 00:00:00 2001 From: Ava Howell Date: Thu, 21 Mar 2024 15:32:17 -0700 Subject: [PATCH 4/6] enforce timestamp resolution in ics20withdrawl to minutes, round to hours in pcli --- crates/bin/pcli/src/command/tx.rs | 4 ++-- crates/core/component/shielded-pool/src/ics20_withdrawal.rs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/bin/pcli/src/command/tx.rs b/crates/bin/pcli/src/command/tx.rs index 18a4efb525..99dae32520 100644 --- a/crates/bin/pcli/src/command/tx.rs +++ b/crates/bin/pcli/src/command/tx.rs @@ -1063,8 +1063,8 @@ impl TxCmd { timeout_timestamp = current_time_ns + 1.728e14 as u64; } - // round to the nearest second - timeout_timestamp += timeout_timestamp % 1_000_000_000; + // round to the nearest hour + timeout_timestamp += timeout_timestamp % 3600_000_000_000; fn parse_denom_and_amount(value_str: &str) -> anyhow::Result<(Amount, Metadata)> { let denom_re = Regex::new(r"^([0-9.]+)(.+)$").context("denom regex invalid")?; diff --git a/crates/core/component/shielded-pool/src/ics20_withdrawal.rs b/crates/core/component/shielded-pool/src/ics20_withdrawal.rs index efff1418e6..94ad359491 100644 --- a/crates/core/component/shielded-pool/src/ics20_withdrawal.rs +++ b/crates/core/component/shielded-pool/src/ics20_withdrawal.rs @@ -78,8 +78,8 @@ impl Ics20Withdrawal { } // in order to prevent clients from inadvertantly identifying themselves by their clock - // skew, enforce that timeout time is rounded to the nearest second - if self.timeout_time % 1_000_000_000 != 0 { + // skew, enforce that timeout time is rounded to the nearest minute + if self.timeout_time % 60_000_000_000 != 0 { anyhow::bail!( "withdrawal timeout timestamp {} is not rounded to one second", self.timeout_time From 58cf12fd7449579b7d93baeb5a88bdeffefb85c1 Mon Sep 17 00:00:00 2001 From: Ava Howell Date: Fri, 22 Mar 2024 00:57:48 -0700 Subject: [PATCH 5/6] change pcli default to round timestamps to 10 minutes --- crates/bin/pcli/src/command/tx.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/bin/pcli/src/command/tx.rs b/crates/bin/pcli/src/command/tx.rs index 99dae32520..8e4d1356b2 100644 --- a/crates/bin/pcli/src/command/tx.rs +++ b/crates/bin/pcli/src/command/tx.rs @@ -1063,8 +1063,8 @@ impl TxCmd { timeout_timestamp = current_time_ns + 1.728e14 as u64; } - // round to the nearest hour - timeout_timestamp += timeout_timestamp % 3600_000_000_000; + // round to the nearest 10 minutes + timeout_timestamp += timeout_timestamp % 600_000_000_000; fn parse_denom_and_amount(value_str: &str) -> anyhow::Result<(Amount, Metadata)> { let denom_re = Regex::new(r"^([0-9.]+)(.+)$").context("denom regex invalid")?; From 1441a971fa7c4093890e59cb2ee3d794f1e46228 Mon Sep 17 00:00:00 2001 From: Ava Howell Date: Mon, 25 Mar 2024 13:34:17 -0700 Subject: [PATCH 6/6] fix error text for timeout timestamp --- crates/core/component/shielded-pool/src/ics20_withdrawal.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/core/component/shielded-pool/src/ics20_withdrawal.rs b/crates/core/component/shielded-pool/src/ics20_withdrawal.rs index 94ad359491..60256c6a74 100644 --- a/crates/core/component/shielded-pool/src/ics20_withdrawal.rs +++ b/crates/core/component/shielded-pool/src/ics20_withdrawal.rs @@ -81,7 +81,7 @@ impl Ics20Withdrawal { // skew, enforce that timeout time is rounded to the nearest minute if self.timeout_time % 60_000_000_000 != 0 { anyhow::bail!( - "withdrawal timeout timestamp {} is not rounded to one second", + "withdrawal timeout timestamp {} is not rounded to one minute", self.timeout_time ); }