💬 fanquake commented on pull request "guix: build for Linux HOSTS with `-static-libgcc`":
(https://github.com/bitcoin/bitcoin/pull/33181#issuecomment-3271268644)
@theuni switched this to using `CMAKE_EXE_LINKER_FLAGS`, which should make it clearer which flags are being used for executables.

💬 instagibbs commented on issue "Difficulty in reliably mapping errors from Bitcoin Core due to unstable error codes and messages":
(https://github.com/bitcoin/bitcoin/issues/33350#issuecomment-3271344694)
Chatting with some other devs, one other motivation for better/easier error codes would be to allow logging to be more sane.

For instance, `sendrawtransaction` may throw an error if already in mempool, which if logged is incredibly noisy and useless for fire-and-forget wallets like CLN.

submitpackage already behaves better in that respect, returning a success instead (as it is in the mempool).

💬 Sjors commented on pull request "wallet: warn against accidental unsafe older() import":
(https://github.com/bitcoin/bitcoin/pull/33135#issuecomment-3271397302)
Rebased. Fixed bug in the functional test found by an AI overlord, apparently using `is` isn't safe: https://stackoverflow.com/a/28864111

> I'll look into modifying miniscript / descriptor classes to avoid the string parsing.

Done, and also added a unit test. It's a bit more code and complexity, but more robust and extendable than parsing strings.

Since c1e6a9f1e7331fa2c1c73a0dd17056570283e443 is no longer relevant for this PR I dropped it.

💬 sipa commented on pull request "Bump SCRIPT_VERIFY flags to 64 bit":
(https://github.com/bitcoin/bitcoin/pull/32998#issuecomment-3271403052)
utACK 417437eb01ac014c57aca47f44d7f8d3da351987

💬 Sjors commented on pull request "index: Fix coinstats overflow":
(https://github.com/bitcoin/bitcoin/pull/30469#issuecomment-3271424867)
Oh oops...

```sh
bitcoin-cli gettxoutsetinfo muhash 913859
```

Both v29.1 and https://github.com/bitcoin/bitcoin/commit/c76797481155754329ec6a6f58e8402569043944:

```json
{
"height": 913859,
"bestblock": "00000000000000000000f7ae1989c423172433aab8aa0164f9fc85fd74ed7f44",
"txouts": 168005479,
"bogosize": 13163671088,
"muhash": "16bf45a4ce9c852aab746826955e3aac8e7e1b964a36af3bce944c17c5e13d55",
"total_amount": 19918082.42878334,
"total_unspendable_amount": 230.07121
...

💬 theuni commented on pull request "guix: build for Linux HOSTS with `-static-libgcc`":
(https://github.com/bitcoin/bitcoin/pull/33181#issuecomment-3271448380)
Switching to `CMAKE_EXE_LINKER_FLAGS` came out of an offline discussion with fanquake.

@fanquake we want all flags passed to all binaries with the exception of `-static-libstdc++ -static-libgcc`, which should only go to exe's.

So I think we want:
`HOST_LDFLAGS="-Wl,--as-needed -Wl,--dynamic-linker=$glibc_dynamic_linker -Wl,-O2"`
`CMAKE_EXE_LINKER_FLAGS="-static-libstdc++ -static-libgcc"`

Then for CMake, still pass `LDFLAGS` via `HOST_LDFLAGS` in addition to `CMAKE_EXE_LINKER_FLAGS`. T
...

💬 fanquake commented on pull request "guix: build for Linux HOSTS with `-static-libgcc`":
(https://github.com/bitcoin/bitcoin/pull/33181#issuecomment-3271470246)
Yea, I need to rework this in any case, as the macOS build breaks with the changes here.

👍 instagibbs approved a pull request: "Bump SCRIPT_VERIFY flags to 64 bit"
(https://github.com/bitcoin/bitcoin/pull/32998#pullrequestreview-3029931042)
ACK 417437eb01ac014c57aca47f44d7f8d3da351987

non-blocking suggestions for coverage

💬 instagibbs commented on pull request "Bump SCRIPT_VERIFY flags to 64 bit":
(https://github.com/bitcoin/bitcoin/pull/32998#discussion_r2213656693)
```Suggestion
BOOST_CHECK_EQUAL(FormatScriptFlags(SCRIPT_VERIFY_P2SH), "P2SH");
BOOST_CHECK_EQUAL(FormatScriptFlags(SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_TAPROOT), "P2SH,TAPROOT");
```

💬 instagibbs commented on pull request "Bump SCRIPT_VERIFY flags to 64 bit":
(https://github.com/bitcoin/bitcoin/pull/32998#discussion_r2213603725)
bit of coverage of multiple unknown bits (didn't validate)
```Suggestion
BOOST_CHECK_EQUAL(FormatScriptFlags(SCRIPT_VERIFY_TAPROOT | (1u<<27)), "TAPROOT,0x08000000");
BOOST_CHECK_EQUAL(FormatScriptFlags(SCRIPT_VERIFY_TAPROOT | (1u<<27) | (1u<<31)), "TAPROOT,0x88000000");
```

💬 instagibbs commented on pull request "Bump SCRIPT_VERIFY flags to 64 bit":
(https://github.com/bitcoin/bitcoin/pull/32998#discussion_r2334116135)
bddcadee82daf3ed1441820a0ffc4c5ef78f64f1

could be here or elsewhere, but round-simply round-tripping adds some coverage:

```
assert(script_verify_flags::from_int(flags.as_int()) == flags);
```

👋 Crypt-iQ's pull request is ready for review: "fuzz: compact block harness"
(https://github.com/bitcoin/bitcoin/pull/33300)

💬 Crypt-iQ commented on pull request "fuzz: compact block harness":
(https://github.com/bitcoin/bitcoin/pull/33300#issuecomment-3271483113)
There is non-determinism [here](https://github.com/bitcoin/bitcoin/blob/c0894a0a2be032cd9a5d5945643689230ab10255/src/node/blockstorage.cpp#L490-L493) because `m_dirty_blockindex` is `std::set<CBlockIndex*>` and sorts based on the pointer. This can be fixed by adding a function object that compares the block hashes. However, I did not know if this should be added just for fuzz code and I have not run benchmarks.

After patching the above non-determinism locally, there is also non-determinism in
...

⚠️ darosior opened an issue: "Higher **reported** memory usage of Bitcoin Core after version 29"
(https://github.com/bitcoin/bitcoin/issues/33351)
For versions of Bitcoin Core after 29.0, common utilities like `top` will report higher RAM usage of the `bitcoind` process than for previous Bitcoin Core versions.

The size of LevelDB files was increased from 2 MiB to 32 MiB in Bitcoin Core 29.0 (PR https://github.com/bitcoin/bitcoin/pull/30039). LevelDB caches its on-disk files using [`mmap`](https://en.wikipedia.org/wiki/Mmap) and will cache up to 1000 files by default, although our fork increased this number to 4096 (see https://github.com/
...

💬 BinaryIgor commented on pull request "rpc, logging: add backgroundvalidation to getblockchaininfo":
(https://github.com/bitcoin/bitcoin/pull/33259#discussion_r2334280301)
nit: typo, should be "validation"

💬 BinaryIgor commented on pull request "rpc, logging: add backgroundvalidation to getblockchaininfo":
(https://github.com/bitcoin/bitcoin/pull/33259#discussion_r2334298532)
As this already is a rather too long test, it would be nice to move these assertions into a separate method, similarly as it done with check_tx_counts

⚠️ yancyribbens opened an issue: "CoinGrinder and SRD disagree on small change"
(https://github.com/bitcoin/bitcoin/issues/33352)
Both CoinGrinder and SRD create a change output, however they disagree on what change output is too small.

Consider SRD selection, which adds `CHANGE_LOWER` to the target value and in so doing, prevents a selection that is less than `CHANGE_LOWER` [ref](https://github.com/bitcoin/bitcoin/blob/c0894a0a2be032cd9a5d5945643689230ab10255/src/wallet/coinselection.cpp#L546C5-L546C47)
```
target_value += CHANGE_LOWER + change_fee;
```

However, CoinGrinder does not add `CHANGE_LOWER` [ref](https://gith
...

💬 yancyribbens commented on issue "CoinGrinder and SRD disagree on small change":
(https://github.com/bitcoin/bitcoin/issues/33352#issuecomment-3271758012)
cc @murchandamus

🤔 w0xlt reviewed a pull request: "test/refactor: use test deque to avoid quadratic iteration"
(https://github.com/bitcoin/bitcoin/pull/33313#pullrequestreview-3202749415)
ACK https://github.com/bitcoin/bitcoin/pull/33313/commits/8c1f10233dc9a843147772c40973031f5bfdbb7c

While the impact of this PR on a few hundred items is negligible, it’s still a good practice and is likely to scale more effectively as test coverage increases.

For reference, the script below demonstrates the performance difference with 1,000,000 items on my machine:

```
List pop(0): 62.790 seconds
Deque popleft(): 0.022 seconds
```

```python
import time
from collections import d
...

💬 murchandamus commented on issue "CoinGrinder and SRD disagree on small change":
(https://github.com/bitcoin/bitcoin/issues/33352#issuecomment-3271804098)
This is intentional.

Knapsack and CoinGrinder use [`GenerateChangeTarget`](https://github.com/bitcoin/bitcoin/blob/c0894a0a2be032cd9a5d5945643689230ab10255/src/wallet/coinselection.cpp#L809) to make a [randomized minimum change](https://github.com/bitcoin/bitcoin/pull/24494), because these two algorithms minimize the overshoot over the minimum change which led to a fingerprint and a clustering of change outputs of the same value. SRD randomly selects inputs, so it already overshoots the minimum
...