📝 naiyoma opened a pull request: "net: make m_mempool optional in PeerManager "
(https://github.com/bitcoin/bitcoin/pull/33029)
This PR is a continuation of →https://github.com/bitcoin/bitcoin/pull/22850, to make m_mempool an optional pointer in PeerManager instead of a reference.

some benefits, as mentioned here:https://github.com/bitcoin/bitcoin/pull/26247#issue-1396302764 are:

1. m_mempool is already a pointer in other parts like:
In Chainstate:[ https://github.com/bitcoin/bitcoin/blob/master/src/validation.h#L532](https://github.com/bitcoin/bitcoin/blob/master/src/validation.h#L532)
In BlockAssembler:[ https
...

💬 glozow commented on pull request "p2p: TxOrphanage revamp cleanups":
(https://github.com/bitcoin/bitcoin/pull/32941#issuecomment-3097406288)
> Looks like the txorphanage_sim inputs might be invalidated with this PR.

Ah good point, yeah.

💬 sipa commented on pull request "p2p: TxOrphanage revamp cleanups":
(https://github.com/bitcoin/bitcoin/pull/32941#discussion_r2219698256)
Use a scripted diff?

💬 sipa commented on pull request "p2p: TxOrphanage revamp cleanups":
(https://github.com/bitcoin/bitcoin/pull/32941#discussion_r2219704726)
That's a lot of fuzzer bytes (32) for very little gain I think. Since it's already fed through a cryptographic RNG anyway, I don't think consuming bytes from the fuzzer has any advantage over using the harness' `rng` (which is already seeded by fuzz input anyway).

💬 sipa commented on pull request "p2p: TxOrphanage revamp cleanups":
(https://github.com/bitcoin/bitcoin/pull/32941#discussion_r2219589388)
Hmm, this means that anyone with a `bitcoin.conf` that lists `maxorphantxs=N` will now get an error. An alternative could be to leave the option in for a while, but make it just emit a warning. Unsure if that's necessary; I doubt many people configure this, but I don't remember how we've dealt with similar changes in recent history.

💬 sipa commented on pull request "p2p: TxOrphanage revamp cleanups":
(https://github.com/bitcoin/bitcoin/pull/32941#discussion_r2219699199)
Use a scripted-diff?

💬 l0rinc commented on pull request "p2p: rename GetAddresses -> GetAddressesUncached":
(https://github.com/bitcoin/bitcoin/pull/32994#discussion_r2219731173)
Actually, first seeing `Uncached` in a name implies to me that it's safer, since it doesn't have side-effects, doesn't pollute the context, it cannot blow up, etc - I don't think it achieves scaring people away.
In other cases the suffix was "Unsafe" if it's important to signal that the user should pay extra attention.

💬 Zeegaths commented on pull request "docs: adds correct updated documentation links":
(https://github.com/bitcoin/bitcoin/pull/32699#issuecomment-3097660338)
thanks for pointing that out. all good now

On Mon, 21 Jul 2025 at 17:19, maflcko ***@***.***> wrote:

> *maflcko* left a comment (bitcoin/bitcoin#32699)
> <https://github.com/bitcoin/bitcoin/pull/32699#issuecomment-3096984862>
>
> No, you haven't replied/addressed the feedback from June: #32699 (comment)
> <https://github.com/bitcoin/bitcoin/pull/32699#discussion_r2150261914>
>
> —
> Reply to this email directly, view it on GitHub
> <https://github.com/bitcoin/bitcoin/pull/32699#iss
...

👋 Crypt-iQ's pull request is ready for review: "log: rate limiting followups"
(https://github.com/bitcoin/bitcoin/pull/33011)

💬 tnndbtc commented on issue "intermittent timeout in wallet_signer.py : 'createwallet' RPC took longer than 1200.000000 seconds":
(https://github.com/bitcoin/bitcoin/issues/32855#issuecomment-3097892555)
@naiyoma For `if(str_std_in.empty())`, it will always return true in this case. However, the empty string is sent by successful calling of `subprocess_close(err_wr_pipe);// close child side of pipe, else get stuck in read below`. If you comment out this line, you will see that read from stdin in signer.py will 100% get stuck.

🤔 l0rinc reviewed a pull request: "[IBD] prevector: store `P2WSH`/`P2TR`/`P2PK` scripts inline"
(https://github.com/bitcoin/bitcoin/pull/32279#pullrequestreview-3039141524)
Thanks, applied most of your comments + a few additional cleanups.
The first push was a simple rebase, the second contains the changes.

💬 l0rinc commented on pull request "[IBD] prevector: store `P2WSH`/`P2TR`/`P2PK` scripts inline":
(https://github.com/bitcoin/bitcoin/pull/32279#discussion_r2219879016)
Sure, modernized the definition in a separate commit

💬 l0rinc commented on pull request "[IBD] prevector: store `P2WSH`/`P2TR`/`P2PK` scripts inline":
(https://github.com/bitcoin/bitcoin/pull/32279#discussion_r2219893434)
Done

💬 l0rinc commented on pull request "[IBD] prevector: store `P2WSH`/`P2TR`/`P2PK` scripts inline":
(https://github.com/bitcoin/bitcoin/pull/32279#discussion_r2219911902)
Done, thanks

💬 l0rinc commented on pull request "[IBD] prevector: store `P2WSH`/`P2TR`/`P2PK` scripts inline":
(https://github.com/bitcoin/bitcoin/pull/32279#discussion_r2219916340)
Makes sense, thanks

📝 brunoerg opened a pull request: "test: check tx is final when there is no locktime"
(https://github.com/bitcoin/bitcoin/pull/33030)
According to https://corecheck.dev/mutation/src/consensus/tx_verify.cpp, there is no proper test for the `tx.nLockTime == 0` check in the `IsFinalTx` function, which is understandable, since this check will only be useful for a specific case where the `nBlockHeight` (block height) is zero. Otherwise, the following check `if ((int64_t)tx.nLockTime < ((int64_t)tx.nLockTime < LOCKTIME_THRESHOLD ? (int64_t)nBlockHeight : nBlockTime))` would catch any of it. This PR adds a test case for it.

💬 murchandamus commented on pull request "FUZZ: Test that BnB finds best solution":
(https://github.com/bitcoin/bitcoin/pull/32894#discussion_r2219993026)
UTXOs are ordered in descending effective value by BnB, so I don’t think the UTXO with largest value being likely to be generated first is a problem.

💬 murchandamus commented on pull request "FUZZ: Test that BnB finds best solution":
(https://github.com/bitcoin/bitcoin/pull/32894#discussion_r2219994826)
BnB traverses the UTXOs after sorting, so I don’t see why this would be of concern.

👍 darosior approved a pull request: "test: check tx is final when there is no locktime"
(https://github.com/bitcoin/bitcoin/pull/33030#pullrequestreview-3039322272)
Makes sense. utACK 7180b82420c0f303140a93ca635f5ac806bea77d. Could add a comment explaining this to the test as it may not be immediately obvious to a reader.

👍 l0rinc approved a pull request: "validation: ensure assumevalid is always used during reindex"
(https://github.com/bitcoin/bitcoin/pull/31615#pullrequestreview-3039244807)
ACK 01e372b25855a21d205c6ded83f6849691111d42

Thanks for fixing it - someone with more knowledge in this area should also validate it.