💬 kevkevinpal commented on pull request "test: Assert that when we add the max orphan amount that we cannot add anymore and that a random orphan gets dropped":
(https://github.com/bitcoin/bitcoin/pull/31040#issuecomment-2428049870)
> Concept ACK for the test While we are trying to add another orphan transaction when the max amount is reached , why are we dropping a random orphan and not just the incoming orphan ? Even in the condition of the node getting DOSed , having to remove a random transaction translates to some amount of work done while its not the case when we just drop the incoming transaction.

If we drop the incoming orphan then an attacker can quickly fill up a node's orphan pool with tx's that will never get
...

🤔 glozow reviewed a pull request: "cluster mempool: Implement changeset interface for mempool"
(https://github.com/bitcoin/bitcoin/pull/31122#pullrequestreview-2383546169)
Concept ACK, did a first pass

💬 glozow commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1809704580)
`StageAddition` and `StageRemoval` seem a bit more intuitive, but it's a nit

💬 glozow commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1809737214)
Question: why isn't this a `shared_ptr` / what's the purpose of the `unique_ptr`? I get that there should only ever be one changeset, but I'm confused by the fact that this carries a copy of the underlying raw pointer, as the `unique_ptr` then doesn't seem very unique.

💬 glozow commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1809735446)
Similarly, `m_stage_tx` and `m_all_conflicts` could be `m_to_add` and `m_to_remove`?

💬 glozow commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1809741165)
You could use `m_subpackage.m_total_modified_fees, m_subpackage.m_total_vsize` here? Unless you feel that `CTxMemPoolChangeSet::GetAggregateFeeRate` will be useful in the future.

👍 rkrux approved a pull request: "rpc: Add support to populate PSBT input utxos via rpc"
(https://github.com/bitcoin/bitcoin/pull/30886#pullrequestreview-2383677163)
tACK 87ceb610a72fda00e02f98426c6ee66b34fd6f54

Rebuilt and reran functional tests.

💬 rkrux commented on pull request "rpc: Add support to populate PSBT input utxos via rpc":
(https://github.com/bitcoin/bitcoin/pull/30886#discussion_r1809795132)
Thanks, this pure function implementation is nice!

📝 Jessiejaymz810s opened a pull request: "Pending changes exported from your codespace"
(https://github.com/bitcoin/bitcoin/pull/31128)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...

✅ Jessiejaymz810s closed a pull request: "Pending changes exported from your codespace"
(https://github.com/bitcoin/bitcoin/pull/31128)

💬 russeree commented on issue "Testnet4 consensus failure due to timewarp related "softfork"":
(https://github.com/bitcoin/bitcoin/issues/30786#issuecomment-2428390283)
> @wiz, @russeree: Are you still mining on Testnet4?

Currently no, I assumed honest HR had started mining on T4 now so I pointed my miners back at Mainnet. Should I reconsider?

💬 laanwj commented on issue "getblockchaininfo `verificationprogress` never reaches 1.0":
(https://github.com/bitcoin/bitcoin/issues/31127#issuecomment-2428465504)
Looks like a duplicate of #26433.

💬 rkrux commented on pull request "rpc: Add support to populate PSBT input utxos via rpc":
(https://github.com/bitcoin/bitcoin/pull/30886#issuecomment-2428493112)
This is far easier to use compared to the earlier way of `signrawtransactionwithwallet` where few details need to be passed manually such as txid, vout, scriptPubKey, witnessScript, amount!

```
➜ bcli28 signrawtransactionwithwallet 0300000001f70daae6a5f540e7ff0216e443f6422200f13c2d3c57fa10ab0cff3f1f2abf3f0000000000fdffffff0120e20f2401000000160014e2370a8c1504f239194d158eb0b1450c932b56a900000000 "[{\"txid\":\"3fbf2a1f3fff0cab10fa573c2d3cf1002242f643e41602ffe740f5a5e6aa0df7\", \"vout\": 0, \"s
...

⚠️ Sandra-Amina-Boss opened an issue: "Bitcoin"
(https://github.com/bitcoin/bitcoin/issues/31129)

✅ willcl-ark closed an issue: "Bitcoin"
(https://github.com/bitcoin/bitcoin/issues/31129)

💬 0xB10C commented on pull request "rpc: net: follow-ups for #30062":
(https://github.com/bitcoin/bitcoin/pull/30183#issuecomment-2428563086)
re-ACK a16917fb5981d1465ffd4c036586f8729e683b73

💬 hebasto commented on pull request "util: Remove RandAddSeedPerfmon":
(https://github.com/bitcoin/bitcoin/pull/31124#issuecomment-2428566032)
Concept ACK.

💬 TheCharlatan commented on issue "Mining Interface doesn't allow for Bitcoin Core to create blocks when it wants":
(https://github.com/bitcoin/bitcoin/issues/31109#issuecomment-2428571666)
> But aren't we holding cs_main throughout the process?

There are brief times where the lock is not held once the `ActivateBestChain` stage is reached during validation.

👍 TheCharlatan approved a pull request: "fees: Remove CLIENT_VERSION serialization"
(https://github.com/bitcoin/bitcoin/pull/29702#pullrequestreview-2384347523)
ACK fa8bd0be8432fda3c7312050433a6deb6722a073

💬 hebasto commented on pull request "depends: add *FLAGS to gen_id":
(https://github.com/bitcoin/bitcoin/pull/31125#issuecomment-2428614747)
Concept ACK.