Skip to content

Regression for sign_tx_request in hsmd when running in liquid-regtest configuration #3487

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

Closed
cdecker opened this issue Feb 6, 2020 · 3 comments · Fixed by #3491
Closed

Comments

@cdecker
Copy link
Member

cdecker commented Feb 6, 2020

It appears that the changes in commit 5c8f881 (part of PR #3363) broke the signature logic when running on an elements blockchain.

20200206152016-3 DEBUG:root:lightningd-1: 2020-02-06T15:23:13.337Z DEBUG 022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59-openingd-chan#1: peer_in WIRE_ACCEPT_CHANNEL
20200206152016-3 DEBUG:root:lightningd-1: 2020-02-06T15:23:13.356Z DEBUG hsmd: Client: Received message 19 from client
20200206152016-3 DEBUG:root:lightningd-1: 2020-02-06T15:23:13.356Z **BROKEN** hsmd: 022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59: tx must have matching witscripts
20200206152016-3 DEBUG:root:lightningd-1: 2020-02-06T15:23:13.356Z **BROKEN** 022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59-openingd-chan#1: STATUS_FAIL_HSM_IO: Bad sign_tx_reply

As it is this completely breaks c-lightning on elements, including liquid and liquid-regtest

Maybe @ksedgwic knows what might have caused this failure?

@cdecker
Copy link
Member Author

cdecker commented Feb 6, 2020

Ok, I think I found the issue: we don't have a witness_script associated with the change output, which in elements-based chains is made explicit in order to allow verification of confidential transaction output values. Unfortunately we did not create mock witness_scripts (they're empty in all cases) so we either need to create those on the lightningd side, or allow a difference in the number of outputs and the number of witness_scripts in hsmd. I have a quick fix doing the latter, but for consistency I'd like to push this down to lightningd freeing hsmd from having to special case that.

I'd also like to co-locate the witness_scripts inside the transaction, so whenever we add an output we also automatically add the scripts to the tx. This will be a cleanup however.

@devrandom
Copy link
Contributor

how do we go about replicating the failure?

@cdecker
Copy link
Member Author

cdecker commented Feb 7, 2020

No problem, I'm working on a fix right now 🙂

In case you still want to reproduce this you'll need to have elementsd on the $PATH so pytest can find it. Then edit config.vars to TEST_NETWORK=liquid-regtest (or set it as an env var if you run pytest directly) and then run the tests either through make check or pytest tests -vvv.

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 a pull request may close this issue.

2 participants