Skip to content
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

build(deps): update tor from v0.4.7.13 to v0.4.8.7 #1537

Merged

Conversation

theborakompanioni
Copy link
Contributor

@theborakompanioni theborakompanioni commented Aug 26, 2023

Updates local tor from v0.4.7.13 to v0.4.8.7.

Release notes: https://gitlab.torproject.org/tpo/core/tor/-/commit/51cefce3e689461492d22275b0bcac36db61ac8e

@AdamISZ
Copy link
Member

AdamISZ commented Aug 26, 2023

Seems like a really worthwhile update.

How does the interaction between updated and non-updated Tor connections to onion services work, I wonder?

Is it just, if you are an unupdated client, then you might be OK, but if DOS connections kick in and you can't provide a PoW, then you just won't be able to connect?

Copy link
Member

@kristapsk kristapsk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK c0dbd61

@theborakompanioni
Copy link
Contributor Author

Seems like a really worthwhile update.

How does the interaction between updated and non-updated Tor connections to onion services work, I wonder?

Is it just, if you are an unupdated client, then you might be OK, but if DOS connections kick in and you can't provide a PoW, then you just won't be able to connect?

I suppose so, but I don't know exactly.

[...] This introduces several torrc options prefixed with "HiddenServicePoW" in order to control this feature. By default, this is disabled.

At first glance, it seems, clients that dont provide PoW when requested, are de-prioritized.
You can find the whole proposal here: https://gitlab.torproject.org/tpo/core/torspec/-/blob/main/proposals/327-pow-over-intro.txt

@theborakompanioni theborakompanioni marked this pull request as ready for review August 28, 2023 09:45
@kristapsk
Copy link
Member

Related Wasabi Wallet discussion about upgrading to Tor v0.4.8.x - WalletWasabi/WalletWasabi#10833.

@theborakompanioni theborakompanioni changed the title build(deps): update tor from v0.4.7.13 to v0.4.8.4 build(deps): update tor from v0.4.7.13 to v0.4.8.6 Sep 21, 2023
@theborakompanioni
Copy link
Contributor Author

Rebased and update to v0.4.8.6 (formerly v0.4.8.4).

@AdamISZ
Copy link
Member

AdamISZ commented Sep 21, 2023

Update on this; I posted this on nostr but that can be flaky, so copy-pasting:

(Also before the post, two things: it's 0.4.8.6 specifically; the bug was still in 0.4.8.5 apparently. And, it seems the update is specifically needed for the onion service side, so, in our case, the directory node, not so much the client (though I'm not 100% sure).


A heads-up to people who are using Tor onion services in their code.

Pre- the latest release (0.4.8.6) of Tor, there was a serious bug causing connection issues. I am not sure that this is the reason that Joinmarket's onion messaging has been having serious reliability issues, but it might be. Either way, this was a problem so if your Tor version is still lower than above and you serve onion services, I suggest you fix it ASAP:

Quote:

"According to our analysis, expiry of ALL introduction points, from both descriptors, all at once, causes an availability issue. Every time a consensus is received (roughly every hour on average), the service is not available for up to two minutes, as the descriptor cache of the clients is valid for two minutes. Until the clients load a new descriptor, the service is not available.
In addition, the expiry of introduction points may lead to unnecessary hidden service descriptor uploads, which may affect the performance of both TOR clients and relays."

From: https://gitlab.torproject.org/tpo/core/tor/-/issues/40858

https://snort.social/e/nevent1qqsfun99t4ts5pfpn368t43aq5fclw0ef5vu2594009q4g7ju732vacpr4mhxue69uhkummnw3ez6vp39e3x7mr59ehkyum9wfmx2u30qgsxwkuyle67y94tj378gw8w2xw2wa6nwmwlqhddlwnz0z7sztsaw2qrqsqqqqqpptrmxv

@AdamISZ
Copy link
Member

AdamISZ commented Sep 21, 2023

Oh, slight qualification on my previous post: I forgot that makers also serve onions, so logically they must update for the same reason, though I don't know if they were having similar availability issues, probably the 1 hour doesn't kick in for them .. well, anyway, everyone should just update :)

@kristapsk
Copy link
Member

I'm having trouble testing this right now. Currently from my development laptop JM scripts fail to connect to any directory nodes both with Tor v0.4.8.6 and v0.4.7.14.

@kristapsk
Copy link
Member

Configured directory nodes - 3kxw6lf5vf6y26emzwgibzhrzhmhqiw6ekrek3nqfjjmhwznb2moonad.onion:5222, jmdirjmioywe2s5jad7ts6kgcqg66rj6wujj6q77n6wbdrgocqwexzid.onion:5222, bqlpq6ak24mwvuixixitift4yu42nxchlilrcqwk2ugn45tdclg42qid.onion:5222.

I''m running bitcoin-qt in parallel, it has no problems connecting to onion peers.

@AdamISZ
Copy link
Member

AdamISZ commented Sep 21, 2023

Configured directory nodes - 3kxw6lf5vf6y26emzwgibzhrzhmhqiw6ekrek3nqfjjmhwznb2moonad.onion:5222, jmdirjmioywe2s5jad7ts6kgcqg66rj6wujj6q77n6wbdrgocqwexzid.onion:5222, bqlpq6ak24mwvuixixitift4yu42nxchlilrcqwk2ugn45tdclg42qid.onion:5222.

I''m running bitcoin-qt in parallel, it has no problems connecting to onion peers.

Confirmed: Everything works perfectly at first, but after a few hours after the directory node was restarted (in this case I think about 3 hours), the same pattern is repeating, something like (a couple of extra debug messages, but same pattern):

2023-09-21 14:14:35,916 [INFO]  Updating status to connected for peer 3kxw6lf5vf6y26emzwgibzhrzhmhqiw6ekrek3nqfjjmhwznb2moonad.onion:5222.
2023-09-21 14:14:35,917 [INFO]  We, xxx.onion:5222, are calling the handshake callback as client.
2023-09-21 14:14:35,918 [INFO]  Sending this handshake: {"app-name": "joinmarket", "directory": false, "location-string": "xxx.onion:5222", "proto-ver": 5, "features": {}, "nick": "yyy", "network": "mainnet"} to peer 3kxw6lf5vf6y26emzwgibzhrzhmhqiw6ekrek3nqfjjmhwznb2moonad.onion:5222
Was sending the handshake as client successful?:  True
2023-09-21 14:14:36,075 [DEBUG]  We got a disconnect event: 3kxw6lf5vf6y26emzwgibzhrzhmhqiw6ekrek3nqfjjmhwznb2moonad.onion:5222
2023-09-21 14:14:36,076 [INFO]  Going to reattempt connection to 3kxw6lf5vf6y26emzwgibzhrzhmhqiw6ekrek3nqfjjmhwznb2moonad.onion:5222 in 103.515625 secon

etc.

This pattern has been largely unchanged for a long time afaict. I'm slightly surprised that it is not related to the aforementioned bug, but it seems, it is not.

@AdamISZ
Copy link
Member

AdamISZ commented Sep 28, 2023

ACK e37f8b4

Checked the hash manually.

@kristapsk please merge if it's OK with you (leave it for you since it's your area).

@theborakompanioni
Copy link
Contributor Author

ACK e37f8b4

Checked the hash manually.

Aaaand v0.4.8.7 is already out 🙈
Release 4 days ago (2023-09-25).

From the release notes

Major bugfixes (conflux):
- Fix an issue that prevented us from pre-building more conflux sets
after existing sets had been used. Fixes bug 40862; bugfix
on 0.4.8.1-alpha.

Update?

@kristapsk
Copy link
Member

@AdamISZ Will get back to this next week.

@kristapsk
Copy link
Member

ACK e37f8b4
Checked the hash manually.

Aaaand v0.4.8.7 is already out 🙈 Release 4 days ago (2023-09-25).

From the release notes

Major bugfixes (conflux):

  • Fix an issue that prevented us from pre-building more conflux sets
    after existing sets had been used. Fixes bug 40862; bugfix
    on 0.4.8.1-alpha.

Update?

Yes, I think it makes sense to go for v0.4.8.7 instead of v0.4.8.6.

@theborakompanioni theborakompanioni changed the title build(deps): update tor from v0.4.7.13 to v0.4.8.6 build(deps): update tor from v0.4.7.13 to v0.4.8.7 Oct 5, 2023
@theborakompanioni
Copy link
Contributor Author

Yes, I think it makes sense to go for v0.4.8.7 instead of v0.4.8.6.

Agree. Rebased and force-pushed. With faster intervals between new tor releases, it will probably need an update again before a new JM release anyway. 🙌

Copy link
Member

@kristapsk kristapsk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK c08e824

@kristapsk kristapsk merged commit 4e9d9e1 into JoinMarket-Org:master Oct 5, 2023
20 checks passed
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 this pull request may close these issues.

3 participants