💬 sipa commented on pull request "ci: detect outbound internet traffic generated while running tests":
(https://github.com/bitcoin/bitcoin/pull/31349#discussion_r2333792206)
This `INTERNET_TRAFFIC_EXPECTED` seems to be unused?

💬 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-3270947536)
We don't have an easy table but I think for example this gives a good example:

https://github.com/lightningnetwork/lnd/blob/master/sweep/fee_bumper.go#L568-L578

In our fee bumper engine we only bump the fee of a transaction if the error is somehow related to an `insufficient fee` error, otherwise we drop this input from the sweeping system because problems like not having the correct signature or being already spent would cause us to never properly sweep this input.

What I am more interested
...

🤔 stickies-v reviewed a pull request: "chore: compare against keys.size() in writeObject comma check"
(https://github.com/bitcoin/bitcoin/pull/33337#pullrequestreview-3201895990)
NACK, seems like busywork, no clear improvement

💬 theStack commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2333831377)
Note that the only musig API function that requires to use this context is [`secp256k1_musig_nonce_gen`](https://github.com/bitcoin-core/secp256k1/blob/36e76952cbf1cf54ddd2d8756cc31a486e2ba1d9/include/secp256k1_musig.h#L338), for all the others you can simply use the static one (`secp256k1_context_static`).

✅ fanquake closed a pull request: "chore: compare against keys.size() in writeObject comma check"
(https://github.com/bitcoin/bitcoin/pull/33337)

💬 sipa commented on pull request "Fix tiebreak when loading blocks from disk (and add tests for comparing chain ties)":
(https://github.com/bitcoin/bitcoin/pull/29640#discussion_r2333834031)
I think just setting the tip to 0 is fine. We don't actually know whether any of the tip's ancestors were activatable earlier than any of their potential competitors.

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