review comments

base_layer/core/src/proof_of_work/difficulty.rs

@@ -20,10 +20,7 @@
 // WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE
 // USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
-use std::{
-    fmt,
-    ops::{Div, Mul, Sub},
-};
+use std::fmt;
 
 use num_format::{Locale, ToFormattedString};
 use serde::{Deserialize, Serialize};
@@ -111,22 +108,44 @@ impl CheckedSub<u64> for Difficulty {
     }
 }
 
-impl Sub<Self> for Difficulty {
+impl From<Difficulty> for u64 {
+    fn from(value: Difficulty) -> Self {
+        value.0
+    }
+}
+
+impl fmt::Display for Difficulty {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        let formatted = self.0.to_formatted_string(&Locale::en);
+        write!(f, "{}", formatted)
+    }
+}
+
+#[cfg(any(test))]
+use std::ops::{Add, Div, Mul, Sub};
+
+// This trait should not be implemented for runtime, here only for testing
+#[cfg(any(test))]
+impl Add<Self> for Difficulty {
     type Output = Self;
 
-    fn sub(self, _rhs: Self) -> Self {
-        unimplemented!("Sub::sub<rhs> must not be used, use `.checked_sub(value)` instead")
+    fn add(self, _rhs: Self) -> Self {
+        unimplemented!("Sub::sub<rhs> must not be used, use `.checked_add(value)` instead")
     }
 }
 
-impl Div for Difficulty {
-    type Output = u64;
+// This trait should not be implemented for runtime, here only for testing
+#[cfg(any(test))]
+impl Sub<Self> for Difficulty {
+    type Output = Self;
 
-    fn div(self, _rhs: Self) -> Self::Output {
-        unimplemented!("Div::div<rhs> must not be used; difficulties should only be added to or subtracted from")
+    fn sub(self, _rhs: Self) -> Self {
+        unimplemented!("Sub::sub<rhs> must not be used, use `.checked_sub(value)` instead")
     }
 }
 
+// This trait should not be implemented for runtime, here only for testing
+#[cfg(any(test))]
 impl Mul for Difficulty {
     type Output = u64;
 
@@ -135,25 +154,24 @@ impl Mul for Difficulty {
     }
 }
 
-impl fmt::Display for Difficulty {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        let formatted = self.0.to_formatted_string(&Locale::en);
-        write!(f, "{}", formatted)
+// This trait should not be implemented for runtime, here only for testing
+#[cfg(any(test))]
+impl Div for Difficulty {
+    type Output = u64;
+
+    fn div(self, _rhs: Self) -> Self::Output {
+        unimplemented!("Div::div<rhs> must not be used; difficulties should only be added to or subtracted from")
     }
 }
 
+// This trait should not be implemented for runtime, here only for testing
+#[cfg(any(test))]
 impl From<u64> for Difficulty {
     fn from(_value: u64) -> Self {
         unimplemented!("Difficulty::from<u64> must not be used, use `Difficulty::from_u64(value)` instead")
     }
 }
 
-impl From<Difficulty> for u64 {
-    fn from(value: Difficulty) -> Self {
-        value.0
-    }
-}
-
 /// General difficulty adjustment algorithm trait. The key method is `get_difficulty`, which returns the target
 /// difficulty given a set of historical achieved difficulties; supplied through the `add` method.
 pub trait DifficultyAdjustment {
@@ -192,6 +210,8 @@ pub mod util {
 
     #[cfg(test)]
     mod test {
+        use std::panic::catch_unwind;
+
         use crate::{
             proof_of_work::{
                 difficulty::{
@@ -225,6 +245,30 @@ pub mod util {
             assert!(d2.checked_add(1).is_none());
         }
 
+        #[test]
+        fn it_blocks_unsupported_traits() {
+            let add_result = catch_unwind(|| {
+                let _ = Difficulty::min() + Difficulty::min();
+            });
+            assert!(add_result.is_err());
+            let sub_result = catch_unwind(|| {
+                let _ = Difficulty::max() - Difficulty::min();
+            });
+            assert!(sub_result.is_err());
+            let mul_result = catch_unwind(|| {
+                let _ = Difficulty::max() * Difficulty::min();
+            });
+            assert!(mul_result.is_err());
+            let div_result = catch_unwind(|| {
+                let _ = Difficulty::max() / Difficulty::min();
+            });
+            assert!(div_result.is_err());
+            let from_result = catch_unwind(|| {
+                let _ = Difficulty::from(10u64);
+            });
+            assert!(from_result.is_err());
+        }
+
         #[test]
         fn subtraction_does_not_underflow() {
             let d1 = Difficulty::from_u64(100).unwrap();