👍 fanquake approved a pull request: "depends: Fix build of Qt for 32-bit platforms with recent glibc"
(https://github.com/bitcoin/bitcoin/pull/29985#pullrequestreview-2030292710)
ACK 2e266f33b5d2be5c233c2c692481f75785714fa1 - at some point qt's open source 5.15.x branch will catch up to where they bumped the internal zlib to >= 1.3 (which contains this change), and we'll be able to drop this patch. Checked that it fixes the build issue in the interim.
(https://github.com/bitcoin/bitcoin/pull/29985#pullrequestreview-2030292710)
ACK 2e266f33b5d2be5c233c2c692481f75785714fa1 - at some point qt's open source 5.15.x branch will catch up to where they bumped the internal zlib to >= 1.3 (which contains this change), and we'll be able to drop this patch. Checked that it fixes the build issue in the interim.
✅ fanquake closed an issue: "depends: Cross-compiling `qt` for `arm-linux-gnueabihf` fails"
(https://github.com/bitcoin/bitcoin/issues/29980)
(https://github.com/bitcoin/bitcoin/issues/29980)
🚀 fanquake merged a pull request: "depends: Fix build of Qt for 32-bit platforms with recent glibc"
(https://github.com/bitcoin/bitcoin/pull/29985)
(https://github.com/bitcoin/bitcoin/pull/29985)
💬 fanquake commented on pull request "depends: Fix build of Qt for 32-bit platforms with recent glibc":
(https://github.com/bitcoin/bitcoin/pull/29985#issuecomment-2084467479)
Backported to 27.x in #29888.
(https://github.com/bitcoin/bitcoin/pull/29985#issuecomment-2084467479)
Backported to 27.x in #29888.
📝 Shutch147 opened 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
...
(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
...
💬 Shutch147 commented on pull request "Update ci.yml":
(https://github.com/bitcoin/bitcoin/pull/30002#issuecomment-2084511110)
@Shutch147
(https://github.com/bitcoin/bitcoin/pull/30002#issuecomment-2084511110)
@Shutch147
💬 Shutch147 commented on pull request "Update ci.yml":
(https://github.com/bitcoin/bitcoin/pull/30002#issuecomment-2084511598)
@dependabot
(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!
(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)
(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
...
(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.
(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.
(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:
(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
(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)
(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
...
(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)
(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.
(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
(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.
(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.