💬 john-moffett commented on pull request "rpc: addpeeraddress: throw on invalid IP":
(https://github.com/bitcoin/bitcoin/pull/33430#issuecomment-3329686831)
Thanks, all!

@vasild

> As an alternative, we could use the `error` field in the response instead of throwing an exception.

I tried to use the same pattern as `setban` does, which I think has a clean boundary between an invalid input and a problem that occurred despite valid input. To me, that category separation makes sense.

@pablomartin4btc

> Would it be useful for the user to differentiate from an empty address vs a provided invalid IP (which obviously includes an empty value)
...

💬 john-moffett commented on pull request "rpc: Always return per-wtxid entries in submitpackage tx-results":
(https://github.com/bitcoin/bitcoin/pull/33427#discussion_r2376310092)
Apologies if it was too minor to refactor! Will try to keep it more surgical in the future for these types of touch-up PRs.

💬 katesalazar commented on pull request "Fix dark mode detection on Linux":
(https://github.com/bitcoin-core/gui/pull/895#issuecomment-3329734282)
Ah, nice and small, thanks. I have a couple of questions.

This is in your code:

> This must be done before creating the QApplication

Does this mean window won't change live when theme changes, on Linux?

This is in your post:

> unlike macOS where it works out of the box.

Does this mean window will change live when theme changes, on macOS?

🤔 hebasto reviewed a pull request: "depends: static libxcb-cursor"
(https://github.com/bitcoin/bitcoin/pull/33434#pullrequestreview-3263686528)
My Guix builds for Linux hosts on both `aarch64` and `x86_64` platforms:
```
cf7f86d91288477c11afdc767969d6b19b0ba1e62c73924e72b22a2527374653 guix-build-eca50854e1cb/output/aarch64-linux-gnu/SHA256SUMS.part
5afe50eb76079f96a5129b5e51e2ede657f10c5031abe24a875410f006c55511 guix-build-eca50854e1cb/output/aarch64-linux-gnu/bitcoin-eca50854e1cb-aarch64-linux-gnu-debug.tar.gz
ba6f7937c5d333e64560abcd95dbb3856ceaa43fbcde766ebcaed1f8e23f2a7e guix-build-eca50854e1cb/output/aarch64-linux-gnu/bitcoi
...

🤔 glozow reviewed a pull request: "bugfix: miner: fix `addPackageTxs` unsigned integer overflow"
(https://github.com/bitcoin/bitcoin/pull/33475#pullrequestreview-3263812697)
nice, utACK b807dfcdc5929c314d43b790c9e705d5bf0a86e8

🤔 furszy reviewed a pull request: "bugfix: miner: fix `addPackageTxs` unsigned integer overflow"
(https://github.com/bitcoin/bitcoin/pull/33475#pullrequestreview-3263830391)
Code ACK b807dfcdc5929c314d43b790c9e705d5bf0a86e8

💬 fjahr commented on pull request "rpc: Fix dumptxoutset rollback with competing forks":
(https://github.com/bitcoin/bitcoin/pull/33444#issuecomment-3329887782)
This seems to implement the same logic that I used in #31117 but this turned out to be too fragile to be seriously considered for merging there. In this context it may work well enough for mainnet because there are not a lot of forks but we have been discussing an alternative approach for a while that does not rely on `invalidateblock`/`reconsiderblock` and this would solve the competing forks problem as well. I have worked on this idea for #31117 primarily but I will open a PR with that approac
...

💬 glozow commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2376540950)
This fails `test_unspent_ephemeral` for me when applied on 00c02b42c61

💬 IdotMaster1 commented on pull request "docs: Undeprecate datacarrier and datacarriersize configuration options":
(https://github.com/bitcoin/bitcoin/pull/33453#issuecomment-3330009658)
Concept ACK, but turn the default OP_RETURN back to normal.

🚀 fanquake merged a pull request: "Backport Cirrus runners to 28.x"
(https://github.com/bitcoin/bitcoin/pull/33406)

💬 enirox001 commented on pull request "rpc: Fix dumptxoutset rollback with competing forks":
(https://github.com/bitcoin/bitcoin/pull/33444#issuecomment-3330024209)
Thanks for the review and context @fjahr . I wasn't aware of that prior work.

You're right that this approach has some fragility concerns. I'd be very interested to see your alternative approach that avoids invalidateblock/reconsiderblock altogether, as that does sound more robust.

Regarding the test failure - let me investigate and fix that. Would you prefer I:
1. Fix the test and keep this PR open for comparison with your upcoming alternative approach, or
2. Close this PR and wait fo
...

💬 instagibbs commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2376581403)
ah right, it will fail with in-mempool sibling because package version will simply fail when it encounters a "parent already has child in mempool" condition it's not meant for RBF cases

💬 glozow commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2376602411)
Noting that this check is redundant with `ReplacementChecks`, since `CalculateChunksForRBF` also calls it before calculating the feerate diagram. I think checking twice is fine, but that one has a slightly different error message that should be unified imo. See this test:

```
diff --git a/test/functional/mempool_cluster.py b/test/functional/mempool_cluster.py
index 3da8b477a2f..8bcc9b2fe65 100755
--- a/test/functional/mempool_cluster.py
+++ b/test/functional/mempool_cluster.py
@@ -4,6 +4
...

💬 plebhash commented on issue "`bitcoin-node` is unkillable after mining IPC connection is established":
(https://github.com/bitcoin/bitcoin/issues/33463#issuecomment-3330050492)
after some more progress on my Rust code, I'm no longer waiting on `waitTipChanged`, since I can get away with only waiting on `waitNext`

after these changes, I'm no longer observing `bitcoin-node` being unkillable via ctrl+c.

so this, plus the odd-looking logs above, make it seem that this is related to how `bitcoin-node` is dealing with `waitTipChanged` internally?

💬 glozow commented on pull request "node: add `BlockTemplateCache`":
(https://github.com/bitcoin/bitcoin/pull/33421#issuecomment-3330050997)
> It seems the new fuzz test in [16d5558](https://github.com/bitcoin/bitcoin/pull/33421/commits/16d55585aa2d26e9177734927b8446faa72570e7)
caught a minor UndefinedBehaviorSanitizer: unsigned-integer-overflow issue on master.

Nice! Sorry for the labeling noise, I meant to label #33475

💬 maflcko commented on issue "Cleanup CFeeRate constructor (sat/vB vs BTC/kvB)":
(https://github.com/bitcoin/bitcoin/issues/23129#issuecomment-3330063270)
@polespinasa I don't think this is resolved. To explain it a bit better looking at the code:

https://github.com/bitcoin/bitcoin/blob/d41b503ae128ac36ef27e652d2935c6fe7981a79/src/wallet/rpc/spend.cpp#L223-L224

`AmountFromValue` is a function that returns a `CAmount` (i64 of satoshis). However, the `CFeeRate` constructor that is called takes a feerate per kvB:

https://github.com/bitcoin/bitcoin/blob/d41b503ae128ac36ef27e652d2935c6fe7981a79/src/policy/feerate.h#L44


So calling `CFeeRate{AmountF
...

💬 l0rinc commented on pull request "[IBD] Tracking PR for speeding up Initial Block Download":
(https://github.com/bitcoin/bitcoin/pull/32043#issuecomment-3330072143)
I have measured the performance of V30 with previous releases (same dbcache, same assumevalid) - on an Intel i9 with an NVMe SSD and an Intel i7 with a HDD.

## Intel Core i9-9900K

Many of our optimization efforts were focusing on the underperforming, cheap hardware, so the i9 may not be representative - though running so many experiments is really slow so we often use the faster one to quickly check if an idea makes any difference.

<details>
<summary>Individual measurements for i9</sum
...

💬 achow101 commented on pull request "cli: Handle arguments that can be either JSON or string":
(https://github.com/bitcoin/bitcoin/pull/33230#discussion_r2376667216)
I think there are a number of other type checking improvements that we can do at the same time, but I would prefer to do those separately.

💬 ryanofsky commented on issue "`bitcoin-node` is unkillable after mining IPC connection is established":
(https://github.com/bitcoin/bitcoin/issues/33463#issuecomment-3330113600)
> [@ryanofsky](https://github.com/ryanofsky) I'm on macOS, would a stack trace from LLDB be equally useful?

Yes a stack trace from all threads after the hang happens should make the cause of the problem clear. lldb command to print that should be `thread backtrace all`. It's also good to know the problem doesn't happen with waitNext, only waitTipChanged since we can look for differences between those methods.

💬 achow101 commented on pull request "[28.x] More backports":
(https://github.com/bitcoin/bitcoin/pull/33415#discussion_r2376694679)
In 1b1f359fc43385cb4d465b15754dac84eef06873 "[doc] manpages for 28.3rc1"

I don't think this is supposed to be removed from the manage.

Same with `-upnp`.