π fanquake merged a pull request: "depends: Add patch for Windows11Style plugin"
(https://github.com/bitcoin/bitcoin/pull/33906)
(https://github.com/bitcoin/bitcoin/pull/33906)
π¬ TheCharlatan commented on pull request "build: Introduce internal kernel library":
(https://github.com/bitcoin/bitcoin/pull/28690#issuecomment-3576666956)
Rebased a9ec7cdc0a1f6d4a3bd532649e79459074ed9f3b -> 4c387e938015de197860215d94b9c9aee9a82da8 ([kernelInternalLib_21](https://github.com/TheCharlatan/bitcoin/tree/kernelInternalLib_21) -> [kernelInternalLib_22](https://github.com/TheCharlatan/bitcoin/tree/kernelInternalLib_22), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelInternalLib_21..kernelInternalLib_22))
(https://github.com/bitcoin/bitcoin/pull/28690#issuecomment-3576666956)
Rebased a9ec7cdc0a1f6d4a3bd532649e79459074ed9f3b -> 4c387e938015de197860215d94b9c9aee9a82da8 ([kernelInternalLib_21](https://github.com/TheCharlatan/bitcoin/tree/kernelInternalLib_21) -> [kernelInternalLib_22](https://github.com/TheCharlatan/bitcoin/tree/kernelInternalLib_22), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelInternalLib_21..kernelInternalLib_22))
π¬ roconnor commented on pull request "Align legacy script policy with P2SH policy in AreInputsStandard":
(https://github.com/bitcoin/bitcoin/pull/33926#discussion_r2560793067)
Maybe it is better to add a new `TxoutType::LEGACY` and just match on that (putting NONSTADARD back on the line above) rather than redoing some of the work already done by Solver.
(https://github.com/bitcoin/bitcoin/pull/33926#discussion_r2560793067)
Maybe it is better to add a new `TxoutType::LEGACY` and just match on that (putting NONSTADARD back on the line above) rather than redoing some of the work already done by Solver.
π¬ l0rinc commented on pull request "doc: clarify and cleanup macOS fuzzing notes":
(https://github.com/bitcoin/bitcoin/pull/33921#issuecomment-3576682607)
@fanquake, what was the urgency to merge here?
(https://github.com/bitcoin/bitcoin/pull/33921#issuecomment-3576682607)
@fanquake, what was the urgency to merge here?
π¬ fanquake commented on pull request "depends: Add patch for Windows11Style plugin":
(https://github.com/bitcoin/bitcoin/pull/33906#issuecomment-3576691652)
Backported to `30.x` in #33609.
(https://github.com/bitcoin/bitcoin/pull/33906#issuecomment-3576691652)
Backported to `30.x` in #33609.
π¬ Sjors commented on pull request "mining: add getMemoryLoad() and track template non-mempool memory footprint":
(https://github.com/bitcoin/bitcoin/pull/33922#issuecomment-3576715015)
`mining_getblocktemplate_longpoll.py` triggered a `stack-use-after-return`, due to `block_template` being `static` (to allow template reuse between RPC calls). I added a commit to move this longpoll template to the node context.
(https://github.com/bitcoin/bitcoin/pull/33922#issuecomment-3576715015)
`mining_getblocktemplate_longpoll.py` triggered a `stack-use-after-return`, due to `block_template` being `static` (to allow template reuse between RPC calls). I added a commit to move this longpoll template to the node context.
π¬ romanz commented on pull request "rest: allow reading partial block data from storage":
(https://github.com/bitcoin/bitcoin/pull/33657#discussion_r2560837987)
Thanks - fixed in [c2088a7458...ff5c3fecea](https://github.com/bitcoin/bitcoin/compare/c2088a74586742cadc0041cab8782c423eda3bd0..ff5c3feceaba496ff25efd8420cfcc32e0864bcc).
(https://github.com/bitcoin/bitcoin/pull/33657#discussion_r2560837987)
Thanks - fixed in [c2088a7458...ff5c3fecea](https://github.com/bitcoin/bitcoin/compare/c2088a74586742cadc0041cab8782c423eda3bd0..ff5c3feceaba496ff25efd8420cfcc32e0864bcc).
π¬ maflcko commented on pull request "kernel: Flush in ChainstateManager destructor":
(https://github.com/bitcoin/bitcoin/pull/31382#issuecomment-3576740793)
> allows for getting rid of the `goto` in `bitcoin-chainstate` as well as making it a bit easier to use for future users of the kernel library.
I don't think this is accurate?
(https://github.com/bitcoin/bitcoin/pull/31382#issuecomment-3576740793)
> allows for getting rid of the `goto` in `bitcoin-chainstate` as well as making it a bit easier to use for future users of the kernel library.
I don't think this is accurate?
π¬ roconnor commented on pull request "Align legacy script policy with P2SH policy in AreInputsStandard":
(https://github.com/bitcoin/bitcoin/pull/33926#issuecomment-3576845925)
My best guess is that making legacy script standard means that randomly generated ScriptPubKeys are now generally mempool acceptable whereas before only very few specifically shaped ScriptPubKeys could be accepted and weren't being generated by the fuzzer. This is exposing some kind of error in the `process_messages` fuzz test harness.
One thing to explore is to see if reverting PR #32822 makes the failure disappear. If so then we maybe narrowed down the problem to the recent changes to thi
...
(https://github.com/bitcoin/bitcoin/pull/33926#issuecomment-3576845925)
My best guess is that making legacy script standard means that randomly generated ScriptPubKeys are now generally mempool acceptable whereas before only very few specifically shaped ScriptPubKeys could be accepted and weren't being generated by the fuzzer. This is exposing some kind of error in the `process_messages` fuzz test harness.
One thing to explore is to see if reverting PR #32822 makes the failure disappear. If so then we maybe narrowed down the problem to the recent changes to thi
...
π¬ Sjors commented on issue "Memory leak when using IPC mining interface":
(https://github.com/bitcoin/bitcoin/issues/33940#issuecomment-3576959068)
@lucasbalieiro does Bitcoin Core behave better if you compile the `30.x` branch from source?
(https://github.com/bitcoin/bitcoin/issues/33940#issuecomment-3576959068)
@lucasbalieiro does Bitcoin Core behave better if you compile the `30.x` branch from source?
π¬ m3dwards commented on pull request "depends, doc: Learn `x86_64-w64-mingw32ucrt` host and document it":
(https://github.com/bitcoin/bitcoin/pull/33857#issuecomment-3577089817)
> Iβm unable to reproduce this in a fresh Trixie container using Docker
I did almost the same apart from I checked out this PR rather than building from master.
This works on x86 but isn't working on Apple ARM for me. I don't think it's anything wrong with this PR, but it's a problem with the UCRT MinGW compiler. That being the case, should this hold up this PR?
(https://github.com/bitcoin/bitcoin/pull/33857#issuecomment-3577089817)
> Iβm unable to reproduce this in a fresh Trixie container using Docker
I did almost the same apart from I checked out this PR rather than building from master.
This works on x86 but isn't working on Apple ARM for me. I don't think it's anything wrong with this PR, but it's a problem with the UCRT MinGW compiler. That being the case, should this hold up this PR?
π Fibonacci747 opened a pull request: "fix: remove redundant mempool lock in ChainImpl::isInMempool()"
(https://github.com/bitcoin/bitcoin/pull/33946)
removes an unnecessary LOCK(mempool->cs) in ChainImpl::isInMempool(). The method calls CTxMemPool::exists(), which already locks mempool->cs internally. Because the mempool mutex is a RecursiveMutex, double-locking was safe but redundant. Dropping the outer lock matches patterns used elsewhere in ChainImpl (e.g., hasDescendantsInMempool() and GetTransactionAncestry() callers) where mempool read APIs are invoked without an additional lock and rely on the calleeβs internal locking. isRBFOptIn() re
...
(https://github.com/bitcoin/bitcoin/pull/33946)
removes an unnecessary LOCK(mempool->cs) in ChainImpl::isInMempool(). The method calls CTxMemPool::exists(), which already locks mempool->cs internally. Because the mempool mutex is a RecursiveMutex, double-locking was safe but redundant. Dropping the outer lock matches patterns used elsewhere in ChainImpl (e.g., hasDescendantsInMempool() and GetTransactionAncestry() callers) where mempool read APIs are invoked without an additional lock and rely on the calleeβs internal locking. isRBFOptIn() re
...
π¬ janb84 commented on pull request "kernel: don't use assert to handle invalid user input":
(https://github.com/bitcoin/bitcoin/pull/33943#discussion_r2561205810)
I find this a bit inconsistent with this enum :
```C
typedef uint8_t btck_ValidationMode;
#define btck_ValidationMode_VALID ((btck_ValidationMode)(0))
#define btck_ValidationMode_INVALID ((btck_ValidationMode)(1))
#define btck_ValidationMode_INTERNAL_ERROR ((btck_ValidationMode)(2))
```
Where `validationmode` runs from 0 to 2 and 2 is the internal error.
Also isn't it better to expose this as an ENUM so that it can be re-used by the handlers ? (not sure because of the single use)
...
(https://github.com/bitcoin/bitcoin/pull/33943#discussion_r2561205810)
I find this a bit inconsistent with this enum :
```C
typedef uint8_t btck_ValidationMode;
#define btck_ValidationMode_VALID ((btck_ValidationMode)(0))
#define btck_ValidationMode_INVALID ((btck_ValidationMode)(1))
#define btck_ValidationMode_INTERNAL_ERROR ((btck_ValidationMode)(2))
```
Where `validationmode` runs from 0 to 2 and 2 is the internal error.
Also isn't it better to expose this as an ENUM so that it can be re-used by the handlers ? (not sure because of the single use)
...
π¬ sdaftuar commented on pull request "Cluster mempool followups":
(https://github.com/bitcoin/bitcoin/pull/33591#discussion_r2561218853)
Fixed in 9e7410479.
(https://github.com/bitcoin/bitcoin/pull/33591#discussion_r2561218853)
Fixed in 9e7410479.
π¬ sdaftuar commented on pull request "Cluster mempool followups":
(https://github.com/bitcoin/bitcoin/pull/33591#discussion_r2561221040)
Took this (with one small fix) in 375682ec
(https://github.com/bitcoin/bitcoin/pull/33591#discussion_r2561221040)
Took this (with one small fix) in 375682ec
π¬ sdaftuar commented on pull request "Cluster mempool followups":
(https://github.com/bitcoin/bitcoin/pull/33591#discussion_r2561222587)
Fixed in 472e3e16
(https://github.com/bitcoin/bitcoin/pull/33591#discussion_r2561222587)
Fixed in 472e3e16
π¬ sdaftuar commented on pull request "Cluster mempool followups":
(https://github.com/bitcoin/bitcoin/pull/33591#discussion_r2561225440)
Replaced this with AssertLockHeld(pool.cs), and added lock annotations to both the single and package TRUC checks in 70893b6b
(https://github.com/bitcoin/bitcoin/pull/33591#discussion_r2561225440)
Replaced this with AssertLockHeld(pool.cs), and added lock annotations to both the single and package TRUC checks in 70893b6b
π¬ sdaftuar commented on pull request "Cluster mempool followups":
(https://github.com/bitcoin/bitcoin/pull/33591#discussion_r2561233005)
I'm going with `chunkweight` and `clusterweight` for now. Not sure what to do for the output of `getmempoolfeeratediagram()` though?
I also took your doc suggestion and created a new file in doc/policy/ that explains the terminology. Would it better to consolidate some of those files into one?
(https://github.com/bitcoin/bitcoin/pull/33591#discussion_r2561233005)
I'm going with `chunkweight` and `clusterweight` for now. Not sure what to do for the output of `getmempoolfeeratediagram()` though?
I also took your doc suggestion and created a new file in doc/policy/ that explains the terminology. Would it better to consolidate some of those files into one?
π¬ pablomartin4btc commented on pull request "Revert "gui, qt: brintToFront workaround for Wayland"":
(https://github.com/bitcoin-core/gui/pull/914#issuecomment-3577355929)
> > The fix is in qt6.3, so this is expected. Your options are:
>
> > ```
> > * Use depends yourself
> > ```
>
> I think I tried that before... I'll check it again, thanks!
Sorry, that (building `depends`) uses `xcb`... and that works well.
<details>
<summary>Built correctly from source both Qt 6.3.2 and 6.10.0, and <code>bringToFront</code> doesn't work on either of them using Wayland. I'll upgrade Ubuntu to 24.04 at some point and will check it again.</summary>
```
2025-11-2
...
(https://github.com/bitcoin-core/gui/pull/914#issuecomment-3577355929)
> > The fix is in qt6.3, so this is expected. Your options are:
>
> > ```
> > * Use depends yourself
> > ```
>
> I think I tried that before... I'll check it again, thanks!
Sorry, that (building `depends`) uses `xcb`... and that works well.
<details>
<summary>Built correctly from source both Qt 6.3.2 and 6.10.0, and <code>bringToFront</code> doesn't work on either of them using Wayland. I'll upgrade Ubuntu to 24.04 at some point and will check it again.</summary>
```
2025-11-2
...
π¬ sdaftuar commented on pull request "Cluster mempool followups":
(https://github.com/bitcoin/bitcoin/pull/33591#discussion_r2561237544)
Oops, fixed in 62b2f2144
(https://github.com/bitcoin/bitcoin/pull/33591#discussion_r2561237544)
Oops, fixed in 62b2f2144