fix both libgmp and num-bigin builds

Author: arvidn

src/more_ops.rs

@@ -436,7 +436,7 @@ pub fn op_div_impl(a: &mut Allocator, input: NodePtr, mempool: bool) -> Response
 
         // this is to preserve a buggy behavior from the initial implementation
         // of this operator.
-        if q == -1 && r != 0 {
+        if q.equal(-1) && r.not_equal(0) {
             q += 1;
         }
         let q1 = ptr_from_number(a, &q)?;
@@ -807,7 +807,7 @@ pub fn op_softfork(a: &mut Allocator, input: NodePtr, max_cost: Cost) -> Respons
         Some((p1, _)) => {
             let n: Number = Number::from_u8(int_atom(&p1, "softfork")?);
             if n.sign() == Sign::Plus {
-                if n > max_cost {
+                if n.greater_than(max_cost) {
                     return err(a.null(), "cost exceeded");
                 }
                 let cost: Cost = n.to_u64();

src/number.rs

@@ -7,17 +7,18 @@ pub use num_bigint::BigInt as Number;
 pub use num_bigint::Sign;
 
 use crate::allocator::{Allocator, NodePtr};
+use crate::number_traits::NumberTraits;
 use crate::reduction::EvalErr;
 
 pub fn ptr_from_number(allocator: &mut Allocator, item: &Number) -> Result<NodePtr, EvalErr> {
-    let bytes: Vec<u8> = item.to_signed_bytes_be();
+    let bytes: Vec<u8> = item.to_signed_bytes();
     allocator.new_atom(bytes.as_slice())
 }
 
 #[cfg(test)]
 #[cfg(feature = "num-bigint")]
 impl crate::number_traits::TestNumberTraits for Number {
-    fn from_str_radix(mut s: &str, radix: i32) -> Number {
+    fn from_str_radix(s: &str, radix: i32) -> Number {
         num_traits::Num::from_str_radix(s, radix as u32).unwrap()
     }
 }
@@ -29,6 +30,19 @@ impl crate::number_traits::NumberTraits for Number {
         i.into()
     }
 
+    fn to_signed_bytes(&self) -> Vec<u8> {
+        let mut ret = self.to_signed_bytes_be();
+
+        // make number minimal by removing leading zeros
+        while (!ret.is_empty()) && (ret[0] == 0) {
+            if ret.len() > 1 && (ret[1] & 0x80 == 0x80) {
+                break;
+            }
+            ret.remove(0);
+        }
+        ret
+    }
+
     fn zero() -> Number {
         <Number as num_traits::Zero>::zero()
     }
@@ -42,17 +56,30 @@ impl crate::number_traits::NumberTraits for Number {
         }
     }
 
-    fn to_u64(n: &Number) -> u64 {
-        n.into()
+    fn to_u64(&self) -> u64 {
+        use std::convert::TryFrom;
+        TryFrom::try_from(self).unwrap()
     }
 
     fn div_mod_floor(&self, denominator: &Number) -> (Number, Number) {
-        self.div_mod_floor(denominator)
+        num_integer::Integer::div_mod_floor(self, denominator)
     }
 
     fn mod_floor(&self, denominator: &Number) -> Number {
         num_integer::Integer::mod_floor(&self, denominator)
     }
+
+    fn equal(&self, other: i64) -> bool {
+        self == &Number::from(other)
+    }
+
+    fn not_equal(&self, other: i64) -> bool {
+        self != &Number::from(other)
+    }
+
+    fn greater_than(&self, other: u64) -> bool {
+        self > &Number::from(other)
+    }
 }
 
 #[test]
@@ -115,4 +142,3 @@ fn test_ptr_from_number() {
     let ptr = ptr_from_number(&mut a, &num).unwrap();
     assert_eq!(&[0x40, 0x00], &a.atom(ptr));
 }
-

src/number_gmp.rs

@@ -1,5 +1,8 @@
 use crate::gmp_ffi as gmp;
 use crate::node::Node;
+use crate::number_traits::NumberTraits;
+#[cfg(test)]
+use crate::number_traits::TestNumberTraits;
 use core::mem::MaybeUninit;
 use std::cmp::Ordering;
 use std::cmp::PartialOrd;
@@ -8,9 +11,6 @@ use std::ops::Drop;
 use std::ops::{
     AddAssign, BitAndAssign, BitOrAssign, BitXorAssign, MulAssign, Not, Shl, Shr, SubAssign,
 };
-use crate::number_traits::NumberTraits;
-#[cfg(test)]
-use crate::number_traits::TestNumberTraits;
 
 #[allow(clippy::enum_variant_names)]
 #[derive(PartialEq)]
@@ -60,6 +60,49 @@ impl NumberTraits for Number {
         ret
     }
 
+    fn to_signed_bytes(&self) -> Vec<u8> {
+        let size = (self.bits() + 7) / 8;
+        let mut ret: Vec<u8> = Vec::new();
+        if size == 0 {
+            return ret;
+        }
+        ret.resize(size + 1, 0);
+        let sign = self.sign();
+        let mut out_size: usize = size;
+        unsafe {
+            gmp::mpz_export(
+                ret.as_mut_slice()[1..].as_mut_ptr() as *mut c_void,
+                &mut out_size,
+                1,
+                1,
+                0,
+                0,
+                &self.v,
+            );
+        }
+        // apparently mpz_export prints 0 bytes to the buffer if the value is 0
+        // hence the special case in the assert below.
+        assert!(out_size == ret.len() - 1);
+        if sign == Sign::Minus {
+            // If the value is negative, we need to convert it to two's
+            // complement. We can't do that in-place.
+            let mut carry = true;
+            for digit in &mut ret.iter_mut().rev() {
+                let res = (!*digit).overflowing_add(carry as u8);
+                *digit = res.0;
+                carry = res.1;
+            }
+            assert!(!carry);
+            assert!(ret[0] & 0x80 != 0);
+            if (ret[1] & 0x80) != 0 {
+                ret.remove(0);
+            }
+        } else if ret[1] & 0x80 == 0 {
+            ret.remove(0);
+        }
+        ret
+    }
+
     fn zero() -> Number {
         let mut v = MaybeUninit::<gmp::mpz_t>::uninit();
         unsafe {
@@ -74,8 +117,8 @@ impl NumberTraits for Number {
         Number::from_signed_bytes_be(v)
     }
 
-    fn to_u64(n: &Number) -> u64 {
-        n.into()
+    fn to_u64(&self) -> u64 {
+        u64::from(self)
     }
 
     // returns the quotient and remained, from dividing self with denominator
@@ -95,6 +138,18 @@ impl NumberTraits for Number {
         }
         r
     }
+
+    fn equal(&self, other: i64) -> bool {
+        self == &other
+    }
+
+    fn not_equal(&self, other: i64) -> bool {
+        self != &other
+    }
+
+    fn greater_than(&self, other: u64) -> bool {
+        self > &other
+    }
 }
 
 impl Number {
@@ -128,49 +183,6 @@ impl Number {
         ret
     }
 
-    pub fn to_signed_bytes_be(&self) -> Vec<u8> {
-        let size = (self.bits() + 7) / 8;
-        let mut ret: Vec<u8> = Vec::new();
-        if size == 0 {
-            return ret;
-        }
-        ret.resize(size + 1, 0);
-        let sign = self.sign();
-        let mut out_size: usize = size;
-        unsafe {
-            gmp::mpz_export(
-                ret.as_mut_slice()[1..].as_mut_ptr() as *mut c_void,
-                &mut out_size,
-                1,
-                1,
-                0,
-                0,
-                &self.v,
-            );
-        }
-        // apparently mpz_export prints 0 bytes to the buffer if the value is 0
-        // hence the special case in the assert below.
-        assert!(out_size == ret.len() - 1);
-        if sign == Sign::Minus {
-            // If the value is negative, we need to convert it to two's
-            // complement. We can't do that in-place.
-            let mut carry = true;
-            for digit in &mut ret.iter_mut().rev() {
-                let res = (!*digit).overflowing_add(carry as u8);
-                *digit = res.0;
-                carry = res.1;
-            }
-            assert!(!carry);
-            assert!(ret[0] & 0x80 != 0);
-            if (ret[1] & 0x80) != 0 {
-                ret.remove(0);
-            }
-        } else if ret[1] & 0x80 == 0 {
-            ret.remove(0);
-        }
-        ret
-    }
-
     pub fn to_bytes_le(&self) -> (Sign, Vec<u8>) {
         let sgn = self.sign();
 
@@ -345,8 +357,8 @@ impl From<usize> for Number {
     }
 }
 
-impl From<Number> for u64 {
-    fn from(n: Number) -> u64 {
+impl From<&Number> for u64 {
+    fn from(n: &Number) -> u64 {
         unsafe {
             assert!(gmp::mpz_sizeinbase(&n.v, 2) <= 64);
             assert!(gmp::mpz_cmp_si(&n.v, 0) >= 0);
@@ -355,8 +367,8 @@ impl From<Number> for u64 {
     }
 }
 
-impl From<Number> for i64 {
-    fn from(n: Number) -> i64 {
+impl From<&Number> for i64 {
+    fn from(n: &Number) -> i64 {
         unsafe {
             assert!(gmp::mpz_sizeinbase(&n.v, 2) <= 64);
             gmp::mpz_get_si(&n.v)
@@ -456,6 +468,9 @@ impl From<&Node<'_>> for Option<Number> {
     }
 }
 
+// TODO: move all tests to number.rs so we can test both the GMP and num-bigint
+// versions
+
 // ==== TESTS ====
 
 #[cfg(test)]
@@ -489,7 +504,7 @@ fn roundtrip_bytes(b: &[u8]) {
             assert!(num.sign() == Sign::Plus);
         }
 
-        let round_trip = num.to_signed_bytes_be();
+        let round_trip = num.to_signed_bytes();
 
         assert_eq!(round_trip, b);
 
@@ -516,7 +531,7 @@ fn roundtrip_bytes(b: &[u8]) {
             unsafe {
                 gmp::mpz_neg(&mut negated.v, &num.v);
             }
-            let magnitude = negated.to_signed_bytes_be();
+            let magnitude = negated.to_signed_bytes();
             assert!(buf_le.iter().eq(magnitude.iter().rev()));
         }
     }
@@ -525,7 +540,7 @@ fn roundtrip_bytes(b: &[u8]) {
     {
         let unsigned_num = Number::from_unsigned_bytes_be(b);
         assert!(unsigned_num.sign() != Sign::Minus);
-        let unsigned_round_trip = unsigned_num.to_signed_bytes_be();
+        let unsigned_round_trip = unsigned_num.to_signed_bytes();
         let unsigned_round_trip = if unsigned_round_trip == &[0] {
             &unsigned_round_trip[1..]
         } else {
@@ -610,7 +625,7 @@ fn roundtrip_u64(v: u64) {
         assert!(num >= v - 1);
     }
 
-    let round_trip: u64 = num.into();
+    let round_trip: u64 = (&num).into();
     assert_eq!(round_trip, v);
 }
 
@@ -656,7 +671,7 @@ fn roundtrip_i64(v: i64) {
     }
 
     assert!(num.bits() <= 64);
-    let round_trip: i64 = num.into();
+    let round_trip: i64 = (&num).into();
     assert_eq!(round_trip, v);
 }
 

src/number_traits.rs

@@ -1,11 +1,16 @@
-
 pub trait NumberTraits {
     fn from_unsigned_bytes_be(v: &[u8]) -> Self;
+    fn to_signed_bytes(&self) -> Vec<u8>;
     fn zero() -> Self;
     fn from_u8(v: &[u8]) -> Self;
-    fn to_u64(n: &Self) -> u64;
-    fn div_mod_floor(&self, denominator: &Self) -> (Self, Self) where Self: Sized;
+    fn to_u64(&self) -> u64;
+    fn div_mod_floor(&self, denominator: &Self) -> (Self, Self)
+    where
+        Self: Sized;
     fn mod_floor(&self, denominator: &Self) -> Self;
+    fn equal(&self, other: i64) -> bool;
+    fn not_equal(&self, other: i64) -> bool;
+    fn greater_than(&self, other: u64) -> bool;
 }
 
 #[cfg(test)]

src/test_ops.rs

@@ -7,10 +7,10 @@ use crate::more_ops::{
     op_point_add, op_pubkey_for_exp, op_sha256, op_softfork, op_strlen, op_substr, op_subtract,
 };
 use crate::number::{ptr_from_number, Number};
+use crate::number_traits::TestNumberTraits;
 use crate::reduction::{Reduction, Response};
 use hex::FromHex;
 use std::collections::HashMap;
-use crate::number_traits::TestNumberTraits;
 
 static TEST_CASES: &str = r#"
 lognot ( 1 2 3 ) => FAIL