Skip to content

Commit

Permalink
Fix time based upgrade (#1826)
Browse files Browse the repository at this point in the history
- building OffsetDateTime from i64::MAX panics as the upper bound on the
timestamp range is much smaller than that so this is fixed by using the
from_utc() function. This function is internally used by
from_unix_timestamp()
- adds test for time based chain config upgrade
  • Loading branch information
imabdulbasit authored Aug 6, 2024
2 parents 91ed045 + bcee671 commit d342aff
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 25 deletions.
39 changes: 30 additions & 9 deletions sequencer/src/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1045,7 +1045,8 @@ mod test {
use espresso_types::{
mock::MockStateCatchup,
v0_1::{UpgradeMode, ViewBasedUpgrade},
FeeAccount, FeeAmount, Header, Upgrade, UpgradeType, ValidatedState,
FeeAccount, FeeAmount, Header, TimeBasedUpgrade, Timestamp, Upgrade, UpgradeType,
ValidatedState,
};
use ethers::utils::Anvil;
use futures::{
Expand All @@ -1067,13 +1068,14 @@ mod test {
};
use jf_merkle_tree::prelude::{MerkleProof, Sha3Node};
use portpicker::pick_unused_port;
use sequencer_utils::test_utils::setup_test;
use sequencer_utils::{ser::FromStringOrInteger, test_utils::setup_test};
use surf_disco::Client;
use test_helpers::{
catchup_test_helper, state_signature_test_helper, status_test_helper, submit_test_helper,
TestNetwork, TestNetworkConfigBuilder,
};
use tide_disco::{app::AppHealth, error::ServerError, healthcheck::HealthStatus};
use time::OffsetDateTime;

use self::{
data_source::{testing::TestableSequencerDataSource, PublicHotShotConfig},
Expand Down Expand Up @@ -1429,9 +1431,33 @@ mod test {
}

#[async_std::test]
async fn test_chain_config_upgrade() {
async fn test_chain_config_upgrade_view_based() {
setup_test();

test_chain_config_upgrade_helper(UpgradeMode::View(ViewBasedUpgrade {
start_voting_view: None,
stop_voting_view: None,
start_proposing_view: 1,
stop_proposing_view: 10,
}))
.await;
}

#[async_std::test]
async fn test_chain_config_upgrade_time_based() {
setup_test();

let now = OffsetDateTime::now_utc().unix_timestamp() as u64;
test_chain_config_upgrade_helper(UpgradeMode::Time(TimeBasedUpgrade {
start_proposing_time: Timestamp::from_integer(now).unwrap(),
stop_proposing_time: Timestamp::from_integer(now + 500).unwrap(),
start_voting_time: None,
stop_voting_time: None,
}))
.await;
}

async fn test_chain_config_upgrade_helper(mode: UpgradeMode) {
let port = pick_unused_port().expect("No ports free");
let anvil = Anvil::new().spawn();
let l1 = anvil.endpoint().parse().unwrap();
Expand All @@ -1446,12 +1472,7 @@ mod test {
upgrades.insert(
<SeqTypes as NodeType>::Upgrade::VERSION,
Upgrade {
mode: UpgradeMode::View(ViewBasedUpgrade {
start_voting_view: None,
stop_voting_view: None,
start_proposing_view: 1,
stop_proposing_view: 10,
}),
mode,
upgrade_type: UpgradeType::ChainConfig {
chain_config: chain_config_upgrade,
},
Expand Down
4 changes: 1 addition & 3 deletions types/src/v0/impls/instance_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,11 +132,9 @@ impl Upgrade {
config.start_proposing_time = t.start_proposing_time.unix_timestamp();
config.stop_proposing_time = t.stop_proposing_time.unix_timestamp();
config.start_voting_time = t.start_voting_time.unwrap_or_default().unix_timestamp();
// this should not panic because Timestamp::max() constructs the maximum possible Unix timestamp
// using i64::MAX
config.stop_voting_time = t
.stop_voting_time
.unwrap_or(Timestamp::max().expect("overflow"))
.unwrap_or(Timestamp::max())
.unix_timestamp();
config.start_proposing_view = 0;
config.stop_proposing_view = u64::MAX;
Expand Down
25 changes: 12 additions & 13 deletions types/src/v0/utils.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,3 @@
use std::{
cmp::{min, Ordering},
fmt::{self, Debug, Display, Formatter},
num::ParseIntError,
str::FromStr,
time::Duration,
};

use anyhow::Context;
use async_std::task::sleep;
use clap::Parser;
Expand All @@ -15,7 +7,16 @@ use rand::Rng;
use sequencer_utils::{impl_serde_from_string_or_integer, ser::FromStringOrInteger};
use serde::{Deserialize, Serialize};
use snafu::Snafu;
use time::{format_description::well_known::Rfc3339 as TimestampFormat, OffsetDateTime};
use std::{
cmp::{min, Ordering},
fmt::{self, Debug, Display, Formatter},
num::ParseIntError,
str::FromStr,
time::Duration,
};
use time::{
format_description::well_known::Rfc3339 as TimestampFormat, macros::time, Date, OffsetDateTime,
};

/// Information about the genesis state which feeds into the genesis block header.
#[derive(Clone, Debug, Default, Deserialize, Serialize, PartialEq, Eq)]
Expand All @@ -40,10 +41,8 @@ impl Timestamp {
self.0.unix_timestamp() as u64
}

pub fn max() -> anyhow::Result<Self> {
Ok(Self(
OffsetDateTime::from_unix_timestamp(i64::MAX).context("overflow")?,
))
pub fn max() -> Self {
Self(OffsetDateTime::new_utc(Date::MAX, time!(23:59)))
}
}

Expand Down

0 comments on commit d342aff

Please sign in to comment.