Implement Debug for generic Address<V: NetworkValidation>

[jirijakes]

Previously Debug was implemented for both Address<NetworkChecked> and Address<NetworkUnchecked>, but not for cases when the NetworkValidation parameter was generic. This change adds this ability. Based on Kixunil's tip.

With previous implementation, the test_address_debug() resulted in error:

The added Debug on NetworkChecked and NetworkUnchecked are required by compiler.

---

While dealing with derives and impls, I also attempted to turn all the derives on Address into manual impls (see Kixunil's suggestion in #1489 (comment)). The motivation behind this was the possibility to remove derives on NetworkChecked and NetworkUnchecked, too. However, even with manual impls, all the traits on NetworkChecked and NetworkUnchecked were still required by compiler in this sort of situations (see also the rest of the same discussion linked above). I do not fully understand why, perhaps limitation of this way of sealing traits?

It can be demonstrated by removing Debug derivation on NetworkUnchecked and NetworkChecked in this PR and running test_address_debug().

Therefore, if we want to allow users of the library to define types generic in NetworkValidation and at the same time derive impls, it seems to me that NetworkChecked and NetworkUnchecked will have to have the same set of impls as Address itself.

[jirijakes]

Unrelated but it's getting difficult to move around address.rs (1700 lines). Would someone object if it's split into multiple files? Perhaps address/mod.rs, address/address.rs, address/payload.rs and address/tests.rs?

[Kixunil]

However, even with manual impls, all the traits on NetworkChecked and NetworkUnchecked were still required by compiler in this sort of situations (see also the rest of the same discussion linked above). I do not fully understand why

I'm not sure if I understand what you're saying correctly, but I think you're experiencing an annoying "poisoning" of downstream generic types which are also forced to do manual impls. I didn't fully realize this when suggesting manual impls. I suspect that just keeping the derive is lesser evil but it depends on how many people and how often will they write generic structs.

I suggest we remove it anyway for now and re-add it if many people complain. I will take a look at it myself since I've done something similar in another project.

[Kixunil]

ACK ebfbe74

Nice PR, thanks!

[jirijakes]

I think you're experiencing an annoying "poisoning" of downstream generic types which are also forced to do manual impls.

Oh, you're right! I didn't think of it before. Yes, manual impl on the generic struct will work.

[tcharding]

ACK ebfbe74

[apoelstra]

Unrelated but it's getting difficult to move around address.rs (1700 lines). Would someone object if it's split into multiple files? Perhaps address/mod.rs, address/address.rs, address/payload.rs and address/tests.rs?

No objection from me. Though we probably want to discuss, in general, the idea of moving unit tests into their own files, because I think it's a common pattern for 50%+ of source files to be unit tests, which can make it hard to navigate.

[apoelstra]

ACK ebfbe74

[tcharding]

I don't think having tests in a separate file is helpful for navigating files, the tests are at the bottom of the file anyways. I'd prefer not to have a proliferation of foo_test.rs files.

FTR I navigate files almost entirely by jumping up and down so file layout is always on my mind.

One thing that could be done, is putting important stuff first, as opposed to putting stuff before its used i.e., error types and code can be at the bottom instead of at the top where it is in many files - I've made an effort to start this already with no push back so far :) Also, private methods after they are called.

[Kixunil]

Yeah, tests aren't that bad. The Rust std does put them in subdirectories though.

Errors could also go to a separate module.

[tcharding]

Yeah, tests aren't that bad. The Rust std does put them in subdirectories though.

Well I've exposed my lack of reading std code here then haven't I. If std does this then I rescind my comment. Does that mean we should but benches in a separate module as well? I see std uses the "new" module naming and has many directories that hold just the test.rs file, I guess we should have bench.rs along side all those.

[tcharding]

How about we do this restructure during the feature freeze right before next release?

[Kixunil]

I'm not trying to imply we have to do what std does, just pointing out that some other, presumably experienced :), Rust devs found that style useful, so might be worth considering. I don't have strong preference right now. Doing it during feature freeze sounds good.

[tcharding]

Cool, I willing to give it a try, time will tell if we like it.