📝 fanquake locked a pull request: "modificación"
(https://github.com/bitcoin/bitcoin/pull/29905)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...

💬 Sjors commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#issuecomment-2063985018)
Either you could generate the mainnet index using the code in this pull request, and then compare it. Or I can take your list and compare it to my result. Whoever gets around to it first :-)

👍 guggero approved a pull request: "ZMQ: Support UNIX domain sockets"
(https://github.com/bitcoin/bitcoin/pull/27679#pullrequestreview-2009075318)
Tested and code review ACK 21d0e6c7b7c7

Thanks a lot for the fix!

💬 ryanofsky commented on pull request "validation: improve performance of CheckBlockIndex":
(https://github.com/bitcoin/bitcoin/pull/28339#discussion_r1570911819)
> ok, yeah. Then it is not an issue but, isn't error-prone to call `best_hdr_chain` something that might not be the best chain then? E.g. it seems that if we ever add a check only for the active chain blocks here, the `best_hdr_chain.Contains()` usage temptation will be there.

I don't love the name `best_hdr_chain` but don't think it is too bad. I agree it could potentially be confused with the active chain in new code, and that would be a bug. In my original suggestion https://github.com/bit
...

💬 pablomartin4btc commented on pull request "Bugfix - don't allow multiple dialogs for same tx in TransactionView":
(https://github.com/bitcoin-core/gui/pull/817#discussion_r1570920312)
Yes, it needs both `activateWindow()` & `raise()`, let me try it on Windows and I'll incorporate your [suggestion](https://github.com/bitcoin-core/gui/pull/817#discussion_r1569483043), thanks!

💬 hebasto commented on pull request "refactor: Use our own implementation of urlDecode":
(https://github.com/bitcoin/bitcoin/pull/29904#issuecomment-2064197562)
Concept ACK.

💬 hebasto commented on pull request "Reintroduce external signer support for Windows":
(https://github.com/bitcoin/bitcoin/pull/29868#issuecomment-2064247329)
> > There are no conflicts with #29865.
>
> Looks like there are now?

Well, there is a conflict in the code, which is being removed in #29865. It won't be a problem for either PR if the other one goes first.

💬 glozow commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1571015884)
(deleted `GetCombinedHash`)

🤔 glozow reviewed a pull request: "p2p: opportunistically accept 1-parent-1-child packages"
(https://github.com/bitcoin/bitcoin/pull/28970#pullrequestreview-2009283703)
Main changes:
- Try same-peer orphanage children (most recent first) before different-peer ones (randomized). Replaced `GetChildren` with `GetChildrenFromSamePeer` and `GetChildrenFromDifferentPeer`.
- Deleted `GetCombinedHash` because it's unused. Also deleted the tests for it
- Changed `GetPackageHash` a bit. Using single-sha256 instead of double-sha256 and using just wtxids instead of serialized vector

💬 glozow commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1571015190)
deleted

💬 glozow commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1571014755)
I figure it's easier to read the test this way; you can easily tell that the lexicographical ordering is what I claim it to be

💬 glozow commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1571015988)
(deleted `GetCombinedHash`)

💬 glozow commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1571006395)
(deleted `GetCombinedHash`)

💬 glozow commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1571005910)
removed

💬 glozow commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1571017712)
Did this now. We first gather all children by the same peer and try by recency order. If we cannot find one that matches, we look for children not by this peer, and try in random order.

💬 glozow commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1571006616)
done, thanks

💬 glozow commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1571006745)
done, thanks

🤔 hebasto reviewed a pull request: "util: remove unused cpp-subprocess options"
(https://github.com/bitcoin/bitcoin/pull/29865#pullrequestreview-2009319290)
Approach ACK 0f847336428153042dbf226caf52ecd43188f22e.

 :rocket:

I have reviewed all commits except the last one (in progress).

The CI run of the combined branch [ this PR + #29868 ] is available here: https://github.com/hebasto/bitcoin/actions/runs/8740663547/job/23984855961

💬 glozow commented on pull request "doc: update release-process.md":
(https://github.com/bitcoin/bitcoin/pull/29645#issuecomment-2064387960)
> I think the "Update repositories and websites for new version" section could be split into 2 distinct steps

Done, I split into website updates and everything else

💬 ryanofsky commented on pull request "refactor: Add util::Result failure values, multiple error and warning messages":
(https://github.com/bitcoin/bitcoin/pull/25665#discussion_r1570820413)
re: https://github.com/bitcoin/bitcoin/pull/25665#discussion_r1552474458

> I have not quite grasped yet what the immediate benefit of these more generic decorators on the Messages are.

The benefit is letting the `Result` struct work with any possible `MessagesType` without placing requirements on it. The PR provides a simple default type to hold error and warning messages:

```c++
struct Messages {
std::vector<bilingual_str> errors{};
std::vector<bilingual_str> warnings{};
}
...