Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: implement slot arithmetic #39

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
[workspace]
members = ["crates/amaru"]
members = ["crates/amaru", "crates/time"]
resolver = "2"

[profile.dev.package]
Expand Down
12 changes: 12 additions & 0 deletions crates/time/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
[package]
name = "time"
version = "0.1.0"
edition = "2021"
description = "Cardano time arithmetic"
license = "Apache-2.0"
authors = ["Amaru Maintainers <[email protected]>"]
repository = "https://github.com/pragma-org/amaru"
homepage = "https://github.com/pragma-org/amaru"
documentation = "https://docs.rs/amaru"
readme = "README.md"
rust-version = "1.81.0"
230 changes: 230 additions & 0 deletions crates/time/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,230 @@
type Slot = u64;

#[derive(Clone, PartialEq, Eq)]
pub struct Bound {
pub time: u64, // Milliseconds
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: either rename to time_ms or have a type Millis = u64 to remove the comment

pub slot: Slot,
pub epoch: u64,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I would also have a type Epoch = u64 but maybe overkill?

}

#[derive(Clone, PartialEq, Eq)]
pub struct EraParams {
epoch_size: u64,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would make the unit explicit, either through the type or name

slot_length: u64, // Milliseconds
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same remark than above

}

impl EraParams {
pub fn new(epoch_size: u64, slot_length: u64) -> Option<Self> {
if epoch_size == 0 {
return None;
}
if slot_length == 0 {
return None;
}
Some(EraParams {
epoch_size,
slot_length,
})
}
}

// The start is inclusive and the end is exclusive. In a valid EraHistory, the
// end of each era will equal the start of the next one.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this another constraint it makes sense to validate / enforce in an EraHistory constructor @KtorZ?

i.e. just pass in a Vec<(Bound, Params)>, and then use the start of the next one as the end of the previous one, with the last one taking a None?

#[derive(Clone, PartialEq, Eq)]
pub struct Summary {
pub start: Bound,
pub end: Bound,
pub params: EraParams,
}

// A complete history of eras that have taken place.
#[derive(PartialEq, Eq)]
pub struct EraHistory {
pub eras: Vec<Summary>,
}

#[derive(Debug, PartialEq, Eq)]
pub enum TimeHorizonError {
PastTimeHorizon,
InvalidEraHistory,
}

#[derive(Clone, Debug, PartialEq, Eq)]
pub struct EpochBounds {
pub start: Slot,
pub end: Slot,
}

// The last era in the provided EraHistory must end at the time horizon for accurate results. The
// horizon is the end of the epoch containing the end of the current era's safe zone relative to
// the current tip. Returns number of milliseconds elapsed since the system start time.
impl EraHistory {
pub fn slot_to_relative_time(&self, slot: u64) -> Result<u64, TimeHorizonError> {
for era in &self.eras {
if era.start.slot > slot {
return Err(TimeHorizonError::InvalidEraHistory)
}
if era.end.slot >= slot {
let slots_elapsed = slot - era.start.slot;
let time_elapsed = era.params.slot_length * slots_elapsed;
let relative_time = era.start.time + time_elapsed;
return Ok(relative_time)
}
}
return Err(TimeHorizonError::PastTimeHorizon)
}

pub fn slot_to_absolute_time(&self, slot: u64, system_start: u64) -> Result<u64, TimeHorizonError> {
self.slot_to_relative_time(slot).map(|t| system_start + t)
}

pub fn relative_time_to_slot(&self, time: u64) -> Result<u64, TimeHorizonError> {
for era in &self.eras {
if era.start.time > time {
return Err(TimeHorizonError::InvalidEraHistory)
}
if era.end.time >= time {
let time_elapsed = time - era.start.time;
let slots_elapsed = time_elapsed / era.params.slot_length;
let slot = era.start.slot + slots_elapsed;
return Ok(slot)
}
}
return Err(TimeHorizonError::PastTimeHorizon)
}

pub fn slot_to_epoch(&self, slot: u64) -> Result<u64, TimeHorizonError> {
for era in &self.eras {
if era.start.slot > slot {
return Err(TimeHorizonError::InvalidEraHistory)
}
if era.end.slot >= slot {
let slots_elapsed = slot - era.start.slot;
let epochs_elapsed = slots_elapsed / era.params.epoch_size;
let epoch_number = era.start.epoch + epochs_elapsed;
return Ok(epoch_number)
}
}
return Err(TimeHorizonError::PastTimeHorizon)
}

pub fn epoch_bounds(&self, epoch: u64) -> Result<EpochBounds, TimeHorizonError> {
for era in &self.eras {
if era.start.epoch > epoch {
return Err(TimeHorizonError::InvalidEraHistory)
}
// We can't answer queries about the upper bound epoch of the era because the bound is
// exclusive.
if era.end.epoch > epoch {
let epochs_elapsed = epoch - era.start.epoch;
let offset = era.start.slot;
let start = offset + era.params.epoch_size * epochs_elapsed;
let end = offset + era.params.epoch_size * (epochs_elapsed + 1);
return Ok(EpochBounds {
start: start,
end: end,
})
}
}
return Err(TimeHorizonError::PastTimeHorizon);
}
}

#[cfg(test)]
mod tests {
use super::*;

#[test]
fn test_slot_to_time() {
let params = EraParams::new(86400, 1000).unwrap();
let eras = EraHistory {
eras: vec![
Summary {
start: Bound {
time: 0,
slot: 0,
epoch: 0,
},
end: Bound {
time: 86400000,
slot: 86400,
epoch: 1,
},
params: params.clone(),
},
Summary {
start: Bound {
time: 86400000,
slot: 86400,
epoch: 1,
},
end: Bound {
time: 172800000,
slot: 172800,
epoch: 2,
},
params: params.clone(),
},
],
};
let t0 = eras.slot_to_relative_time(172801);
assert_eq!(t0, Err(TimeHorizonError::PastTimeHorizon));
let t1 = eras.slot_to_relative_time(172800);
assert_eq!(t1, Ok(172800000));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not a big fan of testing several things in a single test: When the test fails, it's hard to know quickly what's failed, you would have to look at the source code of the test to know exactly what broke, which is annoying.

I would suggest to:

  • not prefix test functions with test_, we know they are tests because there's an annotation just above
  • have more explicit test names (eg. slot_to_time_fails_given_its_past_upper_bound)
  • have each function test a single "thing" (possibly using parameterised tests to avoid duplication)

}

#[test]
fn test_epoch_bounds() {
let params = EraParams::new(86400, 1000).unwrap();
let eras = EraHistory {
eras: vec![
Summary {
start: Bound {
time: 0,
slot: 0,
epoch: 0,
},
end: Bound {
time: 864000000,
slot: 864000,
epoch: 10,
},
params: params,
},
],
};
assert_eq!(eras.epoch_bounds(1).unwrap().start, 86400);
assert_eq!(eras.epoch_bounds(1).unwrap().end, 172800);
assert_eq!(eras.epoch_bounds(10), Err(TimeHorizonError::PastTimeHorizon));
}

#[test]
fn test_slot_to_epoch() {
let params = EraParams::new(86400, 1000).unwrap();
let eras = EraHistory {
eras: vec![
Summary {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could remove duplication by lifting this definition to be a constant within the test module perhaps?

start: Bound {
time: 0,
slot: 0,
epoch: 0,
},
end: Bound {
time: 864000000,
slot: 864000,
epoch: 10,
},
params: params,
},
],
};
let e0 = eras.slot_to_epoch(0);
assert_eq!(e0, Ok(0));
let e1 = eras.slot_to_epoch(86399);
assert_eq!(e1, Ok(0));
let e2 = eras.slot_to_epoch(864000);
assert_eq!(e2, Ok(10));
let e3 = eras.slot_to_epoch(864001);
assert_eq!(e3, Err(TimeHorizonError::PastTimeHorizon));
}
}