Add basic bitcoin signet support #5025

guggero · 2021-02-14T15:49:29Z

Depends on btcsuite/btcd#1692.
Depends on btcsuite/btcwallet#735.

Updates to the latest btcd version (which is why the diff is that large) and adds basic signet support.

chainreg/chainparams.go

guggero · 2021-03-04T15:20:49Z

I added the prefix as decide on in the spec PR (lightning/bolts#844).
Turns out a change in btcwallet is also required so added that PR as well.

guggero · 2021-04-22T11:33:05Z

Rebased. @Roasbeef any particular reason for moving this to 0.14 or just because the dependencies weren't ready yet?

wakiyamap · 2021-05-03T08:08:23Z

I'm using google translate so I'm sorry if it's strange English.

zpay32/decode.go
https://github.com/lightningnetwork/lnd/pull/5025/files#diff-da9268b6d458ca0e80aab05fe9e6a79fe7049567efd48d77895ac50c9996db82R66

var netPrefixLength = len(net.Bech32HRPSegwit) + 2
->
var netPrefixLength = len(expectedPrefix) + 2

At the time of decoding, if it is tbs, the length will be calculated one less.
I get an error on amount, err := decodeAmount(hrp[netPrefixLength:])

exmaple
lntbs200u1psgl2a7pp5xuslr4k4g9n83w6j2mpeh3l84alwd68dh73rfgk9sfvlq9qtptyqdq2w3sk6ctddycqzpgsp5mk3cmhmmfgk2569khklxpg2gq6xfzj7ujv6ackw09cdfxm5g3vms9q8zqqyssqy8evtn0xt46t7xvvm3y4jf50n9zn7g7xs3uyuq6wj7zwpgulevhn5f45s0zevhgh0ssu2jncnepfpx2z7wus6zgk2usvqa5kp8d07qsp027kan

bad len(tb) + 2 -> s200
correct len(tbs) + 2 -> 200

guggero · 2021-05-04T15:39:56Z

Thanks for the input @wakiyamap! Will fix that asap.

wpaulino

I tried a basic open channel end-to-end test between two local nodes (both connected to a local btcd instance) on the default signet and noticed the funding transaction failed to broadcast:

unmatched backend error: -26: TX rejected: transaction 3a7c4ba762cafe3bfe8db312abde4db5ff200e4d066564f21c00393a7a2d64ab has witness data, but segwit isn't active yet

Segwit is indeed active as I received P2WKH funds from a faucet, so it seems there's an issue with btcd determining whether the deployment is active when using signet's segwit deployment parameters.

DeploymentSegwit: {
	BitNumber:  1,
	StartTime:  0,             // Always available for vote
	ExpireTime: math.MaxInt64, // Never expires.
},

wpaulino · 2021-05-06T22:06:57Z

chainreg/chainparams.go

+// BitcoinSigNetParams contains parameters specific to the signet test network.
+var BitcoinSigNetParams = BitcoinNetParams{
+	Params:   &bitcoinCfg.SigNetParams,
+	RPCPort:  "38334",


Port should be 38332.

Right. We didn't do the if wallet then 38334 else 38332 dance in btcd. So had to add a special case in lnd since we always subtract 2 for bitcoind.

guggero · 2021-05-11T12:25:46Z

Thanks for testing this E2E! I figured out what the problem was. Going to fix it in btcd itself, no changes in lnd needed for the deployment. Was my misunderstanding on how the deployments actually work...

Will be fixed in btcsuite/btcd#1718.

wpaulino

The new dependent btcd PR has been merged now as well.

config.go

guggero · 2021-05-12T11:52:15Z

The new dependent btcd PR has been merged now as well.

We don't necessarily need to pull in the dependency as the fix was for running btcd on signet itself. But we can update the dependency in case we want to make sure we use the latest version for itests.
@wpaulino do you think we should update the dep?

wpaulino · 2021-05-12T18:27:40Z

Ah didn't realize the new one wasn't strictly dependent, it could be useful for those (if any) who choose to install btcd through make btcd. I wouldn't consider it a blocker though.

carlaKC

Small set of changes for such a nice feature! LGTM

carlaKC · 2021-05-13T15:36:06Z

config.go

+					str := "%s: Invalid signet challenge, " +
+						"hex decode failed: %v"
+					err := fmt.Errorf(str, funcName, err)


why separate string and error here?

Wanted to follow the style of the rest of the file. But you're right, I like the more compact style more as well, fixed.

With this commit we make lnd compatible with the public signet test network.

The Core devs decided to us the same bech32 HRP for Signet as is used for the current Testnet3. This might be okay for on-chain addresses since they are compatible in theory. But for invoices we want to use a distinct HRP to distinguish testnet from signet. Also see spec PR lightning/bolts#844 for more information about the reasoning.

This commit updates the btcd version to a more recent one in which a bug was fixed that lead to SegWit and Taproot not being activated properly on signet. This update is not strictly necessary for lnd to work but we include it in case anyone wants to install btcd through lnd's Makefile.

guggero · 2021-05-18T11:12:55Z

I added a commit to bump btcd to the most recent version so installing it through lnd's Makefile gives users a signet compatible version.

@Roasbeef, @cfromknecht any chance we can get this in for v0.13.0-beta.rcX?

vwoo · 2021-05-25T23:21:03Z

Default rpc port seems to be incorrectly 38330.

// works
bitcoind.rpchost=x.x.x.x:38332

// fails, see log below
bitcoind.rpchost=x.x.x.x

2021-05-25 12:53:38.521 [INF] LNWL: Opened wallet
2021-05-25 12:53:38.619 [INF] CHRE: Primary chain is set to: bitcoin
2021-05-25 12:54:56.057 [ERR] LTND: unable to create chain control: Post "http://x.x.x.x:38330": dial tcp x.x.x.x:38330: connect: operation timed out
2021-05-25 12:54:56.058 [INF] LTND: Shutdown complete

unable to create chain control: Post "http://x.x.x.x:38330": dial tcp x.x.x.x:38330: connect: operation timed out

Roasbeef · 2021-05-27T01:18:04Z

cc @guggero ^

Roasbeef · 2021-05-27T01:18:55Z

chainreg/chainregistry.go

 						rpcPort = 19443
+					case cfg.Bitcoin.Active && cfg.Bitcoin.SigNet:
+						rpcPort = 38332


@vwoo perhaps something is being morphed before the call is made? We enforce the default port here

If I change the signet port from 38332 to 38334 in chainreg/chainparams.go then the default port works.

Just following the pattern in the rest of chainparams.go where the port is always +2.

bitcoind --help | grep rpcport
mainnet: 8332
testnet: 18332
signet: 38332

chainreg/chainparams.go
mainnet: 8334
testnet: 18334
signet: 38332

Seems like the pattern break is intentional but maybe there was a mixup between chainregistry.go and chainparams.go?

@vwoo see #5351.

Roasbeef · 2021-05-27T01:19:47Z

Gonna merge as is, AFAICT the port information is correct here. Want to include this in the next rc, and can do another one if it turns out there's actually something afoot here.

b3h3rkz · 2021-06-10T16:32:30Z

I have nothing special to add here but to say I love that this came in at this time!!!

Timely and kudos to everyone involved!

guggero added backend Related to the node backend software/interface (e.g. btcd, bitcoin-core) bitcoind Bitcoin Core backend P2 should be fixed if one has time chain handling 32-bit labels Feb 14, 2021

guggero added this to the 0.13.0 milestone Feb 14, 2021

joostjager reviewed Feb 15, 2021

View reviewed changes

chainreg/chainparams.go Show resolved Hide resolved

guggero mentioned this pull request Feb 15, 2021

Bolt 11: add distinct HRP prefix for Bitcoin signet lightning/bolts#844

Merged

guggero mentioned this pull request Mar 4, 2021

Update btcd dependency, add Signet params btcsuite/btcwallet#735

Merged

guggero force-pushed the signet branch from 267a8e0 to d512b0f Compare March 4, 2021 15:19

guggero force-pushed the signet branch from d512b0f to 1c0f036 Compare March 10, 2021 12:52

keblek mentioned this pull request Mar 17, 2021

Signet hsjoberg/blixt-wallet#281

Open

Roasbeef modified the milestones: 0.13.0, v0.14.0 Mar 31, 2021

guggero force-pushed the signet branch from 1c0f036 to af94107 Compare April 22, 2021 11:32

guggero force-pushed the signet branch from af94107 to b618071 Compare May 5, 2021 08:44

guggero requested review from joostjager and wpaulino May 5, 2021 08:45

guggero force-pushed the signet branch from b618071 to 0eefb13 Compare May 5, 2021 08:46

wpaulino suggested changes May 6, 2021

View reviewed changes

guggero force-pushed the signet branch from 0eefb13 to 29f436f Compare May 11, 2021 12:20

guggero force-pushed the signet branch 2 times, most recently from ddbbb56 to a6d6f5e Compare May 11, 2021 12:34

wpaulino reviewed May 11, 2021

View reviewed changes

config.go Show resolved Hide resolved

guggero force-pushed the signet branch from a6d6f5e to f56d9e5 Compare May 12, 2021 11:50

guggero requested review from carlaKC and removed request for joostjager May 12, 2021 11:53

wpaulino approved these changes May 12, 2021

View reviewed changes

carlaKC approved these changes May 14, 2021

View reviewed changes

guggero added 3 commits May 18, 2021 13:06

multi: add signet parameters

4460903

With this commit we make lnd compatible with the public signet test network.

guggero force-pushed the signet branch from f56d9e5 to 367f2c8 Compare May 18, 2021 11:11

Roasbeef reviewed May 27, 2021

View reviewed changes

Roasbeef modified the milestones: v0.14.0, 0.13.0 May 27, 2021

Roasbeef merged commit 0ed72b8 into lightningnetwork:master May 27, 2021

guggero deleted the signet branch May 27, 2021 07:47

Roasbeef mentioned this pull request Jun 1, 2021

build: bump version to v0.13.0-beta.rc4 #5350

Merged

wpaulino mentioned this pull request Jun 1, 2021

config+chainreg: fix default rpc params for signet bitcoind #5351

Merged

Sjors mentioned this pull request Aug 30, 2021

Signet support getumbrel/umbrel#507

Closed

openoms mentioned this pull request Sep 13, 2021

[Feature request] signet support raspiblitz/raspiblitz#2068

Closed

Sjors mentioned this pull request May 10, 2022

c-lightning v0.11.0: strconv.ParseUint: parsing \"s10\": invalid syntax fiatjaf/sparko#18

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add basic bitcoin signet support #5025

Add basic bitcoin signet support #5025

guggero commented Feb 14, 2021 •

edited

Loading

guggero commented Mar 4, 2021

guggero commented Apr 22, 2021

wakiyamap commented May 3, 2021 •

edited

Loading

guggero commented May 4, 2021

wpaulino left a comment

wpaulino May 6, 2021

guggero May 11, 2021

guggero commented May 11, 2021

wpaulino left a comment

guggero commented May 12, 2021

wpaulino commented May 12, 2021

carlaKC left a comment

carlaKC May 13, 2021

guggero May 18, 2021

guggero commented May 18, 2021

vwoo commented May 25, 2021 •

edited

Loading

Roasbeef commented May 27, 2021

Roasbeef May 27, 2021

vwoo May 27, 2021

wpaulino Jun 1, 2021

Roasbeef commented May 27, 2021

b3h3rkz commented Jun 10, 2021

Add basic bitcoin signet support #5025

Add basic bitcoin signet support #5025

Conversation

guggero commented Feb 14, 2021 • edited Loading

guggero commented Mar 4, 2021

guggero commented Apr 22, 2021

wakiyamap commented May 3, 2021 • edited Loading

guggero commented May 4, 2021

wpaulino left a comment

Choose a reason for hiding this comment

wpaulino May 6, 2021

Choose a reason for hiding this comment

guggero May 11, 2021

Choose a reason for hiding this comment

guggero commented May 11, 2021

wpaulino left a comment

Choose a reason for hiding this comment

guggero commented May 12, 2021

wpaulino commented May 12, 2021

carlaKC left a comment

Choose a reason for hiding this comment

carlaKC May 13, 2021

Choose a reason for hiding this comment

guggero May 18, 2021

Choose a reason for hiding this comment

guggero commented May 18, 2021

vwoo commented May 25, 2021 • edited Loading

Roasbeef commented May 27, 2021

Roasbeef May 27, 2021

Choose a reason for hiding this comment

vwoo May 27, 2021

Choose a reason for hiding this comment

wpaulino Jun 1, 2021

Choose a reason for hiding this comment

Roasbeef commented May 27, 2021

b3h3rkz commented Jun 10, 2021

guggero commented Feb 14, 2021 •

edited

Loading

wakiyamap commented May 3, 2021 •

edited

Loading

vwoo commented May 25, 2021 •

edited

Loading