-
Notifications
You must be signed in to change notification settings - Fork 110
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
Integrate trustedcoin clightning plugin #597
Conversation
Hello everyone, I have completely removed the Let me know what you think. Thanks! |
@@ -27,6 +27,7 @@ task: | |||
- scenario: default | |||
- scenario: netns | |||
- scenario: netnsRegtest | |||
- scenario: trustedcoin |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am still not sure if it makes sense to run trustedcoin scenario regurarly in CI.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks again for your work on this. Left a few comments. Did you test whether this actually works and you can use clightning with an unsynced bitcoind?
Hello @jonasnick,
But the The
The output of
Ota |
@seberm Thanks for investigating this. I don't know why clightning's RPC does not get ready in time when trustedcoin is enabled. Maybe while syncing the plugin is in some initialization phase which blocks clightnings RPC startup? It seems like trustedcoin is doing some work despite the timeouts. Does clightning stop restarting on timeout at some after a while or does this go on forever? |
Hello @jonasnick . I did some more testing and found the issue. The reason the clightning RPC socket ( To be concrete, the issue is caused by these options in clightning's systemd service:
If I set See
So the question is how we should handle the network hardening in a case of trustedcoin plugin is enabled? Should I just disable tor enforcing? What's your opinion? Thank you! |
Just to answer your question:
It seems to be restarting forewer :). |
Very nice @seberm. I don't really like to disable Tor in a way that is unexpected for the user. This would be a huge footgun. But as I see it, trustedcoin is anyway only something you'd do for demonstration purposes. So it may be ok if we very clearly document this. Alternatively we can try to tunnel trustedcoin traffic through Tor. |
e86973b
to
4942130
Compare
I agree, I don't like it either.
It seems trustedcoin does not honor the clightning's |
Hello @jonasnick, I also created nbd-wtf/trustedcoin#19 to discuss possible changes in trustedcoin itself. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK a3c6547
This PR tries to add a trustedcoin support into the nix-bitcoin.
The work is based on PRs:
The code is rebased on current master.
I have removed the
clightning.useBcli
option and replaced it withclightning.useBitcoind
. When setting this option totrue
, the bitcoind unit is forcerly disabled andclightning.plugins.trustedcoin.enabled
is set totrue
as suggested in #533 (comment).I also tried to add some basic tests.
@erikarvstedt , could you please take a look and give me some suggestions? Please keep in mind this is only a draft but I would like to move this feature forward.
Thanks!
CC: @neverupdate , @MatthewCroughan , @elsirion
Closes #533