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

💬 brunoerg commented on pull request "test: p2p: check that peer's announced starting height is remembered":
(https://github.com/bitcoin/bitcoin/pull/33990#issuecomment-3601527993)
Can be tested with the following diff:

```diff
diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index 5dac413319..ec417b97ac 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -3535,7 +3535,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
LOCK(pfrom.m_subver_mutex);
pfrom.cleanSubVer = cleanSubVer;
}
- peer->m_starting_height = starting_height;
+ peer->m_starting_height
...

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

Yes, I will take care of it

👍 laanwj approved a pull request: "contrib: Remove brittle, confusing and redundant UTF8 encoding from Python IO"
(https://github.com/bitcoin/bitcoin/pull/33702#pullrequestreview-3529672889)
Re-ACK fad61185861a6a9ed806c387aa63d2b31262b1db
```
$ git range-diff 3bb30658e631ed45b6c8609292facc7ae3dd0f61..fa2fd0ba1fbd4085df24a2c5472636957db28521 b30262dcaa28c40a0a5072847b7194b3db203160..fad61185861a6a9ed806c387aa63d2b31262b1db
```
⋮
* `4: 4444edeecc528ccd88b7dd78ffddbccd69762132 ! 4: fae612424b3e70acd6011a4459518174463b3424 contrib: Remove confusing and redundant encoding from IO` - the only differing commit, checked that it is rebase

⋮

💬 brunoerg commented on pull request "init: point out -stopatheight may be imprecise":
(https://github.com/bitcoin/bitcoin/pull/33993#issuecomment-3601640495)
Rebased

👍 stickies-v approved a pull request: "init: point out -stopatheight may be imprecise"
(https://github.com/bitcoin/bitcoin/pull/33993#pullrequestreview-3529707358)
ACK e4703863655135856d0d6ad54a7021d3326bf071

💬 stickies-v commented on pull request "init: point out -stopatheight may be imprecise":
(https://github.com/bitcoin/bitcoin/pull/33993#discussion_r2580857428)
"Blocks after target height may be processed during shutdown." is a bit more concise and has my preference.

⚠️ plebhash opened an issue: "Mining IPC `createNewBlock` should not return before IBD is over"
(https://github.com/bitcoin/bitcoin/issues/33994)
### Please describe the feature you'd like to see added.

Mining IPC `createNewBlock` should not return before IBD is over

### Is your feature related to a problem, if so please describe it.

on SRI [`bitcoin_core_sv2`](https://github.com/stratum-mining/sv2-apps/tree/main/bitcoin-core-sv2) crate, we want to wait for IBD is over as part of the bootstrapping processes

discussing with @ismaelsadeeq and @Shourya742 during BTrust dev day, we came to the conclusion that `createNewBlock` shouldn't ev
...

💬 hebasto commented on pull request "build: Use clang-cl to build on Windows natively":
(https://github.com/bitcoin/bitcoin/pull/31507#discussion_r2580917867)
Thanks! Reworked.

🤔 alexanderwiederin reviewed a pull request: "kernel: Expose reusable `PrecomputedTransactionData` in script validation"
(https://github.com/bitcoin/bitcoin/pull/33891#pullrequestreview-3529821349)
ACK https://github.com/bitcoin/bitcoin/pull/33891/commits/17d3575cfcb41adfd364e7a1912a4701786f7455

💬 alexanderwiederin commented on pull request "kernel: Expose reusable `PrecomputedTransactionData` in script validation":
(https://github.com/bitcoin/bitcoin/pull/33891#discussion_r2580952377)
nit: wonder if "efficient" here could be misleading.

💬 fanquake commented on pull request "multiprocess: Don't require bitcoin -m argument when IPC options are used":
(https://github.com/bitcoin/bitcoin/pull/33229#issuecomment-3601824876)
Note that 0972f5504021b482b27523fd3bcb8036cf6b439c has broken our `gen-manpages.py` script, by appending additional text (for some bins) to the line containing the version info.

💬 stickies-v commented on pull request "log: Use more severe log level (warn/err) where appropriate":
(https://github.com/bitcoin/bitcoin/pull/33960#discussion_r2581001217)
> I think there will always be a possibility to argue one way or the other,

I agree, so even though I have a different view, it's not that practically important so this shouldn't be a blocker. This PR is an improvement with either approach.

> it seems like it should propagate the error up as a fatal one.

I don't think it needs to be propagated as fatal, the failing subsystem / callsite can just log its issue as a warning (when it has no concept or knowledge on fatality), and then higher
...

💬 stickies-v commented on pull request "log: Use more severe log level (warn/err) where appropriate":
(https://github.com/bitcoin/bitcoin/pull/33960#issuecomment-3601835371)
ACK fa45a1503eee603059166071857215ea9bd7242a

👍 rkrux approved a pull request: "wallet: warn against accidental unsafe older() import"
(https://github.com/bitcoin/bitcoin/pull/33135#pullrequestreview-3530005825)
lgtm re-ACK 76c092ff805833a9adf84f669f0455bc2e0bba8b

💬 brunoerg commented on pull request "mining: add getMemoryLoad() and track template non-mempool memory footprint":
(https://github.com/bitcoin/bitcoin/pull/33922#discussion_r2581145624)
Just ran a mutation testing for this PR and this is the only unkilled mutant - feel free to ignore if it doesn't make sense to address:

```diff
diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp
index f734296b24..13ddbb672f 100644
--- a/src/node/interfaces.cpp
+++ b/src/node/interfaces.cpp
@@ -880,7 +880,7 @@ public:
for (const CTransactionRef& tx : m_block_template->block.vtx | std::views::drop(1)) {
auto ref_count{m_node.template_tx_refs.find(tx)};

...

💬 m3dwards commented on pull request "doc: Add `x86_64-w64-mingw32ucrt` triplet to `depends/README.md`":
(https://github.com/bitcoin/bitcoin/pull/33857#issuecomment-3602078949)
ACK ec8eb013a9bfceb324b309f13b8946b05292a993

🚀 fanquake merged a pull request: "log: Use more severe log level (warn/err) where appropriate"
(https://github.com/bitcoin/bitcoin/pull/33960)