Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

wire+chaincfg: add signet params #1692

Merged
merged 4 commits into from
Apr 26, 2021
Merged

wire+chaincfg: add signet params #1692

merged 4 commits into from
Apr 26, 2021

Conversation

guggero
Copy link
Collaborator

@guggero guggero commented Feb 14, 2021

This commit adds all necessary chain parameters for connecting to the
public signet network.
Reference: bitcoin/bitcoin#18267

Needed for adding signet support to lnd.
Replaces #1536.

cc @Roasbeef, @cfromknecht.

config.go Outdated Show resolved Hide resolved
@coveralls
Copy link

coveralls commented Feb 17, 2021

Pull Request Test Coverage Report for Build 773968254

  • 90 of 131 (68.7%) changed or added relevant lines in 3 files are covered.
  • 12 unchanged lines in 1 file lost coverage.
  • Overall coverage increased (+0.06%) to 52.916%

Changes Missing Coverage Covered Lines Changed/Added Lines %
rpcserver.go 0 2 0.0%
config.go 0 39 0.0%
Files with Coverage Reduction New Missed Lines %
peer/peer.go 12 75.47%
Totals Coverage Status
Change from base Build 745866572: 0.06%
Covered Lines: 21017
Relevant Lines: 39718

💛 - Coveralls

@guggero
Copy link
Collaborator Author

guggero commented Mar 5, 2021

cc @wpaulino

chaincfg/genesis.go Outdated Show resolved Hide resolved
chaincfg/params.go Outdated Show resolved Hide resolved
chaincfg/params.go Outdated Show resolved Hide resolved
chaincfg/params_test.go Show resolved Hide resolved
chaincfg/params.go Outdated Show resolved Hide resolved
chaincfg/params.go Outdated Show resolved Hide resolved
wire/protocol.go Outdated Show resolved Hide resolved
Copy link
Member

@Roasbeef Roasbeef left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about the ability to configure a signet target at runtime? Otherwise if people want to make their own custom signets (which I think is the whole point?), then they'd need to recompile and distribute their binaries, vs being able to say: "yo use these params to join my signet that has covenants implemented".

chaincfg/params.go Outdated Show resolved Hide resolved
@guggero
Copy link
Collaborator Author

guggero commented Mar 15, 2021

@Roasbeef I added the changes necessary to connect to a custom signet. I wasn't able to spin one up locally, but I'll test this as soon as I'm successful.

@guggero
Copy link
Collaborator Author

guggero commented Mar 22, 2021

I've verified that syncing a btcd node against both the "public taproot signet" and a custom local signet work.

Example for connecting to a custom local signet (using bitcoind, see script below):

btcd --signet --signetchallenge=512103e5602bf0701c1565a89695eb618c3d652b0532464dbfca6130c659c06dfc11fd51ae --signetseednode=127.0.0.1 --listen=:38666 --rpclisten=:38662 --rpcuser=lightning --rpcpass=lightning

Client:

btcctl --signet --rpccert ~/.btcd/rpc.cert --rpcuser=lightning --rpcpass=lightning --rpcserver=localhost:38662

Here's my script I used to spin up my own signet (must run inside the src directory of the bitcoin source code with the current master compiled): https://pastebin.com/YJKAPM0p

@guggero guggero requested a review from Roasbeef March 22, 2021 17:33
@guggero
Copy link
Collaborator Author

guggero commented Apr 22, 2021

@Roasbeef PTAL, would be nice to include this in the next lnd release.

This commit adds all necessary chain parameters for connecting to the
public signet network.
Reference: bitcoin/bitcoin#18267
This commit adds the --signet command line flag (or signet config
option) for starting btcd in signet mode.
This commit adds the --signet command line flag to the btcctl utility.
Copy link
Member

@Roasbeef Roasbeef left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for making the signet configuration more flexible! I have some things in mind that would be really dope to expose using a new public signet 😈

@Roasbeef
Copy link
Member

@guggero tried to click on that pastebin link but it's expired, if you recall what it was, can you re-host it somewhere? Thanks!

Copy link
Member

@Roasbeef Roasbeef left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually re-visiting the BIP, this doesn't appear to allow creation of a "custom" signet as no new logic to validate the new block commitment nor the challenge signature has been added. Is the intent here just to add a "light" minimally validating signet client in the short term before following up w/ the proper block generation and validation logic?

Copy link
Member

@Roasbeef Roasbeef left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Chatted w/ Oliver offline and determined that we'll merge this "light client" functionality, then later follow up with the fully logic needed to produce and verify signet blocks so new public btcd based signets will be possible in the future.

@Roasbeef Roasbeef merged commit 7eba688 into btcsuite:master Apr 26, 2021
@guggero guggero deleted the signet branch April 26, 2021 19:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants