go-modularize packages (part I)

[onyb]

Motivation

There are two main motivations of this issue:

• Primary: be able to introduce breaking changes to packages without doing a MAJOR semver bump to btcd.
• Long term: make btcutil package not rely on the main btcd module, and instead import only the necessary ones.

Candidates

I have narrowed down the scope of this PR to the following packages:

• btcec
• chaincfg
• chaincfg/chainhash
• wire

These packages are the ones that are often imported in third-party packages and are isolated enough for a good first version. For the long-term objective of using modules in btcutil, that'd need navigating some circular dependency mess.

Action points

Once this is merged, we need to create some Git tags to bootstrap the initial versions (I chose v0.1.0). If Concept ACK, can someone with commit access create the following tags? (cc: @jcvernaleo or @jakesyl)

• btcec/v0.1.0
• chaincfg/v0.1.0
• chaincfg/chainhash/v0.1.0
• wire/v0.1.0

[jakesylvestre]

@Roasbeef would love to get your input on this- I know lnd has some similar properties and would be curious if you'd tried this over there

[coveralls]

Pull Request Test Coverage Report for Build 301814101

Warning: This coverage report may be inaccurate.

We've detected an issue with your CI configuration that might affect the accuracy of this pull request's coverage report.
To ensure accuracy in future PRs, please see these guidelines.
A quick fix for this PR: rebase it; your next report should be accurate.

• 0 of 0 changed or added relevant lines in 0 files are covered.
• 8 unchanged lines in 1 file lost coverage.
• Overall coverage decreased (-8.2%) to 45.296%

---

Files with Coverage Reduction	New Missed Lines	%
peer/peer.go	8	75.71%

Totals
Change from base Build 289529546:	-8.2%
Covered Lines:	14590
Relevant Lines:	32210

---

💛 - Coveralls

[jcvernaleo]

OK

[jcvernaleo]

Leaving unmerged for another day or two in case anyone else wants to review.

Once merged, someone remind me to make tags please!

[jakesylvestre]

I believe @jalavosus had looked into doing something similar @xplorfin so would love to get his thoughts.

Also, I've had limited success with this kind of project setup so I wanted to run a test. I setup a repo here with a replace github.com/btcsuite/btcd v0.21.0-beta => github.com/onyb/btcd v0.20.1-beta.0.20201012085148-740a56acae3f and was unable to get it to run.

This could be something on my end/my misunderstanding of how gomodules work. My main concern here relates to golang/go#30354.

This is working as designed. replace directives have no effect outside of your own module, so if you invoke go get outside of your own module, they have no effect. - @bcmills

I had an issue earlier this week and have a concise explanation of the effects of replace downstream here. I'm not sure if the same thing applies here (and I know some great work has been done around tooling w/ gopls) but I would like to figure out the issue w/ the btcd-test repo to make sure these kind of downstream effects won't happen with local replace.

[jakesylvestre]

See above

[onyb]

@jakesyl Regarding the error, maybe you need to create a tag called btcec/v0.1.0 for it to work? 🤔

[jakesylvestre]

@onyb on btcd? Possibly - this is kind of my concern. In my experience the effects of replace on downstream workflows are just a huge pain. So as an example, I used the base of your branch in the replace in jakesylvestre/btcd-test#1 and had no issues. Just want to make sure we're not breaking downstream dependencies here

Also, would that mean we'd have to tag every subfolder on release? Btw - if you have an example of another go library that implements this would love to take a look. I've looked at a few different tutorials around this. Most of the ones I've looked at either aren't libraries or have no interdependencies between modules because of the versioning issue.

@ckeyes88 probably articulated this best:

If there's a way to do it, I'm super for it since btcutil pulling in btcd isn't ideal.

[jakesylvestre]

One more thing I'll add, it does look like lnd has tried this approach (see: lightningnetwork/lnd#2602) in a few different places which definitely makes me more comfortable. There's only one case of them doing this with a replace