✅ fanquake closed a pull request: "Update ci.yml"
(https://github.com/bitcoin/bitcoin/pull/30002)

📝 fanquake locked a pull request: "Update ci.yml"
(https://github.com/bitcoin/bitcoin/pull/30002)
@Shutch147

<!--
*** 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 tes
...

💬 willcl-ark commented on pull request "gui: fix misleading signmessage error with segwit":
(https://github.com/bitcoin-core/gui/pull/819#discussion_r1584338116)
Taken all suggestions, thanks.

👍 willcl-ark approved a pull request: "net: Replace ifname check with IFF_LOOPBACK in Discover"
(https://github.com/bitcoin/bitcoin/pull/29984#pullrequestreview-2030591724)
utACK a68fed111be393ddbbcd7451f78bc63601253ee0

Agree this will be less brittle than name-checking. I don't think it's possible that IFF_LOOPBACK would ever be unset for a loopback device unless someone had modified their kernel.

👍 theStack approved a pull request: "p2p: opportunistically accept 1-parent-1-child packages"
(https://github.com/bitcoin/bitcoin/pull/28970#pullrequestreview-2030612104)
Code-review ACK e518a8bf8abf3d7b83c9013f56d0dca18ae04d6f :package:

👍 dergoegge approved a pull request: "fuzz: don't allow adding duplicate transactions to the mempool"
(https://github.com/bitcoin/bitcoin/pull/29990#pullrequestreview-2030618505)
utACK cc15c5bfd1eb3903de246c124702a7c66c687008

🚀 glozow merged a pull request: "fuzz: don't allow adding duplicate transactions to the mempool"
(https://github.com/bitcoin/bitcoin/pull/29990)

💬 glozow commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#issuecomment-2084765381)
Since there are a few ACKs now, listing followups. I plan to open a PR for the first two immediately:
- redundant comment + pass `PackageToValidate` directly https://github.com/bitcoin/bitcoin/pull/28970#pullrequestreview-2025023787
- make `MempoolAcceptResult::m_replaced_transactions` non-optional https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1568781077
- (already in #29974) fix quirks in fuzz/txorphan.cpp https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1576401327
- co
...

🚀 glozow merged a pull request: "test: Don't rely on incentive incompatible replacement in mempool_accept_v3.py"
(https://github.com/bitcoin/bitcoin/pull/29986)

💬 willcl-ark commented on issue ""Migrate Wallet" is unclear to translators":
(https://github.com/bitcoin/bitcoin/issues/29979#issuecomment-2084785331)
Some other suggestions, none I'm particularly favourable towards vs "migration" from an English standpoint tbh:

- transition
- convertion
- transformation

Of these IMO only "transition" is likely to have any chance at being _more_ clear (than "migration" apparently is) when translating into other languages.

🤔 glozow reviewed a pull request: "doc: removed help text saying that peers may not connect automatically"
(https://github.com/bitcoin/bitcoin/pull/29994#pullrequestreview-2030743114)
lgtm 95897ff181c0757e445f0e066a2a590a0a0120d2, cc @vasild

🤔 glozow reviewed a pull request: "test: add MiniWallet tagging support to avoid UTXO mixing, use in `fill_mempool`"
(https://github.com/bitcoin/bitcoin/pull/29939#pullrequestreview-2030752389)
This solution seems really elegant! I don't think the tag is overkill and I can imagine wanting more than 2 wallets so don't think a bool is better.

💬 glozow commented on pull request "test: add MiniWallet tagging support to avoid UTXO mixing, use in `fill_mempool`":
(https://github.com/bitcoin/bitcoin/pull/29939#discussion_r1584443662)
Also I was imagining that, with the ephemeral miniwallet, we can just delete the `miniwallet` param

💬 glozow commented on pull request "test: add MiniWallet tagging support to avoid UTXO mixing, use in `fill_mempool`":
(https://github.com/bitcoin/bitcoin/pull/29939#discussion_r1584441713)
901ffa9e6e57f7c0e0db967fba9c715139bbcf0b shouldn't this be sent from the ephemeral miniwallet?

💬 glozow commented on pull request "test: add MiniWallet tagging support to avoid UTXO mixing, use in `fill_mempool`":
(https://github.com/bitcoin/bitcoin/pull/29939#discussion_r1584437933)
901ffa9e6e57f7c0e0db967fba9c715139bbcf0b nit: did you mean ephemeral? (also in commit message)

💬 dergoegge commented on pull request "Several randomness improvements":
(https://github.com/bitcoin/bitcoin/pull/29625#discussion_r1584428294)
https://github.com/bitcoin/bitcoin/pull/28558 made PeerManager own a FastRandomContext, so we could (should?) use `m_rng` here instead (otherwise `PeerManager::Options::deterministic_rng` still only applies to some of the randomness).

Since this PR kind of makes individual "make this component deterministic" options redudant, we could consider reverting #28558 (not necessarily in this PR)?

I was thinking that in the long run we could break the dependencies between components and the specif
...

👍 dergoegge approved a pull request: "Simplify network-adjusted time warning logic"
(https://github.com/bitcoin/bitcoin/pull/29623#pullrequestreview-2030922611)
utACK c6be144c4b774a03a8bcab5a165768cf81e9b06b

🤔 stickies-v reviewed a pull request: "net: Modernize logging in UPnP and nat-pmp code"
(https://github.com/bitcoin/bitcoin/pull/29978#pullrequestreview-2030936139)
Concept ACK, but it's probably better to use the new macros introduced in #28318? Currently, the conditional macros (which this PR would need) don't take a category parameter, but if you think that's important perhaps it'd be good to chime in on the conversation on #29256.

🤔 stickies-v reviewed a pull request: "refactor: convert string formatting to F-strings"
(https://github.com/bitcoin/bitcoin/pull/29969#pullrequestreview-2030972260)
Concept NACK. Thank you for looking into this, and I support increased f-string usage, but imo this should be done organically whenever these lines are touched for other reasons (or with a scripted-diff across the code base, and enforcing it with linter, should there be support to do that). In its current form, this is not worth the review time imo.

💬 laanwj commented on pull request "net: Modernize logging in UPnP and nat-pmp code":
(https://github.com/bitcoin/bitcoin/pull/29978#issuecomment-2085013529)
i dunno, i prefer being explicit and not using macros myself TBH, what would macros add? The way it's done now, it's clear where the messages come from and what their level is. i'm fine with changing if you really insist.