💬 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)
```

💬 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_r1597506189)
Done, thanks!

💬 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_r1597506230)
Added short explanations to the commit message

💬 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_r1597506366)
Thanks, done

💬 jonatack commented on pull request "contrib: lief updates":
(https://github.com/bitcoin/bitcoin/pull/30084#issuecomment-2106039274)
> Guix build, given it's still using 0.13.2?

Thanks for pointing out this interdependency. Looking at https://github.com/bitcoin/bitcoin/pull/27813 and related preceding changes, would you be concept ACK on updating the guix manifest to a recent lief (0.14 or maybe imminent 0.15), or prefer to hold off for now?

🤔 gatleas17 reviewed a pull request: "debugwindow: update session ID tooltip"
(https://github.com/bitcoin-core/gui/pull/788#pullrequestreview-2051276341)
Help!

💬 hebasto commented on pull request "[27.x] Backports":
(https://github.com/bitcoin/bitcoin/pull/29888#issuecomment-2106108253)
I suggest to add https://github.com/bitcoin-core/gui/pull/813.

💬 rkrux commented on pull request "test: Assumeutxo: import snapshot in a node with a divergent chain":
(https://github.com/bitcoin/bitcoin/pull/29996#discussion_r1597568656)
Yes, that would be good.

💬 rkrux commented on pull request "test: fix MiniWallet script-path spend (missing parity bit in leaf version)":
(https://github.com/bitcoin/bitcoin/pull/30076#discussion_r1597569771)
Yes, this is also good.

💬 rkrux commented on pull request "test: fix MiniWallet script-path spend (missing parity bit in leaf version)":
(https://github.com/bitcoin/bitcoin/pull/30076#discussion_r1597570020)
Great, thanks for sharing, I'm digging into this!

💬 levantah commented on pull request "Testnet4 including PoW difficulty adjustment fix":
(https://github.com/bitcoin/bitcoin/pull/29775#issuecomment-2106151512)
Tested ACK, it works my current tip is

```
2024-05-12T07:22:24Z UpdateTip: new best=00000000000000165fe9709ac837645716d480a230a61ca5745530f6692a235d height=24361 version=0x2e5e2000 log2_work=65.708890 tx=94453 date='2024-05-12T07:22:17Z' progress=1.000000 cache=3.3MiB(27967txo)
```

But, the `bitcoin-cli` in 06c2c713c52b60231efc3e00d2c5eb0bf9e345f9 does not know anything about `testnet4` (different RPC port) and I would personally prefer also `-chain=test4` additional flag (both for `bitc
...