💬 maflcko commented on pull request "test: Fix intermittent failure in p2p_v2_misbehaving.py":
(https://github.com/bitcoin/bitcoin/pull/30420#discussion_r1680462164)
`3` is the peer timeout, so you'll have to change it to bump `2` here. Otherwise, the test will fail:


https://cirrus-ci.com/task/5359619151757312?logs=ci#L3935


```
test 2024-07-17T05:25:54.632000Z TestFramework.p2p (DEBUG): Connected & Listening: 127.0.0.1:15680
node0 2024-07-17T05:25:54.634738Z (mocktime: 2024-07-17T05:25:54Z) [net] [net.cpp:3764] [CNode] [net] Added connection peer=0
node0 2024-07-17T05:25:54.634776Z (mocktime: 2024-07-17T05:25:54Z) [net] [net.cpp:1814] [Cre
...

💬 maflcko commented on pull request "test: Fix intermittent failure in p2p_v2_misbehaving.py":
(https://github.com/bitcoin/bitcoin/pull/30420#discussion_r1680464165)
Also, you'll have to move it up by one line. Otherwise it is racy, because sometimes it will be bumped when the message was sent, and sometimes before the raw message was sent.

💬 meglio commented on issue "gettransaction details does not include send to myself balance changes for imported addresses":
(https://github.com/bitcoin/bitcoin/issues/28555#issuecomment-2232511784)
In my case, `gettransaction()` returns zero for the top-level "amount" key sometimes:

```json
{
"result": {
"amount": 0.00000000,
...
```

However, the "details" have non-zero amounts in them.

Can it be related?

💬 maflcko commented on pull request "blockstorage: XOR blocksdir *.dat files":
(https://github.com/bitcoin/bitcoin/pull/28052#discussion_r1680480182)
Thanks, but actually it doesn't work when running on Windows. Added a new commit with a workaround.

💬 maflcko commented on pull request "blockstorage: XOR blocksdir *.dat files":
(https://github.com/bitcoin/bitcoin/pull/28052#discussion_r1680480364)
Thanks, taken!

💬 maflcko commented on pull request "blockstorage: XOR blocksdir *.dat files":
(https://github.com/bitcoin/bitcoin/pull/28052#discussion_r1680480517)
Thanks, taken!

💬 Sjors commented on pull request "Stratum v2 connman":
(https://github.com/bitcoin/bitcoin/pull/30332#discussion_r1680520000)
I should probably also disable the copy-constructor on `Sv2Client`?

📝 maflcko opened a pull request: "refactor: Make m_last_notified_header private"
(https://github.com/bitcoin/bitcoin/pull/30466)
Seems brittle to expose mutable fields public.

Fix it by making it private.

Fixes https://github.com/bitcoin/bitcoin/pull/30425#discussion_r1677633601

💬 Sjors commented on pull request "Stratum v2 connman":
(https://github.com/bitcoin/bitcoin/pull/30332#issuecomment-2232606942)
Now using mock sockets #30205, thanks @vasild!

💬 Sjors commented on pull request "test: add mocked Sock that can read/write custom data and/or CNetMessages":
(https://github.com/bitcoin/bitcoin/pull/30205#issuecomment-2232609396)
Works like a charm! I'll see if I can add them to the tests in #30332 as well.

💬 hodlinator commented on pull request "blockstorage: XOR blocksdir *.dat files":
(https://github.com/bitcoin/bitcoin/pull/28052#discussion_r1680559019)
Good to see the removal of ` == true` as well in `InitBlocksdirXorKey()`. :+1:

💬 maflcko commented on pull request "qa: Functional test improvements":
(https://github.com/bitcoin/bitcoin/pull/30463#issuecomment-2232618421)
> No behaviour changes for Autotools builds.

I don't think this is true. This is a bugfix.

Previously one could not run the tests in the autotools out-of-source build dir:

```
$ ./test/functional/wallet_disable.py
bash: ./test/functional/wallet_disable.py: No such file or directory
$ ln --symbolic ../../../test/functional/wallet_disable.py ./test/functional/
$ ./test/functional/wallet_disable.py
Traceback (most recent call last):
File "./test/functional/wallet_disable.py", lin
...

💬 maflcko commented on pull request "qa: Functional test improvements":
(https://github.com/bitcoin/bitcoin/pull/30463#issuecomment-2232621185)
Concept ACK on the behavior changes and the bugfix, but please properly describe the changes.

👍 TheCharlatan approved a pull request: "refactor: Make m_last_notified_header private"
(https://github.com/bitcoin/bitcoin/pull/30466#pullrequestreview-2182133067)
lgtm fa927055dd43dda945396574273a210186beec9f

💬 hebasto commented on pull request "depends: Set `CMAKE_SYSTEM_VERSION` for CMake builds":
(https://github.com/bitcoin/bitcoin/pull/30465#issuecomment-2232644321)
My Guix build:
```
x86_64
460732347ffc71500f84d1174f6a6011b887681e82fc1704c1187cb1701bf9af guix-build-5ff8074361e9/output/aarch64-linux-gnu/SHA256SUMS.part
b084b5371e3acff1850c8f3234299f99c3635fd497919f28407acaee27322877 guix-build-5ff8074361e9/output/aarch64-linux-gnu/bitcoin-5ff8074361e9-aarch64-linux-gnu-debug.tar.gz
9f0f40cb3510197338efca2c7423fb7615a34a8b21a90c500c7fa45898875a9d guix-build-5ff8074361e9/output/aarch64-linux-gnu/bitcoin-5ff8074361e9-aarch64-linux-gnu.tar.gz
b571a25f1
...

💬 hebasto commented on pull request "qa: Functional test improvements":
(https://github.com/bitcoin/bitcoin/pull/30463#issuecomment-2232654759)
@maflcko
> Previously one could not run the tests in the autotools out-of-source build dir:
>
> After this fix, it is possible.

That's true. But I did not consider this case as it requires a manual creation of a symlink to a test, which Autottols do not do, but CMake does.

💬 maflcko commented on pull request "qa: Functional test improvements":
(https://github.com/bitcoin/bitcoin/pull/30463#issuecomment-2232664350)
In any case, creating the symlinks seems like a good way to test this, no?

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

💬 hodlinator commented on pull request "blockstorage: XOR blocksdir *.dat files":
(https://github.com/bitcoin/bitcoin/pull/28052#discussion_r1680561263)
nit: Could put cheapest condition first:
```suggestion
if (opts.use_xor && fs::is_empty(opts.blocks_dir)) {
```

💬 hodlinator commented on pull request "blockstorage: XOR blocksdir *.dat files":
(https://github.com/bitcoin/bitcoin/pull/28052#discussion_r1680615465)
Smaller change if we make `util_xor()` take an offset:
```diff
diff --git a/test/functional/feature_reindex.py b/test/functional/feature_reindex.py
index f079a5e343..9f3f5357ba 100755
--- a/test/functional/feature_reindex.py
+++ b/test/functional/feature_reindex.py
@@ -43,15 +43,15 @@ class ReindexTest(BitcoinTestFramework):
NUM_XOR_BYTES = 8 # From InitBlocksdirXorKey::xor_key.size()
xor_dat = xor_f.read(NUM_XOR_BYTES)

- def util_xor(data, key):
+
...