💬 fanquake commented on pull request "depends: build libevent with CMake":
(https://github.com/bitcoin/bitcoin/pull/29835#issuecomment-2178870629)
Guix Build (aarch64):
```bash
db8e2595bb6b4d7088860f1df0c952be13a4ae3db3396a0c3a42de98165529bc guix-build-a5180bffbe78/output/aarch64-linux-gnu/SHA256SUMS.part
e90d5520e9bec9efe81387012d6b9ddd189ed7f4737726d825cd8077c4c0b50b guix-build-a5180bffbe78/output/aarch64-linux-gnu/bitcoin-a5180bffbe78-aarch64-linux-gnu-debug.tar.gz
a0b5ee10285feddbc964a694c134dcd0bb9193065985423063a353c2e518442b guix-build-a5180bffbe78/output/aarch64-linux-gnu/bitcoin-a5180bffbe78-aarch64-linux-gnu.tar.gz
d49594
...

📝 luke-jr opened a pull request: "QA: Expect PACKAGE_NAME rather than constant "Bitcoin Core""
(https://github.com/bitcoin/bitcoin/pull/30308)
Followup to #29144

💬 dergoegge commented on pull request "test: Add Compact Block Encoding test `ReceiveWithExtraTransactions` covering non-empty `extra_txn`":
(https://github.com/bitcoin/bitcoin/pull/30237#discussion_r1646353890)
No need for the actual rng, we can just consume the nonce from the fuzz data.

```suggestion
CBlockHeaderAndShortTxIDs cmpctblock{*block, fuzzed_data_provider.ConsumeIntegral<uint64_t>()};
```

💬 kevkevinpal commented on pull request "QA: Expect PACKAGE_NAME rather than constant "Bitcoin Core"":
(https://github.com/bitcoin/bitcoin/pull/30308#issuecomment-2178928580)
ACK [197b540](https://github.com/bitcoin/bitcoin/pull/30308/commits/197b5404b0f4a1d6e989000845b45c8bd24e0bc6)

we're doing this in these files aswell so seems fine to me
```
test/functional/wallet_multiwallet.py
test/functional/tool_wallet.py
test/functional/interface_bitcoin_cli.py
test/functional/feature_filelock.py
```

💬 maflcko commented on issue "ci: Move more tasks to GHA?":
(https://github.com/bitcoin/bitcoin/issues/30304#issuecomment-2178933486)
If someone finds a solution to all cache issues, then it can be done. (Moving back should be easy in any case)

For now, see the issue description:

> ideally only tasks with `NO_DEPENDS=1` are moved for now. It would be:
>
> * `ci/test/00_setup_env_native_fuzz.sh:export NO_DEPENDS=1`
>
> * `ci/test/00_setup_env_native_tidy.sh:export NO_DEPENDS=1`

💬 kevkevinpal commented on pull request "QA: Expect PACKAGE_NAME rather than constant "Bitcoin Core"":
(https://github.com/bitcoin/bitcoin/pull/30308#issuecomment-2178943305)
I see we're doing something similar in `./test/functional/wallet_backwards_compatibility.py`

```
node_v17.assert_start_raises_init_error(["-wallet=w3"], "Error: Error loading w3: Wallet requires newer version of Bitcoin Core")

and

node_v16.assert_start_raises_init_error([f"-wallet={wallet_name}"], f"Error: Error loading {wallet_name}: Wallet requires newer version of Bitcoin Core")
```

not sure if you want to update here aswell in this PR

💬 TheCharlatan commented on pull request "kernel: De-globalize validation caches":
(https://github.com/bitcoin/bitcoin/pull/30141#discussion_r1646370563)
I was thinking it might be useful to control the caches directly for the tests, but on second thought that does not seem like a good thing. Will revert.

💬 TheCharlatan commented on pull request "kernel: De-globalize validation caches":
(https://github.com/bitcoin/bitcoin/pull/30141#issuecomment-2178956138)
6ad4aa82056030a8a72b1315676e0703c1500d0d -> fa660a266b3aa03f40c47a9330e3bce8be89bb54 ([noGlobalScriptCache_4](https://github.com/TheCharlatan/bitcoin/tree/noGlobalScriptCache_4) -> [noGlobalScriptCache_5](https://github.com/TheCharlatan/bitcoin/tree/noGlobalScriptCache_5), [compare](https://github.com/TheCharlatan/bitcoin/compare/noGlobalScriptCache_4..noGlobalScriptCache_5))

* Addressed @stickies-v's [comment](https://github.com/bitcoin/bitcoin/pull/30141#discussion_r1645856227), changing in
...

💬 maflcko commented on pull request "kernel: De-globalize validation caches":
(https://github.com/bitcoin/bitcoin/pull/30141#discussion_r1646381635)
nit: You can combine the cast by using `()` over `{}`. Also you can specify the template to avoid the `size_t` cast, and ensure the right template is always picked:

```cpp
uint32_t requested_num_elems(std::min<size_t>(

💬 sipa commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1646397855)
@laanwj It's true that you generally shouldn't use `using namespace std;` in headers, but the C++ Core Guidelines actually have an [exception](https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#sf7-dont-write-using-namespace-at-global-scope-in-a-header-file) for the standard literals. The reason being that literals without underscore are reserved for the standard library, so there is no risk of namespace pollution.

✅ 0xB10C closed a pull request: "rpc: introduce getversion RPC"
(https://github.com/bitcoin/bitcoin/pull/30112)

💬 0xB10C commented on pull request "rpc: introduce getversion RPC":
(https://github.com/bitcoin/bitcoin/pull/30112#issuecomment-2178998633)
While this has a few Concept ACKs, I'm not sure the Approach (introducing a new RPC) is really the best route here. I'm closing this for now as I'll be focusing on other projects. Feel free to pick this up if you want.

💬 glozow commented on pull request "test: Add Compact Block Encoding test `ReceiveWithExtraTransactions` covering non-empty `extra_txn`":
(https://github.com/bitcoin/bitcoin/pull/30237#discussion_r1646027082)
Needs documentation, i.e. mentioning the nonce should be randomly generated

💬 Fi3 commented on pull request "Stratum v2 Noise Protocol":
(https://github.com/bitcoin/bitcoin/pull/29346#issuecomment-2179108671)
@Sjors initially the TP was without noise, we added it mainly to have third party TPs (not pool and not miner). About easy of use, if we don't want to implement an sv1 translator in core, the miner still have to deploy a translator proxy. Maybe is easier to just release a translator that is also a JDC (like what I did for demand) rather the implement the JDC in the TP and add custom message to do that.

📝 furszy opened a pull request: "wallet: notify when preset + automatic inputs exceed max weight"
(https://github.com/bitcoin/bitcoin/pull/30309)
Small change. Found it while finishing my review on #29523. This does not interfere with it.

Basically, we are erroring out early when the automatic coin selection process exceeds the maximum weight, but we are not doing so when the user-preselected inputs combined with the wallet-selected inputs exceed the maximum weight.
This change avoids signing all inputs before erroring out and introduces test coverage for `fundrawtransaction`.

📝 m3dwards opened a pull request: "ci: add option for running tests without volume"
(https://github.com/bitcoin/bitcoin/pull/30310)
Fixes: https://github.com/bitcoin/bitcoin/pull/30193#discussion_r1645950272

Cache wasn't being saved when run on GHA because the default behaviour of the CI script was to store cache items in a docker volume. This works on Cirrus CI as the volumes are shared but it does not work on Github Actions in which each run is ephemeral.

Kept the default behaviour the same so hopefully this continues to work as is for the Cirrus CI jobs.

💬 m3dwards commented on pull request "ci: move ASan job to GitHub Actions from Cirrus CI":
(https://github.com/bitcoin/bitcoin/pull/30193#discussion_r1646598411)
https://github.com/bitcoin/bitcoin/pull/30310

👍 brunoerg approved a pull request: "fuzz: Fix wallet_bdb_parser 32-bit unhandled fseek error"
(https://github.com/bitcoin/bitcoin/pull/30307#pullrequestreview-2128908068)
utACK fa7bc9bbca9348cf31b97bee0789ea7caeec635c

💬 fjahr commented on pull request "assumeutxo: Check snapshot base block is not in invalid chain":
(https://github.com/bitcoin/bitcoin/pull/30267#discussion_r1646607479)
Ah, yeah, I gave it a shot and it pretty simple so I am changing the return type as well in the refactoring commit now.

💬 am-sq commented on pull request "doc: loadwallet loads from relative walletdir":
(https://github.com/bitcoin/bitcoin/pull/30302#discussion_r1646609836)
Based on https://github.com/bitcoin/bitcoin/blob/master/doc/files.md#sqlite-database-based-wallets, it appears `.dat` files can be passed to sqlite wallets as well.