Updating signet chainparams for the latest PoW fix

[dr-orlovsky]

Bitcoin Core PR started using Bech32 HRP tb instead of sb, which made c-lightning implementation incompatible. It was also updated with the new genesis block (adjusting PoW nonce). This PR fixes both issues

[kallewoof]

utACK, but not sure you need the comment-line with the genesis hash.

Travis build is failing. I can't determine what the issue is, personally.

[dr-orlovsky]

I put the comment line for the purpose to check the current genesis agains the new one if there will be any upgrades in the future.

Re Travis, I was digging into it and failed to understand. The scope of changes is very small to make the build process changed for some platforms, so have no idea why it fails...

[kallewoof]

Yeah, the genesis hash is not supposed to change again (knock on wood), so it's probably unnecessary, but no strong opinion otherwise.

[dr-orlovsky]

Ok, I can remove the comment line if needed, will wait for the repo maintainers decision

[jsarenik]

@dr-orlovsky I tried to rebase in #4073 but found the tests do not pass even locally so I closed it. There seems to be some collision with using tb also for signet because when I do a simple change of tb (in signet chainparams) to whatever the tests pass.

[jsarenik]

Signet is already in bitcoin/bitcoin@7737603 (current master).

In https://github.com/bitcoin/bitcoin/blob/master/src/chainparams.cpp#L332 there is the bech32_hrp = "tb"; (which is the same for the testnet). What shall we do in c-lightning to avoid the collision? @cdecker @ZmnSCPxj ? Any ideas please?

[dr-orlovsky]

@jsarenik I am using this patch in production with Bitcoin Core and electrs; everything seems to be working well.

Docker images & compose files:
https://github.com/LNP-BP/docker/blob/master/electrs-signet/Dockerfile
https://hub.docker.com/r/lnpbp/electrs/signet
https://github.com/LNP-BP/docker/blob/master/signet/docker-compose.yml

[jsarenik]

Signet is already in bitcoin/bitcoin@7737603 (current master).

No. Sorry for misinformation. Signet is not yet in master of Bitcoin Core.

UPDATE: Or at least I wished that it was not like this. But it is. And is using tb for signet as well. Let's see how it works… because now just by observing a bech32 address it can not be clear where it belongs to. Or am I missing something? @dr-orlovsky

[dr-orlovsky]

Signet got merged into master yesterday: bitcoin/bitcoin#18267

I've tested it with docker one day before the merge, from the signet branch, not master - but the code in the master exactly the same. And it works well with c-lightning after a patch from this PR was applied. So I see no reason why not to apply this PR to c-lightning, b/c without it it fails to work.

Not sure I do understand what concerns you, @jsarenik

[jsarenik]

@dr-orlovsky I totally agree it should be merged. The thing is that c-lightning stops to work because the bolt11 test ends with error. It has a conflict of two "tb" .bip173_names.

[jsarenik]

I wonder how they can distinguish in Bitcoin Core whether a tb... segwit address belongs to the real testnet or a signet. Maybe they don't, but IMHO that is begging for problems in the future. I will play with it on Bitcoin Core now.

[dr-orlovsky]

Regarding Bitcoin Core, it was decided to replace sb with tb and I doubt that this will be reverted. See the discussion here pls: bitcoin/bitcoin#12314 (comment)

I think what is needed is to stop guessing bitcoin network from just a single prefix, which is by definition can be used by multiple networks (testnet, different regtests, now signet, including private signets).

[jsarenik]

Just a note for remembering the AHA moment (from #c-lightning Freenode channel):

09:50 <dr-orlovsky> jasan: Ideally address should match to a set of networks
   and you need to check only that the current network which c-lightning is
   configured to use is within the set provided by HRP address prefix
09:50 <jasan> dr-orlovsky: I thought the whole point of introducing those
   prefixes was to minimize human error.
09:50 <jasan> OK, maybe I am a bit off-topic. I am thinking about humans :)
09:50 <dr-orlovsky> it is believed that using address from one testnet for
   other testnet type would not produce a fund-losing error b/c testnets do not store value
09:51 <jasan> Yes. OK. That makes sense.

[jsarenik]

Even lightningd with this patch is working, just that tests do not pass and stop on bolt11.

I have a setup ready to test.

[jsarenik]

C-Lightning works with @dr-orlovsky 's patch. I have tested it now on signet with latest bitcoind (bitcoin/bitcoin@7737603) and lightningd (jsarenik/lightning@bbcc633). The only thing that I have seen being broken because of the tb prefix conflict is the bolt11 test.

Practically everything works. Feel free to connect to my node on signet.

[kallewoof]

make install fails on this, on macos. It fails on master too, though, but the errors are different.

This PR:

../../../libwally-core/src/internal.c:190:5: error: implicit declaration of function 'memset_s' is
      invalid in C99 [-Werror,-Wimplicit-function-declaration]
    memset_s(dest, len, 0, len);
    ^
../../../libwally-core/src/internal.c:190:5: note: did you mean 'memset'?

Master:

gossipd/gossip_store.c:74:6: error: implicit declaration of function 'pwritev' is invalid in C99
      [-Werror,-Wimplicit-function-declaration]
        if (pwritev(fd, iov, ARRAY_SIZE(iov), *len) != sizeof(hdr) + msglen)
            ^

[jsarenik]

@kallewoof The errors seem unrelated. Please try jsarenik@bbcc633 (which is rebased to current master and a little fix in comment) and make sure you clean your tree before rebuilding.

	git clean -xffd
	git submodule deinit --all --force

[rustyrussell]

Can't force push, so opening a different PR using this commit (#4078)

[kallewoof]

@jsarenik No change.

[rustyrussell]

Merged into #4078