Skip to content

CompactTarget should not be Eq, Ord, and Partial* needs more nuance #2110

@junderw

Description

@junderw

#[derive(Copy, Clone, Debug, Default, PartialEq, Eq, PartialOrd, Ord, Hash)]
#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
#[cfg_attr(feature = "serde", serde(crate = "actual_serde"))]
pub struct CompactTarget(u32);

Right now we just derive the Ord and Eq from the underlying u32.

This causes "incorrect" Ord and Eq implementations.

Ord

0x1b000001 > 0x1a010000 is true, but comparing the targets expanded from them it would be false.

0x1b000001 -> Target(0x0000000000000001000000000000000000000000000000000000000000000000)
0x1a010000 -> Target(0x0000000000000100000000000000000000000000000000000000000000000000)

Eq

0x1b000001 != 0x1a000100 is true, but comparing the targets expanded from them it would be false.

Target(0x0000000000000001000000000000000000000000000000000000000000000000)
Target(0x0000000000000001000000000000000000000000000000000000000000000000)

This also brings into question the validity of Eq and Ord

("window" refers to the 3 byte mantissa of the compact target. The most significant byte of the compact target tells Core where to place the last 3 bytes on the 32 byte target. (3 being the lowest window (least significant 3 bytes) and 32 being the highest window (most significant 3 bytes))

What if the 3 byte "windows" shown by the compact target don't overlap?

I think it's sane to assume "there will be no bits set to 1 above the 3 byte window of this compact target"

But we can't say the same about bits below the 3 byte window. Even if the u32 is equal, unless that value starts with 0x03 (aka the 3 byte window is the 3 least significant bytes) can we really say that "the targets are equal?"

This seems like an arbitrary decision, but I would vote for removing Eq and Ord.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions