vasild
commented at 3:40 PM on July 14, 2023:
contributor
Background
In the i2p::sam::Session class:
Listen() does:
if the session is not created yet
create the control socket and on it:
HELLO
SESSION CREATE ID=sessid
leave the control socked opened
create a new socket and on it:
HELLO
STREAM ACCEPT ID=sessid
read reply (STREAM STATUS), Listen() only succeeds if it contains RESULT=OK
Then a wait starts, for a peer to connect. When connected,
Accept() does:
on the socket from STREAM ACCEPT from Listen(): read the Base64 identification of the connecting peer
Problem
The I2P router may be in such a state that this happens in a quick succession (many times per second, see #22759 (comment)): Listen()-succeeds, Accept()-fails.
Accept() fails because the I2P router sends something that is not Base64 on the socket: STREAM STATUS RESULT=I2P_ERROR MESSAGE="Session was closed"
We only sleep after failed Listen() because the assumption was that if Accept() fails then the next Listen() will also fail.
Solution
Avoid filling the log with "Error accepting:" messages and sleep also after a failed Accept().
Extra changes
Reset the error waiting time after one successful connection. Otherwise the timer will remain high due to problems that have been solved long time in the past.
Increment the wait time less aggressively.
Handle the unexpected "Session was closed" message more gracefully (don't log stupid messages like Cannot decode Base64: "STREAM STATUS...) and destroy the session right way.
DrahtBot
commented at 3:40 PM on July 14, 2023:
contributor
<!--e57a25ab6845829454e8d69fc972939a-->
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
DrahtBot added the label CI failed on Jul 14, 2023
jonatack
commented at 7:46 PM on July 14, 2023:
member
Initial code review ACK (didn't review the test changes yet), will test IRL.
vasild force-pushed on Jul 15, 2023
vasild
commented at 4:54 AM on July 15, 2023:
contributor
e556bc6500...ffa90fceae: try to fix windows compilation
DrahtBot removed the label CI failed on Jul 15, 2023
jonatack
commented at 6:58 PM on July 15, 2023:
member
Initial testing with the Java I2P Router shows that this pull fixes the issue (nice work!)
On master without this PR, thousands of lines of error messages are logged (Error accepting: Cannot decode Base64: "STREAM STATUS RESULT=I2P_ERROR") and bitcoind I2P session re-creation fails over and over, until the I2P router is manually stopped and restarted.
With this pull, the error is logged only once and there is no need to manually shut down and restart the I2P Router. A new persistent I2P session is created on the third try (perhaps a bit more sleep would create a session on the first attempt with less logging).
Laptop auto-sleep for low battery, auto-awakened 2 1/2 hours later on plugging in the PSU:
2023-07-15T15:54:49.493122Z [msghand] [validation.cpp:2604] [UpdateTipLog] UpdateTip: new best=00000000000000000003b9633a1e0e626f80627f5c624fc13fdd48f04f599a24 height=798807 version=0x20018000 log2_work=94.301772 tx=865041663 date='2023-07-15T15:54:44Z' progress=1.000000 cache=149.1MiB(1172872txo)
2023-07-15T18:30:00.022440Z [scheduler] [net_processing.cpp:5212] [CheckForStaleTipAndEvictPeers] Potential stale tip detected, will try using extra outbound peer (last tip update: 9311 seconds ago)
2023-07-15T18:30:00.111461Z [scheduler] [policy/fees.cpp:929] [FlushFeeEstimates] Flushed fee estimates to fee_estimates.dat.
2023-07-15T18:30:00.386387Z [i2paccept] [i2p.cpp:290] [Log] [i2p] Error accepting: Cannot decode Base64: "STREAM STATUS RESULT=I2P_ERROR"
2023-07-15T18:30:01.394270Z [i2paccept] [i2p.cpp:290] [Log] [i2p] Error accepting (will close the session): unexpected reply that hints the session is unusable: STREAM STATUS RESULT=I2P_ERROR MESSAGE="Session was closed"
2023-07-15T18:30:01.394352Z [i2paccept] [i2p.cpp:290] [Log] [i2p] Destroying SAM session 7f0b27f863
2023-07-15T18:30:03.395684Z [i2paccept] [i2p.cpp:290] [Log] [i2p] Creating persistent SAM session 186297974f with 127.0.0.1:7656
2023-07-15T18:30:03.407802Z [i2paccept] [i2p.cpp:290] [Log] [i2p] Error listening: Unexpected reply to "SESSION CREATE STYLE=STREAM ID=186297974f DESTINATION=...": "SESSION STATUS RESULT=I2P_ERROR MESSAGE="Error creating I2PSocketManager: Router client manager is shut down""
2023-07-15T18:30:03.407892Z [i2paccept] [i2p.cpp:290] [Log] [i2p] Control socket error: not connected
2023-07-15T18:30:03.407969Z [i2paccept] [net.cpp:321] [RemoveLocal] RemoveLocal(abc...xyz.b32.i2p:0)
2023-07-15T18:30:06.414223Z [i2paccept] [i2p.cpp:290] [Log] [i2p] Creating persistent SAM session 9e390f52cc with 127.0.0.1:7656
2023-07-15T18:30:06.444280Z [i2paccept] [i2p.cpp:290] [Log] [i2p] Error listening: Unexpected reply to "SESSION CREATE STYLE=STREAM ID=9e390f52cc DESTINATION=...": "SESSION STATUS RESULT=I2P_ERROR MESSAGE="Error creating I2PSocketManager: Router client manager is shut down""
2023-07-15T18:30:06.445189Z [i2paccept] [i2p.cpp:290] [Log] [i2p] Control socket error: not connected
2023-07-15T18:30:09.815885Z [i2paccept] [i2p.cpp:290] [Log] [i2p] Creating persistent SAM session d8a557f871 with 127.0.0.1:7656
...
2023-07-15T18:30:47.694733Z [i2paccept] [i2p.cpp:290] [Log] [i2p] Persistent SAM session d8a557f871 created, my address=abc...xyz.b32.i2p:0
2023-07-15T18:30:47.696563Z [i2paccept] [net.cpp:298] [AddLocal] AddLocal(abc...xyz.b32.i2p:0,4)
<img width="412" alt="Screenshot 2023-07-15 at 12 33 46" src="https://github.com/bitcoin/bitcoin/assets/2415484/f91b4058-1e37-4a6e-a81c-44add2d59662">
Will test with the i2pd router as well.
jonatack
commented at 4:20 PM on July 16, 2023:
member
Verified that the issue doesn't occur with the i2pd router, only with the Java I2P router.
$ i2pd --version
i2pd version 2.48.0 (0.9.59)
Boost version 1.82.0
OpenSSL 3.1.1 30 May 2023
jonatack approved
jonatack
commented at 9:47 PM on July 17, 2023:
member
ACKffa90fceae9b0c0ca2e720fae9e54dd3d094c988
Tested behavior before/after this pull using both the I2P Java router and with i2pd.
fanquake
commented at 9:02 AM on July 18, 2023:
member
The I2P router may be in such a state
Accept() fails because the I2P router sends something that is not Base64 on the socket:
It's not clear to me if this an issue in our code, or an issue with the router (or both), especially if everything "works fine" with one router, but not the other. Have these issues been reported upstream?
vasild
commented at 10:47 AM on July 18, 2023:
contributor
@fanquake, There is an issue with the I2P router - there is a discrepancy between the router behavior and the SAM specification. Then, this unexpected behavior causes Bitcoin Core to log way too many messages. It becomes a problem with Bitcoin Core too. I should report this upstream as per https://geti2p.net/en/faq#bug.
fanquake
commented at 10:24 AM on July 19, 2023:
member
Great. Can you link to the issue once it's reported upstream, as it'd be good to know which (if any) of the changes here are required just to work around buggy router software.
jonatack
commented at 12:04 PM on July 19, 2023:
member
this unexpected behavior causes Bitcoin Core to log way too many messages.
The log flooding rate is very high, but perhaps the more severe issue fixed here is that the I2P connections remain down until the router is manually stopped and then restarted, until which time the log flooding continues, and for anyone running onlynet=i2p, it would mean their node is effectively offline until they notice it and take action.
With this fix, the connections recover on their own, like they do for the other networks. Even if the router is eventually patched upstream, older versions are being used. So, it seems good to fix this and consider backporting.
jonatack
commented at 12:30 PM on July 19, 2023:
member
jonatack
commented at 3:59 PM on July 26, 2023:
member
geti2p.net/en/download has just updated their available macOS version from 1.9 to 2.3 -- will retest.
FWIW, I didn't manage to make the v2.3 jar work and had to reinstall v1.9 (available as a dmg) at https://geti2p.net/en/download/mac. Per that page: "Important Note: The 2.1.0 Mac OSX Easy Install Bundle release is delayed. Please install the 1.9.0 release below. You will be notified in the router console when the 2.1.0 update is available. Thank you for your patience."
fanquake
commented at 4:09 PM on July 26, 2023:
member
FWIW, I didn't manage to make the v2.3 jar work and had to reinstall v1.9 (available as a dmg) at
1.9.0 has known security issues and is not suitable for hosting services or any long-term use. Users are advised to migrate away as soon as possible. Advanced users of the Easy-Install bundle may work around this by compiling the bundle from source and self-signing the software.
The I2P Easy-Install Bundle for Mac has been experiencing stalled updates for the past 2 releases due to the departure of its maintainer.
jonatack
commented at 4:33 PM on July 26, 2023:
member
Yes. I find the lighter-weight i2pd router in C++ to be an easier option for node operators to install: apt install i2pd / brew install i2pd, etc. That router did experience issues with a floodfills attack in late April to mid-May, but otherwise has been reliable for me over the past couple of years. It's also the router used by Raspiblitz and Umbrel. It's good to have a choice, though.
jonatack
commented at 8:53 PM on September 13, 2023:
member
[@zzz](https://i2pgit.org/zzz) · [1 week ago](https://i2pgit.org/i2p-hackers/i2p.i2p/-/issues/399#note_8367)
Maintainer
OP do you have a workaround implemented?
[@idk](https://i2pgit.org/idk) do you have a stack trace from your reproduction?
[@idk](https://i2pgit.org/idk) no, key data is not retained and once an I2CP session is closed it cannot be reopened, so no "resend" is possible, you can't "get the socket back".
NAMING is supported without a current session, so that's not a valid indication that session is alive
SILENT=true is not the default and probably unused by anybody, let's not waste a lot of time worrying about that case
at first glance this appears to have been the java SAM server behavior since at least 2009, so in practice, documenting the current behavior (and OP implementing a workaround if he hasn't already) seems like the best first step, with any other changes to follow later.
getting a message to - and closing - the control socket would be step two
[Vasil Dimov](https://i2pgit.org/vd)
[@vd](https://i2pgit.org/vd) · [1 week ago](https://i2pgit.org/i2p-hackers/i2p.i2p/-/issues/399#note_8373)
Author
OP do you have a workaround implemented?
It is implemented but not merged yet: [#28077](/bitcoin-bitcoin/28077/)
The I2P Easy-Install Bundle for Mac has been experiencing stalled updates for the past 2 releases due to the departure of its maintainer.
At this time, https://geti2p.net/en/download/mac stills provides a dmg only for v1.9.0 and the page hasn't been updated for some time ("Important Note: The 2.1.0 Mac OSX Easy Install Bundle release is delayed. Please install the 1.9.0 release below."). Thus, macOS users like the three that reported the issue in #22759 are likely to continue encountering it. Discussed this privately with a member of the I2P team a couple weeks back and there are no updates yet on hiring someone to replace the departed maintainer and release a updated macOS dmg. I've just pinged the person to ask if any newer update.
Edit: received a new reply that they don't have a new DMG maintainer at this time.
luke-jr referenced this in commit 9d5afbdc36 on Sep 16, 2023
luke-jr referenced this in commit 26e9107fb0 on Sep 16, 2023
zzzi2p
commented at 1:51 PM on September 21, 2023:
none
Sorry for the delay. I'm working on the I2P ticket you filed, linked above. My goal is to resolve it in a couple weeks.
The current behavior has been like this for quite some time, so in practice, the workarounds in this PR are necessary even if we do get a fix out in our next release.
My strategy is:
Check for closed session before returning the "OK", and returning "STREAM SESSION RESULT=I2P_ERROR MESSAGE="Session was closed"" instead
In the rare case where the session is closed after the "OK" (i.e. while waiting for the next incoming socket) it will act as it does now
For both cases, also send an error on the control socket and close it
Update our spec accordingly
I recommend that you change your test at line 193 to be more robust by doing something like:
so you're not dependent on the message text which is undocumented, could change, or may reflect different error cases in the future.
I hope to have a Java I2P patch for you to test by the end of the month, so you may resolve this in time for your 26.0 release.
Concept ACK
vituperative referenced this in commit 6e3457bb1b on Sep 22, 2023
DrahtBot added the label CI failed on Sep 28, 2023
zzzi2p
commented at 10:27 AM on September 29, 2023:
none
Our fixes are approved and merged. Test results are in our ticket linked above. If you wish to test it, I recommend applying the patch to our 2.3.0 release source, not the current HEAD, as our code base is currently unstable. SAM spec updates will follow soon. I hope you can get this PR merged for 26.0. Thanks again for the report.
DrahtBot removed the label CI failed on Oct 3, 2023
i2p: also sleep after errors in Accept()
Background:
`Listen()` does:
* if the session is not created yet
* create the control socket and on it:
* `HELLO`
* `SESSION CREATE ID=sessid`
* leave the control socked opened
* create a new socket and on it:
* `HELLO`
* `STREAM ACCEPT ID=sessid`
* read reply (`STREAM STATUS`)
Then a wait starts, for a peer to connect. When connected,
`Accept()` does:
* on the socket from `STREAM ACCEPT` from `Listen()`: read the
Base64 identification of the connecting peer
Problem:
The I2P router may be in such a state that this happens in a quick
succession (many times per second, see https://github.com/bitcoin/bitcoin/issues/22759#issuecomment-1609907115):
`Listen()`-succeeds, `Accept()`-fails.
`Accept()` fails because the I2P router sends something that is
not Base64 on the socket:
STREAM STATUS RESULT=I2P_ERROR MESSAGE="Session was closed"
We only sleep after failed `Listen()` because the assumption was that
if `Accept()` fails then the next `Listen()` will also fail.
Solution:
Avoid filling the log with "Error accepting:" messages and sleep also
after a failed `Accept()`.
Extra changes:
* Reset the error waiting time after one successful connection.
Otherwise the timer will remain high due to problems that have
vanished long time ago.
* Increment the wait time less aggressively.
762404a68c
i2p: destroy the session if we get an unexpected error from the I2P router
From https://geti2p.net/en/docs/api/samv3:
If SILENT=false was passed, which is the default value, the SAM bridge
sends the client a ASCII line containing the base64 public destination
key of the requesting peer
So, `Accept()` is supposed to receive a Base64 encoded destination of
the connecting peer, but if it receives something like this instead:
STREAM STATUS RESULT=I2P_ERROR MESSAGE="Session was closed"
then destroy the session.
5c8e15c451
vasild force-pushed on Oct 5, 2023
vasild
commented at 12:24 PM on October 5, 2023:
contributor
ffa90fceae...5c8e15c451: rebase and address a suggestion by @zzzi2p - consider the session bricked if we receive a message that contains RESULT=I2P_ERROR instead of the incoming peer's base64 destination (address). Thanks!
vasild renamed this: I2P: also sleep after errors in Accept() & destroy the session if we get "Session was closed" I2P: also sleep after errors in Accept() & destroy the session if we get an unexpected error on Oct 5, 2023
achow101 added this to the milestone 26.0 on Oct 12, 2023
jonatack
commented at 4:09 PM on October 12, 2023:
member
re-ACK5c8e15c451ec870b9dd4eb843ec6ca3ba64cda4f
The only code change since my last review was this line:
- if (ToLower(peer_dest).find("session was closed") != std::string::npos) {
+ if (peer_dest.find("RESULT=I2P_ERROR") != std::string::npos) {
On master without this PR, thousands of lines of error messages are logged (Error accepting: Cannot decode Base64: "STREAM STATUS RESULT=I2P_ERROR") and bitcoind I2P session re-creation fails over and over, until the I2P router is manually stopped and restarted.
With this pull, the error is logged only once and there is no need to manually shut down and restart the I2P Router. A new persistent I2P session is created on the third try (perhaps a bit more sleep would create a session on the first attempt with less logging).
It would be good to include this in v26.0. Per @zzzi2p above:
The current behavior has been like this for quite some time, so in practice, the workarounds in this PR are necessary even if we do get a fix out in our next release.
luke-jr referenced this in commit 3e316169e2 on Oct 19, 2023
luke-jr referenced this in commit 4018d9ae69 on Oct 19, 2023
achow101
commented at 8:01 PM on October 19, 2023:
member
ACK5c8e15c451ec870b9dd4eb843ec6ca3ba64cda4f
achow101 merged this on Oct 19, 2023
achow101 closed this on Oct 19, 2023
Frank-GER referenced this in commit 2389eef67a on Oct 21, 2023
vasild deleted the branch on Oct 23, 2023
HiHat referenced this in commit cc8d726af8 on Oct 15, 2024
This is a metadata mirror of the GitHub repository
bitcoin/bitcoin.
This site is not affiliated with GitHub.
Content is generated from a GitHub metadata backup.
generated: 2026-04-15 00:13 UTC
This site is hosted by @0xB10C More mirrored repositories can be found on mirror.b10c.me
