🤔 maflcko reviewed a pull request: "rpc: require integer verbosity; remove boolean 'verbose'"
(https://github.com/bitcoin/bitcoin/pull/33214#pullrequestreview-3268772493)
not sure where this pull is going, but it could make sense to split out the test changes into a separate pull (replacing the deprecated values with integers and named args), except for one deprecated value for each RPC, so that all code branches remain tests, but most code is using the new way and named args.

💬 maflcko commented on pull request "rpc: require integer verbosity; remove boolean 'verbose'":
(https://github.com/bitcoin/bitcoin/pull/33214#discussion_r2379836497)
this is still moved

💬 maflcko commented on pull request "rpc: require integer verbosity; remove boolean 'verbose'":
(https://github.com/bitcoin/bitcoin/pull/33214#discussion_r2379838271)
seems odd to change the behavior here to allow deprecated values when they were forbidden

💬 maflcko commented on pull request "rpc: require integer verbosity; remove boolean 'verbose'":
(https://github.com/bitcoin/bitcoin/pull/33214#discussion_r2379845951)
when passing integral literal values, it makes sense to use named args, otherwise it is not clear what they mean from reading just the code

💬 m3dwards commented on pull request "ci: remove 3rd party js from windows dll gha job":
(https://github.com/bitcoin/bitcoin/pull/32513#issuecomment-3335185534)
> On the [master](https://github.com/bitcoin/bitcoin/actions/runs/17921155333/job/50956194860) branch:
>
> ```
> VSCMD_ARG_HOST_ARCH: x64
> VSCMD_ARG_TGT_ARCH: x64
> ```
>
> However, with this [PR](https://github.com/bitcoin/bitcoin/actions/runs/17807365691/job/50622408408):
>
> ```
> VSCMD_ARG_HOST_ARCH: x86
> VSCMD_ARG_TGT_ARCH: x86
> ```

What a great catch! Sorted it in this [run](https://github.com/m3dwards/bitcoin/actions/runs/18015235679/job/51258366346), will update PR s
...

📝 waketraindev reopened a pull request: "Add reset button and console commands for clearing output/history"
(https://github.com/bitcoin-core/gui/pull/882)
- Added Reset button with Ctrl+Shift+L shortcut to clear both console output and command history.
- Added new console commands:
- clear -> clears console output
- history-clear -> clears console output and command history
- history -> prints all stored command history

💬 waketraindev commented on issue "No way to clear command history in RPC console or reset the console without restarting the node":
(https://github.com/bitcoin-core/gui/issues/897#issuecomment-3335194642)
My take on the fix,

https://github.com/bitcoin-core/gui/pull/882

💬 maflcko commented on issue "build: `test_bitcoin` link failure with `-flto=thin` & address sanitizer":
(https://github.com/bitcoin/bitcoin/issues/33147#issuecomment-3335205925)
Can be closed as fixed?

💬 maflcko commented on pull request "ci: add libcpp hardening flags to macOS fuzz job":
(https://github.com/bitcoin/bitcoin/pull/33462#issuecomment-3335221230)
Concept ACK. The qa-assets repo has a libc++ debug run, so this isn't required, but it seems fast enough to not hurt.

Would be nice to push my suggested diff, so that the xcode is hardcoded again.

💬 achow101 commented on pull request "wallet: Add `exportwatchonlywallet` RPC to export a watchonly version of a wallet":
(https://github.com/bitcoin/bitcoin/pull/32489#discussion_r2379892167)
Yes, but that's not an issue.

💬 achow101 commented on pull request "wallet: Add `exportwatchonlywallet` RPC to export a watchonly version of a wallet":
(https://github.com/bitcoin/bitcoin/pull/32489#discussion_r2379903393)
Yes. The nodes in this test are not always in sync with each other.

🤔 l0rinc reviewed a pull request: "qa: Improvements to debug_assert_log + busy_wait_for_debug_log"
(https://github.com/bitcoin/bitcoin/pull/33423#pullrequestreview-3268886010)
The error message seems off to me now, we're potentially logging lines as missing that we haven't checked.

💬 l0rinc commented on pull request "qa: Improvements to debug_assert_log + busy_wait_for_debug_log":
(https://github.com/bitcoin/bitcoin/pull/33423#discussion_r2379915262)
Seems this was always eagerly evaluated since https://github.com/bitcoin/bitcoin/commit/fa3e9f7627784ee00980590e5bf044a0e1249999#diff-7f22a082e3a80ca2f212e36193f87dd80237035afedb7f15b116ef7fa265f3eeR245-R248 - the current approach of only calculating it once when the loop also terminates makes sense

nit: I know it's a long line but I would still prefer this merged
```suggestion
self._raise_assertion_error(f'Unexpected message "{unexpected_msg}" found in log:\n\n{join_log(log
...

💬 l0rinc commented on pull request "qa: Improvements to debug_assert_log + busy_wait_for_debug_log":
(https://github.com/bitcoin/bitcoin/pull/33423#discussion_r2379926157)
that's not necessarily true, we have just stopped searching after the first failure, right?

💬 l0rinc commented on pull request "qa: Improvements to debug_assert_log + busy_wait_for_debug_log":
(https://github.com/bitcoin/bitcoin/pull/33423#discussion_r2379920746)
this basically pops until the last element is still in logs, stops otherwise, right?
```suggestion
while remaining_expected and remaining_expected[-1] in log:
remaining_expected.pop()
```

💬 l0rinc commented on pull request "qa: Improvements to debug_assert_log + busy_wait_for_debug_log":
(https://github.com/bitcoin/bitcoin/pull/33423#discussion_r2379922589)
it would be slightly more pythonic to do

```suggestion
if not remaining_expected:
```

💬 maflcko commented on pull request "ci: Remove bash -c from cmake invocation using eval":
(https://github.com/bitcoin/bitcoin/pull/33401#issuecomment-3335311851)
nice, lgtm ACK 8406c1e6321fbc3ce739aa7ccb18c67fb706a4b9

💬 l0rinc commented on pull request "log: print every script verification state change":
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2379948259)
Not sure about this, I have never seen "inchain transaction" but I have seen "onchain". I would also say "it's not in the set" instead of "on the set" but I *would* say "it's on the blockchain" not "in". What do others think?

💬 l0rinc commented on pull request "log: print every script verification state change":
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2379949764)
Wouldn't that cause a race condition if the check misses it?

💬 l0rinc commented on pull request "qa: Improvements to debug_assert_log + busy_wait_for_debug_log":
(https://github.com/bitcoin/bitcoin/pull/33423#discussion_r2379958513)
> Return:
the number of log lines we encountered when matching

This doesn't seem to be the case anymore