From 27c53a0664f5f09f307606994886600a2ce4ba15 Mon Sep 17 00:00:00 2001 From: Theodore Schnepper Date: Fri, 17 Jan 2025 07:49:37 -0700 Subject: [PATCH 1/2] Remove conditionals from `epoch_from_block_number` Upon analyzing the conditional logic of the `epoch_from_block_height` it seems that we really want the epoch to increase on the successor of the multiples of the `epoch_height`. The conditional branches make this less obvious with 3 separate computed results for the calculation. It seems that this can be achieved with a single statement. In addition, extra clarity as to the parameter's intents can be added by renaming `epoch_height` to something that indicates what the parameter is symbolizing. --- crates/types/src/utils.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/crates/types/src/utils.rs b/crates/types/src/utils.rs index 3183b4a508..a0355e5d3d 100644 --- a/crates/types/src/utils.rs +++ b/crates/types/src/utils.rs @@ -241,15 +241,15 @@ pub fn bincode_opts() -> WithOtherTrailing< } /// Returns an epoch number given a block number and an epoch height +/// +/// The epoch has the following properties: +/// - `block_number` 0 is always epoch 0 +/// - `block_number` 1 is the first block in epoch 1 +/// - The epoch increases every `blocks_per_epoch` +/// - Every epoch, other than 0, has `blocks_per_epoch` blocks within it #[must_use] -pub fn epoch_from_block_number(block_number: u64, epoch_height: u64) -> u64 { - if epoch_height == 0 { - 0 - } else if block_number % epoch_height == 0 { - block_number / epoch_height - } else { - block_number / epoch_height + 1 - } +pub fn epoch_from_block_number(block_number: u64, blocks_per_epoch: u64) -> u64 { + (block_number + blocks_per_epoch - 1) / blocks_per_epoch } /// A function for generating a cute little user mnemonic from a hash From 529a6e4aa1bbb795076dfdea0f8336840bc1e9f5 Mon Sep 17 00:00:00 2001 From: Theodore Schnepper Date: Mon, 20 Jan 2025 07:03:51 -0700 Subject: [PATCH 2/2] Add back in consideration for `blocks_per_epoch` of 0 With the previous change the explicit check for `blocks_per_epoch` being zero was removed, thereby removing the protection of division by 0. This check should be re-added making this change somewhat less valuable as we're unable to remove all conditionals. This change has been made to re-add the initial conditional in order to ensure compatibility with the previous implementation. --- crates/types/src/utils.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/crates/types/src/utils.rs b/crates/types/src/utils.rs index a0355e5d3d..85bdc21cb3 100644 --- a/crates/types/src/utils.rs +++ b/crates/types/src/utils.rs @@ -243,13 +243,18 @@ pub fn bincode_opts() -> WithOtherTrailing< /// Returns an epoch number given a block number and an epoch height /// /// The epoch has the following properties: +/// - A `blocks_per_epoch` of 0 is always epoch /// - `block_number` 0 is always epoch 0 /// - `block_number` 1 is the first block in epoch 1 /// - The epoch increases every `blocks_per_epoch` /// - Every epoch, other than 0, has `blocks_per_epoch` blocks within it #[must_use] pub fn epoch_from_block_number(block_number: u64, blocks_per_epoch: u64) -> u64 { - (block_number + blocks_per_epoch - 1) / blocks_per_epoch + if blocks_per_epoch == 0 { + 0 + } else { + (block_number + blocks_per_epoch - 1) / blocks_per_epoch + } } /// A function for generating a cute little user mnemonic from a hash