Skip to content
This repository has been archived by the owner on Sep 12, 2023. It is now read-only.

Commit

Permalink
Implement PartialOrd manually for BitMexPriceEventId
Browse files Browse the repository at this point in the history
This is important because deriving `PartialOrd` means that it looks at
all the struct's fields in order. For this type we want to compare the
`timestamp`, but do not care about the `digits`! More importantly,
someone could unintentionally change this behaviour by reordering the
fields.

We also remove the `Ord` derived implementation because:

- It's unused.
- Deriving `Ord` when manually implementing `PartialOrd` is a very bad
  idea[1].

[1]: rust-lang/rust-clippy#1621.

Co-authored-by: Daniel Karzel <daniel.karzel@coblox.tech>
  • Loading branch information
luckysori and da-kami committed Jul 25, 2022
1 parent 5ba6ea8 commit bd125f7
Showing 1 changed file with 7 additions and 3 deletions.
10 changes: 7 additions & 3 deletions model/src/olivia.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,7 @@ pub struct Attestation {
pub scalars: Vec<SecretKey>,
}

#[derive(
Debug, Clone, Copy, SerializeDisplay, DeserializeFromStr, PartialEq, Eq, Hash, PartialOrd, Ord,
)]
#[derive(Debug, Clone, Copy, SerializeDisplay, DeserializeFromStr, PartialEq, Eq, Hash)]
pub struct BitMexPriceEventId {
/// The timestamp this price event refers to.
timestamp: OffsetDateTime,
Expand Down Expand Up @@ -128,6 +126,12 @@ impl FromStr for BitMexPriceEventId {
}
}

impl PartialOrd for BitMexPriceEventId {
fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> {
self.timestamp.partial_cmp(&other.timestamp)
}
}

impl From<Announcement> for maia_core::Announcement {
fn from(announcement: Announcement) -> Self {
maia_core::Announcement {
Expand Down

0 comments on commit bd125f7

Please sign in to comment.