Initial Signet support #1536
Initial Signet support #1536
Conversation
There was a problem hiding this comment.
Really great stuff! Except for the removed line in .travis.yml, there's only one more thing I could spot: In the README and doc.go files of this package, there's a couple of mentions of "three standard Bitcoin networks". That's not true with signet being added. I'd suggest just removing the mention of how many networks there are altogether.
Also, would it make sense to wait with merging this, until it is merged into Core? In case any changes are made.
.travis.yml
Outdated
| @@ -2,7 +2,6 @@ language: go | |||
| cache: | |||
| directories: | |||
| - $GOCACHE | |||
| - $GOPATH | |||
There was a problem hiding this comment.
what's the reason for changing this?
There was a problem hiding this comment.
@torkelrogstad Sorry, this may look out of context because of rebasing.
Looks like CI has cached my initial version with a failing test and can't invalidate the cache even after providing a code with sucessful tests, e.g.:
https://travis-ci.org/btcsuite/btcd/builds/647711967
https://travis-ci.org/btcsuite/btcd/builds/647712943
https://travis-ci.org/btcsuite/btcd/builds/647769818
These builds worked both locally & on CI for my personal fork:
https://github.com/pavel-main/btcd/pull/1
@jakesyl Was also encountering this issue before in previous PR:
#1503 (comment)
Completely understand if you wish to keep this line for now - I can just try pushing a new branch & opening new PR from scratch
There was a problem hiding this comment.
Hm, I see. I don't really have super strong feelings on this, as I don't have much knowledge of the CI setup here.
There was a problem hiding this comment.
@torkelrogstad Feels like a Travis CI bug, exactly or similar to this one - travis-ci/travis-ci#3055 (comment)
They're storing cached artifacts elsewhere (probably on S3), so something might got broken there. Perhaps a quicker and cleaner workaround is to clear caches on Travis side once and then we'll rollback .travis.yml changes to try again
There was a problem hiding this comment.
If this is flaking tests, let's just remove it
|
@torkelrogstad good catch, I'll follow-up with |
$GOPATH caching has led to flaky tests as per btcsuite#1503 and btcsuite#1536. The speedup is marginal and while the false negatives are a headache, false positives are potentially dangerous.
|
@torkelrogstad Updated |
|
@jcvernaleo (as per #1530)
Well reviewed |
$GOPATH caching has led to flaky tests as per btcsuite#1503 and btcsuite#1536. The speedup is marginal and while the false negatives are a headache, false positives are potentially dangerous.
Initial PR to support default Signet ("Signet Global Test Net III"). Custom Signet is not implemented in this PR.
More info about Signet: