💬 maflcko commented on pull request "init: change shutdown order of load block thread and scheduler":
(https://github.com/bitcoin/bitcoin/pull/30435#issuecomment-2224963680)
I guess it should be backported to 27.x? (My understanding is that this existed "forever", since 0.16.x, because `SyncWithValidationInterfaceQueue` never had a boost interruption point, or other interrupt check)

💬 maflcko commented on pull request "rpc: Use CHECK_NONFATAL over Assert":
(https://github.com/bitcoin/bitcoin/pull/30429#issuecomment-2224984002)
Can you add exact steps to reproduce, starting from a fresh install of your operating system? Otherwise there is nothing that can be done. For a fresh install of Ubuntu 24.04 LTS (noble) or Debian GNU/Linux 12 (bookworm) it passes:

```
git switch --detach fa8b587b91dfa8df28fc13589c511b871902670b && ( git show src | git apply --reverse ) && ./test/lint/lint-assertions.py
HEAD is now at fa8b587 rpc: Use CHECK_NONFATAL over Assert
CHECK_NONFATAL(condition) or NONFATAL_UNREACHABLE should be u
...

💬 maflcko commented on pull request "rpc: Use CHECK_NONFATAL over Assert":
(https://github.com/bitcoin/bitcoin/pull/30429#issuecomment-2224999042)
Same in the CI https://cirrus-ci.com/task/6075434675208192?logs=lint#L755 and when using `( cd ./test/lint/test_runner/ && RUST_BACKTRACE=1 cargo run -- --lint=all_python_linters )` locally. Again, I'll need exact steps to reproduce, otherwise there is little that can be done.

💬 maflcko commented on pull request "logging: Replace LogError and LogWarning with LogAlert":
(https://github.com/bitcoin/bitcoin/pull/30364#discussion_r1675452625)
nit in the scripted-diff: Personally I prefer a non-scripted diff, if the script is larger than the diff. So I think you can split out the 8 `/d` delete statements and three script lines for a simple last commit to just manually delete those lines. If you want to keep the scripted-diff my recommendation would be to split the `/d` parts into a separate one. Otherwise review is harder, because one has to jump back and forth during review because replacements and deletions are mixed with each other
...

💬 maflcko commented on pull request "test: [refactor] Pass TestOpts":
(https://github.com/bitcoin/bitcoin/pull/30407#issuecomment-2225015681)
> I did a grep onto `./src/test/` not sure if we want to use the `TestOpts` struct on any of these

Not sure what you mean, but I am happy to push any changes that make sense, if you provide a diff. Also, I am happy to review follow-ups, if you create one.

💬 maflcko commented on pull request "fuzz: Version handshake":
(https://github.com/bitcoin/bitcoin/pull/30413#issuecomment-2225030941)
> > Are you sure it is completely non-existent?
>
> The coverage we do have comes from the `process_messages` harness, but the harness doesn't fuzz the handshake it only makes sure that the handshake completes, so that the other messages can be fuzzed.

Sorry for the wrong comments above. Not sure why I hallucinated that this was already fuzzed. I guess I remembered seeing the timedata warnings printed in the fuzz runner and somehow assumed this part of the code is already fully fuzzed.


...

💬 Sjors commented on pull request "Stratum v2 Template Provider (take 3)":
(https://github.com/bitcoin/bitcoin/pull/29432#issuecomment-2225041208)
> the framework is nonblocking, so there is no problem with the client making multiple calls at the same time, and the client can also decide what threads in the server the calls run on. So any interface you would like to implement should be possible to implement.

I'm not sure if that helps. I was also a bit confused, so let me rephrase it:

The current interface `createNewBlock` returns a template, which contains 4 MB of serialized transaction data. Ideally I would _not_ include those tran
...

💬 maflcko commented on pull request "assumeutxo: Add dumptxoutset height param, remove shell scripts":
(https://github.com/bitcoin/bitcoin/pull/29553#discussion_r1675479974)
I guess it would still be good to leave a comment about the race in the source code (or in the docs on how to avoid the race). I guess the user would simply have to avoid calling other `*block` RPCs for the duration of `dumptxoutset`? (The p2p is already stopped, so shouldn't be the cause of a race)

💬 rkrux commented on pull request "Fee Estimation: Ignore all transactions that are CPFP'd":
(https://github.com/bitcoin/bitcoin/pull/30079#discussion_r1675498531)
Great, thank you!

👍 alfonsoromanz approved a pull request: "assumeutxo: Don't load a snapshot if it's not in the best header chain"
(https://github.com/bitcoin/bitcoin/pull/30320#pullrequestreview-2174193471)
Re ACK 55b6d7be68a6f6c3882588ffd5b9349d885ed953

🚀 fanquake merged a pull request: "depends: update doc in Qt pwd patch"
(https://github.com/bitcoin/bitcoin/pull/30336)

💬 maflcko commented on pull request "rpc: Avoid getchaintxstats invalid results":
(https://github.com/bitcoin/bitcoin/pull/29720#issuecomment-2225104347)
See https://github.com/bitcoin/bitcoin/issues/29359

📝 hodlinator opened a pull request: "fix: Make TxidFromString() respect string_view length"
(https://github.com/bitcoin/bitcoin/pull/30436)
Prompted by comment in https://github.com/bitcoin/bitcoin/pull/30377#issuecomment-2208857200 (referring to https://github.com/bitcoin/bitcoin/pull/28922#discussion_r1404437378).

💬 hodlinator commented on pull request "refactor: Make uint256S(const char*) consteval":
(https://github.com/bitcoin/bitcoin/pull/30377#issuecomment-2225154172)
> > **transaction_identifier.h** - Fixed dormant bug in `TxidFromString()` where the `string_view` length wasn't respected(!).
>
> This is known, see [#28922 (comment)](https://github.com/bitcoin/bitcoin/pull/28922#discussion_r1404437378). Thanks for picking it up!
>
> Maybe submit the fix first?

PR up now: #30436

💬 maflcko commented on pull request "depends: bump boost to 1.85.0 and use new CMake buildsystem":
(https://github.com/bitcoin/bitcoin/pull/30434#issuecomment-2225163931)
The CI failure https://cirrus-ci.com/task/4950868422819840:

```
Extracting boost...
/ci_container_base/depends/sources/boost-1.85.0-cmake.tar.gz: OK
Preprocessing boost...
Configuring boost...
-- The CXX compiler identification is Clang 18.1.3
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /usr/bin/env - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Boost: using system layout: i
...

💬 fjahr commented on pull request "assumeutxo: Add dumptxoutset height param, remove shell scripts":
(https://github.com/bitcoin/bitcoin/pull/29553#discussion_r1675572668)
I added a small comment and also mention it now in the usage guide as well as the rpc help text.

🚀 fanquake merged a pull request: "util: Use SteadyClock in RandAddSeedPerfmon"
(https://github.com/bitcoin/bitcoin/pull/30372)

💬 maflcko commented on issue "ci: failure in `p2p_unrequested_blocks.py`":
(https://github.com/bitcoin/bitcoin/issues/30430#issuecomment-2225189511)
Dupe of https://github.com/bitcoin/bitcoin/issues/29897 ?

✅ fanquake closed an issue: "ci: failure in `p2p_unrequested_blocks.py`"
(https://github.com/bitcoin/bitcoin/issues/30430)

💬 fanquake commented on issue "ci: failure in `p2p_unrequested_blocks.py`":
(https://github.com/bitcoin/bitcoin/issues/30430#issuecomment-2225193937)
Yea looks like it. I grepped for p2p_unrequested_blocks but somehow did not find that.