💬 stickies-v commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r1945424649)
I'm not sure if this is the best approach. `-DBUILD_KERNEL_LIB=ON -DBUILD_KERNEL_TEST=ON` should imo build the tests even if `-DBUILD_TESTS=OFF`. I think an approach where we update `src/CMakeLists.txt` with the below makes more sense (quick sketch)?

```cmake
if(BUILD_KERNEL_LIB)
add_subdirectory(kernel)
if (BUILD_KERNEL_TEST)
add_subdirectory(test/kernel)
endif()
endif()
```

💬 theuni commented on pull request "cmake: Set top-level target output locations":
(https://github.com/bitcoin/bitcoin/pull/31161#issuecomment-2641008225)
> When would we remove these symlinks? This seems like overkill, and just deferring breakage until later? CMake is already a huge disruption, and it's not clear that we need to add even more workarounds to accomodate a few developers before we've even managed to actually fully make the changeover. Some infra/script/fuzzing etc will need to be updated, but this all seems maangeable.

I agree with this. We've not really attempted any autotools compatibility, I'm not sure why this would be an exc
...

💬 hodlinator commented on pull request "miniscript: account for all `StringType` variants in `Miniscriptdescriptor::ToString()`":
(https://github.com/bitcoin/bitcoin/pull/31734#discussion_r1945435466)
Causes [linter failure](https://github.com/bitcoin/bitcoin/pull/31734/checks?check_run_id=36770622181):
```
[08:12:43.311] Please use bracket syntax includes ("#include <foo.h>") instead of quote syntax includes:
[08:12:43.311] src/script/descriptor.cpp:#include "attributes.h"
```
```suggestion
#include <attributes.h>
```

📝 maflcko opened a pull request: "ci: Bump fuzz task timeout"
(https://github.com/bitcoin/bitcoin/pull/31814)
The fuzz task seems to be the most CPU intense task (going through GB of data through all fuzz inputs for all fuzz targets).

Normally, the task takes 44 minutes (example https://cirrus-ci.com/task/5077976091459584), but under higher load, it may take longer (https://cirrus-ci.com/task/5966231095738368).

I tried to move it to GHA to see how it compares, but it will be even slower there: https://github.com/maflcko/bitcoin-core-with-ci/actions/runs/13182526514/job/36796629409.

The CI machi
...

💬 hodlinator commented on pull request "miniscript: account for all `StringType` variants in `Miniscriptdescriptor::ToString()`":
(https://github.com/bitcoin/bitcoin/pull/31734#discussion_r1945468203)
I ran the *test/lint/test_runner/* runner (`COMMIT_RANGE="HEAD~3..HEAD" cargo run`) on my suggestion (which runs *ruff* if it's installed). It did not emit any errors, so I think you should be fine.

Couldn't find the precise rule against my suggestion, but Q003 seems like it would be in favor of it: https://docs.astral.sh/ruff/rules/#flake8-pytest-style-pt

💬 Randy808 commented on issue "nSequence is not set when spending from satisfiable descriptor with relative timelock":
(https://github.com/bitcoin/bitcoin/issues/31808#issuecomment-2641087658)
@sipa My original usecase was a decaying multisig using `thresh` like your "A 3-of-3 that turns into a 2-of-3 after 90 days" example on https://bitcoin.sipa.be/miniscript/ except I was doing a 2-of-2 that turned into a 1-of-2. In this case my wallet would have one of the private keys available at the time of spending with the `older` clause met. Also, to re-iterate, I can still spend the utxo if I construct the transaction using `createrawtransaction`, `signrawtransactionwithwallet`, and `sendra
...

💬 ryanofsky commented on pull request "cmake: Set top-level target output locations":
(https://github.com/bitcoin/bitcoin/pull/31161#issuecomment-2641101801)
> When would we remove these symlinks? This seems like overkill, and just deferring breakage until later?

> We've not really attempted any autotools compatibility

I don't see any reason to delete the symlinks in the future. And they are not created for compatibility with autotools but for compatibility with cmake defaults and compatibility with our own build.

Also, if we did decide to delete the symlinks in the future it would not be deferring the breakage but replacing **silent breaka
...

💬 ryanofsky commented on pull request "cmake: Set top-level target output locations":
(https://github.com/bitcoin/bitcoin/pull/31161#discussion_r1945504125)
What problem do you see this solving? Creating a symlink pointing at a target that does not exist should be fine. It will still point to the right place.

💬 ryanofsky commented on pull request "cmake: Set top-level target output locations":
(https://github.com/bitcoin/bitcoin/pull/31161#issuecomment-2641120104)
> Did anyone test this on Windows?

Of course would be nice to test this on windows but it should work there according to cmake release notes: https://cmake.org/cmake/help/v3.13/release/3.13.html?highlight=release%20notes#command-line

💬 hebasto commented on pull request "build, ci: Fix linking `bitcoin-chainstate.exe` to `bitcoinkernel.dll` on Windows":
(https://github.com/bitcoin/bitcoin/pull/31158#issuecomment-2641150108)
Converting to a draft after today's offline discussion with @theuni.

📝 hebasto converted_to_draft a pull request: "build, ci: Fix linking `bitcoin-chainstate.exe` to `bitcoinkernel.dll` on Windows"
(https://github.com/bitcoin/bitcoin/pull/31158)
On the master branch @ 9a7206a34e3179d7280de98a818a13374178ee7b, linking `bitcoin-chainstate.exe` to `bitcoinkernel.dll` fails:
```
> cmake -B build --preset vs2022-static -DBUILD_UTIL_CHAINSTATE=ON -DBUILD_KERNEL_LIB=ON -DBUILD_SHARED_LIBS=ON
> cmake --build build --config Release -t bitcoin-chainstate
...
LINK : fatal error LNK1181: cannot open input file 'kernel\Release\bitcoinkernel.lib' [C:\Users\hebasto\source\repos\bitcoin\build-static\src\bitcoin-chainstate.vcxproj]
```

This PR
...

🤔 fjahr reviewed a pull request: "contrib: fix `test_deterministic_coverage.sh` script for out-of-tree builds"
(https://github.com/bitcoin/bitcoin/pull/31588#pullrequestreview-2600123724)
Concept ACK

💬 fjahr commented on pull request "contrib: fix `test_deterministic_coverage.sh` script for out-of-tree builds":
(https://github.com/bitcoin/bitcoin/pull/31588#discussion_r1945553930)
In `gen-manpages.py` and `gen-bitcoin-conf.sh` we use `BUILDDIR` fwiw.

💬 fjahr commented on pull request "contrib: fix `test_deterministic_coverage.sh` script for out-of-tree builds":
(https://github.com/bitcoin/bitcoin/pull/31588#discussion_r1945549602)
I agree, hardcoding a different path isn't much of a fix even if it's the one we use in all our examples. Better to have a way to pass the path in as a parameter.

💬 Willrok91 commented on issue "Generalized fee bumping":
(https://github.com/bitcoin-core/gui/issues/822#issuecomment-2641233162)
bc1q7ppm2rxrw73heugq334msxcmxu5xfr7eh37ne7

💬 Willrok91 commented on issue "Send: ability to (re)view automatically selected coins ":
(https://github.com/bitcoin-core/gui/issues/778#issuecomment-2641236859)
bc1q7ppm2rxrw73heugq334msxcmxu5xfr7eh37ne7

💬 yancyribbens commented on pull request "test: Add expected result assertions":
(https://github.com/bitcoin/bitcoin/pull/31656#issuecomment-2641303013)
> It might be good to also adjust the title of this test for CoinGrinder to "3) Test selection when some selections with sufficient funds would exceed the maximum allowed weight"

I find the use of "selection" twice in the sentence doesn't add much clarity. Now with the added assertions, I feel like the tittle fits the test however I'm open to a better title.

> Not really. The weight-optimality of CoinGrinder solutions is shown per the fuzz target coin_grinder_is_optimal.

Interesting.
...

💬 jarolrod commented on pull request "Prepare "Open Transifex translations for v29.0" release step":
(https://github.com/bitcoin/bitcoin/pull/31809#issuecomment-2641784769)
post merge ack

💬 vasild commented on pull request "Split CConnman":
(https://github.com/bitcoin/bitcoin/pull/30988#discussion_r1945972101)
Done.

💬 vasild commented on pull request "Split CConnman":
(https://github.com/bitcoin/bitcoin/pull/30988#discussion_r1945976586)
No, we do not want to remove entries from `m_listen_permissions` or from `m_listen` when disconnecting a single peer. The entries in those two maps are per listening address. E.g. if we listen on `1.1.1.1:8333` and designate that every peer that connects to that address gets `permissions1` and listen on `2.2.2.2:8333` and peers that connect to that address get `permissions2`. This stays unchanged as peers connect and disconnect. We would want to remove entries from those maps if we stop listenin
...