Remove `Ord` instances for sealed items.

There are two canonical orderings for sealed items: preference
order and encoding order. Removing the `Ord` instances means
that a user can't accidentally choose the wrong ordering;
these orderings are replaced by explicit `preference_order`
and `encoding_order` comparison functions.
This commit is contained in:
Kris Nuttycombe 2022-01-04 08:14:23 -07:00
parent e413f12fb5
commit 2fa73ed368
4 changed files with 27 additions and 65 deletions

View File

@ -25,9 +25,9 @@ pub enum Typecode {
Unknown(u32),
}
impl Ord for Typecode {
fn cmp(&self, other: &Self) -> cmp::Ordering {
match (self, other) {
impl Typecode {
pub fn preference_order(a: &Self, b: &Self) -> cmp::Ordering {
match (a, b) {
// Trivial equality checks.
(Self::Orchard, Self::Orchard)
| (Self::Sapling, Self::Sapling)
@ -55,11 +55,9 @@ impl Ord for Typecode {
(_, Self::P2pkh) => cmp::Ordering::Greater,
}
}
}
impl PartialOrd for Typecode {
fn partial_cmp(&self, other: &Self) -> Option<cmp::Ordering> {
Some(self.cmp(other))
pub fn encoding_order(a: &Self, b: &Self) -> cmp::Ordering {
u32::from(*a).cmp(&u32::from(*b))
}
}
@ -149,11 +147,23 @@ pub(crate) mod private {
use zcash_encoding::CompactSize;
/// A raw address or viewing key.
pub trait SealedItem:
for<'a> TryFrom<(u32, &'a [u8]), Error = ParseError> + cmp::Ord + cmp::PartialOrd + Clone
{
pub trait SealedItem: for<'a> TryFrom<(u32, &'a [u8]), Error = ParseError> + Clone {
fn typecode(&self) -> Typecode;
fn data(&self) -> &[u8];
fn preference_order(a: &Self, b: &Self) -> cmp::Ordering {
match Typecode::preference_order(&a.typecode(), &b.typecode()) {
cmp::Ordering::Equal => a.data().cmp(b.data()),
res => res,
}
}
fn encoding_order(a: &Self, b: &Self) -> cmp::Ordering {
match Typecode::encoding_order(&a.typecode(), &b.typecode()) {
cmp::Ordering::Equal => a.data().cmp(b.data()),
res => res,
}
}
}
/// A Unified Container containing addresses or viewing keys.
@ -337,7 +347,7 @@ pub trait Encoding: private::SealedContainer {
/// * the item list may not contain only transparent items (or no items)
/// * the item list may not contain both P2PKH and P2SH items.
fn try_from_items(mut items: Vec<Self::Item>) -> Result<Self, ParseError> {
items.sort_unstable_by_key(|i| <u32>::from(i.typecode()));
items.sort_unstable_by(Self::Item::encoding_order);
Self::try_from_items_internal(items)
}
@ -385,7 +395,7 @@ pub trait Container {
let mut items = self.items_as_parsed().to_vec();
// Unstable sorting is fine, because all items are guaranteed by construction
// to have distinct typecodes.
items.sort_unstable_by_key(|r| r.typecode());
items.sort_unstable_by(Self::Item::preference_order);
items
}

View File

@ -1,7 +1,6 @@
use super::{private::SealedItem, ParseError, Typecode};
use crate::kind;
use std::cmp;
use std::convert::{TryFrom, TryInto};
/// The set of known Receivers for Unified Addresses.
@ -14,21 +13,6 @@ pub enum Receiver {
Unknown { typecode: u32, data: Vec<u8> },
}
impl cmp::Ord for Receiver {
fn cmp(&self, other: &Self) -> cmp::Ordering {
match self.typecode().cmp(&other.typecode()) {
cmp::Ordering::Equal => self.data().cmp(other.data()),
res => res,
}
}
}
impl cmp::PartialOrd for Receiver {
fn partial_cmp(&self, other: &Self) -> Option<cmp::Ordering> {
Some(self.cmp(other))
}
}
impl TryFrom<(u32, &[u8])> for Receiver {
type Error = ParseError;
@ -166,7 +150,7 @@ mod tests {
fn arb_unified_address_for_typecodes(
mut typecodes: Vec<Typecode>,
) -> impl Strategy<Value = Vec<Receiver>> {
typecodes.sort_unstable_by_key(|tc| <u32>::from(*tc));
typecodes.sort_unstable_by(Typecode::encoding_order);
typecodes
.into_iter()
.map(|tc| match tc {

View File

@ -1,4 +1,3 @@
use std::cmp;
use std::convert::{TryFrom, TryInto};
use super::{
@ -40,21 +39,6 @@ pub enum Fvk {
},
}
impl cmp::Ord for Fvk {
fn cmp(&self, other: &Self) -> cmp::Ordering {
match self.typecode().cmp(&other.typecode()) {
cmp::Ordering::Equal => self.data().cmp(other.data()),
res => res,
}
}
}
impl cmp::PartialOrd for Fvk {
fn partial_cmp(&self, other: &Self) -> Option<cmp::Ordering> {
Some(self.cmp(other))
}
}
impl TryFrom<(u32, &[u8])> for Fvk {
type Error = ParseError;
@ -194,7 +178,7 @@ mod tests {
];
p.prop_map(|mut items| {
items.sort_unstable_by_key(|item| <u32>::from(item.typecode()));
items.sort_unstable_by(Fvk::encoding_order);
items
})
}
@ -209,7 +193,7 @@ mod tests {
transparent in prop::option::of(arb_transparent_fvk()),
) -> Ufvk {
let mut items: Vec<_> = shielded.into_iter().chain(transparent).collect();
items.sort_unstable_by_key(|item| <u32>::from(item.typecode()));
items.sort_unstable_by(Fvk::encoding_order);
Ufvk(items)
}
}

View File

@ -1,4 +1,3 @@
use std::cmp;
use std::convert::{TryFrom, TryInto};
use super::{
@ -45,21 +44,6 @@ pub enum Ivk {
},
}
impl cmp::Ord for Ivk {
fn cmp(&self, other: &Self) -> cmp::Ordering {
match self.typecode().cmp(&other.typecode()) {
cmp::Ordering::Equal => self.data().cmp(other.data()),
res => res,
}
}
}
impl cmp::PartialOrd for Ivk {
fn partial_cmp(&self, other: &Self) -> Option<cmp::Ordering> {
Some(self.cmp(other))
}
}
impl TryFrom<(u32, &[u8])> for Ivk {
type Error = ParseError;
@ -187,7 +171,7 @@ mod tests {
];
p.prop_map(|mut items| {
items.sort_unstable_by_key(|item| <u32>::from(item.typecode()));
items.sort_unstable_by(Ivk::encoding_order);
items
})
}
@ -202,7 +186,7 @@ mod tests {
transparent in prop::option::of(arb_transparent_ivk()),
) -> Uivk {
let mut items: Vec<_> = shielded.into_iter().chain(transparent).collect();
items.sort_unstable_by_key(|item| <u32>::from(item.typecode()));
items.sort_unstable_by(Ivk::encoding_order);
Uivk(items)
}
}