Improve how coin values are represented in the API

[GoogleCodeExporter]

The Utils class assumes that there are 100 million nanocoins in a bitcoin.

>  /** How many nanocoins there are in a BitCoin. */
>  public static final BigInteger COIN = new BigInteger("100000000", 10);
>  /** How many nanocoins there are in 0.01 BitCoins. */
>  public static final BigInteger CENT = new BigInteger("1000000", 10);

Both of these public(!) constants are missing a zero.

As a result the toNanoCoins method does not work correctly.

Will send a patch together with an improved one for issue #1 (and unit tests 
for both)

Original issue reported on code.google.com by thilopl...@googlemail.com on 9 Apr 2011 at 2:17

[GoogleCodeExporter]

Or maybe I was wrong. The Bitcoin wiki says "(1 bitcoin = 100,000,000 
nanocoins)".

That would be weird naming. Can someone please clarify?

Original comment by thilopl...@googlemail.com on 9 Apr 2011 at 3:07

[GoogleCodeExporter]

The constants are correct and the comments/method names are misleading. The 
"nanocoin" terminology needs to die, I agree it's confusing. Got any 
suggestions for a better name?

Original comment by hearn@google.com on 10 Apr 2011 at 7:49

[GoogleCodeExporter]

http://en.wikipedia.org/wiki/Non-SI_unit_prefixes
http://en.wikipedia.org/wiki/SI_prefix

centiMicroCoin

or 

shaCoin  (japanese sha -> 10^-8)

Original comment by bobby.si...@gmail.com on 10 Apr 2011 at 10:32

[GoogleCodeExporter]

I think it is outside of the scope of this project to coin new terminology. We 
should defer this to bitcoin.org.

I was about to propose to make the nano-coin related methods in Utils 
package-private or even delete them altogether, but it seems that the 
public-facing methods in bitcoinj usually deal with BitIntegers of "base units" 
(as opposed to BigDecimals of "bitcoins"), so that does not seem to be an 
option (I am conflicted about which style is better).

So for now, we will have to leave it like it is, I suppose.

I will update my patch for issue #1 to revert back to 100 millions per coin, 
and add some verbage to the Javadocs. 

Original comment by thilopl...@googlemail.com on 10 Apr 2011 at 11:40

[GoogleCodeExporter]

Actually, we are already using a different term in some places: There is a 
method Utils#bitcoinValueToFriendlyString which also converts from "nanocoins". 
Maybe we should use this more neutral term:  Utils#toNanoCoins => 
Utils#toBitcoinValue. 

Original comment by thilopl...@googlemail.com on 11 Apr 2011 at 1:02

[GoogleCodeExporter]

I think we should just introduce a BitCoins class that handles:

 - Conversion between different forms
 - Formatting
 - Exposing unicode Ⓑ character (and maybe others)
 - In future, exchange rate multiplications into other currencies

etc.  That way we get a bit more type safety too as BigIntegers are used in 
other places for unrelated purposes.

Original comment by hearn@google.com on 11 Apr 2011 at 8:03

[GoogleCodeExporter]

That is probably over-designing it.

These four functions seem to be too different to be combined in a single class, 
and especially the exchange rate conversions a bit out of scope for the project 
(i.e. should be in application code, not in the core library).

I think it is okay to have the transaction functions return and accept base 
units as BigIntegers, as they do now. Since it is an integral type, it can 
obviously only mean multiples of the smallest possible unit. Since it is a 
BigInteger (and not just an int), there is probably also a clear intent that 
this is a really big number (i.e. a "nano" coin amount). The only confusion one 
can (and I did) run into is when converting to BitCoins, but this can be dealt 
with by the Utils class, and that confusion is mostly because of the bad "nano" 
terminology.

Original comment by thilopl...@googlemail.com on 11 Apr 2011 at 8:17

[GoogleCodeExporter]

We're just going to end up with a bunch of methods in Utils that are all about 
manipulating coin values in various forms but aren't obviously grouped 
together. I don't think a class is such a big deal, it's not much worse than 
having a String class after all.

Original comment by hearn@google.com on 11 Apr 2011 at 8:22

[GoogleCodeExporter]

Original comment by hearn@google.com on 20 Apr 2011 at 4:34

• Changed title: Improve how coin values are represented in the API
• Added labels: Type-Enhancement
• Removed labels: Type-Defect

[GoogleCodeExporter]

Maybe the exploding exchange rate offers a solution here: Since we might not 
want to deal in bitcoins anymore (when a bitcoin has reached the value of a 
small house), we could have "bitcents" (100 bitcents = 1 bitcoin), which would 
make the smallest unit a "microcent", the millionth part of a cent.

Original comment by thilopl...@googlemail.com on 3 Jun 2011 at 11:37

[GoogleCodeExporter]

I like the term microcent, good idea.

Original comment by hearn@google.com on 6 Jun 2011 at 9:49

[GoogleCodeExporter]

I was just reading the wiki on transactions  
(https://en.bitcoin.it/wiki/Transactions) and in the 'Outputs' paragragh it 
states:

"An output contains instructions for sending bitcoins. Value is the number of 
Satoshi (1 BTC = 100,000,000 Satoshi) that this output will be worth when 
claimed."

Assuming we are happy with its provenance, perhaps we should replace nanocoins 
with 'Satoshi'.

Jim

Original comment by jimburto...@gmail.com on 9 Jun 2011 at 9:04

[GoogleCodeExporter]

The definition of a "satoshi" is not at all clear from the name. I've seen the 
proposal be used before, but I'd rather avoid it for a financial API where 
clarity is essential. The term microcent is better, IMHO.

Original comment by hearn@google.com on 9 Jun 2011 at 9:18

[GoogleCodeExporter]

why dont we keep things simple and stick to just BTC and decimals. Adding this 
extra functionality means that it needs to be maintained. 

I think it would be best to keep any conversion, or break down outside of the 
API, and deal with strictly BTC and decimals. This is easier and more accurate 
as your never having to round numbers to convert between them inside the API

My main reasoning is that each application implementing this API might want to 
choose these own rounding method, or smallest decimal place. With that being 
said they should had the modulation and division.

Further more, how would you protect against people exploiting the API by 
rounding the precision in scale. This would be achieved by changing coins back 
and forth between denominations repeatedly. This is what the guys do in the 
movie Office Space.

Original comment by kraw...@batcountryentertainment.com on 9 Jun 2011 at 4:07

[GoogleCodeExporter]

Why is BigInteger used to represent nanocoins/satoshis/microcents? If there 
will never be any more than 21e6 coins and there are 1e8 nanocoins per coin, 
then the maximum pool will be 2.1e15. A 64-bit signed integer has a maximum 
value of 9.2e18, more than 3 orders of magnitude bigger.

Original comment by nathan.baulch on 21 Jun 2011 at 5:45

[GoogleCodeExporter]

It was just a mechanical translation of uint64 -> BigInteger, to avoid 
potentially subtle problems with signedness.  But you're right, it's not 
necessary. We can just use long.

Original comment by hearn@google.com on 21 Jun 2011 at 9:00

[GoogleCodeExporter]

Original comment by mi...@google.com on 23 Sep 2011 at 8:40

• Added labels: Component-API, Priority-Low
• Removed labels: Priority-Medium

[GoogleCodeExporter]

Issue 23 has been merged into this issue.

Original comment by mi...@google.com on 23 Sep 2011 at 8:45

[GoogleCodeExporter]

Issue 23 has a proposed patch.

Original comment by mi...@google.com on 23 Sep 2011 at 8:45

[GoogleCodeExporter]

Now that protobuf serialization is mostly in place, how difficult will it be to 
still switch this? Is usage of BigInteger carved into the protobuf format?

Original comment by andreas....@gmail.com on 6 Feb 2012 at 2:13

[GoogleCodeExporter]

No. Once everyone is migrated off serialized wallets we can go in and fix this 
more easily. But I guess we should stay compatible with both for at least one 
more release cycle.

We can design the API now though. We don't have to wait.

Original comment by hearn@google.com on 6 Feb 2012 at 2:15

[GoogleCodeExporter]

small addition to remark #15: I've never seen anyone doubt the total coin 
supply, but people seem more open to one day drop the satoshi as smallest 
subdivision of the BTC. There are 3 orders of magnitude of leeway but that 
might not be enough if/when that change happens.

example:
http://bitcoin.stackexchange.com/questions/114/what-is-a-satoshi

Original comment by mike.ros...@gmail.com on 28 Nov 2013 at 4:13

[GoogleCodeExporter]

I thought many times about this issue of what type we should use to represent 
monetary values. Maybe the step from BigInteger to long is too big.

I propose we migrate to a custom type, e.g. org.bitcoinj.Amount

For now, it would store the value as a long internally. It provides similar 
algebraic operations like BigInteger, plus perhaps some useful operations to 
Bitcoin.

That way, dogecoin-like inflated currencies can still quite easily fork the 
framework (it wouldn't be so easy if we just moved to long).

Later, we can decide on further optimization by moving some algorithms to the 
native type (e.g. calculating wallet balance?).

I volunteer to prepare a patch if we have consensus about proceeding.

Original comment by andreas....@gmail.com on 25 Apr 2014 at 1:08

[GoogleCodeExporter]

dogecoin needs >64 bits to represent amounts?!

The main benefit to a wrapper class IMHO is more type safety/clearer APIs and 
it's somewhere we can hook in formatting functions and exchange rate lookup 
code. Not support for dogecoin.

Original comment by mh.in.en...@gmail.com on 25 Apr 2014 at 1:11

[GoogleCodeExporter]

I just wanted to make the point that the refactoring would not break 
"forkability". I thought Dogecoin is inflatonary by design (or by accident?), 
so at some point it will exceed Long.MAX_VALUE. Heck, maybe even Bitcoin will 
exceed that if we need to split Satoshis.

I fully agree on the type safety aspect. I didn't even mention it because I 
thought it's obvious. Do you have any comment about progressing with my 
proposal?

Original comment by andreas....@gmail.com on 25 Apr 2014 at 1:39

[GoogleCodeExporter]

I think a patch would be reasonable, if you want to work on this. Though I'd 
prefer waiting for HDW to land before any huge invasive patches are done. 
Otherwise the rebase will be a nightmare. I'm hoping we can merge HDW to master 
in the next few weeks or maybe after the amsterdam conf.

Original comment by mh.in.en...@gmail.com on 25 Apr 2014 at 3:33

[GoogleCodeExporter]

I can base the pull on your hdw-alpha branch, no problem.

Does HD wallets even touch amounts? I thought this is only about keys and 
chains of keys...

Original comment by andreas....@gmail.com on 25 Apr 2014 at 3:47

[GoogleCodeExporter]

I don't think it does, but it's a huge branch so any kind of big API change 
would result in lots of merge conflicts.

Original comment by mh.in.en...@gmail.com on 25 Apr 2014 at 3:57

[GoogleCodeExporter]

Working on it. Any preference for the naming of the coin class? What about 
"com.google.bitcoin.core.Coin"?

Original comment by andreas....@gmail.com on 25 Apr 2014 at 9:30

• Changed state: Started

[GoogleCodeExporter]

Do transactions still need to be Java serializable? Because in this case, the 
new Coin class also needs to be.

Original comment by andreas....@gmail.com on 26 Apr 2014 at 7:57

[GoogleCodeExporter]

Anwering my own question: Yes, Java serialization is useful.

Original comment by andreas....@gmail.com on 26 Apr 2014 at 10:42

[GoogleCodeExporter]

We don't unit test Java serialization anywhere and it is a bit broken. I'd like 
to remove it. For instance serializing a transaction and unserializing it, then 
trying to add an event listener, will crash. What do you use it for?

Original comment by mh.in.en...@gmail.com on 26 Apr 2014 at 10:54

[GoogleCodeExporter]

> We don't unit test Java serialization anywhere

That's not true: see com.google.bitcoin.core.BlockTest.testJavaSerialiazation()

Original comment by andreas....@gmail.com on 26 Apr 2014 at 11:11

[GoogleCodeExporter]

Android needs to serialize data in order for communicating between components. 
So its still useful.

Original comment by andreas....@gmail.com on 26 Apr 2014 at 11:13

[GoogleCodeExporter]

OK, then we don't unit test it enough :) 

Over time I'd like to strip things like Block and Transaction back to being 
just what's on the wire. At that point p2p protcol serialization should be 
sufficient to represent everything. Whatever replaces it for the Wallet could 
be serializable, but I'd hope it'd not be needed.

Original comment by mh.in.en...@gmail.com on 26 Apr 2014 at 5:03

[GoogleCodeExporter]

Andreas did this: we now have a new Coin class that wraps long and provides 
various helper methods for things like overflow checked math. It's mostly 
source compatible with BigInteger so simply doing a global search and replace 
for BigInteger->Coin should be good enough.

Original comment by mh.in.en...@gmail.com on 30 May 2014 at 1:29

• Changed state: Fixed