💬 l0rinc commented on pull request "ci: Update Clang in "tidy" job":
(https://github.com/bitcoin/bitcoin/pull/31306#discussion_r1860914367)
I'm not sure I understand the reason for the suppression, please see my original suggestion in https://github.com/bitcoin/bitcoin/pull/31306#discussion_r1846343700

💬 furszy commented on pull request "wallet: fix crash during migration due to invalid multisig descriptors":
(https://github.com/bitcoin/bitcoin/pull/31378#issuecomment-2504309436)
> Ruling out consensus-invalid scripts makes sense, but discarding non-standard scripts is a much bigger leap. Also it would be a wack-a-mole: does the migration crash on any non-standard script? How much of that do you want to cover?

Ok, I might have made the PR/commit description too broad when the actual changes are very specific.
Migration does not crash on all non-standard scripts, it only crashes on scripts that fail to be parsed by the descriptors' logic. The specific non-standard cra
...

🤔 mzumsande reviewed a pull request: "rpc: Remove submitblock pre-checks"
(https://github.com/bitcoin/bitcoin/pull/31175#pullrequestreview-2465507312)
ACK 73db95c65c1d372822166045ca8b9f173d5fd883
Reviewed the code (didn't look closely at `bitcoin-chainstate.cpp`) and tested it a little bit.

💬 mzumsande commented on pull request "rpc: Remove submitblock pre-checks":
(https://github.com/bitcoin/bitcoin/pull/31175#discussion_r1860960957)
nit: If, after this PR, we re-submit genesis via `submitblock`, a different code path will be taken than for non-genesis duplicate blocks:
We won't return early in `AcceptBlockHeader`, will actually call `AddToBlockIndex()`, and write an incorrect log message
`Saw new header hash=0f9188f13cb7b2c71f2a335e3a4fc328bf5beb436012afca590b1a11466e2206 height=0` for each `submitblock` call.
Not a real problem though because nothing fails and why would you do that in the first place.

💬 ryanofsky commented on pull request "multiprocess: add bitcoin-mine test program":
(https://github.com/bitcoin/bitcoin/pull/30437#issuecomment-2504369867)
Updated 1f18eefe1e5bc3a97f5f6c9637a3a193e2c2f22e -> 27072547bb22cdd2080d1014eba9c30bc9d47650 ([`pr/mine.13`](https://github.com/ryanofsky/bitcoin/commits/pr/mine.13) -> [`pr/mine.14`](https://github.com/ryanofsky/bitcoin/commits/pr/mine.14), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/mine.13..pr/mine.14)) to fix CI errors from `skip_if_no_multiprocess` check being broken and from default testdir path exceeding maximum socket length

💬 achow101 commented on pull request "rpc: add getdescriptoractivity":
(https://github.com/bitcoin/bitcoin/pull/30708#issuecomment-2504381025)
ACK 37a5c5d83664c31d83fc649d3c8c858bd5f10f21

💬 rkrux commented on pull request "rpc: add getdescriptoractivity":
(https://github.com/bitcoin/bitcoin/pull/30708#issuecomment-2504393638)
re-ACK 37a5c5d83664c31d83fc649d3c8c858bd5f10f21

💬 ryanofsky commented on pull request "multiprocess: Add bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/31375#issuecomment-2504400845)
Updated 19ae652376faca65d972c12cb51cfc8af0560c9e -> 63df9f3deb35be79496f7c240e3303e1d96c6832 ([`pr/wrap.3`](https://github.com/ryanofsky/bitcoin/commits/pr/wrap.3) -> [`pr/wrap.4`](https://github.com/ryanofsky/bitcoin/commits/pr/wrap.4), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/wrap.3..pr/wrap.4)) with fixes for windows and fuzz CI errors, and lint and tidy fixes

👍 rkrux approved a pull request: "rpc: add getdescriptoractivity"
(https://github.com/bitcoin/bitcoin/pull/30708#pullrequestreview-2465616436)
re-ACK 37a5c5d83664c31d83fc649d3c8c858bd5f10f21

🚀 achow101 merged a pull request: "rpc: add getdescriptoractivity"
(https://github.com/bitcoin/bitcoin/pull/30708)

💬 achow101 commented on pull request "contrib: skip missing binaries in gen-manpages":
(https://github.com/bitcoin/bitcoin/pull/30986#issuecomment-2504433940)
ACK ee6185372fc317d3948690997117e42f6b79a5ff

✅ achow101 closed an issue: "gen-manpages.py should skip nonexistent binaries and still work for the existent binaries"
(https://github.com/bitcoin/bitcoin/issues/30985)

🚀 achow101 merged a pull request: "contrib: skip missing binaries in gen-manpages"
(https://github.com/bitcoin/bitcoin/pull/30986)

💬 vasild commented on pull request "ci: detect outbound internet traffic generated while running tests":
(https://github.com/bitcoin/bitcoin/pull/31349#issuecomment-2504438037)
`8799018bd5...803ed4638b`: include https://github.com/bitcoin/bitcoin/pull/31343 into this PR to demonstrate that #31343 works as intended and also to turn the CI here green.

About the false positives - I think it is worth trying this in its current mode where any detected traffic is assumed to have originated from the tests and fails the CI. If this ever fails the CI for another reason (false positive), then it would be easy to turn this into a "report in the logs only but don't fail" by rem
...

🤔 BrandonOdiwuor reviewed a pull request: "ci: detect outbound internet traffic generated while running tests"
(https://github.com/bitcoin/bitcoin/pull/31349#pullrequestreview-2465656461)
Concept ACK

💬 sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1861056105)
Ohh I see, I'll rename that to `parent_wtxid` for the internal context

💬 sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1861058808)
I'll add a comment with the logic for these edge cases and why in some cases this may happen (and why we do allow it).

💬 furszy commented on pull request "wallet: Remove IsMine from migration code":
(https://github.com/bitcoin/bitcoin/pull/30328#issuecomment-2504519664)
Found a few ways to crash migration while reviewing these PR that also occur in master: #31374, #31378. I think we should focus on those before moving forward with this final `IsMine` killing refactoring. They also include further test coverage that would be nice to have here.

👍 vasild approved a pull request: "cmake: Add `CheckLinkerSupportsPIE` module"
(https://github.com/bitcoin/bitcoin/pull/31359#pullrequestreview-2465699855)
ACK d3f42fa08fc385752881afa5740f40287cfb4d8b

Without this patch I get:

```
CMake Warning at CMakeLists.txt:195 (message):
PIE is not supported at link time: PIE (CXX): Change Dir:
'/tmp/build/clang20-fuzz0/CMakeFiles/CMakeScratch/TryCompile-8aYRFx'
...
ld: error: relocation R_X86_64_32S cannot be used against local symbol;
recompile with -fPIC
```

with this patch - no errors.

💬 hebasto commented on issue "Unable to compile for test coverage on Nixos 24.05":
(https://github.com/bitcoin/bitcoin/issues/31087#issuecomment-2504557935)
> ### Operating system and version
>
> NixOS 24.05

I assume the following packages are being used:
- `gcc-13.2.0`
- `lcov-1.16`

The problem is twofold:

1. To workaround path issues, add `-DLCOV_OPTS="--rc geninfo_adjust_src_path=/home/daniela/Developer/bitcoin/build=>/nix/store/zh33l12pk3iij0x5gkjdnargrki3n9gh-boost-1.81.0-dev` to the script invocation:
```
$ cmake -DJOBS=10 -DLCOV_OPTS="--rc geninfo_adjust_src_path=/home/daniela/Developer/bitcoin/build=>/nix/store/zh33l12pk3iij0
...