💬 maflcko commented on pull request "cmake: Switch to generated `ts_files.cmake` file":
(https://github.com/bitcoin/bitcoin/pull/33115#discussion_r2249160955)
There are still some spacing issues here. See also the report https://github.com/maflcko/b-c-gui-translations-review/blob/ab6dc43a4c543c072d916a268a0bc34562ce3f81/README.md

cc https://github.com/maflcko/DrahtBot/issues/49

💬 waketraindev commented on pull request "qt: clear command history when clearing the console":
(https://github.com/bitcoin-core/gui/pull/882#issuecomment-3146305007)
> I don't think this change is practical. Clearing the console should remove the visible output, but it shouldn't also clear the command history. In most terminal environments (e.g., bash, zsh, cmd, PowerShell, etc.), using clear or Ctrl+L clears the screen while preserving command history, which remains accessible via the arrow keys. Users often rely on this history to repeat or modify previous commands, and removing it could unintentionally disrupt that workflow. I suggest keeping the command
...

💬 waketraindev commented on pull request "qt: clear command history when clearing the console":
(https://github.com/bitcoin-core/gui/pull/882#issuecomment-3146310289)
If there is concern about preserving history by default, a compromise could be to clear command history only when holding Shift while clicking the "Clear" button, similar to how modifier keys are used in other GUIs for alternate actions. But personally, my expectation as a user is that "Clear" fully resets the console, including input history.

💬 jesterhodl commented on pull request "cmake: Switch to generated `ts_files.cmake` file":
(https://github.com/bitcoin/bitcoin/pull/33115#discussion_r2249174462)
Thanks, I'll review those

📝 chareice opened a pull request: "wallet: Improve wallet creation error message with usage hint"
(https://github.com/bitcoin/bitcoin/pull/33124)
Improve wallet creation error message with usage hint

💬 maflcko commented on pull request "log: rate limiting followups":
(https://github.com/bitcoin/bitcoin/pull/33011#discussion_r2249189479)
this has to be guarded, see e.g. the `logthreadnames` below

💬 maflcko commented on pull request "wallet: remove use of GetWalletDir and weakly_canonical when creating backup_prefix":
(https://github.com/bitcoin/bitcoin/pull/33122#discussion_r2249190191)

walletname -> wallet name [compound term should be two words for clarity]
if contains slashes -> if it contains slashes [missing pronoun “it”]

💬 fanquake commented on pull request "test: fix p2p_leak_tx.py":
(https://github.com/bitcoin/bitcoin/pull/33121#issuecomment-3146425091)
MSAN failure (https://cirrus-ci.com/task/6435705422872576?logs=ci#L1863) is #33114.

📝 fanquake opened a pull request: "ci: Use mlc `v1` and ignore `depends/work`"
(https://github.com/bitcoin/bitcoin/pull/33125)
Otherwise you might see failures like this, if there are build artifacts in your depends dir:
```bash
[Err ] ./depends/work/build/x86_64-w64-mingw32/qt/6.7.3-64961f1d77c/qtbase/tests/auto/corelib/serialization/qxmlstream/XML-Test-Suite/xmlconf/xmlconf-20031030.htm (39, 55) => /Member/#confidential - Target not found.
[Err ] ./depends/work/build/x86_64-w64-mingw32/qt/6.7.3-64961f1d77c/qtbase/tests/auto/corelib/serialization/qxmlstream/XML-Test-Suite/xmlconf/xmlconf-20020521.htm (39, 55) => /Me
...

💬 yancyribbens commented on pull request "test: add assertions to SRD max weight test":
(https://github.com/bitcoin/bitcoin/pull/33058#issuecomment-3146449310)
> A test being based on knowing that SRD uses the min-heap approach internally would be 1) more complicated, 2) harder to read, 3) more error prone, and 4) a layer violation.

I'm not advocating to test that SRD uses a min-heap. However, the min-heap was added https://github.com/bitcoin/bitcoin/pull/26720 because:

> As the selection of the 1515 small UTXOs exceeds the max allowed tx size, the expectation here is to receive a selection result that only contain the big UTXO. Which is not hap
...

📝 Ataraxia009 opened a pull request: "Allowing multi client support in guix-build"
(https://github.com/bitcoin/bitcoin/pull/33126)
Using `CLIENT_NAME` variable in guix build files to better support forked clients

🤔 furszy reviewed a pull request: "test: add assertions to SRD max weight test"
(https://github.com/bitcoin/bitcoin/pull/33058#pullrequestreview-3081012887)
utACK cc33e4578946c68d6d333f35c48f8cbf3f75f6cf

📝 Ataraxia009 opened a pull request: "Adding alert for failure to prevent dead-end user crash"
(https://github.com/bitcoin/bitcoin/pull/33127)
In some cases after running the client for a bit, the chainstate seems to have blocks with a lower validity level than `VALID_SCRIPTS`. This causes a crash on launch in the `PruneBlockIndexCandidates` function, where the `setBlockIndexCandidates` is empty after filtering by m_chain.

Could not figure out root cause as to why, so for now just added an alert so the user is not deadended.

🤔 furszy reviewed a pull request: "Adding alert for failure to prevent dead-end user crash"
(https://github.com/bitcoin/bitcoin/pull/33127#pullrequestreview-3081017861)
> Could not figure out root cause as to why, so for now just added an alert so the user is not deadended.

Could you share steps to reproduce it?

💬 Ataraxia009 commented on pull request "Adding alert for failure to prevent dead-end user crash":
(https://github.com/bitcoin/bitcoin/pull/33127#issuecomment-3146474055)
> > Could not figure out root cause as to why, so for now just added an alert so the user is not deadended.
>
> Could you share steps to reproduce it?

Couldn't really reproduce it myself, just a had a data dir in a bad state

Which, if this assert is true, should imply we are in a bad data dir state where the chainstate is not included in the setBlockIndexCandidates and all setBlockIndexCandidates are less work.

🤔 hodlinator reviewed a pull request: "rpc: Distinguish between vsize and sigop adjusted mempool vsize"
(https://github.com/bitcoin/bitcoin/pull/32800#pullrequestreview-3081014156)
Reviewed 1523e8d6c981efff1394bd1669fbbbea5ff7ac97

Still had some late suggestions which will hopefully get this in better shape for other reviewers too.

Might be worth pinging some participants from #27591 at this point to see what they think.

💬 hodlinator commented on pull request "rpc: Distinguish between vsize and sigop adjusted mempool vsize":
(https://github.com/bitcoin/bitcoin/pull/32800#discussion_r2249236129)
* Reworded initial sentence to account for fee estimation not being a policy itself.
* Use plain "size" instead of "`vsize`" to avoid loading foot-gun around `getrawtransaction`'s `vsize`.
* Add sentence motivate reasoning behind initial recommendation.
* Bring up the `-bytespersigop` sentence up to section around sigop-adjusted size and split paragraph.
* Use non-breaking space instead of comma as numeric separator for consistency with "4 000 000 units" above.
* Avoid detailing RPCs as the
...

💬 hodlinator commented on pull request "rpc: Distinguish between vsize and sigop adjusted mempool vsize":
(https://github.com/bitcoin/bitcoin/pull/32800#discussion_r2249228894)
Would be better to change "when" to "where" in the beginning of the line. Sorry for not noticing initially!

💬 hodlinator commented on pull request "rpc: Distinguish between vsize and sigop adjusted mempool vsize":
(https://github.com/bitcoin/bitcoin/pull/32800#discussion_r2249224212)
From https://github.com/bitcoin/bitcoin/pull/32800#issuecomment-3126433719:
> Addressed in [46acee8](https://github.com/bitcoin/bitcoin/pull/32800/commits/46acee890064e69b89f6cc0bc65c625972a4250d)

The newline is added back in the second commit now, it would be better to change the first commit to avoid removing it.

🤔 hodlinator reviewed a pull request: "assumevalid: log every script validation state change"
(https://github.com/bitcoin/bitcoin/pull/32975#pullrequestreview-3081040650)
Concept ACK

Good to communicate to users why their sync progress has slowed down.

Seems like it could be related to the reports in #32832, worth referencing in the PR description?