From e2901cab068ff20dffbec75a1abe6d3224a204a3 Mon Sep 17 00:00:00 2001 From: Luke Parker Date: Sun, 13 Aug 2023 04:32:21 -0400 Subject: [PATCH] Revert round-advance on TendermintMachine::new if local clock is ahead of block start It was improperly implemented, as it assumed rounds had a constant time interval, which they do not. It also is against the spec and was meant to absolve us of issues with poor performance when post-starting blockchains. The new, and much more proper, workaround for the latter is a 120-second delay between the Substrate time and the Tributary start time. --- coordinator/src/substrate/mod.rs | 3 ++- coordinator/tributary/tendermint/src/lib.rs | 23 +++++++++------------ 2 files changed, 12 insertions(+), 14 deletions(-) diff --git a/coordinator/src/substrate/mod.rs b/coordinator/src/substrate/mod.rs index 1c3018dd..8a0b6d05 100644 --- a/coordinator/src/substrate/mod.rs +++ b/coordinator/src/substrate/mod.rs @@ -77,7 +77,8 @@ async fn handle_new_set< // Since this block is in the past, and Tendermint doesn't play nice with starting chains after // their start time (though it does eventually work), delay the start time by 120 seconds // This is meant to handle ~20 blocks of lack of finalization for this first block - let time = time + 120; + const SUBSTRATE_TO_TRIBUTARY_TIME_DELAY: u64 = 120; + let time = time + SUBSTRATE_TO_TRIBUTARY_TIME_DELAY; let spec = TributarySpec::new(block.hash(), time, set, set_data); create_new_tributary(db, spec.clone()).await; diff --git a/coordinator/tributary/tendermint/src/lib.rs b/coordinator/tributary/tendermint/src/lib.rs index 3003a9d1..08564100 100644 --- a/coordinator/tributary/tendermint/src/lib.rs +++ b/coordinator/tributary/tendermint/src/lib.rs @@ -265,13 +265,19 @@ impl TendermintMachine { synced_block_result: synced_block_result_recv, messages: msg_send, machine: { + let now = SystemTime::now(); let sys_time = sys_time(last_time); - let time_until = sys_time.duration_since(SystemTime::now()).unwrap_or(Duration::ZERO); + let mut negative = false; + let time_until = sys_time.duration_since(now).unwrap_or_else(|_| { + negative = true; + now.duration_since(sys_time).unwrap_or(Duration::ZERO) + }); log::info!( target: "tendermint", - "new TendermintMachine building off block {} is scheduled to start in {}s", + "new TendermintMachine building off block {} is scheduled to start in {}{}s", last_block.0, - time_until.as_secs() + if negative { "-" } else { "" }, + time_until.as_secs(), ); // If the last block hasn't ended yet, sleep until it has @@ -308,16 +314,7 @@ impl TendermintMachine { // after it, without the standard amount of separation (so their times will be // equivalent or minimally offset) // For callers wishing to avoid this, they should pass (0, GENESIS + N::block_time()) - let start_time = CanonicalInstant::new(last_time); - machine.round(RoundNumber(0), Some(start_time)); - - // If we're past the start time, skip to and only join the next round - let rounds_to_skip = Instant::now().duration_since(start_time.instant()).as_secs() / - u64::from(N::block_time()); - if rounds_to_skip != 0 { - log::trace!("joining mid-block so skipping {rounds_to_skip} rounds"); - machine.round(RoundNumber(rounds_to_skip.try_into().unwrap()), None); - } + machine.round(RoundNumber(0), Some(CanonicalInstant::new(last_time))); machine }, }