💬 marcofleon commented on pull request "fuzz: Faster utxo_snapshot fuzz target":
(https://github.com/bitcoin/bitcoin/pull/30644#issuecomment-2288658870)
Sounds good, I'll review again when it's improved then. Looked to be about 2x when I just tested it.

🤔 furszy reviewed a pull request: "Migrate legacy wallets that are not loaded"
(https://github.com/bitcoin-core/gui/pull/824#pullrequestreview-2238083184)
ACK 8f2522d

💬 epiccurious commented on pull request "feat(build): improve dependency error reporting in autogen.sh":
(https://github.com/bitcoin/bitcoin/pull/30646#issuecomment-2288687022)
Thanks for reviewing. I'll close the PR as won't do.

✅ epiccurious closed a pull request: "feat(build): improve dependency error reporting in autogen.sh"
(https://github.com/bitcoin/bitcoin/pull/30646)

✅ maflcko closed an issue: "contrib: Automation for Bitcoin Full Node Deployment"
(https://github.com/bitcoin/bitcoin/issues/30638)

💬 maflcko commented on issue "contrib: Automation for Bitcoin Full Node Deployment":
(https://github.com/bitcoin/bitcoin/issues/30638#issuecomment-2288699195)
@willcl-ark Maybe leave a new comment in the packaging issue 55 about your dockerfile?

I agree closing this for now. It may be best to continue discussion in https://github.com/bitcoin-core/packaging/issues/55 (yes it is closed, but discussion can continue).

And given that the discussion here seems about a *user* dockerfile, not a *dev* dockerfile, the packaging repo seems more appropriate.

🚀 hebasto merged a pull request: "Migrate legacy wallets that are not loaded"
(https://github.com/bitcoin-core/gui/pull/824)

💬 stickies-v commented on pull request "refactor: Replace ParseHex with consteval ArrayFromHex":
(https://github.com/bitcoin/bitcoin/pull/30377#discussion_r1716909297)
> What is your macOS and XCode version? IIRC Xcode 14 is no longer supported, see also https://github.com/bitcoin/bitcoin/pull/29934, but I think this isn't documented, nor tested, so probably up for debate.

Thanks, I'm on macOS 14.3.1 and just bumped XCode from 14.3.1.0.1.1683849156 to 15.3.0.0.1.1708646388 which resolves the issue 👍

💬 vasild commented on pull request "Halt processing of unrequested transactions v2":
(https://github.com/bitcoin/bitcoin/pull/30572#issuecomment-2288719391)
> @vasild you appear to have disregarded my above message about this change is desirable. Was it unclear?

You mean this one: "At times there have been parties that mass connect..."? If that would be the driving force behind this, then maybe it would be better to put it in the PR description.

So this is not about CPU DoS attack by slow-invalid transactions, but about somebody doing something stupid and using the Bitcoin network in an ineffective way, sending 500 bytes instead of 30? Requiri
...

💬 hebasto commented on pull request "Update translation source file for v28.0 string freeze":
(https://github.com/bitcoin-core/gui/pull/833#issuecomment-2288738565)
@pablomartin4btc @stickies-v

I had to update this PR due to the recently merged bitcoin-core/gui#824.

Sorry for invalidating your ACKs. Mind re-reviewing please?

💬 stickies-v commented on pull request "Update translation source file for v28.0 string freeze":
(https://github.com/bitcoin-core/gui/pull/833#issuecomment-2288796044)
re-ACK 770b0348c0abe2f63b56cc78c7b7a0e6b4057ce1

💬 maflcko commented on pull request "refactor: Replace ParseHex with consteval ArrayFromHex":
(https://github.com/bitcoin/bitcoin/pull/30377#discussion_r1716956530)
So I guess this commit drops support for XCode on macOS Ventura 13. Not sure if `doc/build-osx.md` needs to be adjusted, similar to https://github.com/bitcoin/bitcoin/pull/29934/files

👍 pablomartin4btc approved a pull request: "Update translation source file for v28.0 string freeze"
(https://github.com/bitcoin-core/gui/pull/833#pullrequestreview-2238247544)
re-ACK 770b0348c0abe2f63b56cc78c7b7a0e6b4057ce1

no diffs.

💬 instagibbs commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#issuecomment-2288892100)
personally speaking I'm unsure what is gained by moving the fuzz tests to their own PR since they depend on the refactors. I don't think we want to merge the other commits unless the fuzz harnesses are attached to it. I'm continuing to review either way.

👍 ryanofsky approved a pull request: "log: expand BCLog::LogFlags (categories) to 64 bits"
(https://github.com/bitcoin/bitcoin/pull/26619#pullrequestreview-2238322169)
Code review ACK b31a0cd0378184b2b9eb8f4bd3120cbd32c62005, just dropping mask_bit constant since last review. I still think
#26697 is a better alternative to this PR because it is more type-safe and gets rid of bitwise flags and operations outside the bitset class, but it could also be a followup to this PR.

📝 hebasto opened a pull request: "test: Fix test log file name"
(https://github.com/bitcoin/bitcoin/pull/30654)
https://github.com/bitcoin/bitcoin/pull/19385 dropped `.cpp` infix (see https://github.com/bitcoin/bitcoin/pull/19385#issuecomment-1078960406 and https://github.com/bitcoin/bitcoin/pull/19385#issuecomment-1078970958). However, `src/test/README.md` still refers to the `foo_tests.cpp.log` pattern.

This PR restores the pre-PR19385 behaviour, as appending is easier to implement than replacing when porting this functionality to CMake.

🚀 hebasto merged a pull request: "Update translation source file for v28.0 string freeze"
(https://github.com/bitcoin-core/gui/pull/833)

💬 glozow commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#issuecomment-2289016249)
> I don't think we want to merge the other commits unless the fuzz harnesses are attached to it. I'm continuing to review either way.

Ok sounds good, I'll leave as is. Just hoping to make reviewer easier.

💬 ryanofsky commented on pull request "refactor: Replace ParseHex with consteval ArrayFromHex":
(https://github.com/bitcoin/bitcoin/pull/30377#discussion_r1717082261)
re: https://github.com/bitcoin/bitcoin/pull/30377#discussion_r1716226929

The comment seems not true and not helpful.

- If the concern is _not_ stack space used inside the function, and this warning is only being added because the return type of the function is a variable-sized std::array, then I'm not sure why every function that returns a variable-sized std::array shouldn't have the same warning. This actually seems less deserving of a warning than other functions returning a std::array o
...

💬 glozow commented on pull request "Move maximum timewarp attack threshold back to 600s from 7200s":
(https://github.com/bitcoin/bitcoin/pull/30647#issuecomment-2289038179)
Makes sense to me, sanity checked you can sync with this. Is https://github.com/TheBlueMatt/bips/blob/7f9670b643b7c943a0cc6d2197d3eabe661050c2/bip-XXXX.mediawiki still the right place to see great consensus cleanup spec?