dogecoin / libdohj

Java library for adding altcoin support to bitcoinj
Apache License 2.0
109 stars 89 forks source link

Initial Namecoin support #18

Closed JeremyRand closed 8 years ago

JeremyRand commented 8 years ago

This pull request adds initial support for Namecoin. Included are:

Things not included:

Hopefully I haven't missed anything important.

Cheers!

rnicoll commented 8 years ago

Many thanks for this - obviously will take a while to review, but will try to get back to you with any feedback ASAP!

JeremyRand commented 8 years ago

Sounds good, thanks. libdohj is awesome, coding this with libdohj was so much easier than it would have been to fork BitcoinJ. :D

msgilligan commented 8 years ago

My initial thoughts are that NamecoinJ should be its own project and JAR and depend on libdohj for the Altcoin-support features it needs. (I have not looked at this part of the code, so maybe I'm missing something)

I'm not sure users of libdohj are going to want the jackson-databind dependency, for example.

Of course libdohj would need to be available in a public Maven repository for this to be practical.

rnicoll commented 8 years ago

What I may well do is split libdohj into libdohj-core and libdohj-namecoin, so we can keep the code in the same repo (helps avoid bitrot) without requiring everyone to have the namecoin parts.

JeremyRand commented 8 years ago

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA256

On 06/27/2016 02:19 AM, Ross Nicoll wrote:

What I may well do is split libdohj into libdohj-core and libdohj-namecoin, so we can keep the code in the same repo (helps avoid bitrot) without requiring everyone to have the namecoin parts.

That sounds like a good solution to me.

Which parts of the code do you think should be split out? I would lean toward all of the classes whose names begin with NameLookup, which includes all of the classes that depend on jackson-databind. I'd lean toward keeping the params, name script parsing, and name transaction utils in libdohj-core, but if you prefer to move some of those to libdohj-namecoin that's totally fine with me too.

Would it make sense to name it libdohj-names rather than libdohj-namecoin? A lot of that code isn't specific to the Namecoin chain. On the other hand, there aren't any other chains that implement anything similar to Namecoin, except for non-PoW chains (which I don't think will work with SPV). So perhaps libdohj-namecoin is better, given that it's unlikely that any other chains will be using that code anytime soon.

I definitely prefer the name libdohj-namecoin over namecoinj. Given that @rnicoll put much more work into libdohj than I've put into porting it to Namecoin, I would feel like a plagiarist if I removed libdohj's name from it.

Cheers! -----BEGIN PGP SIGNATURE----- Version: GnuPG v2

iQIcBAEBCAAGBQJXcPeRAAoJEAHN/EbZ1y06jk4P/RP+lUyc/PPD4ZPAKdTazaFc wjQ2pHlTQNdoMfqwv2MqTn3eJbuBovwrYVskn5YyqdIQOrREJwvgKeglrrdfFbYy 4Sc9848Y0CU2pDDml4I44Y5hbtuvfWVqe59aqfBIpZq84KEtbc5aL6RBeuPbnzvW ORipmpsHCWvJJJ4S6ha6MKvbMzdPaqpMtvqGxVZB7Yp9nz1v+sSeUWR3hvSoY73k lFlv4QzkhwrLRzIkyub8MZu95EmZW6h/VGBtjLUydJTZM7dvIPLrGbk3eu1FNQP7 o0mqgN5gQFT+22IX4MvzjHifDpn76PcP0Nilj3Ec2Qo1om5q/LIvjkIxedHs041V wkgqghto7rl3v9QM2YjNapRmIGWmgxCYAODVX/RU1tOneYFfrE9Iu6+jzzPASPwS rGogfrfbUTRHsNaKQd/zeiYx80gvzWTOELFgEjsE/ebFH4SfzLmtaOsroemM8LZP dVJMLDDLFoAaaDeg45ocWmK0FvHdzi1zPvBEvDyy68w+sjHsX7nRMWZA46BmylaG vV4YmKG3bpj037tjmTJ6Qv/mi8A+Um7oQwIZo16H2qqUq9uUNfz+/psWGuXpgEdu kB8SP+lo4MyEYA2e1B4bKLCVGiaV2oIEEm8KmwhITSVdX2OguiJNawrE+Z5qbxbq dY2RTDf/e9hzMwcMElUW =QuDj -----END PGP SIGNATURE-----

rnicoll commented 8 years ago

Do you want to try moving out everything under "org.libdohj.names" first, and then see what dependencies need to go with it?

I've done the basic restructure and added core as the first module, just go ahead and make a new directory for namecoin, and copy the core/pom.xml into it then edit to taste. You'll need to rebase, obviously.

JeremyRand commented 8 years ago

Okay, I've pushed fixes for the override issue and moved the name lookup classes (and the jackson-databind dependency) to libdohj-namecoin. Let me know if anything additional is needed to fix those. (I will squash the override fixes after review but before it's merged, so don't merge yet.)

rnicoll commented 8 years ago

Definitely getting there - I want to tackle some of the TODOs, but we can merge it and I'll then take help out with them, rather than delaying this further. Can you tackle the missing copyright notices and then squash the lot down please?

JeremyRand commented 8 years ago

I've added the missing copyright notices. I also changed the copyright notice on NamecoinMainNetParams. It previously was copyright Google; I've modified it to be copyright both Google and myself; that's because a decent amount of it is copied from Bitcoin's network parameters, but some is also changed by me. Hopefully that's the right way to approach it; let me know if you'd prefer something else.

I've also squashed the Override fixes and copyright notice fixes. Let me know if you'd prefer additional squashing to be done.

rnicoll commented 8 years ago

Great, thanks. I'll try to tackle some of the TODOs in the next few weeks, things are manic here unfortunately.