Add uint256 fuzz test vs TTMath (#34) + Fix multiplication carry bug #36

* Add uint256 fuzz test vs TTMath

* fix the compilation of property_based_uint256

* Fix multiplication carry bug #35

* Change comment in test

src/private/uint_mul.nim

@@ -105,28 +105,35 @@ template extPrecMulImpl*[T](result: var UintImpl[UintImpl[T]], op: untyped, x, y
 
   var z1: type x
 
-  # Low part - z0
+  # Low part and hi part - z0 & z2
   when eqSym(op, `+=`):
     extPrecAddMul(result.lo, x.lo, y.lo)
+    extPrecAddMul(result.hi, x.hi, y.hi)
   else:
     extPrecMul(result.lo, x.lo, y.lo)
+    extPrecMul(result.hi, x.hi, y.hi)
 
-  # Middle part - z1
+  ## TODO - fuse those parts and reduce the number of carry checks
+  # Middle part - z1 - 1st mul
   extPrecMul(z1, x.hi, y.lo)
-  let carry_check = z1
-  extPrecAddMul(z1, x.lo, y.hi)
-  if z1 < carry_check:
-    inc result.hi.lo
-
-  # High part - z2
-  result.hi.lo +=  z1.hi
-  extPrecAddMul(result.hi, x.hi, y.hi)
-
-  # Finalize low part
   result.lo.hi += z1.lo
   if result.lo.hi < z1.lo:
     inc result.hi
 
+  result.hi.lo += z1.hi
+  if result.hi.lo < z1.hi:
+    inc result.hi.hi
+
+  # Middle part - z1 - 2nd mul
+  extPrecMul(z1, x.lo, y.hi)
+  result.lo.hi += z1.lo
+  if result.lo.hi < z1.lo:
+    inc result.hi
+
+  result.hi.lo += z1.hi
+  if result.hi.lo < z1.hi:
+    inc result.hi.hi
+
 func extPrecAddMul[T](result: var UintImpl[UintImpl[T]], u, v: UintImpl[T]) =
   ## Extended precision fused in-place addition & multiplication
   extPrecMulImpl(result, `+=`, u, v)

stint.nimble

@@ -7,8 +7,8 @@ srcDir        = "src"
 
 ### Dependencies
 
-# TODO remove test only requirements: https://github.com/nim-lang/nimble/issues/482
+# TODO remove quicktest and ttmath test only requirements: https://github.com/nim-lang/nimble/issues/482
-requires "nim >= 0.18", "https://github.com/alehander42/nim-quicktest >= 0.18.0"
+requires "nim >= 0.18", "https://github.com/alehander42/nim-quicktest >= 0.18.0", "https://github.com/status-im/nim-ttmath"
 
 proc test(name: string, lang: string = "c") =
   if not dirExists "build":
@@ -34,7 +34,7 @@ task test_debug, "Run all tests - test implementation (StUint[64] = 2x uint32":
 task test_release, "Run all tests - prod implementation (StUint[64] = uint64":
   test "all_tests"
 
-task test_property_debug, "Run random tests (normal mode) - test implementation (StUint[64] = 2x uint32)":
+task test_property_debug, "Run random tests (debug mode) - test implementation (StUint[64] = 2x uint32)":
   requires "quicktest > 0.0.8"
   switch("define", "mpint_test")
   test "property_based"
@@ -45,13 +45,13 @@ task test_property_release, "Run random tests (release mode) - test implementati
   switch("define", "release")
   test "property_based"
 
-task test_property_uint256_debug, "Run random tests (normal mode) vs TTMath on Uint256":
+task test_property_uint256_debug, "Run random tests Uint256 (debug mode) vs TTMath (StUint[256] = 8 x uint32)":
   # TODO: another reference implementation?
   # Note that currently importing both Stint and TTMath will crash the compiler for unknown reason.
   requires "quicktest > 0.0.8"
   test "property_based", "cpp"
 
-task test_property_uint256_release, "Run random tests (release mode) vs TTMath on Uint256":
+task test_property_uint256_release, "Run random tests Uint256 (release mode) vs TTMath (StUint[256] = 4 x uint64)":
   # TODO: another reference implementation?
   # Note that currently importing both Stint and TTMath will crash the compiler for unknown reason.
   requires "quicktest > 0.0.8"
@@ -65,4 +65,5 @@ task test, "Run all tests - test and production implementation":
   exec "nimble test_release"
   exec "nimble test_property_debug"
   exec "nimble test_property_release"
-  # TODO: ttmath tests, but importing both together raises illegal storage access
+  exec "nimble test_property_uint256_debug"
+  exec "nimble test_property_uint256_release"

tests/property_based.nim

@@ -14,9 +14,9 @@ const itercount = 1000
 suite "Property-based testing (testing with random inputs) - uint64 on 64-bit / uint32 on 32-bit":
 
   when defined(release):
-    echo "Testing in release mode with " & $itercount & " random tests for each proc."
+    echo "Testing in release mode with " & $itercount & " random tests for each proc. (StUint[64] = uint64)"
   else:
-    echo "Testing in normal (non-release) mode " & $itercount & " random tests for each proc."
+    echo "Testing in debug mode " & $itercount & " random tests for each proc. (StUint[64] = 2x uint32)"
 
   let hi = 1'u shl (sizeof(uint)*7)
 

tests/property_based_uint256.nim

@@ -14,12 +14,12 @@ import ../src/stint, unittest, quicktest, ttmath_compat
 
 const itercount = 1000
 
-suite "Property-based testing (testing with random inputs) - uint64 on 64-bit / uint32 on 32-bit":
+suite "Property-based testing (testing with random inputs) of Uint256":
 
   when defined(release):
-    echo "Testing in release mode"
+    echo "Testing in release mode. (StUint[256] = 4 x uint64)"
   else:
-    echo "Testing in normal (non-release) mode"
+    echo "Testing in debug mode. (StUint[256] = 8 x uint32)"
 
   let hi = 1'u shl (sizeof(uint)*7)