Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BigInteger in Neo.Json #3037

Open
wants to merge 20 commits into
base: HF_Echidna
Choose a base branch
from
Open
24 changes: 21 additions & 3 deletions src/Neo.Json/JNumber.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
// modifications are permitted.

using System.Globalization;
using System.Numerics;
using System.Text.Json;

namespace Neo.Json
Expand Down Expand Up @@ -40,7 +41,13 @@ public class JNumber : JToken
/// <param name="value">The value of the JSON token.</param>
public JNumber(double value = 0)
{
if (!double.IsFinite(value)) throw new FormatException();
if (!double.IsFinite(value))
shargon marked this conversation as resolved.
Show resolved Hide resolved
throw new ArgumentException("value is not finite", nameof(value));
if (value > MAX_SAFE_INTEGER)
throw new ArgumentException("value is higher than MAX_SAFE_INTEGER", nameof(value));
if (value < MIN_SAFE_INTEGER)
throw new ArgumentException("value is lower than MIN_SAFE_INTEGER", nameof(value));

this.Value = value;
}

Expand Down Expand Up @@ -119,11 +126,22 @@ public static implicit operator JNumber(double value)
return new JNumber(value);
}

public static implicit operator JNumber(long value)
public static implicit operator JNumber(BigInteger value)
{
return new JNumber(value);
return new JNumber((long)value);
}

/// <summary>
/// Check if two JNumber are equal.
/// </summary>
/// <param name="left">Non null <see cref="JNumber"></see> value </param>
/// <param name="right">Nullable <see cref="JNumber"></see> value</param>
/// <returns>bool</returns>
/// <remarks>
/// If the left is null, throw an <exception cref="NullReferenceException"></exception>.
/// If the right is null, return false.
/// If the left and right are the same object, return true.
/// </remarks>
public static bool operator ==(JNumber left, JNumber? right)
Jim8y marked this conversation as resolved.
Show resolved Hide resolved
{
if (right is null) return false;
Expand Down
6 changes: 6 additions & 0 deletions src/Neo.Json/JToken.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
// Redistribution and use in source and binary forms with or without
// modifications are permitted.

using System.Numerics;
using System.Text.Json;
using static Neo.Json.Utility;

Expand Down Expand Up @@ -298,6 +299,11 @@ public static implicit operator JToken(double value)
return (JNumber)value;
}

public static implicit operator JToken(BigInteger value)
{
return (JNumber)value;
}

public static implicit operator JToken?(string? value)
{
return (JString?)value;
Expand Down
7 changes: 2 additions & 5 deletions src/Neo/SmartContract/JsonSerializer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,7 @@ public static JToken Serialize(StackItem item)
}
case Integer num:
{
var integer = num.GetInteger();
if (integer > JNumber.MAX_SAFE_INTEGER || integer < JNumber.MIN_SAFE_INTEGER)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello, #2498.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

He put it in the JNumber. Different exception, but will fail as well.

Copy link
Member Author

@shargon shargon Dec 29, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was moved to the constructor, that was the bug, when we used parse we allowed it...

throw new InvalidOperationException();
return (double)integer;
return num.GetInteger();
}
case Boolean boolean:
{
Expand All @@ -66,7 +63,7 @@ public static JToken Serialize(StackItem item)

foreach (var entry in map)
{
if (!(entry.Key is ByteString)) throw new FormatException();
if (entry.Key is not ByteString) throw new FormatException();

var key = entry.Key.GetString();
var value = Serialize(entry.Value);
Expand Down
33 changes: 30 additions & 3 deletions tests/Neo.Json.UnitTests/UT_JNumber.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
// Redistribution and use in source and binary forms with or without
// modifications are permitted.

using System.Numerics;

namespace Neo.Json.UnitTests
{
enum Woo
Expand Down Expand Up @@ -37,20 +39,45 @@ public void SetUp()
public void TestAsBoolean()
{
maxInt.AsBoolean().Should().BeTrue();
minInt.AsBoolean().Should().BeTrue();
zero.AsBoolean().Should().BeFalse();
}

[TestMethod]
public void TestBigInteger()
{
((JNumber)BigInteger.One).AsNumber().Should().Be(1);
((JNumber)BigInteger.Zero).AsNumber().Should().Be(0);
((JNumber)BigInteger.MinusOne).AsNumber().Should().Be(-1);
}
Copy link
Member

@cschuchardt88 cschuchardt88 Dec 27, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When doing testing, use max and min values please. You will see that using the max or min will fail.... double is to small to hold the data. BigInteger is 32 BYTES and double is 8 BYTES you get overflows. Its like doing

var failed = checked((int)long.MaxValue);

image

image

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ping @shargon

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added ut with min and max

Jim8y marked this conversation as resolved.
Show resolved Hide resolved

[TestMethod]
public void TestNullEqual()
{
JNumber nullJNumber = null;

Assert.IsFalse(maxInt.Equals(null));
Assert.IsFalse(maxInt == null);
Assert.IsFalse(minInt.Equals(null));
Assert.IsFalse(minInt == null);
Assert.IsFalse(zero == null);

Assert.ThrowsException<NullReferenceException>(() => nullJNumber == maxInt);
Assert.ThrowsException<NullReferenceException>(() => nullJNumber == minInt);
Assert.ThrowsException<NullReferenceException>(() => nullJNumber == zero);
}

[TestMethod]
public void TestAsString()
{
Action action1 = () => new JNumber(double.PositiveInfinity).AsString();
action1.Should().Throw<FormatException>();
action1.Should().Throw<ArgumentException>();

Action action2 = () => new JNumber(double.NegativeInfinity).AsString();
action2.Should().Throw<FormatException>();
action2.Should().Throw<ArgumentException>();

Action action3 = () => new JNumber(double.NaN).AsString();
action3.Should().Throw<FormatException>();
action3.Should().Throw<ArgumentException>();
}

[TestMethod]
Expand Down
12 changes: 6 additions & 6 deletions tests/Neo.UnitTests/SmartContract/UT_JsonSerializer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ public void JsonTest_WrongJson()
Assert.ThrowsException<FormatException>(() => JObject.Parse(json));

json = @"{""length"":99999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999}";
Assert.ThrowsException<FormatException>(() => JObject.Parse(json));
Assert.ThrowsException<ArgumentException>(() => JObject.Parse(json));
}

[TestMethod]
Expand Down Expand Up @@ -91,10 +91,10 @@ public void JsonTest_Numbers()

Assert.AreEqual("[1,-2,3.5]", parsed.ToString());

json = "[200.500000E+005,200.500000e+5,-1.1234e-100,9.05E+28]";
json = "[200.500000E+005,200.500000e+5,-1.1234e-100,9.05E+8]";
parsed = JObject.Parse(json);

Assert.AreEqual("[20050000,20050000,-1.1234E-100,9.05E+28]", parsed.ToString());
Assert.AreEqual("[20050000,20050000,-1.1234E-100,905000000]", parsed.ToString());

json = "[-]";
Assert.ThrowsException<FormatException>(() => JObject.Parse(json));
Expand Down Expand Up @@ -216,7 +216,7 @@ public void Serialize_EmptyObject()
public void Serialize_Number()
{
var entry = new VM.Types.Array { 1, 9007199254740992 };
Assert.ThrowsException<InvalidOperationException>(() => JsonSerializer.Serialize(entry));
Assert.ThrowsException<ArgumentException>(() => JsonSerializer.Serialize(entry));
}

[TestMethod]
Expand Down Expand Up @@ -303,7 +303,7 @@ public void Serialize_Array_Bool_Str_Num()
public void Deserialize_Array_Bool_Str_Num()
{
ApplicationEngine engine = ApplicationEngine.Create(TriggerType.Application, null, null);
var items = JsonSerializer.Deserialize(engine, JObject.Parse("[true,\"test\",123,9.05E+28]"), ExecutionEngineLimits.Default);
var items = JsonSerializer.Deserialize(engine, JObject.Parse("[true,\"test\",123,1.05E+4]"), ExecutionEngineLimits.Default);

Assert.IsInstanceOfType(items, typeof(VM.Types.Array));
Assert.AreEqual(((VM.Types.Array)items).Count, 4);
Expand All @@ -313,7 +313,7 @@ public void Deserialize_Array_Bool_Str_Num()
Assert.IsTrue(array[0].GetBoolean());
Assert.AreEqual(array[1].GetString(), "test");
Assert.AreEqual(array[2].GetInteger(), 123);
Assert.AreEqual(array[3].GetInteger(), BigInteger.Parse("90500000000000000000000000000"));
Assert.AreEqual(array[3].GetInteger(), BigInteger.Parse("10500"));
}

[TestMethod]
Expand Down