💬 pinheadmz commented on pull request "test: Add comprehensive txindex reorg test coverage":
(https://github.com/bitcoin/bitcoin/pull/33991#issuecomment-3599888466)
Pretty sure this is just AI slop. "Gittensor" is kinda dead give away.

⚠️ dachengcheng2022 opened an issue: "tx not exist on the testnet4"
(https://github.com/bitcoin/bitcoin/issues/33992)
### Is there an existing issue for this?

- [x] I have searched the existing issues

### Current behaviour

network: testnet4

this is my txinfo: https://mempool.space/zh/testnet4/tx/93cf45c0fb38f62dfde0619ae4abaadc6842a931b6946abd79012d2a963d19e5
tip me confirms already, but i search it use rpc

`curl --location 'http://47.236.168.32:18332' \
--header 'content-type: text/plain;' \
--header 'Authorization: Basic dGVzdDp0ZXN0' \
--data '{"jsonrpc": "1.0", "id": "curltest", "method": "getrawtrans
...

✅ dachengcheng2022 closed an issue: "tx not exist on the testnet4"
(https://github.com/bitcoin/bitcoin/issues/33992)

📝 brunoerg opened a pull request: "init: point out -stopatheight may be imprecise"
(https://github.com/bitcoin/bitcoin/pull/33993)
`-stopatheight` is used to stop running bitcoind after reaching a given height. However, this feature is imprecise since some blocks can still be processed during the shutdown.

There are some previous discussions around it in https://github.com/bitcoin/bitcoin/pull/13713, https://github.com/bitcoin/bitcoin/pull/13490 and https://github.com/bitcoin/bitcoin/issues/13477. However, I'm not sure if it will get fixed since it's undesirable to burden the validation code further with this and we can
...

💬 l0rinc commented on pull request "init: point out -stopatheight may be imprecise":
(https://github.com/bitcoin/bitcoin/pull/33993#discussion_r2580053828)
Wouldn't it suffice to just hint that it's not exact?
```suggestion
argsman.AddArg("-stopatheight", strprintf("Stop running after reaching the approximate height in the main chain (default: %u). It may be imprecise as shutdown is triggered at this height but additional blocks may be processed.", DEFAULT_STOPATHEIGHT), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST);
```

nit: if we're doing this, we could adjust the code comment as well:
https://github.com/bitc
...

💬 Sjors commented on pull request "wallet: warn against accidental unsafe older() import":
(https://github.com/bitcoin/bitcoin/pull/33135#discussion_r2580229022)
Taken (and updated the test)

💬 brunoerg commented on pull request "init: point out -stopatheight may be imprecise":
(https://github.com/bitcoin/bitcoin/pull/33993#discussion_r2580229771)
Happy to address it if others like it but I personally don't like "approximate" because it's kinda subjective (could mean before or after the height) and do not reflect the goal of it. Also, this issue doesn't happen when it's in IBD.

💬 l0rinc commented on pull request "merkle: pre‑reserve leaves to prevent reallocs with odd vtx count":
(https://github.com/bitcoin/bitcoin/pull/32497#discussion_r2580278793)
> can be simplified to -filter='MerkleRoot'.

Yeah, thanks, I know, I explicitly extended it since it produces two results now.

🚀 fanquake merged a pull request: "Cluster mempool followups"
(https://github.com/bitcoin/bitcoin/pull/33591)

💬 fanquake commented on pull request "test: assumeutxo: add missing tests in wallet_assumeutxo.py":
(https://github.com/bitcoin/bitcoin/pull/30455#issuecomment-3601252308)
@alfonsoromanz can you rebase/ followup with the test failures here?

🚀 fanquake merged a pull request: "[30.x] Backports"
(https://github.com/bitcoin/bitcoin/pull/33609)

💬 ajtowns commented on issue "Net split meta issue":
(https://github.com/bitcoin/bitcoin/issues/33958#issuecomment-3601281266)
> Reduce `PeerManager`'s usage of `CNode`
> Move the message handling loop to `PeerManager`

Big fan of these aspects. I like the documentation aspect of NetManagerEvents. Would like to see the PeerManager functions move towards accepting a `Peer&` (vs a `CNode*` and looking up `GetPeerRef`, or an explicit `CNode, Peer` pair) in general too.

> Dropped usage of `CNode` in `PeerManager`
>
> With the event loop moved and nascent interfaces beginning to take shape, `PeerManager` should be able to
...

💬 fanquake commented on pull request "depends, doc: Add `tcl` as build dependency for `sqlite` package":
(https://github.com/bitcoin/bitcoin/pull/33975#issuecomment-3601283102)
Can we just pass the compiler through?

💬 fanquake commented on pull request "build: Use clang-cl to build on Windows natively":
(https://github.com/bitcoin/bitcoin/pull/31507#discussion_r2580569951)
Should be in `compat.h`?

💬 fanquake commented on pull request "doc: improvements to doc/descriptors.md":
(https://github.com/bitcoin/bitcoin/pull/33986#issuecomment-3601316432)
cc @Sjors

💬 Sjors commented on pull request "mining: fix -blockreservedweight shadows IPC option":
(https://github.com/bitcoin/bitcoin/pull/33965#issuecomment-3601375713)
> if you think it would be useful for IPC clients to "have a way to signal their intent to use the node default"

sv2-tp always sets the value (based on `coinbase_output_max_additional_size` in `CoinbaseOutputConstraints`), so it wouldn't benefit. But it does seem better to allow clients to omit this value.

For that to work however, we'd also need to change mining.capnp, e.g.:

```capnp
blockReservedWeight @1 :UInt64 $Proxy.name("block_reserved_weight");
hasBlockReservedWeight @3 :Bool
...

💬 Sjors commented on pull request "mining: fix -blockreservedweight shadows IPC option":
(https://github.com/bitcoin/bitcoin/pull/33965#discussion_r2580623784)
The latest push leaves RPC code untouched.

💬 willcl-ark commented on pull request "doc: Document compiler configuration for native depends packages":
(https://github.com/bitcoin/bitcoin/pull/33902#issuecomment-3601387596)
> See [#33902 (comment)](https://github.com/bitcoin/bitcoin/pull/33902#discussion_r2549756641).

As I describe [here](https://github.com/bitcoin/bitcoin/pull/33902#discussion_r2556778530) I have a commit which if `host_CC` is set, **and**** `CC` is unset, will set `CC` to `host_CC` (and similarly for `CXX` and friends).

In my opinion this would result in the expected behaviour:

- If `{var}` is set, use that
- If not check if `host_{var}` is set, and use that
- Otherwise, fallback to a
...

🤔 rkrux reviewed a pull request: "test: p2p: check that peer's announced starting height is remembered"
(https://github.com/bitcoin/bitcoin/pull/33990#pullrequestreview-3529561300)
Concept ~0 52f96cc235d309d9156eb742036c859984b9a467

I'm on the fence regarding whether this case testing just one field is warranted. I don't see much coverage gained in the `corecheck`.
Though there is a similar `test_desirable_service_flags` case as well but in it the connection is disconnected conditionally, so one can argue that there is more to test over there.

💬 brunoerg commented on pull request "test: p2p: check that peer's announced starting height is remembered":
(https://github.com/bitcoin/bitcoin/pull/33990#discussion_r2580745364)
nit: Perhaps a comment explaining these values (edge cases for 32-bit integers) would be good.