💬 fjahr commented on issue "rpc: dumptxoutset as sqlite file":
(https://github.com/bitcoin/bitcoin/issues/24628#issuecomment-2665978440)
I think this is resolved with the merged of #27432?

🚀 fanquake merged a pull request: "test: remove scanning check on `wallet_importdescriptors`"
(https://github.com/bitcoin/bitcoin/pull/31893)

💬 hodlinator commented on issue "test: feature_assumeutxo.py Windows timeout":
(https://github.com/bitcoin/bitcoin/issues/31894#issuecomment-2666012863)
```
[shutoff] [...\httpserver.cpp:536] [StopHTTPServer] [http] Waiting for 1 connections to stop HTTP server
[http] [...\httpserver.cpp:355] [ThreadHTTP] [http] Exited http event loop
[net] [...\util\thread.cpp:22] [TraceThread] net thread exit
```
The fact that we don't get "Stopped HTTP server" or even "Waiting for HTTP event thread to exit" after these indicates that we are stuck in the lower part of `StopHTTPServer()`:

https://github.com/bitcoin/bitcoin/blob/790c509a4796ec47be2275d86b35370f
...

💬 maflcko commented on pull request "ci: switch MSAN to use prebuilt Clang binaries":
(https://github.com/bitcoin/bitcoin/pull/31850#issuecomment-2666017905)
> Needs a fixup for MSAN fuzz.

What is the error?

💬 sipa commented on pull request "p2p: improve TxOrphanage denial of service bounds and increase -maxorphantxs":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r1959969112)
In commit "[bench] TxOrphanage::LimitOrphans"

Would it make sense to introduce this benchmark earlier (and the other ones below), so we can see what effect the previous commit has on it?

💬 sipa commented on pull request "p2p: improve TxOrphanage denial of service bounds and increase -maxorphantxs":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r1959814286)
In commit "[txorphanage] add per-peer iterator list and announcements accounting"

Use `tx, std::move(announcer_list_pos), ...` to avoid an allocation.

💬 sipa commented on pull request "p2p: improve TxOrphanage denial of service bounds and increase -maxorphantxs":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r1959842788)
In commit "[txorphanage] add per-peer iterator list and announcements accounting"

Would it be possible to do the `list_pos` updating here without needing to return an iteration to push that responsibility to the caller? It would mean `RemoveIterAt` would need to know what peer it's operating in, so that means it's perhaps more appropriate to have it as ` TxOrphanage` member function rather than a `PeerOrphanInfo` member function.

💬 sipa commented on pull request "p2p: improve TxOrphanage denial of service bounds and increase -maxorphantxs":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r1959859482)
In commit "[txorphanage] add per-peer iterator list and announcements accounting"

I think this line, and the 7 lines before it, can be replaced with `RemoveIterAt(it->second.list_pos)`, especially if it can be changed to do the `list_pos` updating internally?

Not very important as the code disappears in the next commit, but would make it more obviously correct.

✅ hebasto closed an issue: "qa: `wallet_importdescriptors` failure "TypeError: 'bool' object is not subscriptable""
(https://github.com/bitcoin/bitcoin/issues/31881)

💬 hebasto commented on issue "qa: `wallet_importdescriptors` failure "TypeError: 'bool' object is not subscriptable"":
(https://github.com/bitcoin/bitcoin/issues/31881#issuecomment-2666041425)
Resolved in https://github.com/bitcoin/bitcoin/pull/31893.

📝 maflcko opened a pull request: "refactor: Remove redundant and confusing calls to IsArgSet"
(https://github.com/bitcoin/bitcoin/pull/31896)
`IsArgSet` is problematic:

* It returns whether an arg has been set, even if it has been negated. `IsArgSet` is sometimes used to check for a truthy value, which is wrong, but usually harmless. Cleanup of those cases may or may not be done in a follow-up.
* In most other cases, calling it is redundant, because the immediately following `Get*Arg` calls can already return an `std::optional` nullopt value to indicate an unset arg.

So relieve both issues by removing all `IsArgSet` that are re
...

💬 maflcko commented on pull request "scripted-diff: Type-safe settings retrieval":
(https://github.com/bitcoin/bitcoin/pull/31260#discussion_r1959977366)
nit: I find the `!Value(gArgs).isNull()` with the double negation and verbosity over just `Get(gArgs)` a bit ugly and I think it would be better to remove `IsArgSet` as much as possible. In most cases it is not needed anyway, or used incorrectly. About this one, I left a comment here: https://github.com/bitcoin/bitcoin/pull/31887/files#r1959923000

💬 hodlinator commented on pull request "doc: clarify loadwallet path loading for wallets":
(https://github.com/bitcoin/bitcoin/pull/30302#discussion_r1959978900)
Agree on double-quotes in more places giving nicer output, but would not rephrase or remove legacy wallets at this point.
```suggestion
{"filename", RPCArg::Type::STR, RPCArg::Optional::NO, "The path to the wallet directory (absolute or relative to the \"wallets\" directory), or a legacy wallet's .dat file (within the \"wallets\" directory). The \"wallets\" directory is set by the -walletdir option and defaults to the \"wallets\" folder within the data directory."},
```
I
...

📝 Sjors opened a pull request: "mining: drop unused -nFees and sigops from CBlockTemplate"
(https://github.com/bitcoin/bitcoin/pull/31897)
For the coinbase `vTxFees` used a dummy value of -nFees.

Similarly the first `vTxSigOpsCost` entry was calculated from
the dummy coinbase transaction.

This was introduced in #2115, but the values were never returned by the RPC or used in a test.

Set both to 0 instead and add code comments to prevent confusion.

💬 jonatack commented on pull request "doc: clarify loadwallet path loading for wallets":
(https://github.com/bitcoin/bitcoin/pull/30302#discussion_r1960001093)
I think invalidating ACKs is more a consideration for critical or difficult-to-review changes.

If loading legacy wallets will be disabled in the next release, it may be worth not adding a mention here that would require attention/removal.

🚀 fanquake merged a pull request: "build: remove ENABLE_HARDENING condition from check-security"
(https://github.com/bitcoin/bitcoin/pull/31892)

💬 Sjors commented on pull request "Add waitNext() to BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/31283#discussion_r1960003712)
I opened #31897 to drop this `-nFees` dummy.

💬 jonatack commented on pull request "doc: clarify loadwallet path loading for wallets":
(https://github.com/bitcoin/bitcoin/pull/30302#discussion_r1960005999)
> Agree on double-quotes in more places giving nicer output

The suggested diff doesn't only add double quotes but also proposes a rewrite for clarity between wallet directory and wallets directory.

✅ prusnak closed an issue: "rpc: dumptxoutset as sqlite file"
(https://github.com/bitcoin/bitcoin/issues/24628)

💬 prusnak commented on issue "rpc: dumptxoutset as sqlite file":
(https://github.com/bitcoin/bitcoin/issues/24628#issuecomment-2666086329)
> I think this is resolved with the merged of [#27432](https://github.com/bitcoin/bitcoin/pull/27432)?

Yes