💬 maflcko commented on issue "Networking tests fail on emulated big-endian systems":
(https://github.com/bitcoin/bitcoin/issues/31812#issuecomment-2640966745)
I suspect this is a macOS issue, because on my local CI system using Linux the s390x task was last reported to have passed on Feb 2nd.

💬 glozow commented on pull request "TxOrphanage: account for size of orphans and count announcements":
(https://github.com/bitcoin/bitcoin/pull/31810#discussion_r1945412927)
done

💬 glozow commented on pull request "TxOrphanage: account for size of orphans and count announcements":
(https://github.com/bitcoin/bitcoin/pull/31810#discussion_r1945413178)
good point, done

💬 hebasto commented on issue "Release Schedule for 29.0":
(https://github.com/bitcoin/bitcoin/issues/31029#issuecomment-2640982819)
> ## 2025-02-06 🚧
>
> * Open Transifex translations for `v29.0`
>
> * Soft translation string freeze (no large or non-critical string changes until release)
>
> * Finalize and close translations for `v27.0`

First set of tasks completed.

See the announcement on Transifex: https://app.transifex.com/bitcoin/communication/d:df052ee8-69d4-4739-a529-ba835673a9f0/.

👍 theuni approved a pull request: "depends: Avoid using the `-ffile-prefix-map` compiler option"
(https://github.com/bitcoin/bitcoin/pull/31800#pullrequestreview-2599895291)
utACK 407062f2ac93624f350e9e8a4f641c882a2aaf2f

👍 hodlinator approved a pull request: "logging: Ensure -debug=0/none behaves consistently with -nodebug"
(https://github.com/bitcoin/bitcoin/pull/31767#pullrequestreview-2599809742)
ACK 1f4aff75c308226d58a45460982f75287e15cb81

### Concept

Having `-debug=0` and `-debug=none` perform the same effect as `-nodebug` in discarding all preceding categories makes more sense, instead of disabling all categories including the ones following them.

### Testing

As well as running the new test as-is, I also verified stated behavior of `-nodebug` prior to the C++ change by modifying included test.

### Suggestions

Only found nits, the `last_negated` calculation in particu
...

💬 hodlinator commented on pull request "logging: Ensure -debug=0/none behaves consistently with -nodebug":
(https://github.com/bitcoin/bitcoin/pull/31767#discussion_r1945367933)
nit: I tried to replicate what [ChatGPT suggested to ryanofsky](https://github.com/bitcoin/bitcoin/pull/30529#discussion_r1930956563), but `std::views` wouldn't compile (didn't look into modifying my setup). Was able to combine it with your version though into:
```suggestion
auto last_negated = std::find_if(categories.rbegin(), categories.rend(),
[](const std::string& cat) { return cat == "0" || cat == "none"; });
```

💬 hodlinator commented on pull request "logging: Ensure -debug=0/none behaves consistently with -nodebug":
(https://github.com/bitcoin/bitcoin/pull/31767#discussion_r1945369539)
mega-nit: I find this marginally nicer, but still not great:
```suggestion
for (auto it = last_negated == categories.rend() ? categories.begin() : last_negated.base();
it != categories.end(); ++it) {
if (!LogInstance().EnableCategory(*it)) {
return util::Error{strprintf(_("Unsupported logging category %s=%s."), "-debug", *it)};
```

💬 hodlinator commented on pull request "logging: Ensure -debug=0/none behaves consistently with -nodebug":
(https://github.com/bitcoin/bitcoin/pull/31767#discussion_r1945371836)
nit: I tested breaking out an initial commit which only adds the test for `-nodebug` to prove existing behavior, before the commit performing the C++ change and adding `0` + `none` tests (it passed). You might want to rewrite the commit history to do that as well, but I'm okay with the current approach.

💬 hodlinator commented on pull request "logging: Ensure -debug=0/none behaves consistently with -nodebug":
(https://github.com/bitcoin/bitcoin/pull/31767#discussion_r1945373795)
nit: Could add invalid `abc` category prior to disabling, and verify that *multiple* categories after the disabling work.
```suggestion
self.restart_node(0, ['-debug=http', '-debug=abc', debug_opt, '-debug=rpc', '-debug=net'])
logging = self.nodes[0].logging()
assert logging['rpc']
assert logging['net']
```

💬 fanquake commented on pull request "cmake: Set top-level target output locations":
(https://github.com/bitcoin/bitcoin/pull/31161#issuecomment-2640989842)
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.

> Also, the suggested commit might not work on Windows, where symbolic links are not enabled by default.

Did an
...

💬 davidgumberg commented on pull request "wallet: Utilize IsMine() and CanProvide() in migration to cover edge cases":
(https://github.com/bitcoin/bitcoin/pull/31495#discussion_r1945420839)
nit: could also drop the checks below in `test_no_privkeys` for `watchonly0` and `watchonly1` (R411, R442)

```diff
@@ -408,9 +408,7 @@ class WalletMigrationTest(BitcoinTestFramework):
_, watchonly0 = self.migrate_and_get_rpc("watchonly0")
assert_equal("watchonly0_watchonly" in self.master_node.listwallets(), False)
info = watchonly0.getwalletinfo()
- assert_equal(info["descriptors"], True)
assert_equal(info["private_keys_enabled"], False)
-
...

💬 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.