🚀 fanquake merged a pull request: "test: Avoid duplicate curl call in get_previous_releases.py"
(https://github.com/bitcoin/bitcoin/pull/30703)

🤔 furszy reviewed a pull request: "Wallet, Bugfix: Lock Wallet Context mutex Before Adding/Removing Settings"
(https://github.com/bitcoin/bitcoin/pull/30697#pullrequestreview-2258765600)
I don't think using the context wallet's mutex is the best approach for this issue. The problem lies in how we update settings, which is not atomic. Today, the race is in the wallets flow, but tomorrow it could easily affect another settings option.

Updates are executed in three separate steps:
1) Obtain settings value while acquiring the settings lock.
2) Modify settings value.
3) Overwrite settings value while acquiring the settings lock.

This process is not thread-safe. Different thr
...

💬 furszy commented on pull request "Wallet, Bugfix: Lock Wallet Context mutex Before Adding/Removing Settings":
(https://github.com/bitcoin/bitcoin/pull/30697#discussion_r1730021378)
This could be shorter:
```
// This test verifies that wallet settings can be added and removed
// concurrently, ensuring no race conditions occur during either process.
BOOST_FIXTURE_TEST_CASE(write_wallet_settings_concurrently, TestingSetup)
{
WalletContext context;
context.chain = m_node.chain.get();
const auto NUM_WALLETS{5};
// Since we're counting the number of wallets, ensure we start without any.
BOOST_REQUIRE(context.chain->getRwSetting("wallet").isNull());
...

📝 jamesob opened a pull request: "rpc: add getdescriptoractivity"
(https://github.com/bitcoin/bitcoin/pull/30708)
The RPC command `scanblocks` provides a useful way to get a set of blockhashes that have activity relevant to a set of descriptors (`relevant_blocks`). However actually extracting the activity from those blocks is left as an exercise to the end user.

This process involves not only generating the (potentially ranged) set of scripts for the descriptor set on the client side (maybe via `deriveaddresses`), but then the user must retrieve each block's contents one-by-one using `getblock <hash>`,
...

👍 theStack approved a pull request: "Use MiniWallet in functional test rpc_signrawtransactionwithkey."
(https://github.com/bitcoin/bitcoin/pull/30701#pullrequestreview-2258787624)
lgtm ACK 96d34fb2e493a2c37907e76808215c5ab0c3cf70

🚀 fanquake merged a pull request: "test: replace deprecated secp256k1 context flags usage"
(https://github.com/bitcoin/bitcoin/pull/30687)

🚀 fanquake merged a pull request: "fuzz: remove repeated word in note"
(https://github.com/bitcoin/bitcoin/pull/30651)

🤔 tdb3 reviewed a pull request: "rpc: add getdescriptoractivity"
(https://github.com/bitcoin/bitcoin/pull/30708#pullrequestreview-2258846712)
Concept ACK

This seems to be a great way to simplify the chain of RPC commands needed to obtain targeted transaction history.

Thought about a potential alternative to update/enhance `scanblocks` instead of adding a new RPC (with the advantage of not needing to provide block hashes). Adding `getdescriptoractivity` seems like a better solution, since it avoids breaking RPC compatibility. Carrying blockhashes from `scanblocks` to `getdescriptoractivity` seems like a reasonably price to pay fo
...

💬 l0rinc commented on pull request "refactor: Replace ParseHex with consteval ""_hex literals":
(https://github.com/bitcoin/bitcoin/pull/30377#discussion_r1730107698)
I also like the suggestion, but there are other hex conversion methods in uint256 which could use some compile-time love - I'm fine with doing it in a separate PR.

💬 l0rinc commented on pull request "refactor: Replace ParseHex with consteval ""_hex literals":
(https://github.com/bitcoin/bitcoin/pull/30377#issuecomment-2308510085)
I'm not sure what's with the `Instruction clone failed` CI failures, but the change looks good to me

ACK 92e599d57c507f35d889c3a804d2a7020dcc6b1d

💬 1440000bytes commented on issue "DNS seed "seed.bitcoinstats.com" doesn't support filtering while the comments says it does":
(https://github.com/bitcoin/bitcoin/issues/29911#issuecomment-2308513735)
> Only bumping this because there's a good chance this seed will serve zero addresses by release time.

[Pristine node count](https://21.ninja/dns-seeds/pristine-count/) is already zero.


🤔 jonatack reviewed a pull request: "doc: fix 3 simple CI codespell warnings"
(https://github.com/bitcoin/bitcoin/pull/30700#pullrequestreview-2258924418)
Missing the following one with current codespell 2.3.0

```
src/qt/forms/optionsdialog.ui:344: incomin ==> incoming
```

Perhaps allow "highTS" but not "hights", as the latter could be an misspelling of "heights"

```diff
--- a/test/lint/spelling.ignore-words.txt
+++ b/test/lint/spelling.ignore-words.txt
hashIn
+highTS
inflight
```

💬 jonatack commented on pull request "doc: fix 3 simple CI codespell warnings":
(https://github.com/bitcoin/bitcoin/pull/30700#discussion_r1730112466)
maybe add "re-use" to the ignore list

https://www.oxfordreference.com/display/10.1093/acref/9780195135084.001.0001/acref-9780195135084-e-1902

```diff
--- a/test/lint/spelling.ignore-words.txt
+++ b/test/lint/spelling.ignore-words.txt
outIn
+re-use
requestor
```

💬 jamesob commented on pull request "rpc: add getdescriptoractivity":
(https://github.com/bitcoin/bitcoin/pull/30708#issuecomment-2308521018)
> Thought about a potential alternative that updates/enhances `scanblocks` instead of adding a new RPC (with the advantage of not needing to provide block hashes). Adding `getdescriptoractivity` seems like a better solution, since it avoids breaking RPC compatibility. Carrying block hashes from `scanblocks` to `getdescriptoractivity` seems like a reasonably price to pay for compatibility.

I thought about this initially and I think it's probably worth doing at some point via an additional `wit
...

💬 maaku commented on pull request "Have miner account for timewarp mitigation, activate on regtest, lower nPowTargetTimespan to 144 and add test":
(https://github.com/bitcoin/bitcoin/pull/30681#issuecomment-2308521346)
Hate to be that guy, but this form of the time-warp mitigation closes off the possibility of using something like forward blocks for scaling bitcoin on-chain in the future. That was my voiced objection to the "create consensus clean-up" on the mailing list. Of course it only applies to testnet4, but in practice this ends up throwing up a hurdle to later deployment of something like forward blocks, as it can no longer be tested on the primary testnet. Has there been consideration given to limitin
...

💬 l0rinc commented on pull request "doc: fix 3 simple CI codespell warnings":
(https://github.com/bitcoin/bitcoin/pull/30700#discussion_r1730121698)
We have a few cases of both, but I don't recall seeing it spelled this way anywhere else. My vote goes for fixing them instead.

💬 l0rinc commented on pull request "doc: fix 3 simple CI codespell warnings":
(https://github.com/bitcoin/bitcoin/pull/30700#issuecomment-2308524353)
> src/qt/forms/optionsdialog.ui:344: incomin ==> incoming

I've seen this one before, it's not a typo and we can't really whitelist it either: `incomin&amp;g` (I think it means that `g` is a shortcut key)

🤔 ismaelsadeeq reviewed a pull request: "Wallet, Bugfix: Lock Wallet Context mutex Before Adding/Removing Settings"
(https://github.com/bitcoin/bitcoin/pull/30697#pullrequestreview-2258946638)
> Today, the race is in the wallets flow, but tomorrow it could easily affect another settings option.

I agree with this point. All clients that update settings will be victim to the same bug.

> Given this context, I've implemented a different solution that makes the underlying settings update operation atomic: [furszy@de05ba1](https://github.com/furszy/bitcoin-core/commit/de05ba1144bd6332d1d903d07e49463268412dc0). This commit also shortens the test code.

This seems like a great idea an
...

💬 jonatack commented on pull request "doc: fix 3 simple CI codespell warnings":
(https://github.com/bitcoin/bitcoin/pull/30700#discussion_r1730128226)
Yes, I mentioned it because it doesn't need fixing as it's not incorrect, and the linter raising on it will just annoy contributors needlessly.

💬 jonatack commented on pull request "doc: fix 3 simple CI codespell warnings":
(https://github.com/bitcoin/bitcoin/pull/30700#issuecomment-2308532905)
> I've seen this one before, it's not a typo and we can't really whitelist it either: `incomin&amp;g` (I think it means that `g` is a shortcut key)

Probably preferable to avoid needlessly raising. It will need to handled in any case when the CI is updated to a current version of codespell.

```diff
--- a/test/lint/spelling.ignore-words.txt
+++ b/test/lint/spelling.ignore-words.txt
+incomin
inflight
```