Remove all p2p dependency from network

[rustaceanrob]

Motivated by moving the p2p module to its own crate. TryFrom and From are already implement for converting to and from Network/Magic. The methods related to Magic are removed from Network, as well as any reference to p2p in the documentation, as bitcoin would no longer depend on p2p.

The deser roundtrip test are relocated to p2p/mod.rs

[github-actions]

🚨 API BREAKING CHANGE DETECTED

To see the changes click details on "Check semver breaks / PR Semver - stable toolchain" job then expand "Run semver checker script" and scroll to the end of the section.

[Kixunil]

NACK removing the conversions.

These conversions are critical for network-agnostic applications (e.g. Firefish) and removing them is a breakage that cannot be worked around downstream.

We need a different approach: separate Network into a crate (I think bitcoincore-basic-api would be an appropriate name) and implement the conversions there.

[tcharding]

ACK 608736f

[tcharding]

These conversions are critical for network-agnostic applications (e.g. Firefish) and removing them is a breakage that cannot be worked around downstream.

You can still use From and TryFrom, what is the issue?

[Kixunil]

Oh, yes, but why remove the conversions then? Presumably Network depends on Magic, not the other way around.

[coveralls]

Pull Request Test Coverage Report for Build 15326023324

Details

• 28 of 28 (100.0%) changed or added relevant lines in 1 file are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.03%) to 84.639%

---

Totals
Change from base Build 15304145287:	0.03%
Covered Lines:	23490
Relevant Lines:	27753

---

💛 - Coveralls

[tcharding]

This is preparation for splitting the p2p crate out.

[apoelstra]

Oh, yes, but why remove the conversions then? Presumably Network depends on Magic, not the other way around.

Magic is p2p specific while Network is a general type used all over the rust-bitcoin crate ecosystem. So Magic depends on Network.

[Kixunil]

The thing is, they are perhaps on the same level technically but if Network depends on Magic then changes to the enum would have less bad effect on code that doesn't take Network directly but converts it (what we're doing already in some places).

[apoelstra]

Code that converts it uses the Into<Network> generic, which is unaffected by removing these conversion methods.

The types are not on the same level technically -- Network is a ecosystem-wide type used for all sorts of things while Magic technically only has meaning for the p2p layer. (Though it's sometimes used as a sort of 'network ID' elsewhere.)

[apoelstra]

Needs rebase.

[github-actions]

🚨 API BREAKING CHANGE DETECTED

To see the changes click details on "Check semver breaks / PR Semver - stable toolchain" job then expand "Run semver checker script" and scroll to the end of the section.

[github-actions]

🚨 API BREAKING CHANGE DETECTED

To see the changes click details on "Check semver breaks / PR Semver - stable toolchain" job then expand "Run semver checker script" and scroll to the end of the section.

[tcharding]

ACK cbe04b0

[apoelstra]

ACK cbe04b0; successfully ran local tests

Dismissing review -- your initial complaint, that there was no workaround, was untrue because there are still From/TryFrom conversions (and also these methods can be implemented entirely outside the library if somebody wants). Your secondary complaint, that there was no reason to remove them since Network depends on Magic, is also untrue; the dependency runs the opposite direction, which is the reason for this PR in the first place.