Make MemoryPool a singleton and expose a generic intern method

[GoogleCodeExporter]

If I could design bitcoinj again, I'd make Transaction fully immutable and not 
have any attached objects. Unfortunately, that's not how the code is written. 
This means that you can end up with multiple Transaction objects representing 
the same logical wire transaction.

This problem has been solved for a long time in the core code using the 
MemoryPool class, which essentially resolves the issue that we might download a 
transaction multiple times from different peers, thus ending up with different 
Java objects when really we only want one. This is both for memory efficiency 
reasons, and more importantly because we store state in Transaction, like the 
confidence object.

Andreas encountered an issue with the bluetooth support in BW. If the sending 
device has internet, it will send via BT and also via the P2P network. If the 
tx arrives via the P2P network first, the internet-created object becomes 
canonical and the confidence listener on the BT-created version doesn't work 
right. The fix is for Andreas to put canonicalise his BT object via the 
MemoryPool as well, therefore:

1) MemoryPool should become a global singleton, as Andreas doesn't always have 
access to a PeerGroup and anyway, it would probably make multiple PeerGroups 
(untested) work better.

2) MemoryPool needs a new intern method that doesn't require you to provide a 
PeerAddress. Currently all the methods are written on the assumption that you 
got the object from a P2P download (possibly in parallel with other peers also 
downloading the tx, hence the design).

Longer term, we need to find some kind of design that makes all this less 
opaque. Probably:

1) Transaction becomes totally immutable and represents just the wire data.

2) MutableTransaction allows you to edit the inputs/outputs as today.

3) MemoryPool gets renamed (it's got nothing to do with the bitcoin-qt memory 
pool concept), probably to TransactionTable or something like that.

4) TransactionConfidence also becomes immutable.

4) TransactionTable vends these immutable confidence objects and other things 
that are logically "attached" to a Transaction in a global sense, given the TX 
hash. It has an atomic compareAndSwap, and will call event listeners when the 
confidence object changes.

Original issue reported on code.google.com by hearn@google.com on 18 Oct 2013 at 4:51

[GoogleCodeExporter]

Good suggestions! May I add to your upper list a

3) Make all methods that accept transactions call MemoryPool.intern(), so the 
API user do not need to care about it.

(Of course, if immutability and a well defined definition of equalness is in 
place it will not be needed any more.)

It's possible that if the memory pool is a single in future we probably will 
need some purging mechanism. Otherwise memory constrained apps will run out of 
memory at some time. Currently in the Android app, PeerGroup is not alive for 
more than a few minutes which keeps the memory pool at a reasonable size.

Original comment by andreas....@gmail.com on 18 Oct 2013 at 5:05

[GoogleCodeExporter]

Actually the MemoryPool class uses weak references. So it only does 
deduplication for as long as there's an instance of the underlying Transaction 
referenced elsewhere. Once the last non-MemoryPool reference is gone, the 
MemoryPool will automatically clean itself up and the Transaction will be freed.

Original comment by hearn@google.com on 21 Oct 2013 at 9:25

[GoogleCodeExporter]

Suggestion for rename: TransactionFactory

Original comment by andreas....@gmail.com on 5 Nov 2013 at 9:01

[GoogleCodeExporter]

It's not a factory though. I think TransactionTable is a better name.

Original comment by hearn@google.com on 5 Nov 2013 at 10:30