💬 jonatack commented on pull request "doc: improve NODE_NETWORK_LIMITED documentation per BIP159":
(https://github.com/bitcoin/bitcoin/pull/31805#issuecomment-2660261042)
Opened https://github.com/bitcoin/bips/pull/1768 and putting this in draft for now.

📝 jonatack converted_to_draft a pull request: "doc: improve NODE_NETWORK_LIMITED documentation per BIP159"
(https://github.com/bitcoin/bitcoin/pull/31805)
Per BIP159, the definition is:

NODE_NETWORK_LIMITED || bit 10 (0x400) || If signaled, the peer <I>MUST</I> be capable of serving at least the last 288 blocks (~2 days).

Fix our documentation related to this, and add BIP159 mentions where relevant.

---

Originally this PR also tried to clarify the following, but it was dropped in response to review:

`NODE_NETWORK_LIMITED` is set by default, and it only signals a pruned or otherwise limited node if `NODE_NETWORK` is not also set. See
...

👋 l0rinc's pull request is ready for review: "optimization: speed up `CheckBlock` input checks (duplicate detection & nulls)"
(https://github.com/bitcoin/bitcoin/pull/31682)

🚀 achow101 merged a pull request: "rpc: Remove deprecated dummy alias for listtransactions::label"
(https://github.com/bitcoin/bitcoin/pull/31413)

💬 TheCharlatan commented on pull request "init: Take lock on blocks directory in BlockManager ctor":
(https://github.com/bitcoin/bitcoin/pull/31860#discussion_r1956747029)
I've been thinking about allocating it on the heap instead, but I kind of like that declaring it as the first field forces us to initialize it first, which a pointer type would not necessarily. Then again, moving the lock as a unique pointer from the options to the actual class would also probably be easier than defining a move constructor the directory lock class.

💬 achow101 commented on pull request "validation: In case of a continued reindex, only activate chain in the end":
(https://github.com/bitcoin/bitcoin/pull/31439#issuecomment-2660323933)
ACK c9136ca90605bbe29f005f538b92ff96ca360a13

🚀 achow101 merged a pull request: "validation: In case of a continued reindex, only activate chain in the end"
(https://github.com/bitcoin/bitcoin/pull/31439)

💬 mzumsande commented on pull request "p2p: improve TxOrphanage denial of service bounds and increase -maxorphantxs":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r1956712751)
This seems very cautious. The test first adds 150 txns, then another `DEFAULT_MAX_ORPHAN_ANNOUNCEMENTS` (so there is a 1 to 1 relation between announcements and txns).
If all txns are distinct, this should be enough to assert that `Size()` should shrink by 150 compared to prev_count, not just 1.

💬 mzumsande commented on pull request "p2p: improve TxOrphanage denial of service bounds and increase -maxorphantxs":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r1956737572)
the numbers don't add up for me: the tests creates 100 large orphans, sends 20, sends 1 small tx, then sends another 40 large orphans, and finally asserts that there are less than 101 entries in the orphanage. This would also be true without any eviction.

💬 achow101 commented on pull request "wallet: abandon orphan coinbase txs, and their descendants, during startup":
(https://github.com/bitcoin/bitcoin/pull/31794#issuecomment-2660362190)
ACK 3c89cab06a6259fa70eaf3a78c01ee42780c4e27

⚠️ davidgumberg opened an issue: "guix: Unable to reproduce macOS SDK tarball on Fedora 40"
(https://github.com/bitcoin/bitcoin/issues/31873)
Following the instructions in [`contrib/macdeploy/README.md`](https://github.com/bitcoin/bitcoin/tree/master/contrib/macdeploy#readme) to generate the macOS SDK tarball that is used during GUIX builds on Fedora 40, I am not able to reproduce the hash in the readme for the generated tarball. I have reproduced this on two different Fedora 40 machines, and with a Fedora 40 docker image, but the issue does not appear when generating the tarball in an Ubuntu 24.04 docker image, or on a macOS Sequoia
...

💬 achow101 commented on pull request "doc: build: Fix instructions for msvc gui builds":
(https://github.com/bitcoin/bitcoin/pull/31851#issuecomment-2660364547)
ACK c3fa043ae560289759b6f836ac62736d97ba91bf

🚀 achow101 merged a pull request: "doc: build: Fix instructions for msvc gui builds"
(https://github.com/bitcoin/bitcoin/pull/31851)

💬 achow101 commented on pull request "build: move `rpc/external_signer` to node library":
(https://github.com/bitcoin/bitcoin/pull/31865#issuecomment-2660374335)
ACK e501246e77cc36cff222fb07aed2fd1316e11e19

💬 sipa commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r1956785027)
Indeed, fixed.

💬 sipa commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r1956785498)
Done. I have actually added a bunch of functions (`IsAcceptable()`, `IsOptimal()`, `NeedsSplitting()`, and further on also `IsOversized()`) and mostly rewritten the long conditions involving `m_quality` to use these functions instead.

💬 sipa commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r1956785650)
I have expanded on this. LMK if it's clearer now.

💬 sipa commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r1956785706)
Done.

📝 hodlinator opened a pull request: "qa wallet: Activate dormant checks in wallet_multisig_descriptor_psbt.py"
(https://github.com/bitcoin/bitcoin/pull/31874)
Conditions had virtually no effect, lets activate them.

Also adds `assert_true()` which (unlike built-in `assert`) is kept in optimized Python executions, discussed in https://github.com/bitcoin/bitcoin/pull/30529#discussion_r1893029530

💬 sipa commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r1956786541)
I see this was a bit confusing, I have expanded the explanation.

But what I meant to convey was that (M) **in staging** means that if the transaction does not exist in main, it doesn't exist in staging either. A (P) in staging just means it exists there. LMK if the new explanation is clearer.