💬 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/

💬 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_r1597506104)
Indeed, added the values to the error message:
```
test/base58_tests.cpp:102: error: in "base58_tests/base58_random_encode_decode_with_optional_spaces": Decoding should fail for `invalidSmallResultLength` (61)
test/base58_tests.cpp:104: error: in "base58_tests/base58_random_encode_decode_with_optional_spaces": Decoding should succeed within sufficiently large result length (134)
```