💬 hebasto commented on pull request "build: Introduce CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/30454#issuecomment-2311866895)
> doc/developer-notes.md needs to be updated too.

@l0rinc has already started to work on it in https://github.com/hebasto/bitcoin/pull/337. It will be converted in a follow up PR once this PR is merged.

💬 hebasto commented on pull request "build: Introduce CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1732369832)
Correct. Will be updated in a follow up PR, along with a few other amendments such as https://github.com/bitcoin/bitcoin/pull/30712.

🤔 glozow reviewed a pull request: "Pre-28.x branch off version bump and doc updates"
(https://github.com/bitcoin/bitcoin/pull/30719#pullrequestreview-2262535007)
lgtm, should also do doc/release-notes/release-notes-27064.md

💬 TheCharlatan commented on pull request "bench: [refactor] iwyu":
(https://github.com/bitcoin/bitcoin/pull/30716#issuecomment-2311898883)
Still seems to be:
```
bench/bench.h should add these lines:
#include "nanobench.h" // for Bench (ptr only)

bench/bench.h should remove these lines:

The full include-list for bench/bench.h:
#include <bench/nanobench.h>
#include <util/fs.h> // for path
#include <util/macros.h> // for PASTE2, STRINGIZE
#include <chrono> // for milliseconds
#include <cstdint> // for uint8_t
#include <functional> // for function
#include <map> // for map
#inclu
...

💬 ismaelsadeeq commented on pull request "Use MiniWallet in functional test rpc_signrawtransactionwithkey.":
(https://github.com/bitcoin/bitcoin/pull/30701#discussion_r1732382841)
nit: add spaces between passed arguments
```suggestion
tx = self.wallet.send_to(from_node=self.nodes[0], scriptPubKey=script_pub_key, amount=int(amount * COIN))
```

💬 ismaelsadeeq commented on pull request "Use MiniWallet in functional test rpc_signrawtransactionwithkey.":
(https://github.com/bitcoin/bitcoin/pull/30701#discussion_r1732394215)
8c64048c443cddce201b6d54184b32f3894ab806 can be squashed together with the first commit

💬 ismaelsadeeq commented on pull request "Use MiniWallet in functional test rpc_signrawtransactionwithkey.":
(https://github.com/bitcoin/bitcoin/pull/30701#discussion_r1732387713)
It might be better to return a tuple here
```suggestion
return tx["txid"], tx["sent_vout"]
```

💬 ismaelsadeeq commented on pull request "Use MiniWallet in functional test rpc_signrawtransactionwithkey.":
(https://github.com/bitcoin/bitcoin/pull/30701#discussion_r1732385781)
Why are you converting the amount to an integer here? Won’t that change the assumptions in the test? I see 49.999 explicitly passed in the test—don’t we want to maintain precision?

💬 hebasto commented on pull request "build: Introduce CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/30454#issuecomment-2311917823)
> I was surprised Qt wasn't at least enabled by default.

The default build options were tailored to meet the needs of the majority of this repository's contributors.

> > The "auto" value is no longer available; non-default values must be specified explicitly.
>
> I assume this is a CMake standard practice?

It is.

> Or is there some other reason to not autodetect bdb/portmapping/qt/zmq?

Package autodetection can introduce unsolicited changes in the build configuration, which is
...

💬 hodlinator commented on pull request "bench: [refactor] iwyu":
(https://github.com/bitcoin/bitcoin/pull/30716#issuecomment-2311922032)
Concept ACK

Thanks for taking the initiative on this! (Should hopefully free contributors from considering `#include`s when reviewing, recent example of me doing so: https://github.com/bitcoin/bitcoin/pull/30571#pullrequestreview-2253654936).

💬 glozow commented on pull request "chainparams: Remove seed.bitcoinstats.com":
(https://github.com/bitcoin/bitcoin/pull/30720#issuecomment-2311926209)
lgtm looking at discussion on #29911

cc @cdecker

💬 cdecker commented on pull request "chainparams: Remove seed.bitcoinstats.com":
(https://github.com/bitcoin/bitcoin/pull/30720#issuecomment-2311931636)
I'm in the process of fixing the seeder. It should be back online by the end of the week, if that's acceptable

💬 hebasto commented on pull request "Pre-28.x branch off version bump and doc updates":
(https://github.com/bitcoin/bitcoin/pull/30719#issuecomment-2311933156)
> > This should be post-branching...?
>
> I don't think it particularly matters since that step has to be done in the 28.x branch too, in addition to the other version numbers being bumped.

During the previous release cycle, it was done just _after_ the branching off.

👍 TheCharlatan approved a pull request: "bench: [refactor] iwyu"
(https://github.com/bitcoin/bitcoin/pull/30716#pullrequestreview-2262609868)
ACK fab0e834b8cf906c286ebbeed86eca8d65854f75

💬 hodlinator commented on issue "ci: ConnectionRefusedError: [WinError 10061] No connection could be made because the target machine actively refused it":
(https://github.com/bitcoin/bitcoin/issues/30390#issuecomment-2311947739)
(Btw, b22529529823c0cb5916ac318c8536e9107b7e78 did touch code raising `JSONRPCException` `-342` in the beginning of June. Don't see how that change would cause issues, but if it correlates with when you started seeing this, maybe it could be?)

💬 hodlinator commented on pull request "bench: [refactor] iwyu":
(https://github.com/bitcoin/bitcoin/pull/30716#issuecomment-2311968223)
ACK fab0e834b8cf906c286ebbeed86eca8d65854f75

https://cirrus-ci.com/task/4676750536343552 log only shows **bench/** issues with **bench/bench.h** itself, [as TheCharlatan previously mentioned](https://github.com/bitcoin/bitcoin/pull/30716#issuecomment-2311898883).

💬 hebasto commented on pull request "bench: [refactor] iwyu":
(https://github.com/bitcoin/bitcoin/pull/30716#issuecomment-2311972969)
> Still seems to be:
>
> ```
> bench/bench.h should add these lines:
> #include "nanobench.h" // for Bench (ptr only)
>
> bench/bench.h should remove these lines:
>
> The full include-list for bench/bench.h:
> #include <bench/nanobench.h>
> #include <util/fs.h> // for path
> #include <util/macros.h> // for PASTE2, STRINGIZE
> #include <chrono> // for milliseconds
> #include <cstdint> // for uint8_t
> #include <functional> // for function
> #include <
...

👍 hebasto approved a pull request: "bench: [refactor] iwyu"
(https://github.com/bitcoin/bitcoin/pull/30716#pullrequestreview-2262651379)
ACK fab0e834b8cf906c286ebbeed86eca8d65854f75.

📝 hodlinator opened a pull request: "refactor: Testnet4 - Replace uint256S("str") -> uint256{"str"}"
(https://github.com/bitcoin/bitcoin/pull/30721)
Ran scripted-diff from 2d9d752e4fc30aabf2ddd36ca7a63432d26d4fea:
```
sed -i --regexp-extended -e 's/\buint256S\("(0x)?([^"]{64})"\)/uint256{"\2"}/g' $(git grep -l uint256S)
```

Follow-up to Testnet4 introduction #29775 which overlapped with work on `uint256` `consteval` ctor #30560 (the latter includes the scripted-diff commit).

Going forward `uint256{}` should be used for constants instead of `uint256S()`.

💬 maflcko commented on issue "ci: ConnectionRefusedError: [WinError 10061] No connection could be made because the target machine actively refused it":
(https://github.com/bitcoin/bitcoin/issues/30390#issuecomment-2312002464)
> This could potentially be an issue between two different tests as well, if they shut down/start up quickly enough to still be within the TCP `TIME_WAIT` period.

I don't think different tests affect each other, because the test_runner assigns non-overlapping spans of ports for each test case (rpc, p2p, ...).

> Edit: I guess TIME_WAIT does not directly explain why connection still fails after 2400s. But it could be a contributing factor.

I don't think Bitcoin Core will re-try to start t
...