💬 hebasto commented on pull request "Replace dead gnome link notificator.cpp":
(https://github.com/bitcoin/bitcoin/pull/32629#issuecomment-2919418223)
Please [squash](https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits) your commits and update the PR title and description.

💬 ismaelsadeeq commented on pull request "Replace cluster linearization algorithm with SFL":
(https://github.com/bitcoin/bitcoin/pull/32545#issuecomment-2919479775)
> I'm interested in seeing benchmarks of this on various systems, especially high-end/modern ones (as their performance is likely most predictive of what future hardware will be like).

Below are benchmarks on MacBook M2 PRO 2023.
Compiled using clang with only build bench cmake option.
```
cmake -B build -DBUILD_BENCH=ON && cmake --build build
```

<details>
<summary>master 9a887baadebce7c2e76df831fad54c5fa81d309e</summary>

```terminal
abubakarismail@Abubakars-MacBook-Pro ~/D/W/b/b
...

💬 sipa commented on pull request "Replace cluster linearization algorithm with SFL":
(https://github.com/bitcoin/bitcoin/pull/32545#issuecomment-2919488733)
@ismaelsadeeq Not an apples-to-apples comparison I'm afraid, because the benchmark cases are changed in this PR too. Can you run the `LinearizeBoundedExample?` benchmarks too?

💬 ismaelsadeeq commented on pull request "cluster mempool: add TxGraph reorg functionality":
(https://github.com/bitcoin/bitcoin/pull/31553#issuecomment-2919520179)
reACK e1f501cab224608004e3a1eee7a48eec3cea25fc [deab145ab8..e1f501cab2](https://github.com/bitcoin/bitcoin/compare/538e9ff804f8dfba6f6a808e83572fdeab145ab8..e1f501cab224608004e3a1eee7a48eec3cea25fc)

Following the last light ACK, review comments by @instagibbs were addressed:
- Increased fuzz test coverage
- Clearer and corrected comments
- An Assume statement was added

✅ fanquake closed a pull request: "Replace dead gnome link notificator.cpp"
(https://github.com/bitcoin/bitcoin/pull/32629)

💬 fanquake commented on pull request "Replace dead gnome link notificator.cpp":
(https://github.com/bitcoin/bitcoin/pull/32629#issuecomment-2919554277)
I'll cherry pick this into a different branch.

💬 TheCharlatan commented on pull request "build: Introduce internal kernel library":
(https://github.com/bitcoin/bitcoin/pull/28690#issuecomment-2919581594)
Rebased 7e6e43a581c6fbf4f979206b0cabadc5476a35bd -> e4d80d721b8336047fcd75eb1891e08ac156a903 ([kernelInternalLib_17](https://github.com/TheCharlatan/bitcoin/tree/kernelInternalLib_17) -> [kernelInternalLib_18](https://github.com/TheCharlatan/bitcoin/tree/kernelInternalLib_18), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelInternalLib_17..kernelInternalLib_18))

Fixed confilict with #31375

💬 ismaelsadeeq commented on pull request "Replace cluster linearization algorithm with SFL":
(https://github.com/bitcoin/bitcoin/pull/32545#issuecomment-2919605052)
> @ismaelsadeeq Not an apples-to-apples comparison I'm afraid, because the benchmark cases are changed in this PR too

Oops sorry haven't take a look, I was hoping I can compare locally myself. I saw a link to the bench results comparing them in the description 👍🏾

> Can you run the LinearizeBoundedExample? benchmarks too?

Yep I did and updated the bench results. previous one was not pointing to your latest push.

💬 theuni commented on pull request "threading: remove ancient CRITICAL_SECTION macros":
(https://github.com/bitcoin/bitcoin/pull/32592#issuecomment-2919662378)
> https://cirrus-ci.com/task/6734127736553472?logs=ci#L3223
>
> ```shell
> [12:12:21.305] /ci_container_base/src/test/reverselock_tests.cpp:23:9: error: cannot call function 'AssertLockNotHeldInline' while mutex 'mutex' is held [-Werror,-Wthread-safety-analysis]
> [12:12:21.305] 23 | AssertLockNotHeld(mutex);
> [12:12:21.305] | ^
> [12:12:21.305] /ci_container_base/src/sync.h:141:31: note: expanded from macro 'AssertLockNotHeld'
> [12:12:21.305] 141 | #define A
...

⚠️ rando876 opened an issue: "Bug in PartiallyDownloadedBlock fuzz test logic or behavior"
(https://github.com/bitcoin/bitcoin/issues/32640)
There may be a bug in the fuzz test implementation for `PartiallyDownloadedBlock` in `src/test/fuzz/partially_downloaded_block.cpp`. Please investigate the following:

- The fuzz test performs various operations relating to compact blocks and partial downloads, including marking transactions as available, manipulating mempool/extra_txn, and reconstructing blocks.
- It uses assertions to check the state after calling `InitData` and `FillBlock` on `PartiallyDownloadedBlock`.
- The test may not pro
...

🤔 instagibbs reviewed a pull request: "p2p: improve TxOrphanage denial of service bounds and increase -maxorphantxs"
(https://github.com/bitcoin/bitcoin/pull/31829#pullrequestreview-2878472725)
reviewed through ea3a65e698f519afee23484ce1b399e9a4c62529

💬 instagibbs commented on pull request "p2p: improve TxOrphanage denial of service bounds and increase -maxorphantxs":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2114070740)
c9a34d4cdb5f5eca385304dc5c836960fad2a74a

```Suggestion
node::TxOrphanageImpl orphanage_low_mem(/*max_global_ann=*/10, /*reserved_peer_usage=*/TX_SIZE);
```

💬 instagibbs commented on pull request "p2p: improve TxOrphanage denial of service bounds and increase -maxorphantxs":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2114084831)
c9a34d4cdb5f5eca385304dc5c836960fad2a74a

might as well use and assert return of `CheckNumEvictions`

💬 instagibbs commented on pull request "p2p: improve TxOrphanage denial of service bounds and increase -maxorphantxs":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2114089251)
c9a34d4cdb5f5eca385304dc5c836960fad2a74a

nit: s/peer0/dos_peer/

💬 instagibbs commented on pull request "p2p: improve TxOrphanage denial of service bounds and increase -maxorphantxs":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2114080543)
c9a34d4cdb5f5eca385304dc5c836960fad2a74a

could run these checks just before additions to assert no-op

💬 instagibbs commented on pull request "p2p: improve TxOrphanage denial of service bounds and increase -maxorphantxs":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2114103109)
c9a34d4cdb5f5eca385304dc5c836960fad2a74a

`EraseForPeer` and `EraseTx` coverage here would be good. IIUC max memory goes up IFF a peer has a live orphan announcement?

💬 instagibbs commented on pull request "p2p: improve TxOrphanage denial of service bounds and increase -maxorphantxs":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2114084722)
c9a34d4cdb5f5eca385304dc5c836960fad2a74a

might as well use and assert return of `CheckNumEvictions`

💬 instagibbs commented on pull request "p2p: improve TxOrphanage denial of service bounds and increase -maxorphantxs":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2114085090)
c9a34d4cdb5f5eca385304dc5c836960fad2a74a

might as well use and assert return of `CheckNumEvictions`

💬 instagibbs commented on pull request "p2p: improve TxOrphanage denial of service bounds and increase -maxorphantxs":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2114091408)
c9a34d4cdb5f5eca385304dc5c836960fad2a74a
```Suggestion
BOOST_CHECK_EQUAL(orphanage.AnnouncementsFromPeer(peer1), 0);
BOOST_CHECK_EQUAL(orphanage.AnnouncementsFromPeer(peer2), 0);
```

💬 instagibbs commented on pull request "p2p: improve TxOrphanage denial of service bounds and increase -maxorphantxs":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2114104811)
c9a34d4cdb5f5eca385304dc5c836960fad2a74a

Single scenario that boots out 2+ txns with a single `CheckNumEvictions` seems apt