💬 sipa commented on issue "nSequence is not set when spending from satisfiable descriptor with relative timelock":
(https://github.com/bitcoin/bitcoin/issues/31808#issuecomment-2640347090)
We discussed this a bit off-line.

There are a number of issues here, and the simple ones can be solved in specialized cases, but the general problem is really a sign of a deeper issue.

The superficial problems are at least these, which stem from the specific order decisions are made: (1) coin selection, (2) transaction building, (3) input signing.
* Coin selection may be picking coins which are not (yet) spendable (assuming the intent is to spend right now, and not something to be presigned an
...

📝 instagibbs opened a pull request: "test: test_inv_block, use mocktime instead of waiting"
(https://github.com/bitcoin/bitcoin/pull/31811)
Performance issue reported in https://github.com/bitcoin/bitcoin/pull/31437#issuecomment-2640221382

It seems that code as-is waits for wall-clock time to pass to synchronize mempools. Locally, sometimes the subtest takes a couple seconds, sometimes it takes an additional minute.

Just use mocktime?

⚠️ l0rinc opened an issue: "Networking tests fail on emulated big-endian systems"
(https://github.com/bitcoin/bitcoin/issues/31812)
### Is there an existing issue for this?

- [x] I have searched the existing issues

### Current behaviour

Running the test with an emulated big-endian system (via `docker run --platform linux/s390x -it ubuntu:24.10 /bin/bash`, as described in https://github.com/bitcoin/bitcoin/issues/31344#issuecomment-2493612110), I'm seeing mismatched byte orders in IP address representations.
I've noticed this in https://github.com/bitcoin/bitcoin/pull/29852#issuecomment-2614010124, but it reproduces on mas
...

🤔 murchandamus reviewed a pull request: "Wallet: don't underestimate the fees when spending a Taproot output"
(https://github.com/bitcoin/bitcoin/pull/26573#pullrequestreview-2599346386)
Concept ACK

💬 murchandamus commented on pull request "Wallet: don't underestimate the fees when spending a Taproot output":
(https://github.com/bitcoin/bitcoin/pull/26573#discussion_r1945108666)
Do I understand right that this test will try the whole battery of different script trees seen below and uses the maximal estimate across script trees, so if there are multiple different leaves that we can satisfy, our transaction might overpay, but no longer underpay?

💬 darosior commented on issue "nSequence is not set when spending from satisfiable descriptor with relative timelock":
(https://github.com/bitcoin/bitcoin/issues/31808#issuecomment-2640490507)
> I have searched the existing issues

Hmm https://github.com/bitcoin/bitcoin/issues/27527. :)

> Transaction building may be setting nLockTime / nSequence values which aren't actually compatible with what we can sign for.

To add to this, the coin selection algorithm right now may even be selecting coins that are incompatible with each other: https://github.com/bitcoin/bitcoin/issues/27526.

💬 pinheadmz commented on pull request "Split CConnman":
(https://github.com/bitcoin/bitcoin/pull/30988#discussion_r1945120612)
if `WaitMany()` returns false, we would have already waited the duration of `SELECT_TIMEOUT` -- do we need to `sleep_for()` again?

📝 vijayabhaskar78 opened a pull request: "Fix MSVC ccache reporting (Fixes #31771)"
(https://github.com/bitcoin/bitcoin/pull/31813)

Fixes #31771

### Summary
This PR resolves the incorrect reporting of `ccache` usage for MSVC builds by:
1. Removing forced compiler launcher configuration
2. Improving status message accuracy
3. Documenting proper MSVC+ccache workflows

### Key Changes
- **Deleted**: `cmake/ccache.cmake` (Eliminates conflicts with user/CI configurations like `sccache`)
- **Updated**: `CMakeLists.txt` status reporting
```cmake
if(CMAKE_CXX_COMPILER_LAUNCHER)
message(STATUS "Com
...

💬 vijayabhaskar78 commented on pull request "Fix MSVC ccache reporting (Fixes #31771)":
(https://github.com/bitcoin/bitcoin/pull/31813#issuecomment-2640653595)
@hebasto Can you help me regarding this PR please

💬 glozow commented on pull request "mining: bugfix: Fix duplicate coinbase tx weight reservation":
(https://github.com/bitcoin/bitcoin/pull/31384#discussion_r1945226453)
Or have a test for the expected behavior? A comment in the options-handling code? I don't think it should be unspecified.

💬 marcofleon commented on pull request "fuzz: add targets for PCP and NAT-PMP port mapping requests":
(https://github.com/bitcoin/bitcoin/pull/31676#discussion_r1945193198)
Should the PCP target include IPv6 or no? Something like `(domain == AF_INET || domain == AF_INET6)` might improve the test.

💬 marcofleon commented on pull request "fuzz: add targets for PCP and NAT-PMP port mapping requests":
(https://github.com/bitcoin/bitcoin/pull/31676#discussion_r1945195855)
I don't see this used anywhere.

💬 marcofleon commented on pull request "fuzz: add targets for PCP and NAT-PMP port mapping requests":
(https://github.com/bitcoin/bitcoin/pull/31676#discussion_r1945222662)
The `ConsumeData` here was the culprit for the bad coverage.

Basically, this function needs to set `name_len` to 16 or 28 for the fuzzer to continue past `SetSockAddr` in `PCPRequestPortMap`. In the PCP case, `name_len` started here as 128 and then `ConsumeData` would look to return the min of 128 or the remaining bytes of the input. It would consume the rest of the input every time because the fuzzer was optimizing for the `SetSockAddr` check. So, there would always be nothing left in `fuzz
...

🤔 marcofleon reviewed a pull request: "fuzz: add targets for PCP and NAT-PMP port mapping requests"
(https://github.com/bitcoin/bitcoin/pull/31676#pullrequestreview-2599495421)
I ran `pcp_request_port_map` with the patch I discuss below and looks like coverage is good to go now.

https://marcofleon.github.io/coverage/pcp_request_port_map/coverage/root/bitcoin/src/common/pcp.cpp.html

💬 mzumsande commented on pull request "Double check all block rules in `ConnectBlock`, not only `CheckBlock`":
(https://github.com/bitcoin/bitcoin/pull/31792#issuecomment-2640677459)
Given that this will likely make IBD a bit slower due to repeated checks, I think there should be more tangible benefits than just simpler reasoning about edge cases - or maybe it would be good to explain these edge cases a bit more to be able to judge how relevant they are.

Once we are synced to the tip, block acception and connection typically happen right after each other in `ProcessNewBlock()`, so I can only think of of rather far-fetched scenarios for the repeated checks to become releva
...

💬 glozow commented on pull request "mining: bugfix: Fix duplicate coinbase tx weight reservation":
(https://github.com/bitcoin/bitcoin/pull/31384#discussion_r1945227927)
Meh good point miner policy is policy, resolving.

💬 darosior commented on pull request "Double check all block rules in `ConnectBlock`, not only `CheckBlock`":
(https://github.com/bitcoin/bitcoin/pull/31792#issuecomment-2640695928)
This becomes a bit likelier in IBD, but i agree. As i [said yesterday on IRC](https://bitcoin-irc.chaincode.com/bitcoin-core-dev/2025-02-05#1738786490-1738786524;) i am not sure this change is worth doing especially since we can never really solve the issue of a non-upgraded node having accepted invalid blocks short of re-connecting the whole chain when upgrading.

💬 darosior commented on pull request "Double check all block rules in `ConnectBlock`, not only `CheckBlock`":
(https://github.com/bitcoin/bitcoin/pull/31792#issuecomment-2640697895)
I'll try to bug @sdaftuar, who introduced this comment, to see if we are missing something.

💬 Prabhat1308 commented on pull request "Fix MSVC ccache reporting (Fixes #31771)":
(https://github.com/bitcoin/bitcoin/pull/31813#discussion_r1945244783)
There are a lot of changes in this file that are not required . Please adhere to not changing the code if not necessary. Please remove the extra whitespaces and syntax highlighting.

💬 Prabhat1308 commented on pull request "Fix MSVC ccache reporting (Fixes #31771)":
(https://github.com/bitcoin/bitcoin/pull/31813#discussion_r1945245975)
Similar irrelevant changes introduced in this file .