💬 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-3270998612)
Speaking only for myself since I'm more familiar with both LN (by means of CLN) and Bitcoin Core sides...

> Yes I am really surprised that people are not using those error codes to decided based on that what is wrong with their transaction what they for example trying to broadcast.

I think it's honestly quite rare. People will craft transactions using information gleaned Core (like mempool minfee or fee estimator) and proactively avoid violating it. The software can't do anything about other e
...

💬 fjahr commented on pull request "index: Fix coinstats overflow":
(https://github.com/bitcoin/bitcoin/pull/30469#issuecomment-3271072287)
> `getblockstats` gives the same result for both at height 913,859

Did you mean `gettxoutsetinfo` here? :)

🤔 sipa reviewed a pull request: "Fix tiebreak when loading blocks from disk (and add tests for comparing chain ties)"
(https://github.com/bitcoin/bitcoin/pull/29640#pullrequestreview-3202025791)
utACK 0465574c127907df9b764055a585e8281bae8d1d

Happy with either the "whole active chain is set to nSequence 0" or the "only tip is set to nSequence 0" approaches.

🤔 sipa reviewed a pull request: "headerssync: Correct unrealistic unit test behavior"
(https://github.com/bitcoin/bitcoin/pull/32579#pullrequestreview-3202085490)
utACK 33d550d3044f9075cc866093c453158288f12dec

💬 sipa commented on pull request "headerssync: Correct unrealistic unit test behavior":
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2333959308)
In commit "refactor(headerssync): Process spans of headers"

Nit: slightly simpler: `std::span{first_chain}.subspan(1)`.

💬 sipa commented on pull request "headerssync: Correct unrealistic unit test behavior":
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2333960814)
In commit "refactor(headerssync): Process spans of headers"

Same here: `std::span{second_chain}.subspan(1)`.

💬 mzumsande commented on pull request "test: fix p2p_leak_tx.py":
(https://github.com/bitcoin/bitcoin/pull/33121#issuecomment-3271246212)
> My understanding of [`Peer::TxRelay::m_tx_inventory_known_filter`](https://github.com/bitcoin/bitcoin/blob/e04cb9c1bdf2199127cc8cf9c87f25e46b8cac7b/src/net_processing.cpp#L297-L300) is that it will [occasionally indicate](https://github.com/bitcoin/bitcoin/blob/e04cb9c1bdf2199127cc8cf9c87f25e46b8cac7b/src/common/bloom.h#L91-L92) that a transaction is known to a peer when it isn't, and then we won't include it in an INV:

Good point, I didn't think of that possibility.

> Maybe this is also
...

💬 ziggie1984 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-3271254766)
Ok thank you for the information so far, feels like there is currently no easy way for us to proceed so we continue mapping the error strings. What would be very helpful is like a release-note highlight in case the error reporting for the RPC changes.

Keeping this issue open for a bit in case somebody else has a better idea, thank you so far for your thoughts.

💬 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)