From 44ca39bfd7df0068a7aedb1780a8ad954e1cd50c Mon Sep 17 00:00:00 2001 From: Andreas Schildbach Date: Wed, 2 Dec 2015 10:27:27 +0100 Subject: [PATCH] Coin: Remove chain-specific coin limit. Add some checks for arithmetic over/underflows instead. --- .../src/main/java/org/bitcoinj/core/Coin.java | 4 -- .../java/org/bitcoinj/uri/BitcoinURI.java | 2 + .../test/java/org/bitcoinj/core/CoinTest.java | 45 ++++++++++--------- .../org/bitcoinj/utils/ExchangeRateTest.java | 24 ---------- 4 files changed, 27 insertions(+), 48 deletions(-) diff --git a/core/src/main/java/org/bitcoinj/core/Coin.java b/core/src/main/java/org/bitcoinj/core/Coin.java index 5ba7d0f4..dd7884a7 100644 --- a/core/src/main/java/org/bitcoinj/core/Coin.java +++ b/core/src/main/java/org/bitcoinj/core/Coin.java @@ -84,9 +84,6 @@ public final class Coin implements Monetary, Comparable, Serializable { public final long value; private Coin(final long satoshis) { - long maxSatoshis = COIN_VALUE * NetworkParameters.MAX_COINS; - checkArgument(-maxSatoshis <= satoshis && satoshis <= maxSatoshis, - "%s satoshis exceeds maximum possible quantity of Bitcoin.", satoshis); this.value = satoshis; } @@ -115,7 +112,6 @@ public final class Coin implements Monetary, Comparable, Serializable { checkArgument(cents >= 0); checkArgument(coins >= 0); final Coin coin = COIN.multiply(coins).add(CENT.multiply(cents)); - checkArgument(coin.compareTo(NetworkParameters.MAX_MONEY) <= 0); return coin; } diff --git a/core/src/main/java/org/bitcoinj/uri/BitcoinURI.java b/core/src/main/java/org/bitcoinj/uri/BitcoinURI.java index b68bfc21..a336d6c5 100644 --- a/core/src/main/java/org/bitcoinj/uri/BitcoinURI.java +++ b/core/src/main/java/org/bitcoinj/uri/BitcoinURI.java @@ -219,6 +219,8 @@ public class BitcoinURI { // Decode the amount (contains an optional decimal component to 8dp). try { Coin amount = Coin.parseCoin(valueToken); + if (params != null && amount.isGreaterThan(params.getMaxMoney())) + throw new BitcoinURIParseException("Max number of coins exceeded"); if (amount.signum() < 0) throw new ArithmeticException("Negative coins specified"); putWithValidation(FIELD_AMOUNT, amount); diff --git a/core/src/test/java/org/bitcoinj/core/CoinTest.java b/core/src/test/java/org/bitcoinj/core/CoinTest.java index 2ac1b150..4230c8fd 100644 --- a/core/src/test/java/org/bitcoinj/core/CoinTest.java +++ b/core/src/test/java/org/bitcoinj/core/CoinTest.java @@ -24,7 +24,6 @@ import static org.junit.Assert.assertTrue; import static org.junit.Assert.assertFalse; import static org.junit.Assert.fail; -import org.junit.Assert; import org.junit.Test; public class CoinTest { @@ -53,21 +52,10 @@ public class CoinTest { assertEquals(NEGATIVE_SATOSHI, valueOf(-1)); assertEquals(MAX_MONEY, valueOf(MAX_MONEY.value)); assertEquals(MAX_MONEY.negate(), valueOf(MAX_MONEY.value * -1)); - try { - valueOf(MAX_MONEY.value + 1); - org.junit.Assert.fail("should not have accepted too-great a monetary value"); - } catch (IllegalArgumentException e) { - } - try { - valueOf( (MAX_MONEY.value * -1) - 1); - org.junit.Assert.fail("should not have accepted too-little a monetary value"); - } catch (IllegalArgumentException e) { - } - - try { - valueOf(Long.MIN_VALUE); - fail(); - } catch (IllegalArgumentException e) {} + valueOf(MAX_MONEY.value + 1); + valueOf((MAX_MONEY.value * -1) - 1); + valueOf(Long.MAX_VALUE); + valueOf(Long.MIN_VALUE); try { valueOf(1, -1); @@ -99,6 +87,26 @@ public class CoinTest { assertFalse(valueOf(2).isLessThan(valueOf(1))); } + @Test(expected = ArithmeticException.class) + public void testMultiplicationOverflow() { + Coin.valueOf(Long.MAX_VALUE).multiply(2); + } + + @Test(expected = ArithmeticException.class) + public void testMultiplicationUnderflow() { + Coin.valueOf(Long.MIN_VALUE).multiply(2); + } + + @Test(expected = ArithmeticException.class) + public void testAdditionOverflow() { + Coin.valueOf(Long.MAX_VALUE).add(Coin.SATOSHI); + } + + @Test(expected = ArithmeticException.class) + public void testSubstractionUnderflow() { + Coin.valueOf(Long.MIN_VALUE).subtract(Coin.SATOSHI); + } + @Test public void testToFriendlyString() { assertEquals("1.00 BTC", COIN.toFriendlyString()); @@ -123,10 +131,7 @@ public class CoinTest { assertEquals("54321.12345", parseCoin("54321.12345").toPlainString()); assertEquals("654321.123456", parseCoin("654321.123456").toPlainString()); assertEquals("7654321.1234567", parseCoin("7654321.1234567").toPlainString()); - try { - assertEquals("87654321.12345678", parseCoin("87654321.12345678").toPlainString()); - Assert.fail(); // More than MAX_MONEY - } catch (Exception e) {} + assertEquals("87654321.12345678", parseCoin("87654321.12345678").toPlainString()); // check there are no trailing zeros assertEquals("1", parseCoin("1.0").toPlainString()); diff --git a/core/src/test/java/org/bitcoinj/utils/ExchangeRateTest.java b/core/src/test/java/org/bitcoinj/utils/ExchangeRateTest.java index 4f7ee19e..b2a24a31 100644 --- a/core/src/test/java/org/bitcoinj/utils/ExchangeRateTest.java +++ b/core/src/test/java/org/bitcoinj/utils/ExchangeRateTest.java @@ -55,30 +55,6 @@ public class ExchangeRateTest { rate.fiatToCoin(Fiat.parseFiat("USD", "1")); } - @Test(expected = ArithmeticException.class) - public void fiatToCoinTooLarge() throws Exception { - ExchangeRate rate = new ExchangeRate(Fiat.parseFiat("XXX", "1")); - rate.fiatToCoin(Fiat.parseFiat("XXX", "21000001")); - } - - @Test(expected = ArithmeticException.class) - public void fiatToCoinTooSmall() throws Exception { - ExchangeRate rate = new ExchangeRate(Fiat.parseFiat("XXX", "1")); - rate.fiatToCoin(Fiat.parseFiat("XXX", "-21000001")); - } - - @Test(expected = ArithmeticException.class) - public void coinToFiatTooLarge() throws Exception { - ExchangeRate rate = new ExchangeRate(Fiat.parseFiat("XXX", "1000000000")); - rate.coinToFiat(Coin.parseCoin("1000000")); - } - - @Test(expected = ArithmeticException.class) - public void coinToFiatTooSmall() throws Exception { - ExchangeRate rate = new ExchangeRate(Fiat.parseFiat("XXX", "1000000000")); - rate.coinToFiat(Coin.parseCoin("-1000000")); - } - @Test(expected = IllegalArgumentException.class) public void constructMissingCurrencyCode() { new ExchangeRate(Fiat.valueOf(null, 1));