💬 kevkevinpal commented on pull request "mempool / rpc: followup to getprioritisedtransactions and delete a mapDeltas entry when delta==0":
(https://github.com/bitcoin/bitcoin/pull/28885#discussion_r1402215512)
thanks! fixed in [e3f2fc0](https://github.com/bitcoin/bitcoin/pull/28885/commits/e3f2fc026a705148221de1bd0ecb37a26c3a4b36)
(https://github.com/bitcoin/bitcoin/pull/28885#discussion_r1402215512)
thanks! fixed in [e3f2fc0](https://github.com/bitcoin/bitcoin/pull/28885/commits/e3f2fc026a705148221de1bd0ecb37a26c3a4b36)
👍 pablomartin4btc approved a pull request: "coins: make sure PoolAllocator uses the correct alignment"
(https://github.com/bitcoin/bitcoin/pull/28913#pullrequestreview-1744665014)
Tested ACK d5b4c0b69e543de51bb37d602d488ee0949ba185
<details>
<summary>Reproduced the issue on <code>master</code> using emulated ARM 32 getting the OOM at height
318617.</summary>
```
2023-11-22T08:24:03Z UpdateTip: new best=00000000000000001771de0c2dc0ff95d619943d4160308c93f9184fcee8d5f8 height=318617 version=0x00000002 log2_work=80.467750 tx=45766401 date='2014-09-01T14:44:48Z' progress=0.049383 cache=107.7MiB(14296037txo)
[44951.075682] b-addcon invoked oom-killer: gfp_mask=0x140cc
...
(https://github.com/bitcoin/bitcoin/pull/28913#pullrequestreview-1744665014)
Tested ACK d5b4c0b69e543de51bb37d602d488ee0949ba185
<details>
<summary>Reproduced the issue on <code>master</code> using emulated ARM 32 getting the OOM at height
318617.</summary>
```
2023-11-22T08:24:03Z UpdateTip: new best=00000000000000001771de0c2dc0ff95d619943d4160308c93f9184fcee8d5f8 height=318617 version=0x00000002 log2_work=80.467750 tx=45766401 date='2014-09-01T14:44:48Z' progress=0.049383 cache=107.7MiB(14296037txo)
[44951.075682] b-addcon invoked oom-killer: gfp_mask=0x140cc
...
💬 ryanofsky commented on pull request "multiprocess: Add basic type conversion hooks":
(https://github.com/bitcoin/bitcoin/pull/28921#discussion_r1402240033)
> As I understand it, this comment implies that (without `&& std::is_same_v<LocalType, std::remove_cv_t<std::remove_reference_t<LocalType>>>`) the following would not take precedence, even though it has `Priority<2>`?
>
> ```c++
> template <typename LocalType, typename Output>
> void CustomBuildField(TypeList<LocalType>, Priority<2>, InvokeContext& invoke_context, const CTransaction& value, Output&& output)
> ```
Yes that's right, but it would be more correct to say "would not *reliably
...
(https://github.com/bitcoin/bitcoin/pull/28921#discussion_r1402240033)
> As I understand it, this comment implies that (without `&& std::is_same_v<LocalType, std::remove_cv_t<std::remove_reference_t<LocalType>>>`) the following would not take precedence, even though it has `Priority<2>`?
>
> ```c++
> template <typename LocalType, typename Output>
> void CustomBuildField(TypeList<LocalType>, Priority<2>, InvokeContext& invoke_context, const CTransaction& value, Output&& output)
> ```
Yes that's right, but it would be more correct to say "would not *reliably
...
🤔 pablomartin4btc reviewed a pull request: "getrawtransaction implementation"
(https://github.com/bitcoin-core/gui/pull/777#pullrequestreview-1744793504)
Concept ACK
As @luke-jr perhaps we can create a separate menu item like "Tools" or "Utils" with this feature in it (and would be more suitable if we need to add for example other features as @willcl-ark [mentioned](https://github.com/bitcoin-core/gui/pull/777#issuecomment-1810068057)).
Light CR.
(https://github.com/bitcoin-core/gui/pull/777#pullrequestreview-1744793504)
Concept ACK
As @luke-jr perhaps we can create a separate menu item like "Tools" or "Utils" with this feature in it (and would be more suitable if we need to add for example other features as @willcl-ark [mentioned](https://github.com/bitcoin-core/gui/pull/777#issuecomment-1810068057)).
Light CR.
💬 pablomartin4btc commented on pull request "getrawtransaction implementation":
(https://github.com/bitcoin-core/gui/pull/777#discussion_r1402321260)
Since you are here (refactoring commit 10b3cd129e1fa8ece229a255d6fff89f419751c0) I'd extract all these settings into a function like "`initializeAboutModeWindow`" or something like that, same for the other modes, making the code a bit more maintainable and easier to follow.
(https://github.com/bitcoin-core/gui/pull/777#discussion_r1402321260)
Since you are here (refactoring commit 10b3cd129e1fa8ece229a255d6fff89f419751c0) I'd extract all these settings into a function like "`initializeAboutModeWindow`" or something like that, same for the other modes, making the code a bit more maintainable and easier to follow.
💬 ismaelsadeeq commented on pull request "Use Txid in COutpoint":
(https://github.com/bitcoin/bitcoin/pull/28922#issuecomment-1823047081)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/28922#issuecomment-1823047081)
Concept ACK
💬 fanquake commented on pull request "rpc: add 'getnetmsgstats', new rpc to view network message statistics":
(https://github.com/bitcoin/bitcoin/pull/27534#issuecomment-1823088898)
Picked up in #28926.
(https://github.com/bitcoin/bitcoin/pull/27534#issuecomment-1823088898)
Picked up in #28926.
💬 fanquake commented on pull request "rpc: add 'getnetmsgstats' RPC":
(https://github.com/bitcoin/bitcoin/pull/28926#issuecomment-1823090489)
```bash
clang-tidy-17 -p=/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu -quiet -load=/tidy-build/libbitcoin-tidy.so /ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/rpc/net.cpp
rpc/net.cpp:726:30: error: use emplace_back instead of push_back [modernize-use-emplace,-warnings-as-errors]
726 | path.push_back("totals");
| ^~~~~~~~~~
| emplace_back(
```
(https://github.com/bitcoin/bitcoin/pull/28926#issuecomment-1823090489)
```bash
clang-tidy-17 -p=/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu -quiet -load=/tidy-build/libbitcoin-tidy.so /ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/rpc/net.cpp
rpc/net.cpp:726:30: error: use emplace_back instead of push_back [modernize-use-emplace,-warnings-as-errors]
726 | path.push_back("totals");
| ^~~~~~~~~~
| emplace_back(
```
💬 fanquake commented on pull request "[26.x] Changes for rc3":
(https://github.com/bitcoin/bitcoin/pull/28872#discussion_r1402376062)
No worries, fixed up.
(https://github.com/bitcoin/bitcoin/pull/28872#discussion_r1402376062)
No worries, fixed up.
🚀 fanquake merged a pull request: "lint: Report all lint errors instead of early exit"
(https://github.com/bitcoin/bitcoin/pull/28862)
(https://github.com/bitcoin/bitcoin/pull/28862)
💬 brunoerg commented on pull request "rpc: add 'getnetmsgstats' RPC":
(https://github.com/bitcoin/bitcoin/pull/28926#issuecomment-1823155627)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/28926#issuecomment-1823155627)
Concept ACK
💬 fanquake commented on pull request "build: Fix regression in "ARMv8 CRC32 intrinsics" test":
(https://github.com/bitcoin/bitcoin/pull/28919#issuecomment-1823164420)
> The vmull_p64 is a part of the Crypto extensions from the ACLE. They are optional extensions, so they get enabled with a +crypto for architecture flags.
I guess this isn't true for all aarch64, as the checks currently work fine, for example, on Apple arm64 (M1):
```bash
checking for AVX2 intrinsics... no
checking for x86 SHA-NI intrinsics... no
checking whether C++ compiler accepts -march=armv8-a+crc... yes
checking whether C++ compiler accepts -march=armv8-a+crypto... yes
checking fo
...
(https://github.com/bitcoin/bitcoin/pull/28919#issuecomment-1823164420)
> The vmull_p64 is a part of the Crypto extensions from the ACLE. They are optional extensions, so they get enabled with a +crypto for architecture flags.
I guess this isn't true for all aarch64, as the checks currently work fine, for example, on Apple arm64 (M1):
```bash
checking for AVX2 intrinsics... no
checking for x86 SHA-NI intrinsics... no
checking whether C++ compiler accepts -march=armv8-a+crc... yes
checking whether C++ compiler accepts -march=armv8-a+crypto... yes
checking fo
...
👍 hebasto approved a pull request: "[26.x] Changes for rc3"
(https://github.com/bitcoin/bitcoin/pull/28872#pullrequestreview-1744950215)
re-ACK f868852d3d7321aa522d6b168f33bef1e5eba2dc.
(https://github.com/bitcoin/bitcoin/pull/28872#pullrequestreview-1744950215)
re-ACK f868852d3d7321aa522d6b168f33bef1e5eba2dc.
💬 hebasto commented on pull request "build: Fix regression in "ARMv8 CRC32 intrinsics" test":
(https://github.com/bitcoin/bitcoin/pull/28919#issuecomment-1823172024)
> > The vmull_p64 is a part of the Crypto extensions from the ACLE. They are optional extensions, so they get enabled with a +crypto for architecture flags.
>
> I guess this isn't true for all aarch64, as the checks currently work fine, for example, on Apple arm64 (M1)
Right. Mind suggesting a more precise wording?
(https://github.com/bitcoin/bitcoin/pull/28919#issuecomment-1823172024)
> > The vmull_p64 is a part of the Crypto extensions from the ACLE. They are optional extensions, so they get enabled with a +crypto for architecture flags.
>
> I guess this isn't true for all aarch64, as the checks currently work fine, for example, on Apple arm64 (M1)
Right. Mind suggesting a more precise wording?
💬 fanquake commented on pull request "build: Fix regression in "ARMv8 CRC32 intrinsics" test":
(https://github.com/bitcoin/bitcoin/pull/28919#issuecomment-1823179632)
> I guess this isn't true for all aarch64, as the checks currently work fine, for example, on Apple arm64 (M1):
Actually, I think this is just an Apple special case, and because Apple Clang just enables `+crypto` and +`crc` as part of the default compile flags.
(https://github.com/bitcoin/bitcoin/pull/28919#issuecomment-1823179632)
> I guess this isn't true for all aarch64, as the checks currently work fine, for example, on Apple arm64 (M1):
Actually, I think this is just an Apple special case, and because Apple Clang just enables `+crypto` and +`crc` as part of the default compile flags.
❤1
✅ fanquake closed an issue: "win: non-static libssp in libbitcoinconsensus DLL"
(https://github.com/bitcoin/bitcoin/issues/28104)
(https://github.com/bitcoin/bitcoin/issues/28104)
❤1
🚀 fanquake merged a pull request: "build: Windows SSP roundup"
(https://github.com/bitcoin/bitcoin/pull/28461)
(https://github.com/bitcoin/bitcoin/pull/28461)
🚀 fanquake merged a pull request: "build: Fix regression in "ARMv8 CRC32 intrinsics" test"
(https://github.com/bitcoin/bitcoin/pull/28919)
(https://github.com/bitcoin/bitcoin/pull/28919)
💬 hebasto commented on pull request "build: Fix regression in "ARMv8 CRC32 intrinsics" test":
(https://github.com/bitcoin/bitcoin/pull/28919#issuecomment-1823187990)
Is it worth backporting to 26.x?
(https://github.com/bitcoin/bitcoin/pull/28919#issuecomment-1823187990)
Is it worth backporting to 26.x?
💬 fanquake commented on pull request "build: Fix regression in "ARMv8 CRC32 intrinsics" test":
(https://github.com/bitcoin/bitcoin/pull/28919#issuecomment-1823188307)
Backported in #28872.
(https://github.com/bitcoin/bitcoin/pull/28919#issuecomment-1823188307)
Backported in #28872.
👍 hebasto approved a pull request: "[26.x] Changes for rc3"
(https://github.com/bitcoin/bitcoin/pull/28872#pullrequestreview-1744989681)
re-ACK 2f86d3053314c382dfce5cf314e98811ed433859, only https://github.com/bitcoin/bitcoin/pull/28919 backported since my [recent](https://github.com/bitcoin/bitcoin/pull/28872#pullrequestreview-1744950215) review.
(https://github.com/bitcoin/bitcoin/pull/28872#pullrequestreview-1744989681)
re-ACK 2f86d3053314c382dfce5cf314e98811ed433859, only https://github.com/bitcoin/bitcoin/pull/28919 backported since my [recent](https://github.com/bitcoin/bitcoin/pull/28872#pullrequestreview-1744950215) review.