fix(p/int256): prevent negative zero (#2750)

# Description - Added a check to prevent appending a minus sign to zero values. This ensures that "0" always returned for zero values. - Fix negative number handling in the Arithmetic function - Add divide by zero panic in `Div` function <details><summary>Contributors' checklist...</summary> - [x] Added new tests, or not needed, or not feasible - [ ] Provided an example (e.g. screenshot) to aid review or the PR is self-explanatory - [ ] Updated the official documentation or not needed - [ ] No breaking changes were made, or a `BREAKING CHANGE: xxx` message was included in the description - [ ] Added references to related issues and PRs - [ ] Provided any useful hints for running manual tests - [ ] Added new benchmarks to [generated graphs](https://gnoland.github.io/benchmarks), if any. More info [here](https://github.com/gnolang/gno/blob/master/.benchmarks/README.md). </details>
gnolang · Sep 13, 2024 · 1239f1d · 1239f1d
1 parent 5244f33
commit 1239f1d
Show file tree

Hide file tree

Showing 4 changed files with 140 additions and 70 deletions.
diff --git a/examples/gno.land/p/demo/int256/arithmetic.gno b/examples/gno.land/p/demo/int256/arithmetic.gno
@@ -5,23 +5,23 @@ import "gno.land/p/demo/uint256"
 func (z *Int) Add(x, y *Int) *Int {
 	z.initiateAbs()
 
-	neg := x.neg
-
 	if x.neg == y.neg {
-		// x + y == x + y
-		// (-x) + (-y) == -(x + y)
-		z.abs = z.abs.Add(x.abs, y.abs)
+		// If both numbers have the same sign, add their absolute values
+		z.abs.Add(x.abs, y.abs)
+		z.neg = x.neg
 	} else {
-		// x + (-y) == x - y == -(y - x)
-		// (-x) + y == y - x == -(x - y)
-		if x.abs.Cmp(y.abs) >= 0 {
-			z.abs = z.abs.Sub(x.abs, y.abs)
-		} else {
-			neg = !neg
-			z.abs = z.abs.Sub(y.abs, x.abs)
+		switch x.abs.Cmp(y.abs) {
+		case 1: // x > y
+			z.abs.Sub(x.abs, y.abs)
+			z.neg = x.neg
+		case -1: // x < y
+			z.abs.Sub(y.abs, x.abs)
+			z.neg = y.neg
+		case 0: // x == y
+			z.abs = uint256.NewUint(0)
 		}
 	}
-	z.neg = neg // 0 has no sign
+
 	return z
 }
 
@@ -66,22 +66,27 @@ func AddDeltaOverflow(z, x *uint256.Uint, y *Int) bool {
 func (z *Int) Sub(x, y *Int) *Int {
 	z.initiateAbs()
 
-	neg := x.neg
 	if x.neg != y.neg {
-		// x - (-y) == x + y
-		// (-x) - y == -(x + y)
-		z.abs = z.abs.Add(x.abs, y.abs)
+		// If sign are different, add the absolute values
+		z.abs.Add(x.abs, y.abs)
+		z.neg = x.neg
 	} else {
-		// x - y == x - y == -(y - x)
-		// (-x) - (-y) == y - x == -(x - y)
-		if x.abs.Cmp(y.abs) >= 0 {
-			z.abs = z.abs.Sub(x.abs, y.abs)
-		} else {
-			neg = !neg
-			z.abs = z.abs.Sub(y.abs, x.abs)
+		switch x.abs.Cmp(y.abs) {
+		case 1: // x > y
+			z.abs.Sub(x.abs, y.abs)
+			z.neg = x.neg
+		case -1: // x < y
+			z.abs.Sub(y.abs, x.abs)
+			z.neg = !x.neg
+		case 0: // x == y
+			z.abs = uint256.NewUint(0)
 		}
 	}
-	z.neg = neg // 0 has no sign
+
+	// Ensure zero is always positive
+	if z.abs.IsZero() {
+		z.neg = false
+	}
 	return z
 }
 
@@ -107,7 +112,7 @@ func (z *Int) Mul(x, y *Int) *Int {
 	z.initiateAbs()
 
 	z.abs = z.abs.Mul(x.abs, y.abs)
-	z.neg = x.neg != y.neg // 0 has no sign
+	z.neg = x.neg != y.neg && !z.abs.IsZero() // 0 has no sign
 	return z
 }
 
@@ -126,12 +131,13 @@ func (z *Int) MulUint256(x *Int, y *uint256.Uint) *Int {
 func (z *Int) Div(x, y *Int) *Int {
 	z.initiateAbs()
 
-	z.abs.Div(x.abs, y.abs)
-	if x.neg == y.neg {
-		z.neg = false
-	} else {
-		z.neg = true
+	if y.abs.IsZero() {
+		panic("division by zero")
 	}
+
+	z.abs.Div(x.abs, y.abs)
+	z.neg = (x.neg != y.neg) && !z.abs.IsZero() // 0 has no sign
+
 	return z
 }
 

diff --git a/examples/gno.land/p/demo/int256/arithmetic_test.gno b/examples/gno.land/p/demo/int256/arithmetic_test.gno
@@ -15,8 +15,9 @@ func TestAdd(t *testing.T) {
 		{"1", "1", "2"},
 		{"1", "2", "3"},
 		// NEGATIVE
-		{"-1", "1", "-0"}, // TODO: remove negative sign for 0 ??
+		{"-1", "1", "0"},
 		{"1", "-1", "0"},
+		{"3", "-3", "0"},
 		{"-1", "-1", "-2"},
 		{"-1", "-2", "-3"},
 		{"-1", "3", "2"},
@@ -188,10 +189,10 @@ func TestSub(t *testing.T) {
 		{"1", "1", "0"},
 		{"-1", "1", "-2"},
 		{"1", "-1", "2"},
-		{"-1", "-1", "-0"},
-		{"-115792089237316195423570985008687907853269984665640564039457584007913129639935", "-115792089237316195423570985008687907853269984665640564039457584007913129639935", "-0"},
+		{"-1", "-1", "0"},
+		{"-115792089237316195423570985008687907853269984665640564039457584007913129639935", "-115792089237316195423570985008687907853269984665640564039457584007913129639935", "0"},
 		{"-115792089237316195423570985008687907853269984665640564039457584007913129639935", "0", "-115792089237316195423570985008687907853269984665640564039457584007913129639935"},
-		{x: "-115792089237316195423570985008687907853269984665640564039457584007913129639935", y: "1", want: "-0"},
+		{x: "-115792089237316195423570985008687907853269984665640564039457584007913129639935", y: "1", want: "0"},
 	}
 
 	for _, tc := range tests {
@@ -351,46 +352,45 @@ func TestMulUint256(t *testing.T) {
 
 func TestDiv(t *testing.T) {
 	tests := []struct {
-		x, y, want string
+		x, y, expected string
 	}{
+		{"1", "1", "1"},
 		{"0", "1", "0"},
-		{"0", "-1", "-0"},
-		{"10", "0", "0"},
-		{"10", "1", "10"},
-		{"10", "-1", "-10"},
-		{"-10", "0", "-0"},
-		{"-10", "1", "-10"},
-		{"-10", "-1", "10"},
-		{"10", "-3", "-3"},
-		{"10", "3", "3"},
+		{"-1", "1", "-1"},
+		{"1", "-1", "-1"},
+		{"-1", "-1", "1"},
+		{"-6", "3", "-2"},
+		{"10", "-2", "-5"},
+		{"-10", "3", "-3"},
+		{"7", "3", "2"},
+		{"-7", "3", "-2"},
+		{"115792089237316195423570985008687907853269984665640564039457584007913129639935", "2", "57896044618658097711785492504343953926634992332820282019728792003956564819967"}, // Max uint256 / 2
 	}
 
-	for _, tc := range tests {
-		x, err := FromDecimal(tc.x)
-		if err != nil {
-			t.Error(err)
-			continue
-		}
-
-		y, err := FromDecimal(tc.y)
-		if err != nil {
-			t.Error(err)
-			continue
-		}
-
-		want, err := FromDecimal(tc.want)
-		if err != nil {
-			t.Error(err)
-			continue
-		}
-
-		got := New()
-		got.Div(x, y)
-
-		if got.Neq(want) {
-			t.Errorf("Div(%s, %s) = %v, want %v", tc.x, tc.y, got.ToString(), want.ToString())
-		}
+	for _, tt := range tests {
+		t.Run(tt.x+"/"+tt.y, func(t *testing.T) {
+			x := MustFromDecimal(tt.x)
+			y := MustFromDecimal(tt.y)
+			result := Zero().Div(x, y)
+			if result.ToString() != tt.expected {
+				t.Errorf("Div(%s, %s) = %s, want %s", tt.x, tt.y, result.ToString(), tt.expected)
+			}
+			if result.abs.IsZero() && result.neg {
+				t.Errorf("Div(%s, %s) resulted in negative zero", tt.x, tt.y)
+			}
+		})
 	}
+
+	t.Run("Division by zero", func(t *testing.T) {
+		defer func() {
+			if r := recover(); r == nil {
+				t.Errorf("Div(1, 0) did not panic")
+			}
+		}()
+		x := MustFromDecimal("1")
+		y := MustFromDecimal("0")
+		Zero().Div(x, y)
+	})
 }
 
 func TestDivUint256(t *testing.T) {

diff --git a/examples/gno.land/p/demo/int256/conversion.gno b/examples/gno.land/p/demo/int256/conversion.gno
@@ -82,5 +82,6 @@ func (z *Int) ToString() string {
 	if z.neg {
 		return "-" + t
 	}
+
 	return t
 }
diff --git a/examples/gno.land/p/demo/int256/conversion_test.gno b/examples/gno.land/p/demo/int256/conversion_test.gno
@@ -169,3 +169,66 @@ func TestSetUint256(t *testing.T) {
 		}
 	}
 }
+
+func TestToString(t *testing.T) {
+	tests := []struct {
+		name     string
+		setup    func() *Int
+		expected string
+	}{
+		{
+			name: "Zero from subtraction",
+			setup: func() *Int {
+				minusThree := MustFromDecimal("-3")
+				three := MustFromDecimal("3")
+				return Zero().Add(minusThree, three)
+			},
+			expected: "0",
+		},
+		{
+			name: "Zero from right shift",
+			setup: func() *Int {
+				return Zero().Rsh(One(), 1234)
+			},
+			expected: "0",
+		},
+		{
+			name: "Positive number",
+			setup: func() *Int {
+				return MustFromDecimal("42")
+			},
+			expected: "42",
+		},
+		{
+			name: "Negative number",
+			setup: func() *Int {
+				return MustFromDecimal("-42")
+			},
+			expected: "-42",
+		},
+		{
+			name: "Large positive number",
+			setup: func() *Int {
+				return MustFromDecimal("115792089237316195423570985008687907853269984665640564039457584007913129639935")
+			},
+			expected: "115792089237316195423570985008687907853269984665640564039457584007913129639935",
+		},
+		{
+			name: "Large negative number",
+			setup: func() *Int {
+				return MustFromDecimal("-115792089237316195423570985008687907853269984665640564039457584007913129639935")
+			},
+			expected: "-115792089237316195423570985008687907853269984665640564039457584007913129639935",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			z := tt.setup()
+			result := z.ToString()
+			if result != tt.expected {
+				t.Errorf("ToString() = %s, want %s", result, tt.expected)
+			}
+		})
+	}
+}