💬 mzumsande commented on pull request "test: add missing sync to feature_fee_estimation.py":
(https://github.com/bitcoin/bitcoin/pull/31016#issuecomment-2387293853)
> Should also fix #30640?

Yes! I didn't analyze that in detail but I think that's very likely. I'll add it to the OP.

👍 tdb3 approved a pull request: "test: add missing sync to feature_fee_estimation.py"
(https://github.com/bitcoin/bitcoin/pull/31016#pullrequestreview-2341658200)
code review ACK a1576edab356053c4c736691e4950b58e9a14f76

👍 tdb3 approved a pull request: "doc: add testnet4 section header for config file"
(https://github.com/bitcoin/bitcoin/pull/31007#pullrequestreview-2341683240)
ACK 61cdb1c9d83778b95f4f9596f34617b7a191d0a5

Good find. Thanks.

💬 RandyMcMillan commented on pull request "doc/build-osx.md:brew relinking note":
(https://github.com/bitcoin/bitcoin/pull/30993#discussion_r1783713469)
This PR isn't in response to and issue or comment. I just noticed it may be useful to have a note about relinking.

👍 tdb3 approved a pull request: "refactor: move util/pcp and util/netif to common/"
(https://github.com/bitcoin/bitcoin/pull/31011#pullrequestreview-2341666390)
ACK fd38711217cafbd62e8abd22d2b43f85fede8cde

cherry-pick'ed fd38711217cafbd62e8abd22d2b43f85fede8cde into master (intentionally without d51edecddcb7fa52349c8aa5ef88b01f72be44c7), ran `check-deps.sh`, and verified (with `echo $?`) that the script exited with 1 (vs 0 on master without the cherry-pick'ed commit).

💬 tdb3 commented on pull request "refactor: move util/pcp and util/netif to common/":
(https://github.com/bitcoin/bitcoin/pull/31011#discussion_r1783673649)
femto nit. Ignore if not already updating the file for something else.

```diff
- #include <common/pcp.h>
- #include <common/netif.h>
+ #include <common/netif.h>
+ #include <common/pcp.h>
```

💬 glozow commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1783599080)
Added the `Assume`. I agree it makes more sense.

💬 glozow commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1783611446)
Ah, sorry for missing that. I'm not really convinced this is more efficient overall?

💬 glozow commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1783578259)
edited somewhat. I like the annotations there but only did first row.

💬 glozow commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1783602138)
Thanks, done

💬 glozow commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1783604279)
More efficient to shuffle ints than objects. I've changed to use the ctor with the size.

💬 glozow commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1783572311)
edited to no longer be for a chain

💬 glozow commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1783604666)
done

💬 glozow commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1783602611)
Unsure, it's just a habit I guess.

💬 glozow commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1783609907)
This is part of a block of code that was moved from `PeerManagerImpl`. The members still have the same name, so I don't think this is incorrect.

💬 glozow commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1783569179)
added

💬 tdb3 commented on pull request "rpc: add getorphantxs":
(https://github.com/bitcoin/bitcoin/pull/30793#issuecomment-2387502458)
Pushed to b6368fc285bf00b3033061dcd4e29298b227c6df (rebased on top of fc642c33ef28829eda0119a0fe39fd9bc4b84051)

<details>
<summary>Saw the following CI error for macOS 14 with e6853592361341c27103ed74b25470ac1e098d6d</summary>

This seems to be related to PR #30684

```
node0 2024-10-02T00:15:45.783638Z [init] [src/noui.cpp:57] [noui_InitMessage] init message: Verifying wallet(s)…
Error: node0 2024-10-02T00:15:45.783653Z [init] [src/noui.cpp:31] [noui_ThreadSafeMessageBox] [error] In
...

💬 luisschwab commented on pull request "rpc: add getorphantxs":
(https://github.com/bitcoin/bitcoin/pull/30793#issuecomment-2387536123)
Concept ACK

💬 maflcko commented on pull request "test: add missing sync to feature_fee_estimation.py":
(https://github.com/bitcoin/bitcoin/pull/31016#issuecomment-2387683977)
lgtm ACK a1576edab356053c4c736691e4950b58e9a14f76

💬 maflcko commented on pull request "net: fuzz: bypass network magic and checksum validation":
(https://github.com/bitcoin/bitcoin/pull/31012#issuecomment-2387699942)
> I added a commit to bypass some asserts in `ConnmanTestMsg` that would fail since we're bypassing network magic and checksum validation. Also, I think we will have to change `p2p_transport_bidirectional` target since there are some sanity checks that would fail with the bypasses, especially the assertions about `ReceivedBytes`/`GetReceivedMessage`. Any thoughts?

Can you explain why the asserts and sanity checks would be failing? They are unrelated to the checksum, so if they are failing, it
...