-
Notifications
You must be signed in to change notification settings - Fork 912
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
generate a static tor onion address persistent unique to the node id #3155
generate a static tor onion address persistent unique to the node id #3155
Conversation
10e2779
to
80c6316
Compare
20141dc
to
ea68f2d
Compare
@cdecker rebased and ready to rumble ... 👍 |
3afbb39
to
cf2643c
Compare
9417bcf
to
9233e15
Compare
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.
There is a lot of error-prone string parsing in the PR, I wonder if using
tal_strreg
might simplify the code and make it more flexible for future changes.
As it is now this resembles more a key-value bag than an address, so maybe we
should embrace that and actualy use :
as the key-value separator in a stream
of pairs that use a different separator? Similar to the URL encoding commonly
used on the web, and making the relationship beween torport
and 45321
clearer (I fell for that myself, not realizing that these are effectively
key-value pairs and not understanding the format 🙁). We are using /
elsewhere in the RPC as separator sometimes.
If we were to introduce backwards incompatible changes I'd advocate for a
format similar to this:
statictor=127.0.0.1:9151/torport=45321/torblob=11234567890123456789012345678901
The use of :
as separator is really causing a bit of trouble since it is
commonly used as <address>:<port>
separator, and for internal formatting of
IPv6 addresses (who came up with that design...🤬)
As a side note, I had to review this as a complete diff, since the incremental changes made it really hard to analyse the commits individually (I'd also be doing a squash merge on this since we can't guarantee bisectability).
We tend to normally have self-contained commits that are minimal in what they do, and then we fix them up using either fixup!
commits (like I usually do) or by amending commits in-place (rusty uses guilt
iirc). Both methods result in a clean and easy to follow commit history that is easier to review, and to audit later, whereas just adding incremental changes in cleanup commits added to the end of the PR make things hard. I'd strongly suggest using ones of those processes for future pull requests 😉
0d87016
to
478b541
Compare
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.
addressed the most/all points of your review. Thanks. 👍
Please review again
Its now rebased, bisectable and structured in 3 parts.
1cb6281
to
5942998
Compare
c0c4197
to
c11566c
Compare
A bit of side show, but almost all PR not only this, fail the Travis lottery on first or second push with random errors unrelated to the changes. |
I'm restarting failed tests and keeping track of which tests were flaky, so you don't need to push just to get Travis to like your PR 😉 Pushing the same PR just to retry Travis just makes tracking changes rather hard: http://46.101.246.115:8888/ElementsProject/lightning/pull/3155 |
d4d1563
to
2d23a62
Compare
We want to have a static Tor service created from a blob bound to our node on cmdline Changelog-added: persistent Tor address support Changelog-added: allow the Tor inbound service port differ from 9735 Signed-off-by: Saibato <saibato.naga@pm.me> Add base64 encode/decode to common We need this to encode the blob for the tor service Signed-off-by: Saibato <saibato.naga@pm.me>
Signed-off-by: Saibato <saibato.naga@pm.me>
Signed-off-by: Saibato <saibato.naga@pm.me>
2d23a62
to
14882ec
Compare
very cool, thanks @Saibato ! |
We now can generate a V3 tor service address from a blob
--addr=statictor:127.0.0.1:9051[/torblob=[blob]]
Or generate a unique static tor address bound to our node id /pub key
--addr=statictor:127.0.0.1:9051
TODO:
Documentation
Write some simple py-tests
EDIT: With this you are able to keep your public known or group known tor address static
and independent of torrc file or the tor instance ( if multiple ), in case you have to
move your node to an other PC or system/phone, as long as there is a tor service
your node will be reachable even if ip, NAT, firewall have changed by that static V3 address.
EDIT: related issues #2476 #3085
EDIT_2:
With this PR we can now also select the extern tor port we assign our local port to
--addr=statictor:127.0.0.1:9051[/torblob=[blob]][/torport=EXTERNPORT]
Or generate a unique static tor address bound to our node id /pub key
--addr=statictor:127.0.0.1:9051[/torport=EXTERNPORT]