💬 rex4539 commented on pull request "bench: fix used file that is not opened":
(https://github.com/bitcoin/bitcoin/pull/31693#issuecomment-2603929593)
> What are the exact steps to reproduce or test this problem and fix?

`cppcheck src/bench/load_external.cpp`

💬 maflcko commented on pull request "bench: fix used file that is not opened":
(https://github.com/bitcoin/bitcoin/pull/31693#issuecomment-2603966705)
> > What are the exact steps to reproduce or test this problem and fix?
>
> `cppcheck src/bench/load_external.cpp`

cppcheck is a static code analysis tool. A finding does not imply a bug and removing a finding does not imply a fix.

With "reproduce" I meant how a real user/developer can run into this problem.

Using static analysers to find (and fix) bugs will overall result in more bugs (Source: https://www.sqlite.org/testing.html#static_analysis). Looking at the diff in this pull con
...

💬 rex4539 commented on pull request "bench: fix used file that is not opened":
(https://github.com/bitcoin/bitcoin/pull/31693#issuecomment-2604000730)
Hey, if everyone feels this PR is not helpful, feel free to close. No hard feelings :)

👍 hodlinator approved a pull request: "descriptor: Add proper Clone function to miniscript::Node"
(https://github.com/bitcoin/bitcoin/pull/30866#pullrequestreview-2563823652)
re-ACK 17e59eccec2a94417fe932f8bfcecee46c85d9a3

`git range-diff master 352391c 17e59ec`

- Added private (cheers!) constructor, simplifying `Clone()`.

💬 hodlinator commented on pull request "descriptor: Add proper Clone function to miniscript::Node":
(https://github.com/bitcoin/bitcoin/pull/30866#discussion_r1923277352)
nit: Could make less verbose:
```suggestion
return std::unique_ptr<Node>{new Node{internal::NoDupCheck{}, node.m_script_ctx, node.fragment, std::move(new_subs), node.keys, node.data, node.k}};
```
Good call on adding the `make_unique` comment.

💬 maflcko commented on pull request "bench: fix used file that is not opened":
(https://github.com/bitcoin/bitcoin/pull/31693#issuecomment-2604049186)
The burden to motivate a change and explain the change is on the pull request author.

If you genuinely think there is benefit in this change and a real user/developer can run into this problem, it would be good to explain it and put it into context. Also, the pull request author should answer any outstanding review questions.

💬 Sjors commented on pull request "Add multiprocess binaries to release build (except Windows, OpenBSD)":
(https://github.com/bitcoin/bitcoin/pull/30975#issuecomment-2604052263)
Another CI failure on ` rpc_misc.py` this time on centOS. I'm unsure if it's the same issue:

https://cirrus-ci.com/task/6027377219731456?logs=ci#L4105

```
raise AssertionError("Unexpected stderr {} != {}".format(stderr, expected_stderr))
AssertionError: Unexpected stderr terminate called after throwing an instance of 'kj::ExceptionImpl'
what(): mp/proxy.cpp:242: disconnected: write(m_post_fd, &buffer, 1): Broken pipe
```

Pushing again with only the last commit date changed to see i
...

💬 rex4539 commented on pull request "bench: fix used file that is not opened":
(https://github.com/bitcoin/bitcoin/pull/31693#issuecomment-2604106619)
This PR doesn't have any seriousness. I had some free time and ran `cppcheck` for fun. I tested a fix proposal that I'm not sure if it's even correct. It appears to fix the issue and not introduce any new bugs (by my testing and CI). I definitely cannot motivate or convince anyone about it. It was made purely for fun.

✅ maflcko closed a pull request: "bench: fix used file that is not opened"
(https://github.com/bitcoin/bitcoin/pull/31693)

💬 hebasto commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2604118572)
Rebased.

💬 dergoegge commented on pull request "fuzz: add targets for PCP and NAT-PMP port mapping requests":
(https://github.com/bitcoin/bitcoin/pull/31676#discussion_r1923371490)
Here's my theory for the MSan failure:

`data` might hold less bytes than `consumed_bytes` indicated because the fuzzed data provider might not have enough bytes left. This means only `data.size()` bytes of `buf` will be initialized with the memcpy but `consumed_bytes` is returned as the number of bytes "received".

This should work:

```suggestion
const auto data{ConsumeRandomLengthByteVector(fuzzed_data_provider, len)};
if (!data.empty()) std::memcpy(buf, data.data(), d
...

👍 hebasto approved a pull request: "Update leveldb subtree to latest upstream"
(https://github.com/bitcoin/bitcoin/pull/31671#pullrequestreview-2564012838)
ACK 910a11fa66305f90b0f3a8aa9d2055b58a2d8d80, I've performed a subtree update locally and got the same changes.

I also verified that the `LEVELDB_IS_BIG_ENDIAN` macro is no longer used in the code.

💬 Sjors commented on pull request "psbt: add non-default sighash types to PSBTs and unify sighash type match checking":
(https://github.com/bitcoin/bitcoin/pull/31622#discussion_r1923390174)
Ah, I see. This is only called by `descriptorprocesspsbt` and `utxoupdatepsbt`. The former passes in any private key info it needs, and the latter doesn't pass private keys at all.

💬 Sjors commented on pull request "psbt: add non-default sighash types to PSBTs and unify sighash type match checking":
(https://github.com/bitcoin/bitcoin/pull/31622#discussion_r1923397245)
Do you mean in a followup PR? I'm still seeing this in `CreateSig` at the last commit of this PR:

```cpp
// BASE/WITNESS_V0 signatures don't support explicit SIGHASH_DEFAULT, use SIGHASH_ALL instead.
const int hashtype = nHashType == SIGHASH_DEFAULT ? SIGHASH_ALL : nHashType;
```

👍 brunoerg approved a pull request: "descriptor: Add proper Clone function to miniscript::Node"
(https://github.com/bitcoin/bitcoin/pull/30866#pullrequestreview-2564059690)
reACK 17e59eccec2a94417fe932f8bfcecee46c85d9a3

352391c2cf1a45231ae92ca92d2415b3786ab9ad..17e59eccec2a94417fe932f8bfcecee46c85d9a3: nice simplification!

💬 hebasto commented on pull request "[WIP] build: remove need to test for endianness":
(https://github.com/bitcoin/bitcoin/pull/29852#issuecomment-2604285343)
> > MSVC
>
> Has someone reported the request to them? If not, it seems less likely they'll fix it.

https://developercommunity.visualstudio.com/t/Missed-optimization-for-consecutive-byte/10831400

@theuni
> ... MSVC (an unsupported ... compiler)...

Since when?

✅ fanquake closed an issue: "P2TR Spending Bug - Signing Transaction Failed"
(https://github.com/bitcoin/bitcoin/issues/31589)

🚀 fanquake merged a pull request: "descriptors: Try pubkeys of both parities when retrieving the private keys for an xonly pubkey in a descriptor"
(https://github.com/bitcoin/bitcoin/pull/31590)

💬 hebasto commented on pull request "depends: Override default build type for `libevent`":
(https://github.com/bitcoin/bitcoin/pull/31661#issuecomment-2604352308)
> This change is not deterministic.

Thanks! Should be fixed now.

📝 maflcko opened a pull request: " test: Check that reindex with prune wipes blk files "
(https://github.com/bitcoin/bitcoin/pull/31696)
This adds missing test coverage for `CleanupBlockRevFiles`.