💬 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

💬 hodlinator commented on pull request "log: print every script verification state change":
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2379991579)
But you've only sent it one block?

💬 hodlinator commented on pull request "log: print every script verification state change":
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2379988259)
Blocks are "in the chain" to me, like metal links being in a chain. Maybe on-chain tx is more in contrast to off-chain tx, but a tx is in a block, in a chain.

Not important enough that I reacted upon first review, but would definitely suggest changing if you re-touch.