💬 l0rinc commented on pull request "refactor: reenable `implicit-integer-sign-change` check for `serialize.h`":
(https://github.com/bitcoin/bitcoin/pull/32296#discussion_r2088818525)
Nice, I like that a lot more - addressed the reformatting and unification in separate commits, let me know if you think this is better.
Pushed a rebase separately to simplify review from GitHub as well (I know you do range-diff), and to make sure the latest CI is run against it.

💬 fanquake commented on pull request "test: fix two intermittent failures in wallet_basic.py":
(https://github.com/bitcoin/bitcoin/pull/32483#issuecomment-2880037334)
Backported to `29.x` in #32292.

⚠️ Sjors opened an issue: "Dropping unused legacy wallet code"
(https://github.com/bitcoin-core/gui/issues/873)
Tracking pull request(s) that remove any remaining legacy wallet code, either here or in the main repo:

- https://github.com/bitcoin/bitcoin/pull/32459

💬 luke-jr commented on pull request "[DRAFT] Expose AssumeUTXO Load Snapshot Functionality To The GUI":
(https://github.com/bitcoin-core/gui/pull/870#issuecomment-2880052845)
Seems like this would be more appropriate for the first-run intro page?

💬 Sjors commented on pull request "qt: drop unused watch-only functionality":
(https://github.com/bitcoin/bitcoin/pull/32459#issuecomment-2880054651)
Doesn't seem worth moving, and this PR also touches the interface, but I opened a tracking issue on the GUI repo that points here. There's probably more than what I found here.

💬 maflcko commented on pull request "refactor: reenable `implicit-integer-sign-change` check for `serialize.h`":
(https://github.com/bitcoin/bitcoin/pull/32296#discussion_r2088837960)
nit in the second commit: Would be good to just set this to the final version `template<typename Stream> void Serialize(Stream& s, int8_t a ) { ser_writedata8(s, uint8_t(a)); }` from the beginning. Otherwise there will be three style-fixup commits on top of each other, making review harder and the git log and git blame depth tedious to follow.

💬 maflcko commented on issue "Dropping unused legacy wallet code":
(https://github.com/bitcoin-core/gui/issues/873#issuecomment-2880070206)
No objection, but not sure a tracking issue is needed for a single, simple pull request?

💬 TheCharlatan commented on pull request "kernel: Separate UTXO set access from validation functions":
(https://github.com/bitcoin/bitcoin/pull/32317#discussion_r2088847208)
Sweet, will apply this!

💬 Sjors commented on issue "Dropping unused legacy wallet code":
(https://github.com/bitcoin-core/gui/issues/873#issuecomment-2880140236)
I don't think I found all the code, regular GUI contributors might find more.

💬 maflcko commented on issue "Dropping unused legacy wallet code":
(https://github.com/bitcoin-core/gui/issues/873#issuecomment-2880157568)
I think they can just open a new issue or pull request, if they find something? I guess it can't hurt to open/close this issue as a notification, but absent any bug or any concrete work to do, I am not sure what needs to be done here and when this can be closed.

📝 l0rinc opened a pull request: "merkle: pre‑reserve leaves to prevent reallocs with odd vtx count"
(https://github.com/bitcoin/bitcoin/pull/32497)
### Summary
`ComputeMerkleRoot` duplicates the last hash when the input size is odd:
https://github.com/bitcoin/bitcoin/blob/39b6c139bd6be33699af781f1d71f6fed303d468/src/consensus/merkle.cpp#L54-L56
If the caller gave it a `std::vector` whose capacity equals its size (the common case when the vector was created with `resize()`), that single `push_back` forces a reallocation (doubling its size).
We pay this penalty on every Merkle‑root calculation even though we know the final size in advance
...

💬 ismaelsadeeq commented on pull request "interfaces: refactor: move `Mining` and `BlockTemplate` implementation to miner":
(https://github.com/bitcoin/bitcoin/pull/32378#issuecomment-2880169560)
@Sjors I've taken your suggestion in this push [0d774ab59..62fc42d475](https://github.com/bitcoin/bitcoin/compare/51344920ebe1772d8dd3bece789a6510d774ab59..62fc42d475df4f23bd93313f95ee7b7eb0d4683f)

If the `GetTip` method from the mining interface isn't going to be used by the TP, it can be deleted as well?

💬 Sjors commented on issue "Dropping unused legacy wallet code":
(https://github.com/bitcoin-core/gui/issues/873#issuecomment-2880178818)
In that case I can close this once https://github.com/bitcoin/bitcoin/pull/32459 is merged, unless there's another issue found by then.

💬 l0rinc commented on pull request "refactor: reenable `implicit-integer-sign-change` check for `serialize.h`":
(https://github.com/bitcoin/bitcoin/pull/32296#discussion_r2088900114)
we could, but I would be mixing changes in the same commit - the commit message wouldn't represent the behavior. Or you mean I should just squash the last few changes as "(un)serialization consistency"?

💬 instagibbs commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#discussion_r2088901333)
Directionally I want them marked for removal, even if it sits around for a few years. This can re-litigated with new information perhaps, but I don't find it "honest" to not mark it as such unless the project changes direction.

💬 maflcko commented on pull request "refactor: reenable `implicit-integer-sign-change` check for `serialize.h`":
(https://github.com/bitcoin/bitcoin/pull/32296#discussion_r2088912362)
You can just append a section to the commit message:

```
Also, fixup whitespace while touching...
```

I am just mentioning it, because I often use `git log -S 'string'` and it is tedious to walk back the history if `string` is modified three times just to fixup some whitespace.

💬 maflcko commented on issue "MSVC 17.12.0 internal compiler error ":
(https://github.com/bitcoin/bitcoin/issues/31303#issuecomment-2880212546)
https://developercommunity.visualstudio.com/t/C1001:-Internal-compiler-error-with-st/10830350#T-N10903514 . So I guess this was fixed in Visual Studio 2022 v17.14?

👍 Sjors approved a pull request: "interfaces: refactor: move `Mining` and `BlockTemplate` implementation to miner"
(https://github.com/bitcoin/bitcoin/pull/32378#pullrequestreview-2840153694)
ACK 62fc42d475df4f23bd93313f95ee7b7eb0d4683f

We could do another round of interface pruning like #31196 closer to the v30 branch-off.

Although `getTip()` isn't used by the TP, it does seem generally useful and trivial to maintain. But we can decide later if that's a good enough reason.

💬 hebasto commented on issue "MSVC 17.12.0 internal compiler error ":
(https://github.com/bitcoin/bitcoin/issues/31303#issuecomment-2880215710)
> https://developercommunity.visualstudio.com/t/C1001:-Internal-compiler-error-with-st/10830350#T-N10903514 . So I guess this was fixed in Visual Studio 2022 v17.14?

Correct.

🤔 danielabrozzoni reviewed a pull request: "config: allow setting -proxy per network"
(https://github.com/bitcoin/bitcoin/pull/32425#pullrequestreview-2840193686)
Code Review ACK e98c51fcce9ae3f441a416cab32a5c85756c6c64

I have reviewed the code, and it looks good to me.
I run the tests locally and did some very limited manual testing, mimicking the functional test behavior (passing in `-proxy=...=network` and checking the `getnetworkinfo` output)