-
Notifications
You must be signed in to change notification settings - Fork 119
Labels
rustIssues that affect or pull requests that update Rust codeIssues that affect or pull requests that update Rust code
Description
We currently implement Ord for StorageSlot:
protocol/crates/miden-protocol/src/account/storage/slot/storage_slot.rs
Lines 104 to 114 in dc1f0f3
| impl Ord for StorageSlot { | |
| fn cmp(&self, other: &Self) -> core::cmp::Ordering { | |
| self.name().cmp(&other.name) | |
| } | |
| } | |
| impl PartialOrd for StorageSlot { | |
| fn partial_cmp(&self, other: &Self) -> Option<core::cmp::Ordering> { | |
| Some(self.cmp(other)) | |
| } | |
| } |
This is incorrect because Ord and Eq must agree, but Eq compares the slot content while Ord only compares the key. This was only implemented as a convenience to be able to easily sort a Vec<StorageSlot>. We should remove this and replace slot.sort_unstable with sort_unstable_by or sort_unstable_by_key to sort by the storage slot's name instead.
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
rustIssues that affect or pull requests that update Rust codeIssues that affect or pull requests that update Rust code