💬 purpleKarrot commented on pull request "cmake: Fix `-pthread` flags in summary":
(https://github.com/bitcoin/bitcoin/pull/31724#issuecomment-2668512824)
There are two general problems with printing a build summary at configure time:

1. It is annoying visual clutter. The cmake configure log should contain status events and important warnings as well as errors. Detailed settings can be analyzed and modified in the cache editor such as `ccmake`. Currently, when using `ccmake`, this dreaded summary has to be dismissed with `e` after each configure run with `c`. This is because make treats the summary as a warning.

2. There is no way to know al
...

📝 maflcko opened a pull request: "test: Assert unused port to debug intermittent issue 30030"
(https://github.com/bitcoin/bitcoin/pull/31903)
`p2p_i2p_ports.py` is intermittently failing in tests on Cirrus, GHA, and locally.

However, the reason is unclear.

Assert that the port is unused to possibly get more details for further actions.

💬 maflcko commented on issue "Intermittent issue in p2p_i2p_ports.py AssertionError: [node 0] Expected messages "['Error connecting to [...].b32.i2p:0: Cannot connect to 127.0.0.1:60000']" does not partially match log:":
(https://github.com/bitcoin/bitcoin/issues/30030#issuecomment-2668550971)
Not sure what to do here. Let's print the offending program name? See https://github.com/bitcoin/bitcoin/pull/31903

💬 Sjors commented on pull request "Have createNewBlock() wait for tip, make rpc handle shutdown during long poll and wait methods":
(https://github.com/bitcoin/bitcoin/pull/31785#issuecomment-2668567278)
Four CI machines consistently failing `rpc_blockchain.py` (either `--v1transport` or `--v2transport`) at:

```py
def assert_waitforheight(height, timeout=2):
assert_equal(
node.waitforblockheight(height=height, timeout=timeout)['height'],
current_height)
```

With "Remote end closed connection without response", but not much useful info otherwise. Can't reproduce locally yet.

💬 maflcko commented on pull request "Have createNewBlock() wait for tip, make rpc handle shutdown during long poll and wait methods":
(https://github.com/bitcoin/bitcoin/pull/31785#discussion_r1961627062)
```suggestion
BlockRef current_block{*CHECK_NONFATAL(miner.getTip())};
// Abort if RPC came out of warmup too early

```

There is no need for optional, if the value can never be nullopt

💬 hodlinator commented on pull request "doc: clarify loadwallet path loading for wallets":
(https://github.com/bitcoin/bitcoin/pull/30302#discussion_r1961635509)
I think "DriveLetter:" is more confusing than "C:". This is an example, not some kind of catch-all pattern.

💬 hodlinator commented on pull request "doc: clarify loadwallet path loading for wallets":
(https://github.com/bitcoin/bitcoin/pull/30302#discussion_r1961638926)
(I'm fine with the "path\\to\\walletname"-part though).

💬 Sjors commented on pull request "Have createNewBlock() wait for tip, make rpc handle shutdown during long poll and wait methods":
(https://github.com/bitcoin/bitcoin/pull/31785#issuecomment-2668606129)
Replaced `NONFATAL_UNREACHABLE` with `CHECK_NONFATAL` again by avoiding the intermediate `std::optional`, see https://github.com/bitcoin/bitcoin/pull/31785#discussion_r1961627062

Rebased and pushed again, though I haven't figured out the CI issue.

💬 vasild commented on pull request "Have createNewBlock() wait for tip, make rpc handle shutdown during long poll and wait methods":
(https://github.com/bitcoin/bitcoin/pull/31785#issuecomment-2668649415)
> Remote end closed connection without response

That probably means `bitcoind` crashed.

> node0 stderr ... optional ... Assertion 'this->_M_is_engaged()' failed.

✅ maflcko closed a pull request: "test: Assert unused port to debug intermittent issue 30030"
(https://github.com/bitcoin/bitcoin/pull/31903)

💬 maflcko commented on pull request "test: Assert unused port to debug intermittent issue 30030":
(https://github.com/bitcoin/bitcoin/pull/31903#issuecomment-2668650948)
```
COMMAND PID USER FD TYPE DEVICE SIZE/OFF NODE NAME
[08:12:30.628] python3 50589 root 11u IPv4 179599797 0t0 TCP localhost:60000->localhost:19216 (ESTABLISHED)
[08:12:30.628] bitcoind 50594 root 21u IPv4 179704903 0t0 TCP localhost:19216->localhost:60000 (ESTABLISHED)

📝 l0rinc opened a pull request: "refactor: modernize outdated trait patterns using helper aliases (C++14)"
(https://github.com/bitcoin/bitcoin/pull/31904)
The use of [`std::underlying_type_t<T>`](https://en.cppreference.com/w/cpp/types/underlying_type) or [`std::is_enum_v<T>`](https://en.cppreference.com/w/cpp/types/is_enum) (and similar ones, introduced in C++14) replace the `typename std::underlying_type<T>::type` and `std::is_enum<T>::value` constructs (available in C++11).

The `_t` and `_v` helper alias templates offer a more concise way to extract the type and value directly.

I've modified the instances I found in the codebase one-by-o
...

⚠️ goodthebest opened an issue: "ERROR: AcceptBlockHeader: prev block not found"
(https://github.com/bitcoin/bitcoin/issues/31905)
I noticed in debug.log there are numerous such errors. I'm wondering is it meant to be on local server's blockchain storage or it's kind of network error? Blockchain apparently fully synced yet these errors?


2025-02-16T03:43:19Z ERROR: AcceptBlockHeader: prev block not found
2025-02-16T03:43:27Z ERROR: AcceptBlockHeader: prev block not found
2025-02-16T03:44:01Z ERROR: AcceptBlockHeader: prev block not found
2025-02-16T03:44:37Z ERROR: AcceptBlockHeader: prev block not found
2025-02-16T03:45:2
...

💬 eval-exec commented on pull request "random: move VerifyRNDRRS above InitHardwareRand":
(https://github.com/bitcoin/bitcoin/pull/31902#issuecomment-2668670182)
Why a `CI failed ` label is added?

🤔 rkrux reviewed a pull request: "wallet: fix crash on double block disconnection"
(https://github.com/bitcoin/bitcoin/pull/31757#pullrequestreview-2626627692)
I reviewed till df87cf2

I had thought about adding the functional test earlier but had ruled it out because I thought technically it's a bug fix and not a functionality per se.
But after reading this test, I'm in favour of this test, makes me think from the POV of what's happening to the node overall.

💬 rkrux commented on pull request "wallet: fix crash on double block disconnection":
(https://github.com/bitcoin/bitcoin/pull/31757#discussion_r1961582654)
Can add a comment here highlighting that this RPC call is added to ensure the block disconnected notification is handled by the wallet.

💬 rkrux commented on pull request "wallet: fix crash on double block disconnection":
(https://github.com/bitcoin/bitcoin/pull/31757#discussion_r1961643854)
Can add a comment here: `Disconnect tip again`

💬 rkrux commented on pull request "wallet: fix crash on double block disconnection":
(https://github.com/bitcoin/bitcoin/pull/31757#discussion_r1961641424)
```diff
+ # Abort process abruptly to force an unclean shutdown (no flush) so that the best block locator changes are not persisted
```

💬 rkrux commented on pull request "wallet: fix crash on double block disconnection":
(https://github.com/bitcoin/bitcoin/pull/31757#discussion_r1961637704)
This test passes in my system but one thing that's on my mind is that can this test ever become flaky? Can there be a scenario that the best block locator changes are written to the disk before the node is killed here? Do you think a time delay should be added before killing the node here?

💬 rkrux commented on pull request "wallet: fix crash on double block disconnection":
(https://github.com/bitcoin/bitcoin/pull/31757#discussion_r1961669843)
Might not hurt to check for wallet balance again. But more importantly it would be nice to check that the best block hash is not `tip` now like below. Should we stop the node again (cleanly this time) and check this on start?

```diff
+ assert_equal(node.getbestblockhash() == tip, False)
```