✅ fanquake closed a pull request: "Replace dead gnome link notificator.cpp"
(https://github.com/bitcoin/bitcoin/pull/32629)
(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.
(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
(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.
(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
...
(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
...
(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
(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);
```
(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`
(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/
(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
(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?
(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`
(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`
(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);
```
(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
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2114104811)
c9a34d4cdb5f5eca385304dc5c836960fad2a74a
Single scenario that boots out 2+ txns with a single `CheckNumEvictions` seems apt
💬 fanquake commented on pull request "deps: Bump lief to 0.16.5":
(https://github.com/bitcoin/bitcoin/pull/32431#issuecomment-2919737881)
Actually, we can now use 0.16.6: https://github.com/lief-project/LIEF/releases/tag/0.16.6, which includes that change.
(https://github.com/bitcoin/bitcoin/pull/32431#issuecomment-2919737881)
Actually, we can now use 0.16.6: https://github.com/lief-project/LIEF/releases/tag/0.16.6, which includes that change.
💬 ryanofsky commented on pull request "Mining: Avoid copying template CBlocks":
(https://github.com/bitcoin/bitcoin/pull/32547#issuecomment-2919753784)
> The constness of methods is a property of the interface. It is a way to say calling it will not change the object
Yes, I understand the intent of marking `interfaces::BlockTemplate` methods `const` is to say calling those methods will not change the object. And I agree using `const` to prevent changing the object would be a nice goal, but unfortunately that's not what `const` keyword does here.
Marking the methods `const` does not mark the block template data `const` or do anything that
...
(https://github.com/bitcoin/bitcoin/pull/32547#issuecomment-2919753784)
> The constness of methods is a property of the interface. It is a way to say calling it will not change the object
Yes, I understand the intent of marking `interfaces::BlockTemplate` methods `const` is to say calling those methods will not change the object. And I agree using `const` to prevent changing the object would be a nice goal, but unfortunately that's not what `const` keyword does here.
Marking the methods `const` does not mark the block template data `const` or do anything that
...
📝 fanquake converted_to_draft a pull request: "Mining: Avoid copying template CBlocks"
(https://github.com/bitcoin/bitcoin/pull/32547)
Refactoring to avoid unnecessary copies/complexity, at least in the mainnet paths.
Also abstracts out a new `TemplateToJSON` function to keep track of variable scoping better.
(https://github.com/bitcoin/bitcoin/pull/32547)
Refactoring to avoid unnecessary copies/complexity, at least in the mainnet paths.
Also abstracts out a new `TemplateToJSON` function to keep track of variable scoping better.
💬 fanquake commented on pull request "guix: accomodate migration to codeberg":
(https://github.com/bitcoin/bitcoin/pull/32439#issuecomment-2919777106)
Backported to `27.x` in #32479.
(https://github.com/bitcoin/bitcoin/pull/32439#issuecomment-2919777106)
Backported to `27.x` in #32479.