💬 glozow commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1734817526)
Right. I'm also building my orphan resolution branch that effectively skips adding orphans for peers that aren't "registered" (we need to know their connection info to decide some params). These tests started failing there, so I had to add `ConnectPeer`. Figured it makes sense to do this now.

📝 maflcko opened a pull request: "ci: Re-add configs removed in cmake migration"
(https://github.com/bitcoin/bitcoin/pull/30740)
In commit 9730288a0cd3f33021ef00fb2d95e5216d10ab61 many configs were removed from the CI without explanation.

Fix it by adding them back.

Can be reviewed by looking at:

* the parity table https://gist.github.com/hebasto/2ef97d3a726bfce08ded9df07f7dab5e
* the installed packages
* the CI logs from before the cmake migration and the CI logs of this pull request

💬 maflcko commented on pull request "build: Introduce CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/30454#issuecomment-2315591429)
ci regressions fixed in https://github.com/bitcoin/bitcoin/pull/30740

✅ glozow closed an issue: "descriptors: represent multiple derivation paths within one descriptor"
(https://github.com/bitcoin/bitcoin/issues/17190)

🚀 glozow merged a pull request: "descriptors: Be able to specify change and receiving in a single descriptor string"
(https://github.com/bitcoin/bitcoin/pull/22838)

📝 fanquake converted_to_draft a pull request: "guix: (explicitly) build Linux GCC with `--enable-cet`"
(https://github.com/bitcoin/bitcoin/pull/30438)
Similar to #29695, and in the same vein of explicitly configuring hardening options in our release toolchain.

See https://gcc.gnu.org/install/configure.html:

>` --enable-cet`

> Enable building target run-time libraries with control-flow instrumentation, see `-fcf-protection option`. When --enable-cet is specified target libraries are configured to add `-fcf-protection` and, if needed, other target specific options to a set of building options.

> `--enable-cet=auto` is default. CET is
...

👋 fanquake's pull request is ready for review: "guix: Bump time machine to 53396a22afc04536ddf75d8f82ad2eafa5082725"
(https://github.com/bitcoin/bitcoin/pull/30730)

💬 maflcko commented on pull request "test: Fix RANDOM_CTX_SEED use with parallel tests":
(https://github.com/bitcoin/bitcoin/pull/30737#discussion_r1734852679)
This isn't set in the fuzz tests, IIRC. So, as explained in https://github.com/bitcoin/bitcoin/issues/30696#issuecomment-2307116476 it would fail if there were two fuzz tests that set up a (Basic)TestingSetup each. No?

💬 martinsaposnic commented on pull request "Use MiniWallet in functional test rpc_signrawtransactionwithkey.":
(https://github.com/bitcoin/bitcoin/pull/30701#discussion_r1734854686)
@theStack what do you think?

💬 maflcko commented on pull request "test: Fix RANDOM_CTX_SEED use with parallel tests":
(https://github.com/bitcoin/bitcoin/pull/30737#discussion_r1734856457)
Also, obviously, the issue would remain if the same test is run in parallel

💬 maflcko commented on pull request "test: [refactor] Use m_rng directly":
(https://github.com/bitcoin/bitcoin/pull/30571#issuecomment-2315614557)
Is this test-only refactor (and small bugfix) with two acks rfm, or does it need more review?

📝 l0rinc opened a pull request: "Update documentation generation example in developer-notes.md"
(https://github.com/bitcoin/bitcoin/pull/30741)
Moved over from https://github.com/hebasto/bitcoin/pull/337

In [the other readmes](https://github.com/bitcoin/bitcoin/blob/6ce50fd9d0ae6850d54bf883e7a7c1bcb6912c5c/src/test/README.md?plain=1#L19) we've provided a default build directory instead, unified the `developer-notes.md` to specify it explicitly.

In the next commit I've used this default to go over each reference to our binaries and changed their in-source references to the build directory.
Some of these changes were in example out
...

💬 marcofleon commented on pull request "test: [refactor] Use m_rng directly":
(https://github.com/bitcoin/bitcoin/pull/30571#issuecomment-2315625892)
In the process of reviewing now.

💬 fanquake commented on pull request "Update documentation generation example in developer-notes.md":
(https://github.com/bitcoin/bitcoin/pull/30741#discussion_r1734870234)
Not sure that making this less generic is an improvement?

💬 maflcko commented on pull request "build: Add option to use C++23 for testing":
(https://github.com/bitcoin/bitcoin/pull/30735#issuecomment-2315636061)
Actually I am not sure on the suggestion on a second thought. Users shouldn't be able to modify this at all, because building with anything other than C++20 is not supported and will lead to a broken result. The goal here is to only allow C++23 for testing.

💬 fanquake commented on pull request "build: Add option to use C++23 for testing":
(https://github.com/bitcoin/bitcoin/pull/30735#issuecomment-2315639969)
Looks like this also runs into: https://github.com/bitcoin-core/leveldb-subtree/issues/43.

💬 l0rinc commented on pull request "Update documentation generation example in developer-notes.md":
(https://github.com/bitcoin/bitcoin/pull/30741#discussion_r1734875898)
Seems to me that was the way we've preceded before: https://github.com/bitcoin/bitcoin/blob/6ce50fd9d0ae6850d54bf883e7a7c1bcb6912c5c/src/test/README.md?plain=1#L19

And since we need to update the build directories in the commits (at least that's what I tried doing here), we need a sensible default.

Do you have a better suggestion?

💬 fanquake commented on pull request "Update documentation generation example in developer-notes.md":
(https://github.com/bitcoin/bitcoin/pull/30741#discussion_r1734875943)
This isn't correct. We want `./src/`.

💬 ismaelsadeeq commented on pull request "Use MiniWallet in functional test rpc_signrawtransactionwithkey.":
(https://github.com/bitcoin/bitcoin/pull/30701#discussion_r1734876179)
> It's failing because `CTxOut`'s `nValue` is an `int`, so when you pass a `float`, the `to_bytes` method throws an error.

I can also verify that this fails locally.

I'm generally not a fan of magic numbers, but the obvious explanation is that some satoshis were subtracted from the UTXO value to account for transaction fees.

If that's the case, then it's fine to round it down.

> I'm happy to create a separate issue for it and tackle it.

+1

💬 maflcko commented on pull request "build: Add option to use C++23 for testing":
(https://github.com/bitcoin/bitcoin/pull/30735#issuecomment-2315643568)
> Looks like this also runs into: [bitcoin-core/leveldb-subtree#43](https://github.com/bitcoin-core/leveldb-subtree/issues/43).

Force pushed trivial CI fixup to temporarily work around the issue for now.