📝 maflcko opened a pull request: " refactor: Make node_id a const& in RemoveBlockRequest "
(https://github.com/bitcoin/bitcoin/pull/31282)
Currently, `valgrind` is not usable on a default build with GCC. Specifically, `p2p_compactblocks.py --valgrind` gives a false-positive in `RemoveBlockRequest` when comparing `node_id` with `from_peer`. According to the upstream bug report, this happens because both symbols are on the stack and the compiler can more aggressively optimize the compare (order). See https://bugs.kde.org/show_bug.cgi?id=472329#c7

It is possible to work around this bug by pulling at least one value from the stack.
...

🚀 fanquake merged a pull request: "doc: mention `descriptorprocesspsbt` in psbt.md"
(https://github.com/bitcoin/bitcoin/pull/31277)

🤔 Sjors reviewed a pull request: " refactor: Make node_id a const& in RemoveBlockRequest "
(https://github.com/bitcoin/bitcoin/pull/31282#pullrequestreview-2432510141)
Concept ACK

Only studied the main one line change.

💬 Sjors commented on pull request " refactor: Make node_id a const& in RemoveBlockRequest ":
(https://github.com/bitcoin/bitcoin/pull/31282#discussion_r1839882117)
fa91206b3c570dc17ee18565ade04067a88e4ef8: this seems fine to me. The code is nicer and happens to fix a false positive.

💬 fanquake commented on pull request "guix: scope pkg-config to Linux only":
(https://github.com/bitcoin/bitcoin/pull/31276#issuecomment-2473092574)
Guix build (aarch64):
```bash
02cdb4ec345e0e4b2f3876dded48a468c864fe58515ae7e4d7f9a8c1c4911337 guix-build-7edc54b086ac/output/aarch64-linux-gnu/SHA256SUMS.part
83d376375b1d5309108acc9e02711138c826d38fca0a73e7cc6048b21f3c4ab1 guix-build-7edc54b086ac/output/aarch64-linux-gnu/bitcoin-7edc54b086ac-aarch64-linux-gnu-debug.tar.gz
25677323242badfd9be8c5fdbc71ecf9b663239842deb244bbca4bc90a311d4d guix-build-7edc54b086ac/output/aarch64-linux-gnu/bitcoin-7edc54b086ac-aarch64-linux-gnu.tar.gz
de94c7
...

💬 fanquake commented on pull request "guix: scope pkg-config to Linux only":
(https://github.com/bitcoin/bitcoin/pull/31276#issuecomment-2473152977)
Dropped another patch and pruned a Qt libtool archive.

💬 fjahr commented on pull request "test: Introduce ensure_for helper":
(https://github.com/bitcoin/bitcoin/pull/30893#discussion_r1840013926)
reverted it

💬 fjahr commented on pull request "test: Introduce ensure_for helper":
(https://github.com/bitcoin/bitcoin/pull/30893#discussion_r1840014484)
done

💬 fjahr commented on pull request "test: Introduce ensure_for helper":
(https://github.com/bitcoin/bitcoin/pull/30893#discussion_r1840014778)
done

💬 fjahr commented on pull request "test: Introduce ensure_for helper":
(https://github.com/bitcoin/bitcoin/pull/30893#issuecomment-2473228711)
Addressed latest feedback from @maflcko

👍 vasild approved a pull request: "ci: skip Github CI on branch pushes for forks"
(https://github.com/bitcoin/bitcoin/pull/30487#pullrequestreview-2432802018)
ACK 8610bcef9d030013f9e36cffe0c58dd2cfe85d66

👍 vasild approved a pull request: "addrman: cap the `max_pct` to not exceed the maximum number of addresses"
(https://github.com/bitcoin/bitcoin/pull/31235#pullrequestreview-2432819520)
ACK 9c5775c331e02dab06c78ecbb58488542d16dda7

💬 polespinasa commented on pull request "rpc: print P2WSH witScript in getrawtransaction":
(https://github.com/bitcoin/bitcoin/pull/31252#discussion_r1840096537)
I agree, it needs to be changed in one more place apart from this line but yes I will apply it.
Thanks!

💬 naumenkogs commented on pull request "Package validation: accept packages of size 1":
(https://github.com/bitcoin/bitcoin/pull/31096#discussion_r1840103944)
Expanding on this thought, I sketched a two-step improvement in handling single-package tx wrt code readability. Wanna [take a look](https://github.com/naumenkogs/bitcoin/commits/2024-10-submitpackage-singleton-v1/)?

I certainly might be missing something.

👍 rkrux approved a pull request: "test: Rework wallet_migration.py to use previous releases"
(https://github.com/bitcoin/bitcoin/pull/31248#pullrequestreview-2432399968)
tACK a76ad56a80d9c9a60352bb98b363131e359a383b

Certainly useful because it reflects how the end users would do wallet migration after the upcoming release.

Make is successful but around 15 functional tests are failing in my system that seem unrelated to this change because they are failing in master as well. All of the failures are due to timeouts, probably an issue with my setup.

Asked few questions and left suggestions. Found few really good tests in this file in the end!

💬 rkrux commented on pull request "test: Rework wallet_migration.py to use previous releases":
(https://github.com/bitcoin/bitcoin/pull/31248#discussion_r1839858333)
Should this not be called `create_legacy_wallet_and_get_rpc` now highlighting that it returns the wallet RPC from the old node?

💬 rkrux commented on pull request "test: Rework wallet_migration.py to use previous releases":
(https://github.com/bitcoin/bitcoin/pull/31248#discussion_r1839822834)
Not in the diff but still

```diff
- # Now migrate and test that we still see have the same balance/transactions
+ # Now migrate and test that we still have the same balance/transactions
```

💬 rkrux commented on pull request "test: Rework wallet_migration.py to use previous releases":
(https://github.com/bitcoin/bitcoin/pull/31248#discussion_r1839812117)
Nit
```
# restart master node and verify that everything is still there
```

💬 rkrux commented on pull request "test: Rework wallet_migration.py to use previous releases":
(https://github.com/bitcoin/bitcoin/pull/31248#discussion_r1839933824)
This `notloaded` wallet migration testing is removed because it is effectively tested in `test_unloaded_by_path` below?

💬 rkrux commented on pull request "test: Rework wallet_migration.py to use previous releases":
(https://github.com/bitcoin/bitcoin/pull/31248#discussion_r1839954334)
+1 for adding this. Is the reason for the absence of this _unsetting_ here not causing any issue earlier when `wallet.setmocktime(curr_time)` was added that no subsequent test in this file below relies on mock time?