💬 maflcko commented on issue "ci: libFuzzer disabled leak detection after every mutation":
(https://github.com/bitcoin/bitcoin/issues/32681#issuecomment-2943527766)
See https://github.com/bitcoin/bitcoin/pull/31481

💬 m3dwards commented on pull request "test: enabling wallet migration functional test on windows":
(https://github.com/bitcoin/bitcoin/pull/32219#discussion_r2128468936)
Taken, thank you.

📝 Sjors opened a pull request: "wallet: have external signer use PSBT error code EXTERNAL_SIGNER_NOT_FOUND"
(https://github.com/bitcoin/bitcoin/pull/32682)
When attempting to sign a transaction involving an external signer, if the device is connected we throw an `std::runtime_error`. This prevents the (mainly GUI) code that's actually supposed to handle this case from running.

This PR returns an `PSBTError::EXTERNAL_SIGNER_NOT_FOUND` instead of throwing.

The first commit is a refactor to have `GetExternalSigner()` return a `util::Result<ExternalSigner>` so the caller can decide how to handle the error. There are two other places where call `G
...

💬 Sjors commented on issue "external signer: PSBT error code `EXTERNAL_SIGNER_NOT_FOUND` is never returned":
(https://github.com/bitcoin/bitcoin/issues/32426#issuecomment-2943569685)
Fixed in #32682. It has the nice benefit of actually using the existing translations for "External signer not found".

💬 m3dwards commented on pull request "test: enabling wallet migration functional test on windows":
(https://github.com/bitcoin/bitcoin/pull/32219#discussion_r2128483548)
@hodlinator agreed and taken

💬 stickies-v commented on pull request "log: Mitigate disk filling attacks by rate limiting LogPrintf, LogInfo, LogWarning, LogError":
(https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2128488386)
This logic should be encapsulated in `ResetWindows` imo, so probably this statement could just be:

```suggestion
m_limiter.MaybeResetWindow();
```

💬 m3dwards commented on pull request "test: enabling wallet migration functional test on windows":
(https://github.com/bitcoin/bitcoin/pull/32219#discussion_r2128518144)
It was renamed to prevent a clash with the now imported platform package. I have reworked the variables to be more clear.

💬 m3dwards commented on pull request "test: enabling wallet migration functional test on windows":
(https://github.com/bitcoin/bitcoin/pull/32219#discussion_r2128520456)
Taken and applied everywhere.

🤔 BrandonOdiwuor reviewed a pull request: "test: wallet: cover wallet passphrase with a null char"
(https://github.com/bitcoin/bitcoin/pull/32675#pullrequestreview-2899781298)
Code Review ACK 7cfbb8575e1ffbad5c48e2c461b45dd6ac63d064

🤔 janb84 reviewed a pull request: "ci: Rewrite test-each-commit as rust script"
(https://github.com/bitcoin/bitcoin/pull/32680#pullrequestreview-2899822942)
Concept ACK

What do you think about the idea to use cargo for this (maybe not now because it's still in 'unstable) ?

💬 janb84 commented on pull request "ci: Rewrite test-each-commit as rust script":
(https://github.com/bitcoin/bitcoin/pull/32680#discussion_r2128559420)
```suggestion
git rebase --exec "git merge --no-commit origin/${GITHUB_BASE_REF} && cargo -Zscript ./.github/ci-test-each-commit-exec.rs && git reset --hard" ${{ env.TEST_BASE }}
```

This removes the extra execution step if cargo is used. Do note that this is stil in "unstable features" .

💬 janb84 commented on pull request "ci: Rewrite test-each-commit as rust script":
(https://github.com/bitcoin/bitcoin/pull/32680#discussion_r2128554692)
```suggestion
---cargo
[package]
edition = "2024"
---
// Copyright (c) The Bitcoin Core developers
```

nit / idea, add embedded manifest for direct cargo execution

💬 marcofleon commented on pull request "(RFC) kernel: Replace leveldb-based BlockTreeDB with flat-file based store":
(https://github.com/bitcoin/bitcoin/pull/32427#issuecomment-2943783645)
Concept ACK

I've differentially [fuzzed](https://github.com/marcofleon/bitcoin/blob/084d430dcb87e708e0fc9a65dc3d0c4d1f468f85/src/test/fuzz/block_index_diff.cpp) `BlockTreeDB` and `BlockTreeStore` for ~5000 cpu hours so far and no issues. Happy to continue testing (differentially fuzzing or otherwise) once the final approach is implemented.

💬 maflcko commented on pull request "ci: Rewrite test-each-commit as rust script":
(https://github.com/bitcoin/bitcoin/pull/32680#discussion_r2128644660)
Sure, happy to review a pull doing that once it is stable

💬 maflcko commented on pull request "ci: Rewrite test-each-commit as rust script":
(https://github.com/bitcoin/bitcoin/pull/32680#discussion_r2128646681)
I think you disabled warnings. Otherwise: https://github.com/bitcoin/bitcoin/pull/32680#discussion_r2128644660

🤔 maflcko reviewed a pull request: "[WIP] rpc: add `clearmempool` command for regtest mode"
(https://github.com/bitcoin/bitcoin/pull/32418#pullrequestreview-2900025631)
I am mostly thinking that this is a test-only regtest-only feature, and the mentioned alternatives are ways to achieve the same already today on vanilla Bitcoin Core

💬 maflcko commented on pull request "[WIP] rpc: add `clearmempool` command for regtest mode":
(https://github.com/bitcoin/bitcoin/pull/32418#discussion_r2128677281)
this is the wrong order

💬 maflcko commented on pull request "[WIP] rpc: add `clearmempool` command for regtest mode":
(https://github.com/bitcoin/bitcoin/pull/32418#discussion_r2128677493)
node0 stderr Assertion failed: detected inconsistent lock order for '::cs_main' in rpc/mempool.cpp:1150 (in thread 'httpworker.3'), details in debug log.

💬 maflcko commented on pull request "wallet, rpc: Change `OutputType` items from `string` into compile-time constants `string_view`":
(https://github.com/bitcoin/bitcoin/pull/32432#issuecomment-2943998197)
> > Not sure what you mean by "won't work". What is the exact diff to reproduce the compile failure or error?
>
> If you try to concatenate the RPC description with any of the output type string variables, you will get an empty output type string. The reason seems to be that they are not yet initialized when this operation occurs.

Again, it would be good to have the exact code to reproduce. I guess you are talking about SIOF: https://en.cppreference.com/w/cpp/language/siof.html, which has
...

💬 maflcko commented on pull request "uint256 cxx-20 constexpr patch":
(https://github.com/bitcoin/bitcoin/pull/32663#issuecomment-2944018762)
see https://github.com/bitcoin/bitcoin/blob/ae024137bda9fe189f4e7ccf26dbaffd44cbbeb6/.github/workflows/ci.yml#L121-L128