💬 ryanofsky commented on pull request "wallet: Ensure best block matches wallet scan state":
(https://github.com/bitcoin/bitcoin/pull/30221#issuecomment-2863875857)
re: https://github.com/bitcoin/bitcoin/pull/30221#issuecomment-2861121747

> 🚧 At least one of the CI tasks failed. Task `fuzzer,address,undefined,integer, no depends`: https://github.com/bitcoin/bitcoin/runs/41829354924 LLM reason (✨ experimental): The CI failure is caused by a dynamic-type-mismatch detected by UndefinedBehaviorSanitizer during fuzz testing.

This seems unrelated to the other CI failures, but it looks like failure is caused by a bug in the fuzz test:

https://github.com/
...

💬 achow101 commented on pull request "wallet: init, don't error out when loading legacy wallets":
(https://github.com/bitcoin/bitcoin/pull/32449#discussion_r2080218220)
In 86f05d81a12851ab68c33c2447a000cce52ee25c "wallet: init, don't error out when loading legacy wallets"

`output` is overwritten here, given the previous line, I assume it's supposed to be appended to?

💬 furszy commented on pull request "wallet: init, don't error out when loading legacy wallets":
(https://github.com/bitcoin/bitcoin/pull/32449#discussion_r2080266825)
upps, yeah. Fixed.

💬 Bitcoin3554 commented on pull request "contrib: add xor-blocks tool to obfuscate blocks directory":
(https://github.com/bitcoin/bitcoin/pull/32451#issuecomment-2863960189)
alguém ai ta usando o btc3 versão 3.0 https://github.com/Bitcoin3554/Bitcoin3.0

👍 ismaelsadeeq approved a pull request: "doc: warn that CheckBlock() underestimates sigops"
(https://github.com/bitcoin/bitcoin/pull/31624#pullrequestreview-2826075627)
ACK a04f17a1882407db09b0a07338e12877ac1d9e92

Thanks for taking my suggestion and providing the context.

----
I will prefer if the size check is also clarified in same PR.
Combining the two in same PR might be better to not miss writing the follow-up.

🤔 musaHaruna reviewed a pull request: "wallet, rpc: Change `OutputType` items from `string` into compile-time constants `string_view`"
(https://github.com/bitcoin/bitcoin/pull/32432#pullrequestreview-2826091815)
Concept ACK [2fee8f4](https://github.com/bitcoin/bitcoin/pull/32432/commits/2fee8f4886e1421099dfb8ba959b9d0c7e4b0aa8)

After reading the comment [#32429](https://github.com/bitcoin/bitcoin/pull/32429#discussion_r2076251627)

The PR is addresses the comment very well.

Overall code looks good to me. The additional documentation added to the outputtype.h file help makes the code clearer. A little suggestion which you can feel free to ignore. Since PR is also making updates to the documentat
...

🤔 mzumsande reviewed a pull request: "refactor: validation: mark CheckBlockIndex as const"
(https://github.com/bitcoin/bitcoin/pull/32364#pullrequestreview-2826150227)
Code Review ACK 3e6ac5bf772751c66cdcd015dcb7e6ce4ea2cc77

💬 ismaelsadeeq commented on pull request "Wallet: Add `maxfeerate` wallet startup option":
(https://github.com/bitcoin/bitcoin/pull/29278#issuecomment-2864081597)
Rebased to fix merge conflict.

👍 ryanofsky approved a pull request: "qa: Verify clean shutdown on startup failure"
(https://github.com/bitcoin/bitcoin/pull/30660#pullrequestreview-2826013832)
Code review ACK 10845cd7cc89fc83b4ee844dc577b391720bba9c. Only changes since last review were adding a new commit tweaking assert_raises_message(), extending the new test to have a self-check, and to pass through all options to child tests instead of a hardcoded list of options. I left some cleanup suggestions below but they are not important.

I think I will go ahead and merge this with 2 current acks and many stale ones, since this had has a lot of heavy review already and it only affects t
...

💬 ryanofsky commented on pull request "qa: Verify clean shutdown on startup failure":
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r2080245499)
In commit "qa: assert_raises_message() - Stop assuming certain structure for exceptions" (28e282ef9ae94ede4aace6b97ff18c66cb72a001)

Would be nice if commit message mentioned why this change is being made. Currently there's no explanation and the change seems pretty random.

Also could replace `{repr(e)}` with just `{e!r}`

💬 ryanofsky commented on pull request "qa: Verify clean shutdown on startup failure":
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r2080321673)
In commit "qa: Add feature_framework_startup_failures.py" (10845cd7cc89fc83b4ee844dc577b391720bba9c)

Would suggest `s/only modifying tmpdir/just appending a unique suffix to tmpdir/` to give a little more context and help the next part of the comment make sense.

💬 ryanofsky commented on pull request "qa: Verify clean shutdown on startup failure":
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r2080292165)
In commit "qa: Add feature_framework_startup_failures.py" (10845cd7cc89fc83b4ee844dc577b391720bba9c)

I think all these nested string operations make this code hard to decipher, and flattening could make it more readable. Would suggest:

```diff
--- a/test/functional/feature_framework_startup_failures.py
+++ b/test/functional/feature_framework_startup_failures.py
@@ -71,12 +71,13 @@ class FeatureFrameworkStartupFailures(BitcoinTestFramework):

def run_test(self):
self.lo
...

💬 ryanofsky commented on pull request "qa: Verify clean shutdown on startup failure":
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r2080314268)
In commit "qa: Add feature_framework_startup_failures.py" (10845cd7cc89fc83b4ee844dc577b391720bba9c)

This line is pretty dense. Might suggest adding a comment like "Get position and value of the first --tmpdir option, or the end position and self.options.tmpdir if none is specified." so it's not necessary to spend time deciphering this to understand the rest of the code.

Also I think it would probably be more correct to `s/enumerate(parent_args)/reversed(enumerate(parent_args))/` to use th
...

💬 ryanofsky commented on pull request "qa: Verify clean shutdown on startup failure":
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r2080377055)
re: https://github.com/bitcoin/bitcoin/pull/30660#discussion_r2080132673

Suggestion here seems to be to replace latest error `repr` with ignored errors `str`. Could also do the opposite and use `repr` for both. Either way seems find and would be nice to be consistent

💬 ryanofsky commented on pull request "qa: Verify clean shutdown on startup failure":
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r2080383095)
re: https://github.com/bitcoin/bitcoin/pull/30660#discussion_r2080170811

I didn't look into it closely but I think I don't understand why wait_for_rpc_connection should not be called if stop if called. If there's no cookie file because password authentication is used instead for example, i'd think you should still need for the RPC interface to wait before sending the stop RPC

💬 l0rinc commented on pull request "contrib: add xor-blocks tool to obfuscate blocks directory":
(https://github.com/bitcoin/bitcoin/pull/32451#discussion_r2080400271)
To speed this up significantly, we could probably process the data in 8 byte chunks, cast it to 64 bits, xor it with a 64 bit key and do the last bytes (if any) byte-by-byte against the current vector key - similarly to https://github.com/bitcoin/bitcoin/pull/31144

💬 kevkevinpal commented on pull request "test: added fuzz coverage for consensus/merkle.cpp":
(https://github.com/bitcoin/bitcoin/pull/32243#discussion_r2080404024)
Good catch! You're right it doesnt make sense to always have the `MAX_SCRIPT_SIZE` we should instead use in that range

updated in [0fc30f2](https://github.com/bitcoin/bitcoin/pull/32243/commits/0fc30f215f0f9c630b0c84a6ce2b89c6d4135237)

💬 fjahr commented on pull request "Embed default ASMap as binary dump header file":
(https://github.com/bitcoin/bitcoin/pull/28792#discussion_r2080421333)
Good idea. Done, also made a BE version of it.

💬 fjahr commented on pull request "Embed default ASMap as binary dump header file":
(https://github.com/bitcoin/bitcoin/pull/28792#discussion_r2080421426)
Done

💬 fjahr commented on pull request "Embed default ASMap as binary dump header file":
(https://github.com/bitcoin/bitcoin/pull/28792#discussion_r2080421642)
Hm, right, that's a left-over from previous iterations. Done.