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

Implement Expired variant, Scheduled #606

Merged
merged 5 commits into from
Dec 28, 2021
Merged

Implement Expired variant, Scheduled #606

merged 5 commits into from
Dec 28, 2021

Conversation

orkunkl
Copy link
Contributor

@orkunkl orkunkl commented Dec 24, 2021

This PR implements Scheduled type. The reason behind this is, Expiration type represents an expiration event although the logic is great for representing future events.

  • Duplicated Expiration and deleted Scheduled::Never

@orkunkl orkunkl changed the title Implement Expired variant, Scheduled. Implement Expired variant, Scheduled Dec 24, 2021
Comment on lines +31 to +64
impl Scheduled {
#[allow(dead_code)]
pub fn is_triggered(&self, block: &BlockInfo) -> bool {
match self {
Scheduled::AtHeight(height) => block.height >= *height,
Scheduled::AtTime(time) => block.time >= *time,
}
}
}

impl Add<Duration> for Scheduled {
type Output = StdResult<Scheduled>;

fn add(self, duration: Duration) -> StdResult<Scheduled> {
match (self, duration) {
(Scheduled::AtTime(t), Duration::Time(delta)) => {
Ok(Scheduled::AtTime(t.plus_seconds(delta)))
}
(Scheduled::AtHeight(h), Duration::Height(delta)) => Ok(Scheduled::AtHeight(h + delta)),
_ => Err(StdError::generic_err("Cannot add height and time")),
}
}
}

impl PartialOrd for Scheduled {
fn partial_cmp(&self, other: &Scheduled) -> Option<Ordering> {
match (self, other) {
// compare if both height or both time
(Scheduled::AtHeight(h1), Scheduled::AtHeight(h2)) => Some(h1.cmp(h2)),
(Scheduled::AtTime(t1), Scheduled::AtTime(t2)) => Some(t1.cmp(t2)),
_ => None,
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

It'd be usefull to test those implementations, in case someone (even by accident) changes it...

Copy link
Contributor

@ueco-jb ueco-jb left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link
Member

@ethanfrey ethanfrey left a comment

Choose a reason for hiding this comment

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

lgtm

@ethanfrey ethanfrey merged commit b872e22 into main Dec 28, 2021
@ethanfrey ethanfrey deleted the add-scheduled branch December 28, 2021 11:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants