💬 glozow commented on pull request "p2p: TxOrphanage revamp cleanups":
(https://github.com/bitcoin/bitcoin/pull/32941#issuecomment-3097204926)
This is ready for review. Would also be great to get https://github.com/bitcoin-core/qa-assets/pull/231 in for the 2 new fuzzies

💬 marcofleon commented on pull request "p2p: TxOrphanage revamp cleanups":
(https://github.com/bitcoin/bitcoin/pull/32941#issuecomment-3097235779)
Looks like the `txorphanage_sim` inputs might be invalidated after this PR. Not sure if we should wait to add those inputs until after this gets in or if it's worth adding them before.

💬 mzumsande commented on pull request "p2p: Advance pindexLastCommonBlock early after connecting blocks":
(https://github.com/bitcoin/bitcoin/pull/32180#issuecomment-3097243452)
[b7ff6a6](https://github.com/bitcoin/bitcoin/commit/b7ff6a611a220e9380f6cd6428f1d3483c8d566f) to [3bdff9a](https://github.com/bitcoin/bitcoin/commit/3bdff9a154733f8f9f379448f5595a2e90474bc6): rebased due to conflict with #32868

📝 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.