From 957ac48da4556e84f12744646ba97cff78889001 Mon Sep 17 00:00:00 2001
From: Nirvan Tyagi <nirvantyagi@users.noreply.github.com>
Date: Fri, 20 Nov 2020 12:54:13 -0500
Subject: [PATCH] Apply suggestions from code review

Co-authored-by: Pratyush Mishra <pratyushmishra@berkeley.edu>
---
 src/bits/uint.rs  | 26 ++++++++++++++------------
 src/bits/uint8.rs | 20 ++++++++++----------
 2 files changed, 24 insertions(+), 22 deletions(-)

diff --git a/src/bits/uint.rs b/src/bits/uint.rs
index 20bbcd4..40c9d95 100644
--- a/src/bits/uint.rs
+++ b/src/bits/uint.rs
@@ -330,23 +330,25 @@ macro_rules! make_uint {
             }
 
             impl<ConstraintF: Field> CondSelectGadget<ConstraintF> for $name<ConstraintF> {
-                #[tracing::instrument(target = "r1cs")]
+                #[tracing::instrument(target = "r1cs", skip(cond, true_value, false_value))]
                 fn conditionally_select(
                     cond: &Boolean<ConstraintF>,
                     true_value: &Self,
                     false_value: &Self,
                 ) -> Result<Self, SynthesisError> {
-                    let selected_bits = true_value.bits.iter().zip(&false_value.bits)
-                        .map(|(true_bit, false_bit)| {
-                            cond.select(true_bit, false_bit)
-                        }).collect::<Result<Vec<Boolean<ConstraintF>>, SynthesisError>>()?;
-                    let selected_value = match (cond.value(), true_value.value(), false_value.value()) {
-                        (Ok(true), Err(_), _) => None,
-                        (Ok(true), Ok(v), _) => Some(v),
-                        (Ok(false), _, Err(_)) => None,
-                        (Ok(false), _, Ok(v)) => Some(v),
-                        (Err(_), _, _) => None,
-                    };
+                    let selected_bits = true_value
+                    	.bits
+                    	.iter()
+                    	.zip(&false_value.bits)
+                        .map(|(t, f)| cond.select(t, f))
+                        .collect::<Result<Vec<_>, SynthesisError>>()?;
+                    let selected_value = cond.value().ok().and_then(|cond| {
+                    	if cond {
+                    		true_value.value().ok()
+                    	} else {
+                    		false_value.value().ok()
+                    	}
+                    });
                     Ok(Self {
                         bits: selected_bits,
                         value: selected_value,
diff --git a/src/bits/uint8.rs b/src/bits/uint8.rs
index 512fbab..8e294e9 100644
--- a/src/bits/uint8.rs
+++ b/src/bits/uint8.rs
@@ -285,7 +285,7 @@ impl<ConstraintF: Field> EqGadget<ConstraintF> for UInt8<ConstraintF> {
 }
 
 impl<ConstraintF: Field> CondSelectGadget<ConstraintF> for UInt8<ConstraintF> {
-    #[tracing::instrument(target = "r1cs")]
+    #[tracing::instrument(target = "r1cs", skip(cond, true_value, false_value))]
     fn conditionally_select(
         cond: &Boolean<ConstraintF>,
         true_value: &Self,
@@ -295,15 +295,15 @@ impl<ConstraintF: Field> CondSelectGadget<ConstraintF> for UInt8<ConstraintF> {
             .bits
             .iter()
             .zip(&false_value.bits)
-            .map(|(true_bit, false_bit)| cond.select(true_bit, false_bit))
-            .collect::<Result<Vec<Boolean<ConstraintF>>, SynthesisError>>()?;
-        let selected_value = match (cond.value(), true_value.value(), false_value.value()) {
-            (Ok(true), Err(_), _) => None,
-            (Ok(true), Ok(v), _) => Some(v),
-            (Ok(false), _, Err(_)) => None,
-            (Ok(false), _, Ok(v)) => Some(v),
-            (Err(_), _, _) => None,
-        };
+            .map(|(t, f)| cond.select(t, f))
+            .collect::<Result<Vec<_>, SynthesisError>>()?;
+                    let selected_value = cond.value().ok().and_then(|cond| {
+                    	if cond {
+                    		true_value.value().ok()
+                    	} else {
+                    		false_value.value().ok()
+                    	}
+                    });
         Ok(Self {
             bits: selected_bits,
             value: selected_value,