💬 maflcko commented on pull request "ci: Skip read-write of default env vars":
(https://github.com/bitcoin/bitcoin/pull/31678#issuecomment-2650891965)
> My understanding is that #28138 was only needed for `DANGER_RUN_CI_ON_HOST`. That's why removing it here isn't a regression to pre 28138, correct?

I do not know if they were ever needed to be set inside the CI pod, given that they fall back to the default anyway (see pull description). This may have just been added out of caution, as on the outside the pod, they are required sometimes to be set, see the docs:

https://github.com/bitcoin/bitcoin/blame/86528937e5c4da2e12c46085fc41e87ed75925
...

💬 eval-exec commented on pull request "Limit retries in GetRNDRRS to avoid infinite loop":
(https://github.com/bitcoin/bitcoin/pull/31826#discussion_r1950895954)
Thank you, updated.

💬 0xB10C commented on pull request "ci: Skip read-write of default env vars":
(https://github.com/bitcoin/bitcoin/pull/31678#issuecomment-2650904601)
Makes sense, thanks.

ACK fa952acdb6ef1b07b7927202d1373fa7479fd5e4

💬 davidgumberg commented on pull request "build: Make config warnings fatal if -DWCONFIGURE_ERROR=ON":
(https://github.com/bitcoin/bitcoin/pull/31665#issuecomment-2650954493)
> > Summarizing #31476, the CentOS CI task has been configuring a build with python `3.9` (the latest version in the Stream 9 core repo) which is below the minimum version of `3.10`
>
> I don't think this is entirely right. appstream ships with 3.11 and 3.12 on centos. (You can install them just like you can install g++-14 on centos 9).

By 'core repo' I meant what I guess CentOS calls the 'BaseOS' repository, fixed in the description.

> > Making this matter a bit worse, we now also [use
...

💬 vasild commented on pull request "Add waitNext() to BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/31283#discussion_r1950980811)
> it's possible to start the node and quickly shut it down. If that happens then `m_tip_block` remains null.
> If you call `createNewBlock()` during that time I'm not sure what happens

The RPC is not yet serving requests when [`m_tip_block` is assigned a value](https://github.com/bitcoin/bitcoin/blob/86528937e5c4da2e12c46085fc41e87ed759258e/src/node/kernel_notifications.cpp#L53-L58), but IPC is started before that. I put a long sleep just before assigning to `m_tip_block` but I am not sure h
...

💬 sipa commented on pull request "random: Check `GetRNDRRS` is supported in `InitHardwareRand` to avoid infinite loop":
(https://github.com/bitcoin/bitcoin/pull/31826#discussion_r1951006504)
There is one difference, in that `SeedHardwareFast` only tries to get 64 bits of entropy, while `SeedHardwareSlow` tries to get a full (and fresh) 256 bits, if possible. But without the rndrrs instruction, the difference isn't all that significant (especially as `SeedSlow` also gets 256 bit of entropy from the OS.

💬 Sjors commented on pull request "Add waitNext() to BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/31283#discussion_r1951009074)
> it's possible

In theory, I don't think this is easy to hit in practice. And #31785 should fix it entirely. You could try modifying `bitcoin-mine` in #30437.

⚠️ hebasto opened an issue: "Wallet symlinks are not rejected as expected"
(https://github.com/bitcoin/bitcoin/issues/31842)
Since https://github.com/bitcoin/bitcoin/pull/10885, `bitcoind` is expected to reject symlinks to wallets. This behaviour is also intended to be tested in the following test:https://github.com/bitcoin/bitcoin/blob/86528937e5c4da2e12c46085fc41e87ed759258e/test/functional/wallet_multiwallet.py#L168-L170

However, this is not the case:
```
$ ls -l ~/.bitcoin/regtest/wallets/
total 3
drwx------ 2 hebasto hebasto 3 Feb 11 14:49 w8
lrwxrwxrwx 1 hebasto hebasto 41 Feb 11 14:11 w8_symlink_abs -> /home/
...

⚠️ fanquake opened an issue: "build: `-static-pie` builds no-longer working with CMake"
(https://github.com/bitcoin/bitcoin/issues/31843)
Using 28.x, it's easy to produce a `-static-pie` binary doing:
```bash
# aarch64 alpine, 28.x (32efe850438ef22e2de39e562af557872a402c31)
make -C depends/ NO_QT=1 NO_WALLET=1 NO_ZMQ=1 NO_USDT=1 NO_UPNP=1 NO_NATPMP=1 LDFLAGS="-static-pie"
./autogen.sh
CONFIG_SITE=/bitcoin/depends/aarch64-unknown-linux-musl/share/config.site ./configure
make src/bitcoind
# link line
libtool: link: /usr/bin/ccache g++ -std=c++20 <snip> -fPIE <snip> -pie -static-pie -o bitcoind bitcoind-bitcoind.o ....
# file src/bit
...

🤔 laanwj reviewed a pull request: "random: Check `GetRNDRRS` is supported in `InitHardwareRand` to avoid infinite loop"
(https://github.com/bitcoin/bitcoin/pull/31826#pullrequestreview-2609096562)
Code review ACK a2ae62fb4db5e014c03a7ad41ba94ca49fd3806e
i do not have RNDR-supporting hardware to test this

💬 sipa commented on pull request "random: Check `GetRNDRRS` is supported in `InitHardwareRand` to avoid infinite loop":
(https://github.com/bitcoin/bitcoin/pull/31826#discussion_r1951059294)
Nit: the entire loop can be skipped here (replace `if (g_rndr_supported) {` with `if (g_rndrrs_support) {`).

💬 laanwj commented on pull request "random: Check `GetRNDRRS` is supported in `InitHardwareRand` to avoid infinite loop":
(https://github.com/bitcoin/bitcoin/pull/31826#discussion_r1951061433)
Eh yes, that would be better.

💬 eval-exec commented on pull request "random: Check `GetRNDRRS` is supported in `InitHardwareRand` to avoid infinite loop":
(https://github.com/bitcoin/bitcoin/pull/31826#discussion_r1951069695)
Yes, Updated.

💬 sipa commented on pull request "random: Check `GetRNDRRS` is supported in `InitHardwareRand` to avoid infinite loop":
(https://github.com/bitcoin/bitcoin/pull/31826#issuecomment-2651173341)
Code review ACK 585aba6eec858e5b1411ae9a8684ef8f82a7e435. I have not tested this.

💬 mzumsande commented on pull request "test: add missing sync to p2p_tx_download.py":
(https://github.com/bitcoin/bitcoin/pull/31837#discussion_r1951078538)
didn't get to it before merge.

>not many remaining cases

well, I count ~130 still, so definitely some churn involved changing most of these. Still, it would make sense to me.

💬 laanwj commented on pull request "random: Check `GetRNDRRS` is supported in `InitHardwareRand` to avoid infinite loop":
(https://github.com/bitcoin/bitcoin/pull/31826#issuecomment-2651183942)
re-ACK a2ae62fb4db5e014c03a7ad41ba94ca49fd3806e

i could test it on amazon graviton again like i did the original PR, but only the good-weather case, that won't reproduce the edge case that led to this.

Would be good if @giahuy98 (who opened #31817) could test it on the specific target hardware.

💬 instagibbs commented on pull request "test: add missing sync to p2p_tx_download.py":
(https://github.com/bitcoin/bitcoin/pull/31837#discussion_r1951081533)
seems to cause quite a few nuisance issues in tests, may be worth it going forward

💬 furszy commented on pull request "descriptors: inference process, do not return unparsable multisig descriptors":
(https://github.com/bitcoin/bitcoin/pull/31404#discussion_r1951082137)
That's a more delicate topic. `InferScript` can return `nullptr` in valid scenarios too. It shouldn't just fail in every case as you're doing there.

For example, if the script is a P2SH output and the provider lacks the redeem script (watch-only scenario), the expected behavior is to return a `addr()`. But with your changes, it would return `nullptr instead.

We need a different way to signal errors from inner `InferScript()` calls. Could be a boolean or.. a different return object like a
...

💬 sipa commented on pull request "descriptors: inference process, do not return unparsable multisig descriptors":
(https://github.com/bitcoin/bitcoin/pull/31404#discussion_r1951097542)
Rather than making the RPC output field optional, can we not fallback to always returning a `raw` / `addr` descriptor if inferring returns something that would be invalid?

💬 ryanofsky commented on issue "wallet: wrong balance and crash after reorg and unclean shutdown":
(https://github.com/bitcoin/bitcoin/issues/31824#issuecomment-2651239390)
> I actually implemented this in [#25297](https://github.com/bitcoin/bitcoin/pull/25297), three years ago :). Will see how much work is needed to revive it next week.

@furszy since I mentioned it the other day, this is a link to the [`PtrOrValue`](https://github.com/ryanofsky/libmultiprocess/blob/ce4814f46d3c38d0bdd08df2253bc69b084f22ee/include/mp/util.h#L136-L148) utility I used in libmultiprocess to allow functions to lock internally or optionally accept an external lock, without requiring tw
...