[WIP] grs use wally sha256 and warp to v.0.7.1

[ghost]

No description provided.

[gruve-p]

Just compiled it to test it out.
When generating a new address for p2sh mainnet it generates a p2sh address for testnet:

~/lightning/cli#./lightning-cli newaddr p2sh-segwit
{
   "address" : "2N7sPwsy4HyoBXKFsphX5a6kdPfq7Qn5vYV",
   "p2sh-segwit" : "2N7sPwsy4HyoBXKFsphX5a6kdPfq7Qn5vYV"
}

[gruve-p]

There seems to be compatibilty issues between c-lightning and eclair.
C-lightning nodes can open channels to each other.
Eclair nodes can open channels to each other.
However channels can't be opened between c-lightning nodes and eclair nodes because of a bad signature error message in the logs.

Eclair test binary can be found here: https://github.com/Groestlcoin/eclair/releases/download/v0.2-alpha3-grs3/eclair-node-gui-0.3.2-SNAPSHOT-7ad2e45.jar (don't mind the old release tag, we uploaded recent binary on the latest available release tag)

Question here is which implementation is using the right hash for signature and which implementation should change the signature hash to make it compatible?
Keep in mind, we will be getting LND implementation and Electrum LN implementation also somewhere this year which needs to use the same hash for signature.

[ghost]

AFAICS both impl. have issues to eclair.
Same for LND a bit opposite.

see lightningnetwork/lnd#3126

I will take a look in that.
In general LND leaks IP over DNS by it's suboptimal Tor implementation, so for a coin that's states secure/private.
LND is not the best of choices.

And when it comes to further impl. in GRS
If I could chose in sight of QC and other fast statistical factoriesation methods
, between

A) Schnorr music party
B) Lamport dinner
C) Picnic with 3 bears

I would craft a Picnic fork :-) and some halving

EDIT: in elcair there is no transition from double sha256 to groestl so
I can not see how that is a grs lightning. And have you withdrawn successully funds of a closed channel from eclair-ln to a legacy wallet?

If doublesha256 breaks there is no backup so grs makes some sense,but if the eclair-ln does not use groestl for double hash
What use is there for grs lightning without groestl channels?

[gruve-p]

Eclair uses https://github.com/Groestlcoin/groestlcoin-lib as dependency where all necessary hashes are changed from double sha256 to double groestl.

I have withdrawn succesfully funds of closed channels from eclair.

Even master branch of https://github.com/Groestlcoin/lightning/ does not work with our eclair. So this issue is not related to this PR.

[gruve-p]

We had found some bugs on Eclair (invoices have wrong hrp; they start with bc1 instead of grs1) which is currently fixed on Github. HashEngineering will return next week from vacation and will make a new build to test compatibility with c-lightning 0.7.1.

Thank you for your patience

[ghost]

Here a eclair compat c-ln less groestl more sha256 :-( i.e for testing
https://github.com/dflatemaster/lightning/commit/caa65d6c06d25cf7b0c578606f4ec0d62b0ddb30
My QD Scala groestcoin-lib to test with regtest
https://github.com/dflatemaster/groestlcoin-lib/commit/5dbcb5da66d52af33c220589812edc83447f9978

[gruve-p]

Thanks for fixing regtest on our groestlcoin-lib. https://github.com/dflatemaster/groestlcoin-lib/commit/5dbcb5da66d52af33c220589812edc83447f9978 works fine on our regtest

I also tested https://github.com/dflatemaster/lightning/commit/caa65d6c06d25cf7b0c578606f4ec0d62b0ddb30 and the compatibility between c-lightning and eclair is indeed fixed with this.

We keep track of everything that needs sha256, sha256d and groestlhash on Groestlcoin here:
https://github.com/Groestlcoin/grs-dev-docs/blob/master/hashes.md

We have to decide next week if we want to use your fix on c-lightning (which works fine) and keep our eclair as it is or if we want to fix this on eclair side and not use your fix.

Much appreciate your troubleshooting efforts

[gruve-p]

We are brainstorming about this.

sha256_double_groestl seem to be introducing a new kind of hash: groestl(sha256(message)) for block hashes. In Groestlcoin Core our block header are hashed with GroestlHash. And our Block hashes are groestlHash digests of the block header instead of sha256d digests.

For consistency we should stick to these 3 hashes:
sha256 (for transactions and messages signed by user)
sha256d (merkleblock)
groestl512d (aka groestlhash used for Block headers, Base58 Checksums and P2P network messages)

Maybe we should rename sha256_double_groestl to groestl512_double and change groestl(sha256(message)) to groestl(groestl(message)) to be consistent with Groestlcoin Core?

Any thoughts on this approach?

[ghost]

Double a sha3 function ( groestl ) ?
Length extension attacks are a problem for sha-2 (sha256) so single Groestl is ok and makes sense.

AFAICS double any hash is never a good idea. as that yields to some mapping attacks, if one knows the prime/primes and the generator of the sha-x modular group, so single sha256 the tx is better than sha256d

groestl(sha256( ... is a bit much for P2P message ̶b̶u̶t̶ ̶I̶ ̶w̶o̶u̶l̶d̶ ̶a̶l̶w̶a̶y̶s̶ ̶p̶r̶e̶f̶e̶r̶ ̶a̶ ̶s̶i̶n̶g̶l̶e̶ ̶h̶a̶s̶h̶.̶a

So that are just my thoughts on that.

[yura-pakhuchiy]

@dflatemaster @gruve-p I propose to use exactly the same hashes in Groestlcoin Lightning as Bitcoin uses, except for the cases where we must use different hashes due to Groestlcoin Core and Bitcoin Core differences. I believe lightning developers carefully weighted all the trade-offs and it is not wise to randomly change hashes in GRS Lightning. Also usage of Bitcoin hashes will make Groestlcoin lightning implementations maintenance easier.

[gruve-p]

Thank you for your thoughts @dflatemaster and @yura-pakhuchiy
I agree in general with Yura to stick to the same hashes as Bitcoin uses except for the already differences between Bitcoin Core and Groestlcoin Core.

@dflatemaster Let's not use single groestl hash anywhere in c-lightning please. It will bring alot of confusion between the GRS devs, We value your opinion but we need to make it also easy to maintain eclair, lnd and c-lightning.

Lets stick to these guidelines:
single sha256 (for transactions and messages signed by user)
double sha256 (merkleblock)
double groestl512 (aka groestlhash; for OP_HASH256, Block headers, Base58 Checksums and P2P network messages)

Can you adjust c-lightning based on the above?
I guess that would mean renaming sha256_double_groestl to groestl512_double and change groestl(sha256(message)) to groestl(groestl(message))? What are you using for bs58check and block headers in c-lightning? I assume double groestl ? If not that needs to be corrected also. The aim is that all 3 implementations uses the same hashes for everything and are compatible with each other.

@HashEngineering Will be adjusting groestlcoin-lib for eclair also next week to the above guidelines

[ghost]

I see your point, since almost all moved to the crypto lib, a simple sha is probably enough here.

As you probably know LN devs are not free of external influences. And I see patterns of classical strategies like promoting or upvoting trivials or conferences when work is needed or delegating to large sub comissions that never deliver or assigning hard task to not the best in there field.
What discurages the better ones. A similar pattern you see in Bitcoin in general.

I think the more GRS does his own way and have his own impl, it may have a future.

So thanks for your remarks and please never hesitate to say frank what you think. even if I don't react fast or with a smile.
FOSS needs no confident channels or subgroups, we say what we want free and open and then we decide.

[yura-pakhuchiy]

@dflatemaster I suppose that this was your reply to #16 (comment)

OK, I do not want argue about whether LN devs are doing best effort or not, anyway as you said "simple sha is probably enough here".

However you have not addressed real issues with your current code I have pointed there. Here is expanded a bit version:

1. It is very confusing and error prone that sha256_double_done computes single SHA despite name. Especially when sha256_double actually computes double SHA as advertised. Developers who are not aware of this change can lose hours debugging. It is like adding #define true false somewhere to headers, but wrong hash value is not as easy to spot as true/false replacement.
2. It is extremely fragile. Upstream devs may refactor some code to use sha256_double instead sha256_double_done (or vice-versa) in next release and this trivial change will completely break your implementation.
3. Hash is replaced in many random places, not only in places which are required for Groestlcoin Core compatibility. This will make harder to develop another compatible Lightning implementations, because internal implementation details may differ and by doing similar replacement in other codebases hash will not be replaced exactly in the same cases.

So I propose:

1. Introduce sha256_done() for single SHA and update code only in required places for Core compatibility to use it.
2. Do not change hashes at any new places except where GRS Core changed. If you strongly believe that at some place in Lightning hash should be changed, then please submit PR to docs with detailed description as well.

I understand it is much easier to replace code in sha256_double instead of studying c-lightning codebase and GRS docs and replace hash only in required places, but unfortunately this approach is not acceptable due to reasons I've listed above.

[gruve-p]

I agree with @yura-pakhuchiy in general.
If we really want to change it then I would propose to change sha256_double_done to single_sha256_done . Feel free to send a PR with the suggested fixes.