search

bitcoinj-cash / bitcoinj

A library for working with Bitcoin
http://bitcoinj.cash
Apache License 2.0
65 stars 35 forks source link

cleanup networkparameters classes #12

Open Danconnolly opened 6 years ago

Danconnolly commented 6 years ago
  1. merge NetworkParameters & AbstractBitcoinNetParams - Is there a need to have these two classes? can they be merged into one?
  2. these classes are supposed to just contain parameters - is there any logic that should be moved out?
  3. TestNet2 no longer exists - merge class with RegTestParams?
HashEngineering commented 6 years ago

There was a time when bitcoinj had no AbstractBitcoinNetParams class and MainNetParams was derived from NetworkParameters. This was with version 0.12. AbstractBlockChain contained the difficulty checks.

With 0.13, various network specific functions were moved to AbstractBitcoinNetParams, which was mainly the checkDifficultyTransitions (they are different between mainnet and testnet) and then TestNet3Params modified the DAA according to the extra rules that it hash.

I think we should keep the AbstractBitcoinNetParams and NetworkParameters separate.

Regarding TestNet2, which many have been a real thing at one point with Bitcoin Core, I would be in favor of removing it and merging it with RegTest. An old network that was never used with Bitcoin Cash is not needed.

HashEngineering commented 6 years ago

My previous comment was based on the cash-0.14 branch, but not the cash branch (which has version 0.15) regarding keeping AbstractBitcoinNetParams and NetworkParameters separate.