novitski / bitcoinj

Automatically exported from code.google.com/p/bitcoinj
Apache License 2.0
0 stars 0 forks source link

wrong java.lang.ArithmeticException in com.google.bitcoin.uri.BitcoinURI.<init> #407

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
On bitcoinj 0.8. This is most likely due to a malformed Bitcoin URI, but I 
don't have a log of that.

The point of this bug report is the BitcoinURI constructor is supposed to throw 
BitcoinURIParseException in this case.

java.lang.ArithmeticException: Rounding necessary
at java.math.BigDecimal.toBigIntegerExact(BigDecimal.java:2450)
at com.google.bitcoin.core.Utils.toNanoCoins(Utils.java:111)
at com.google.bitcoin.uri.BitcoinURI.parseParameters(BitcoinURI.java:202)
at com.google.bitcoin.uri.BitcoinURI.<init>(BitcoinURI.java:170)
at 
de.schildbach.wallet.ui.SendCoinsFragment.onActivityResult(SendCoinsFragment.jav
a:532)

Original issue reported on code.google.com by andreas....@gmail.com on 17 May 2013 at 11:23

GoogleCodeExporter commented 9 years ago
I think this can be caused if the Bitcoin value encoded into a URI has more 
decimal places than the protocol can support. I wish the URI spec required 
values to be specified in Satoshis, then this mistake couldn't happen. Anyway 
yes. It should be fixed. One question is should we round off the price if such 
a URI exists, or should we refuse to process it? My inclination is to reject 
and hope the creators of these URIs fix things.

Original comment by hearn@google.com on 17 May 2013 at 4:52

GoogleCodeExporter commented 9 years ago
I think we should rethrow the exception as BitcoinURIParseException now, 
because that's an obvious bug. Wether we relax the parsing in this case is 
another issue that can be decided later.

Original comment by andreas....@gmail.com on 17 May 2013 at 7:07

GoogleCodeExporter commented 9 years ago
Just reproduced the exception. Indeed, it happens if you have too many decimal 
places:

bitcoin:15P7W9X5xWVLewpnSk5gjLWVakvZ3NRUGN?amount=0.99999999999

Original comment by andreas....@gmail.com on 17 May 2013 at 8:06

GoogleCodeExporter commented 9 years ago
I've written a bunch of test cases for this and some related bugs:

    @Test(expected = BitcoinURIParseException.class)
    public void testBad_AmountTooPrecise() throws BitcoinURIParseException {
        new BitcoinURI(MainNetParams.get(), BitcoinURI.BITCOIN_SCHEME + ":" + MAINNET_GOOD_ADDRESS
                + "?amount=0.123456789");
    }

    @Test(expected = BitcoinURIParseException.class)
    public void testBad_NegativeAmount() throws BitcoinURIParseException {
        new BitcoinURI(MainNetParams.get(), BitcoinURI.BITCOIN_SCHEME + ":" + MAINNET_GOOD_ADDRESS
                + "?amount=-1");
    }

    @Test(expected = BitcoinURIParseException.class)
    public void testBad_TooLargeAmount() throws BitcoinURIParseException {
        new BitcoinURI(MainNetParams.get(), BitcoinURI.BITCOIN_SCHEME + ":" + MAINNET_GOOD_ADDRESS
                + "?amount=100000000");
    }

Original comment by andreas....@gmail.com on 18 May 2013 at 7:10

GoogleCodeExporter commented 9 years ago
I've rebased my bitcoin-uri-tests branch on current master.

Original comment by andreas....@gmail.com on 18 Jun 2013 at 10:47

GoogleCodeExporter commented 9 years ago

Original comment by hearn@google.com on 11 Jul 2013 at 2:53

GoogleCodeExporter commented 9 years ago
The last two testcases in this issue are not fixed.

Original comment by andreas....@gmail.com on 11 Jul 2013 at 3:16

GoogleCodeExporter commented 9 years ago
And can we have a testcase for the "too precise" case so it is clear what 
result to expect?

Original comment by andreas....@gmail.com on 11 Jul 2013 at 3:23

GoogleCodeExporter commented 9 years ago
This issue was closed by revision 719a786db148.

Original comment by hearn@google.com on 11 Jul 2013 at 3:33