💬 luke-jr commented on pull request "Add option dbfilesize to control LevelDB target ("max") file size":
(https://github.com/bitcoin/bitcoin/pull/30059#discussion_r1597483759)
LevelDB itself enforces it. Would it make sense to check it redundantly on our end to control error behaviour? (But then we have to remember to keep it in sync with LevelDB... but maybe just having the docs necessitates that)

💬 hebasto commented on pull request "kernel: Streamline util library":
(https://github.com/bitcoin/bitcoin/pull/29015#issuecomment-2105975730)
> I also added a [script](https://github.com/ryanofsky/bitcoin/blob/pr/rmutil/test/util/check-deps.sh) which turned up a few other other unexpected dependencies (not related to util), which could be addressed separately.

It would be great to have such a tool at our disposal.

👍 hebasto approved a pull request: "debugwindow: update session ID tooltip"
(https://github.com/bitcoin-core/gui/pull/788#pullrequestreview-2051240925)
ACK 3bf00e13609eefa6ddb11353519bb1aec2342513.

🚀 hebasto merged a pull request: "debugwindow: update session ID tooltip"
(https://github.com/bitcoin-core/gui/pull/788)

👍 hebasto approved a pull request: "Don't permit port in proxy IP option"
(https://github.com/bitcoin-core/gui/pull/813#pullrequestreview-2051243691)
ACK 10c5275ba4532fb1bf54057d2f61fc35b51f1e85, tested on Ubuntu 24.04.

✅ hebasto closed an issue: "gui: node shutting down after incorrect proxy IP address input"
(https://github.com/bitcoin-core/gui/issues/809)

🚀 hebasto merged a pull request: "Don't permit port in proxy IP option"
(https://github.com/bitcoin-core/gui/pull/813)

💬 Ayush170-Future commented on pull request "fuzz: wallet, add target for `Crypter`":
(https://github.com/bitcoin/bitcoin/pull/28074#issuecomment-2105985842)
A soft ping. Can we move forward with this PR if this looks good?

💬 jonatack commented on pull request "doc: removed help text saying that peers may not connect automatically":
(https://github.com/bitcoin/bitcoin/pull/29994#issuecomment-2105995375)
@vasild interesting 👍

here on a non-clearnet node (with addnode to a colleague via ipv4)

```
$ ./src/bitcoin-cli -addrinfo
{
"addresses_known": {
"ipv4": 1,
"ipv6": 0,
"onion": 17487,
"i2p": 2297,
"cjdns": 4,
"total": 19789
}
}

$ ./src/bitcoin-cli getnodeaddresses 0 |jq 'map(select(.port != 8333)) |map(select(.network != "i2p")) |length'
16

$ ./src/bitcoin-cli getnodeaddresses 0 |jq 'map(select(.port != 8333)) |map(select(.network != "i2p"))
...

💬 paplorinc commented on pull request "refactor: Model the bech32 charlimit as an Enum":
(https://github.com/bitcoin/bitcoin/pull/30047#discussion_r1597498451)
Awesome

💬 TheCharlatan commented on pull request "blockstorage: XOR blocksdir *.dat files":
(https://github.com/bitcoin/bitcoin/pull/28052#discussion_r1597496149)
Nit:
This prints as:
```
Using xor key for *.dat files: '0000000000000000'`
```
But the log lines informing the user on data obfuscation in the other DBs use:
```
Using obfuscation key for /home/drgrid/.bitcoin/blocks/index: 0000000000000000
Using obfuscation key for /home/drgrid/.bitcoin/chainstate: a3b154cf1d9714d7
```
I think it would be nice to be consistent here, both by listing the full directory and using the same text.

👍 TheCharlatan approved a pull request: "blockstorage: XOR blocksdir *.dat files"
(https://github.com/bitcoin/bitcoin/pull/28052#pullrequestreview-2051219287)
ACK faaf3cf535999c2c9a84337414c8b35435384862

💬 TheCharlatan commented on pull request "blockstorage: XOR blocksdir *.dat files":
(https://github.com/bitcoin/bitcoin/pull/28052#discussion_r1597462109)
Just a comment: This test implicitly checks if the blockfiles are XORed. Would a simple additional check on the undo data make sense?

💬 TheCharlatan commented on pull request "blockstorage: XOR blocksdir *.dat files":
(https://github.com/bitcoin/bitcoin/pull/28052#discussion_r1597499049)
Nit: I found this a bit hard to follow and a bit unfortunate that a new random key would be generated every time whether it were used or not. How about:
```diff
@@ -1166,4 +1165,0 @@ static auto InitBlocksdirXorKey(const BlockManager::Options& opts)
- // Initialize random fresh key, ...
- FastRandomContext{}.fillrand(xor_key);
- // ... but allow legacy or user-disabled key.
- if (!fs::is_empty(opts.blocks_dir) || opts.xor_key == false) std::fill(xor_key.begin(), xor_key.end(),
...

💬 paplorinc commented on pull request "test: Add a few more corner cases to the base58 test suite":
(https://github.com/bitcoin/bitcoin/pull/30035#discussion_r1597500420)
I don't feel strongly about either, removed the space

💬 paplorinc commented on pull request "refactor: Model the bech32 charlimit as an Enum":
(https://github.com/bitcoin/bitcoin/pull/30047#discussion_r1597502072)
Awesome!
I just noticed my email was wrong there, would you be so kind and change it to `Lőrinc <pap.lorinc@gmail.com>`?

💬 paplorinc commented on pull request "test: Add a few more corner cases to the base58 test suite":
(https://github.com/bitcoin/bitcoin/pull/30035#discussion_r1597504325)
Didn't realize this, thanks!
```
test/base58_tests.cpp:40: info: check '["271F359E","zzzzy"]: got "zzzzy"' has passed
```

💬 paplorinc commented on pull request "test: Add a few more corner cases to the base58 test suite":
(https://github.com/bitcoin/bitcoin/pull/30035#discussion_r1597504354)
```
check '["271F35A1","211112"]: got "271f35a1"' has passed
```

👍 TheCharlatan approved a pull request: "blockstorage: Separate reindexing from saving new blocks"
(https://github.com/bitcoin/bitcoin/pull/29975#pullrequestreview-2051255096)
ACK 9cf475ffffb869cd55c2b2f3be84d7c90b199521

💬 TheCharlatan commented on pull request "blockstorage: Separate reindexing from saving new blocks":
(https://github.com/bitcoin/bitcoin/pull/29975#discussion_r1597503981)
Nit: s/the also/also the/