Whitelist electrs

[kroese]

Newer versions of Electrs use the P2P protocol instead of the RPC protocol to connect to the local Bitcoin node. But currently the IP of electrs is not whitelisted.

Two of the benefits of doing so are:

• If there are no free slots for incoming connections (because maxconnections has been reached) it will still allow electrs to connect because it will drop an existing peer to make room.
• If the maxuploadtarget is reached it will allow electrs to continue downloading blocks for indexing as the limit is not applied to whitelisted peers.

[nmfretz]

Thanks for this @Kroese. Sorry for the delay in responding, we are just now looking to update the bitcoin app.

We're cautious about hardcoding an individual app's IP in the conf file because we want to avoid additional overhead should we change the IP of an app in the future and then require users to update multiple apps at the same time in order for everything to function correctly. We think a solution could be to whitelist the entire Umbrel IP range instead (whitelist=10.21.0.0/16). This would also solve this problem for other apps that require P2P connection within the Umbrel network.

I have tested the following, but would really appreciate your thoughts on this and any additional testing:

• set a low maxconnections on your node
• connect to max # of peers so all peer slots are taken up
• try connecting in via another clearnet peer --> it should fail
• try connecting in via a Tor peer --> it should fail... this is the primary purpose of the test, since we want to make sure that whitelisting the entire Umbrel internal ip range does not inadvertently whitelist all Tor peers.
• try connecting in via a peer in the Umbrel IP range (e.g., electrs), it should pass since it's whitelisted

FYI, Electrs will fail to connect to a bitcoind instance that has maxconnections set to less than 12 with this error:

[2023-08-25T04:35:04.946Z WARN  electrs::p2p] failed to shutdown p2p connection: Transport endpoint is not connected (os error 107)
[2023-08-25T04:35:04.946Z INFO  electrs::db] closing DB at /data/db/bitcoin
Error: electrs failed

Caused by:
    receiving on an empty and disconnected channel

More info on maxconnections here: https://github.com/bitcoin/bitcoin/blob/master/doc/reduce-memory.md#number-of-peers

We plan to include maxconnections in Advanced Settings, so we will need to properly warn users about the implications of setting maxconnections too low.

[nmfretz]

Here are the results of my testing:

1. Connected node 1 to node 2 over both clearnet and Tor via rpc addnode. This worked.

2. Removed manual connections for the following whitelist test.

3. Node 2: stopped electrs. set maxconnections=12 + whitelist=10.21.0.0/16 and restart bitcoind. Allow connections to fill up.

4. Node 1: connect to node 2 using rpc addnode. Both clearnet and onion connection fail... no new connections seen on node 2 and node 1 shows failed connection state:

   $ sudo docker exec bitcoin_bitcoind_1 bitcoin-cli getaddednodeinfo
   [
   {
   "addednode": "<redacted-ip>",
   "connected": false,
   "addresses": [
   ]
   },
   {
   "addednode": "<redacted-onion-address..onion:8333",
   "connected": false,
   "addresses": [
   ]
   }
   ]

5. Node 2: start electrs back up. connects to bitcoind successfully.

When maxconnections are set too low you will see this error in the elecrs logs:

[2023-08-25T04:35:04.946Z WARN  electrs::p2p] failed to shutdown p2p connection: Transport endpoint is not connected (os error 107)
[2023-08-25T04:35:04.946Z INFO  electrs::db] closing DB at /data/db/bitcoin
Error: electrs failed

Caused by:
    receiving on an empty and disconnected channel

[kroese]

Yes, this is a known issue that you cannot go below 12. I once read an explanation about it, I believe it had something to do with maxconnections referring to the sum of incoming AND outgoing connections, of which 11 are already reserved for outbound.

You could make it less confusing for the user by just adding 11 to the value specified in the UI. So when the user wants 3 incoming connections, you can set it to 14 (11 + 3) internally.

And yes, hardcoding the IP address of Electrum was a quick fix, so I totally agree that whitelisting the entire range is a much nicer solution.

[nmfretz]

Thanks again for this @kroese! This will show up in the next Bitcoin app update (soon).

You could make it less confusing for the user by just adding 11 to the value specified in the UI. So when the user wants 3 incoming connections, you can set it to 14 (11 + 3) internally.

We will add some form of warning or check so that users are aware of the implications.

[kroese]

@nmfretz In the new release you did not implement any check to make sure maxconnections is set to at least 12 when its not 0.

It was even better if you just added 11 to the value specified, so if the user specified 3 that it would set maxconnections=14, that way it becomes much more logical to the end-user.

But since you did not implement that, it should at least validate the value to be 0 or > 11.

[nmfretz]

ah, you are right. We only have logic such that bitcoind will not error out. Thanks for catching this and for the reminder. We will include logic (and helpful description) in the next update. Also please feel free to open a PR.