Repository: ergoplatform/sigma-rust (language: rust)

Issue:
scalar_bigint_roundtrip test failed 
```
---- sigma_protocol::dlog_group::tests::scalar_bigint_roundtrip stdout ----
thread 'sigma_protocol::dlog_group::tests::scalar_bigint_roundtrip' panicked at 'assertion failed: `(left == right)`
  left: `Scalar(Scalar4x64([12438879760993867047, 15668063604945817822, 4375183397110540183, 53500923189448437]))`,
 right: `Scalar(Scalar4x64([11513238136387086080, 8080818871156661932, 13242305237725190105, 13696236336498799932]))`', ergotree-ir/src/sigma_protocol/dlog_group.rs:219:9
```


Discussion / hints:
Found another case:
```
---- sigma_protocol::dlog_group::tests::scalar_bigint_roundtrip stdout ----
thread 'sigma_protocol::dlog_group::tests::scalar_bigint_roundtrip' panicked at 'assertion failed: `(left == right)`
  left: `Scalar(Scalar4x64([4041210275284044675, 6942116427424140532, 1121630985750299385, 4394074963199624]))`,
 right: `Scalar(Scalar4x64([1532162344980546304, 6294374344463021112, 10436371246433368416, 1124883190579103759]))`', ergotree-ir\src\sigma_protocol\dlog_group.rs:219:9
```
Should be suppressed in #325 via limiting BigInt256 range in test.
Experienced in rebased #346 (after #326 merge).
```
---- sigma_protocol::dlog_group::tests::scalar_bigint256_roundtrip stdout ----
thread 'sigma_protocol::dlog_group::tests::scalar_bigint256_roundtrip' panicked at 'assertion failed: `(left == r
ight)`
  left: `Scalar(Scalar4x64([12081056610554858139, 10753050667248866626, 17360340428558366986, 40113997526394505])
)`,
 right: `Scalar(Scalar4x64([12144231992548563712, 4216103832986665639, 17028572020649560725, 10269183366756993520
]))`', ergotree-ir/src/sigma_protocol/dlog_group.rs:223:9
```

Fix patch:
diff --git a/ergo-lib/src/wallet/tx_builder.rs b/ergo-lib/src/wallet/tx_builder.rs
index 01313c392..58d138e1c 100644
--- a/ergo-lib/src/wallet/tx_builder.rs
+++ b/ergo-lib/src/wallet/tx_builder.rs
@@ -583,11 +583,10 @@ mod tests {
             let tx = tx_builder.build().unwrap();
             prop_assert!(outputs.into_iter().all(|i| tx.output_candidates.iter().any(|o| *o == i)),
                          "tx.output_candidates is missing some outputs");
-            let tx_all_inputs_vals: Vec<BoxValue> = tx.inputs.iter()
-                                                             .map(|i| inputs.iter()
-                                                                  .find(|ib| ib.box_id() == i.box_id).unwrap().value)
-                                                             .collect();
-            let tx_all_inputs_sum = checked_sum(tx_all_inputs_vals.into_iter()).unwrap();
+            let tx_all_inputs_vals = tx.inputs.iter()
+                .map(|i| inputs.iter()
+                    .find(|ib| ib.box_id() == i.box_id).unwrap().value);
+            let tx_all_inputs_sum = checked_sum(tx_all_inputs_vals).unwrap();
             let expected_change = tx_all_inputs_sum.checked_sub(&all_outputs).unwrap();
             prop_assert!(tx.output_candidates.iter().any(|b| {
                 b.value == expected_change && b.ergo_tree == change_address.script().unwrap()
diff --git a/ergoscript-compiler/src/ast.rs b/ergoscript-compiler/src/ast.rs
index 8396aab4d..1ae41321e 100644
--- a/ergoscript-compiler/src/ast.rs
+++ b/ergoscript-compiler/src/ast.rs
@@ -15,7 +15,7 @@ impl AstError {
     }
 
     pub fn pretty_desc(&self, source: &str) -> String {
-        pretty_error_desc(&source, self.span, &self.msg)
+        pretty_error_desc(source, self.span, &self.msg)
     }
 }
 
@@ -57,6 +57,7 @@ impl Ident {
     }
 }
 
+#[allow(clippy::enum_variant_names)]
 #[derive(Debug)]
 pub enum Expr {
     Ident(Ident),
@@ -155,7 +156,7 @@ impl Literal {
     pub fn parse(&self) -> Result<LiteralValue, AstError> {
         let text = self.0.first_token().unwrap().text().to_string();
         if text.ends_with('L') {
-            text.strip_suffix("L")
+            text.strip_suffix('L')
                 .unwrap()
                 .parse()
                 .ok()
diff --git a/ergoscript-compiler/src/binder.rs b/ergoscript-compiler/src/binder.rs
index fb7f957e4..f59033eb8 100644
--- a/ergoscript-compiler/src/binder.rs
+++ b/ergoscript-compiler/src/binder.rs
@@ -19,7 +19,7 @@ impl BinderError {
     }
 
     pub fn pretty_desc(&self, source: &str) -> String {
-        pretty_error_desc(&source, self.span, &self.msg)
+        pretty_error_desc(source, self.span, &self.msg)
     }
 }
 
diff --git a/ergoscript-compiler/src/compiler.rs b/ergoscript-compiler/src/compiler.rs
index a9e313e35..8bb9c411e 100644
--- a/ergoscript-compiler/src/compiler.rs
+++ b/ergoscript-compiler/src/compiler.rs
@@ -76,7 +76,7 @@ pub fn compile(source: &str, env: ScriptEnv) -> Result<ErgoTree, CompileError> {
 }
 
 pub(crate) fn compile_hir(source: &str) -> Result<hir::Expr, CompileError> {
-    let parse = super::parser::parse(&source);
+    let parse = super::parser::parse(source);
     if !parse.errors.is_empty() {
         return Err(CompileError::ParseError(parse.errors));
     }
diff --git a/ergoscript-compiler/src/hir.rs b/ergoscript-compiler/src/hir.rs
index 3630b2533..93c122f81 100644
--- a/ergoscript-compiler/src/hir.rs
+++ b/ergoscript-compiler/src/hir.rs
@@ -26,7 +26,7 @@ pub fn lower(ast: ast::Root) -> Result<Expr, HirLoweringError> {
     let first_expr = exprs
         .first()
         .ok_or_else(|| AstError::new(format!("Cannot parse empty root: {:?}", ast), ast.span()))?;
-    Expr::lower(&first_expr)
+    Expr::lower(first_expr)
 }
 
 #[derive(Debug, PartialEq, Clone)]
@@ -48,7 +48,7 @@ impl HirLoweringError {
     }
 
     pub fn pretty_desc(&self, source: &str) -> String {
-        pretty_error_desc(&source, self.span, &self.msg)
+        pretty_error_desc(source, self.span, &self.msg)
     }
 }
 
diff --git a/ergoscript-compiler/src/mir/lower.rs b/ergoscript-compiler/src/mir/lower.rs
index fc1fd3edb..6d85d1066 100644
--- a/ergoscript-compiler/src/mir/lower.rs
+++ b/ergoscript-compiler/src/mir/lower.rs
@@ -22,7 +22,7 @@ impl MirLoweringError {
     }
 
     pub fn pretty_desc(&self, source: &str) -> String {
-        pretty_error_desc(&source, self.span, &self.msg)
+        pretty_error_desc(source, self.span, &self.msg)
     }
 }
 
diff --git a/ergoscript-compiler/src/type_infer.rs b/ergoscript-compiler/src/type_infer.rs
index 8656b562f..65be0c4e1 100644
--- a/ergoscript-compiler/src/type_infer.rs
+++ b/ergoscript-compiler/src/type_infer.rs
@@ -18,7 +18,7 @@ impl TypeInferenceError {
     }
 
     pub fn pretty_desc(&self, source: &str) -> String {
-        pretty_error_desc(&source, self.span, &self.msg)
+        pretty_error_desc(source, self.span, &self.msg)
     }
 }
 
diff --git a/ergotree-interpreter/src/sigma_protocol/prover.rs b/ergotree-interpreter/src/sigma_protocol/prover.rs
index 3f8d93075..ddfd7e01c 100644
--- a/ergotree-interpreter/src/sigma_protocol/prover.rs
+++ b/ergotree-interpreter/src/sigma_protocol/prover.rs
@@ -705,7 +705,7 @@ fn proving<P: Prover + ?Sized>(
                                 .find(|prover_input| prover_input.public_image() == us.proposition)
                             {
                                 let z = dlog_protocol::interactive_prover::second_message(
-                                    &priv_key,
+                                    priv_key,
                                     us.randomness_opt.ok_or_else(|| {
                                         ProverError::Unexpected(format!(
                                             "empty randomness in {:?}",
diff --git a/ergotree-interpreter/src/sigma_protocol/prover/context_extension.rs b/ergotree-interpreter/src/sigma_protocol/prover/context_extension.rs
index 83d34afaa..b5da657db 100644
--- a/ergotree-interpreter/src/sigma_protocol/prover/context_extension.rs
+++ b/ergotree-interpreter/src/sigma_protocol/prover/context_extension.rs
@@ -102,14 +102,12 @@ mod arbitrary {
         fn arbitrary_with(_args: Self::Parameters) -> Self::Strategy {
             vec(any::<Constant>(), 0..10)
                 .prop_map(|constants| {
-                    let pairs: Vec<(u8, Constant)> = constants
+                    let pairs = constants
                         .into_iter()
                         .enumerate()
                         .map(|(idx, c)| (idx as u8, c))
                         .collect();
-                    Self {
-                        values: pairs.into_iter().collect(),
-                    }
+                    Self { values: pairs }
                 })
                 .boxed()
         }
diff --git a/ergotree-interpreter/src/sigma_protocol/prover/prover_result.rs b/ergotree-interpreter/src/sigma_protocol/prover/prover_result.rs
index e4ece69df..01332b844 100644
--- a/ergotree-interpreter/src/sigma_protocol/prover/prover_result.rs
+++ b/ergotree-interpreter/src/sigma_protocol/prover/prover_result.rs
@@ -61,7 +61,7 @@ impl SigmaSerializable for ProofBytes {
             ProofBytes::Empty => w.put_u16(0)?,
             ProofBytes::Some(bytes) => {
                 w.put_u16(bytes.len() as u16)?;
-                w.write_all(&bytes)?;
+                w.write_all(bytes)?;
             }
         }
         Ok(())
diff --git a/ergotree-interpreter/src/sigma_protocol/sig_serializer.rs b/ergotree-interpreter/src/sigma_protocol/sig_serializer.rs
index 569cf500a..ee7a5f3e5 100644
--- a/ergotree-interpreter/src/sigma_protocol/sig_serializer.rs
+++ b/ergotree-interpreter/src/sigma_protocol/sig_serializer.rs
@@ -173,7 +173,7 @@ fn parse_sig_compute_challnges_reader<R: SigmaByteRead>(
 
                 let (last, rest) = cor.items.split_last();
                 for it in rest {
-                    children.push(parse_sig_compute_challnges_reader(&it, r, None)?);
+                    children.push(parse_sig_compute_challnges_reader(it, r, None)?);
                 }
                 let xored_challenge = children
                     .clone()
diff --git a/ergotree-ir/src/address.rs b/ergotree-ir/src/address.rs
index 2fb85aa04..af4e2aa3b 100644
--- a/ergotree-ir/src/address.rs
+++ b/ergotree-ir/src/address.rs
@@ -349,7 +349,7 @@ impl AddressEncoder {
         };
 
         let network_prefix = (bytes[0] & 0xF0).try_into()?;
-        AddressEncoder::unchecked_parse_address_from_bytes(&bytes).map(|addr| NetworkAddress {
+        AddressEncoder::unchecked_parse_address_from_bytes(bytes).map(|addr| NetworkAddress {
             address: addr,
             network: network_prefix,
         })
@@ -388,7 +388,7 @@ impl AddressEncoder {
 
     /// encode address as Base58 encoded string
     pub fn address_to_str(&self, address: &Address) -> String {
-        AddressEncoder::encode_address_as_string(self.network_prefix, &address)
+        AddressEncoder::encode_address_as_string(self.network_prefix, address)
     }
 
     /// encode address as Base58 encoded string
@@ -406,7 +406,7 @@ impl AddressEncoder {
     pub fn encode_address_as_string(network_prefix: NetworkPrefix, address: &Address) -> String {
         bs58::encode(AddressEncoder::encode_address_as_bytes(
             network_prefix,
-            &address,
+            address,
         ))
         .into_string()
     }
diff --git a/ergotree-ir/src/bigint256.rs b/ergotree-ir/src/bigint256.rs
index dc8ef64b2..397bac981 100644
--- a/ergotree-ir/src/bigint256.rs
+++ b/ergotree-ir/src/bigint256.rs
@@ -6,6 +6,8 @@ use std::ops::{Add, BitAnd, BitOr, BitXor, Deref, Div, Mul, Neg, Rem, Sub};
 
 use num256::int256::Int256;
 use num_bigint::BigInt;
+use num_bigint::BigUint;
+use num_bigint::ToBigInt;
 use num_derive::{One, Zero};
 use num_traits::{Bounded, CheckedAdd, CheckedDiv, CheckedMul, CheckedNeg, CheckedSub, Num};
 
@@ -30,6 +32,22 @@ impl TryFrom<BigInt> for BigInt256 {
     }
 }
 
+impl TryFrom<BigUint> for BigInt256 {
+    type Error = String;
+
+    fn try_from(value: BigUint) -> Result<Self, Self::Error> {
+        #[allow(clippy::unwrap_used)]
+        if value > Self::max_value().0 .0.to_biguint().unwrap() {
+            Err(format!(
+                "BigInt256: Value {} is larger than 2^255 - 1",
+                value
+            ))
+        } else {
+            Ok(Self(Int256(value.to_bigint().unwrap())))
+        }
+    }
+}
+
 impl TryFrom<&[u8]> for BigInt256 {
     type Error = String;
 
diff --git a/ergotree-ir/src/serialization/data.rs b/ergotree-ir/src/serialization/data.rs
index 3d89cd897..ff1ea18a2 100644
--- a/ergotree-ir/src/serialization/data.rs
+++ b/ergotree-ir/src/serialization/data.rs
@@ -32,7 +32,7 @@ impl DataSerializer {
             Value::BigInt(v) => {
                 let bytes = v.to_signed_bytes_be();
                 w.put_u16(bytes.len() as u16)?;
-                w.write_all(&&bytes)?
+                w.write_all(&bytes)?
             }
             Value::GroupElement(ecp) => ecp.sigma_serialize(w)?,
             Value::SigmaProp(s) => s.value().sigma_serialize(w)?,
diff --git a/ergotree-ir/src/sigma_protocol/dlog_group.rs b/ergotree-ir/src/sigma_protocol/dlog_group.rs
index 42b1db247..ea91c673e 100644
--- a/ergotree-ir/src/sigma_protocol/dlog_group.rs
+++ b/ergotree-ir/src/sigma_protocol/dlog_group.rs
@@ -24,7 +24,10 @@ use crate::serialization::{
 use k256::elliptic_curve::ff::PrimeField;
 use k256::elliptic_curve::sec1::ToEncodedPoint;
 use k256::{ProjectivePoint, PublicKey, Scalar};
+use num_bigint::BigUint;
 use num_bigint::Sign;
+use num_bigint::ToBigUint;
+use num_traits::ToPrimitive;
 use rand::RngCore;
 use std::convert::TryFrom;
 use std::ops::{Add, Mul, Neg};
@@ -123,27 +126,39 @@ pub fn random_scalar_in_group_range(rng: impl RngCore) -> Scalar {
 /// (FFFFFFFF FFFFFFFF FFFFFFFF FFFFFFFE BAAEDCE6 AF48A03B BFD25E8C D0364141)
 /// it might not fit into 256-bit BigInt because BigInt uses 1 bit for sign.
 pub fn scalar_to_bigint256(s: Scalar) -> Option<BigInt256> {
-    let r_g_array = s.to_bytes();
-    let r_b_array: &[u8] = r_g_array.as_slice();
-    BigInt256::try_from(r_b_array).ok()
+    // from https://github.com/RustCrypto/elliptic-curves/blob/fe737c56add103e4e8ff270d0c05ffdb6107b8d6/k256/src/arithmetic/scalar.rs#L598-L602
+    let bytes = s.to_bytes();
+    #[allow(clippy::unwrap_used)]
+    let bu: BigUint = bytes
+        .iter()
+        .enumerate()
+        .map(|(i, w)| w.to_biguint().unwrap() << ((31 - i) * 8))
+        .sum();
+    BigInt256::try_from(bu).ok()
+}
+
+fn biguint_to_bytes(x: &BigUint) -> [u8; 32] {
+    // from https://github.com/RustCrypto/elliptic-curves/blob/fe737c56add103e4e8ff270d0c05ffdb6107b8d6/k256/src/arithmetic/scalar.rs#L587-L588
+    let mask = BigUint::from(u8::MAX);
+    let mut bytes = [0u8; 32];
+    #[allow(clippy::needless_range_loop)]
+    #[allow(clippy::unwrap_used)]
+    for i in 0..32 {
+        bytes[i] = ((x >> ((31 - i) * 8)) as BigUint & &mask).to_u8().unwrap();
+    }
+    bytes
 }
 
 /// Attempts to create Scalar from BigInt256
 /// Returns None if not in the range [0, modulus).
 pub fn bigint256_to_scalar(bi: BigInt256) -> Option<Scalar> {
-    let (sign, bytes_be) = bi.to_bytes_be();
-
-    if Sign::Minus == sign {
+    if Sign::Minus == bi.sign() {
         return None;
     }
-
-    let bytes = bytes_be.as_slice();
-    debug_assert!(bytes.len() <= 32);
-    let mut bytes_32 = [0; 32];
-    for (i, v) in bytes.iter().enumerate() {
-        bytes_32[i] = *v;
-    }
-    Scalar::from_repr(bytes_32.into())
+    #[allow(clippy::unwrap_used)] // since it's 256-bit BigInt it should always fit into BigUint
+    let bu = bi.to_biguint().unwrap();
+    let bytes = biguint_to_bytes(&bu);
+    Scalar::from_repr(bytes.into())
 }
 
 impl SigmaSerializable for EcPoint {
@@ -201,8 +216,48 @@ mod tests {
     use super::*;
     use crate::mir::expr::Expr;
     use crate::serialization::sigma_serialize_roundtrip;
+    use num_bigint::BigUint;
+    use num_bigint::ToBigUint;
     use proptest::prelude::*;
-    use rand::thread_rng;
+
+    // the following Scalar <-> BigUint helpers are from k256::arithmetic::scalar
+
+    /// Converts a byte array (big-endian) to BigUint.
+    fn bytes_to_biguint(bytes: &[u8; 32]) -> BigUint {
+        bytes
+            .iter()
+            .enumerate()
+            .map(|(i, w)| w.to_biguint().unwrap() << ((31 - i) * 8))
+            .sum()
+    }
+
+    fn scalar_to_biguint(scalar: &Scalar) -> Option<BigUint> {
+        Some(bytes_to_biguint(scalar.to_bytes().as_ref()))
+    }
+
+    fn biguint_to_scalar(x: &BigUint) -> Scalar {
+        debug_assert!(x < &modulus_as_biguint());
+        let bytes = biguint_to_bytes(x);
+        Scalar::from_repr(bytes.into()).unwrap()
+    }
+
+    /// Returns the scalar modulus as a `BigUint` object.
+    fn modulus_as_biguint() -> BigUint {
+        scalar_to_biguint(&Scalar::one().negate()).unwrap() + 1.to_biguint().unwrap()
+    }
+
+    prop_compose! {
+        fn scalar()(bytes in any::<[u8; 32]>()) -> Scalar {
+            let mut res = bytes_to_biguint(&bytes);
+            let m = modulus_as_biguint();
+            // Modulus is 256 bit long, same as the maximum `res`,
+            // so this is guaranteed to land us in the correct range.
+            if res >= m {
+                res -= m;
+            }
+            biguint_to_scalar(&res)
+        }
+    }
 
     proptest! {
 
@@ -211,14 +266,22 @@ mod tests {
             let e: Expr = v.into();
             prop_assert_eq![sigma_serialize_roundtrip(&e), e];
         }
-    }
 
-    #[test]
-    fn scalar_bigint256_roundtrip() {
-        // Shift right to make sure that the MSB is 0, so that the Scalar can be
-        // converted to a BigInt256 and back
-        let rand_scalar: Scalar = random_scalar_in_group_range(thread_rng()) >> 1;
-        let as_bigint256: BigInt256 = scalar_to_bigint256(rand_scalar).unwrap();
-        assert_eq!(rand_scalar, bigint256_to_scalar(as_bigint256).unwrap());
+        #[test]
+        fn scalar_biguint_roundtrip(scalar in scalar()) {
+            let bu = scalar_to_biguint(&scalar).unwrap();
+            let to_scalar = biguint_to_scalar(&bu);
+            prop_assert_eq!(scalar, to_scalar);
+        }
+
+        #[test]
+        fn scalar_bigint256_roundtrip(scalar in scalar()) {
+            // Shift right to make sure that the MSB is 0, so that the Scalar can be
+            // converted to a BigInt256
+            let shifted_scalar = scalar >> 1;
+            let as_bigint256: BigInt256 = scalar_to_bigint256(shifted_scalar).unwrap();
+            let to_scalar = bigint256_to_scalar(as_bigint256).unwrap();
+            prop_assert_eq!(shifted_scalar, to_scalar);
+        }
     }
 }
diff --git a/ergotree-ir/src/types/stype.rs b/ergotree-ir/src/types/stype.rs
index bed936aa4..4f23e7812 100644
--- a/ergotree-ir/src/types/stype.rs
+++ b/ergotree-ir/src/types/stype.rs
@@ -122,7 +122,7 @@ impl SType {
 
     pub(crate) fn with_subst(self, subst: &HashMap<STypeVar, SType>) -> Self {
         match self {
-            SType::STypeVar(ref tpe_var) => subst.get(&tpe_var).cloned().unwrap_or(self),
+            SType::STypeVar(ref tpe_var) => subst.get(tpe_var).cloned().unwrap_or(self),
             SType::SOption(tpe) => SType::SOption(tpe.with_subst(subst).into()),
             SType::SColl(tpe) => SType::SColl(tpe.with_subst(subst).into()),
             SType::STuple(stup) => SType::STuple(stup.with_subst(subst)),


Test patch: