💬 Shutch147 commented on pull request "Update ci.yml":
(https://github.com/bitcoin/bitcoin/pull/30002#issuecomment-2084511598)
@dependabot

💬 vasild commented on issue "Rethink thread_local (take 2)":
(https://github.com/bitcoin/bitcoin/issues/29952#issuecomment-2084511638)
It just occurred to me on Friday evening and I forgot about this during the weekend - we may have a bug in our code and FreeBSD may just be the messenger - we return a reference to the `thread_local`, store it in `CLockLocation`, from there in the global `lockdata` / `lock_stack`. It looks like the reference in `lockdata` may still be existent after the thread has exited.

@maflcko, thanks for the `string_view` hint!

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