From 9a1808d127a417b9fd27f93cb3f0cbe3a37a7b60 Mon Sep 17 00:00:00 2001 From: "Nelson R. Perez" Date: Thu, 23 Nov 2017 22:58:22 -0500 Subject: [PATCH] Modified AssetAmount#multiply() and AssetAmount#divide() to not cast its value to a long primitive and thus preventing some overflow scenarios that the idea of using the UnsignedLong class is suposed to prevent --- .../cy/agorise/graphenej/AssetAmount.java | 9 ++++-- .../cy/agorise/graphenej/AssetAmountTest.java | 29 +++++++++++++++++-- 2 files changed, 34 insertions(+), 4 deletions(-) diff --git a/graphenej/src/main/java/cy/agorise/graphenej/AssetAmount.java b/graphenej/src/main/java/cy/agorise/graphenej/AssetAmount.java index 312662e..9170581 100644 --- a/graphenej/src/main/java/cy/agorise/graphenej/AssetAmount.java +++ b/graphenej/src/main/java/cy/agorise/graphenej/AssetAmount.java @@ -17,6 +17,7 @@ import java.io.DataOutput; import java.io.DataOutputStream; import java.io.IOException; import java.lang.reflect.Type; +import java.math.BigDecimal; import java.math.RoundingMode; import cy.agorise.graphenej.errors.IncompatibleOperation; @@ -91,7 +92,9 @@ public class AssetAmount implements ByteSerializable, JsonSerializable { * @return The same AssetAmount instance, but with the changed amount value. */ public AssetAmount multiplyBy(double factor, RoundingMode roundingMode){ - this.amount = UnsignedLong.valueOf(DoubleMath.roundToLong(this.amount.longValue() * factor, roundingMode)); + BigDecimal originalAmount = new BigDecimal(amount.bigIntegerValue()); + BigDecimal decimalResult = originalAmount.multiply(new BigDecimal(factor)); + this.amount = UnsignedLong.valueOf(DoubleMath.roundToBigInteger(decimalResult.doubleValue(), roundingMode)); return this; } @@ -111,7 +114,9 @@ public class AssetAmount implements ByteSerializable, JsonSerializable { * @return: The same AssetAMount instance, but with the divided amount value */ public AssetAmount dividedBy(double divisor, RoundingMode roundingMode){ - this.amount = UnsignedLong.valueOf(DoubleMath.roundToLong(this.amount.longValue() / divisor, roundingMode)); + BigDecimal originalAmount = new BigDecimal(amount.bigIntegerValue()); + BigDecimal decimalAmount = originalAmount.divide(new BigDecimal(divisor), 18, RoundingMode.HALF_UP); + this.amount = UnsignedLong.valueOf(DoubleMath.roundToBigInteger(decimalAmount.doubleValue(), roundingMode)); return this; } diff --git a/graphenej/src/test/java/cy/agorise/graphenej/AssetAmountTest.java b/graphenej/src/test/java/cy/agorise/graphenej/AssetAmountTest.java index 8589d93..4113957 100644 --- a/graphenej/src/test/java/cy/agorise/graphenej/AssetAmountTest.java +++ b/graphenej/src/test/java/cy/agorise/graphenej/AssetAmountTest.java @@ -1,13 +1,14 @@ package cy.agorise.graphenej; import com.google.common.primitives.UnsignedLong; + import org.junit.Before; import org.junit.Test; -import static org.junit.Assert.*; +import static org.junit.Assert.assertEquals; /** - * Created by nelson on 2/23/17. + * Testing AssetAmount operations. */ public class AssetAmountTest { private final int LARGE_VALUE = 1000; @@ -27,4 +28,28 @@ public class AssetAmountTest { assertEquals(large.subtract(small).getAmount(), new AssetAmount(UnsignedLong.valueOf(LARGE_VALUE - SMALL_VALUE), testAsset).getAmount()); assertEquals(small.subtract(large).getAmount(), new AssetAmount(UnsignedLong.valueOf(Math.abs(SMALL_VALUE - LARGE_VALUE)), testAsset).getAmount()); } + + @Test + public void testMultiplication(){ + // Testing a simple multiplication by a double + AssetAmount result = large.multiplyBy(0.5); + assertEquals(500, result.getAmount().longValue()); + + // Testing the multiplication of a number that would normally give an overflow + AssetAmount max = new AssetAmount(UnsignedLong.valueOf(Long.MAX_VALUE), testAsset); + AssetAmount overMaxLong = max.multiplyBy(1.5); + assertEquals("13835058055282163712", overMaxLong.getAmount().toString(10)); + } + + @Test + public void testDivision(){ + // Testing a simple division by a double + AssetAmount result = large.dividedBy(0.5); + assertEquals(2000, result.getAmount().longValue()); + + // Testing a division of a number that would normally give an overflow + AssetAmount max = new AssetAmount(UnsignedLong.valueOf(Long.MAX_VALUE), testAsset); + AssetAmount overMaxLong = max.dividedBy(0.8); + assertEquals("11529215046068469760", overMaxLong.getAmount().toString()); + } } \ No newline at end of file