👍 TheCharlatan approved a pull request: "Fee Estimator updates from Validation Interface/CScheduler thread"
(https://github.com/bitcoin/bitcoin/pull/28368#pullrequestreview-1744286806)
Re-ACK 91504cbe0de2b74ef1aa2709761aaf0597ec66a2
(https://github.com/bitcoin/bitcoin/pull/28368#pullrequestreview-1744286806)
Re-ACK 91504cbe0de2b74ef1aa2709761aaf0597ec66a2
💬 Sjors commented on pull request "wallet: batch all individual spkms setup db writes in a single db txn":
(https://github.com/bitcoin/bitcoin/pull/28894#issuecomment-1822850560)
re-utACK f05302427386fe63f4929a7198652cb1e4ab3bcc
CI failure seems spurious
(https://github.com/bitcoin/bitcoin/pull/28894#issuecomment-1822850560)
re-utACK f05302427386fe63f4929a7198652cb1e4ab3bcc
CI failure seems spurious
💬 maflcko commented on pull request "blockstorage: XOR blocksdir *.dat files":
(https://github.com/bitcoin/bitcoin/pull/28052#issuecomment-1822887374)
rebased
(https://github.com/bitcoin/bitcoin/pull/28052#issuecomment-1822887374)
rebased
💬 dergoegge commented on pull request "multiprocess: Add basic type conversion hooks":
(https://github.com/bitcoin/bitcoin/pull/28921#discussion_r1401997881)
I'm gonna try to summarize my understanding of this to make sure I actually got it right.
We want this specialization of `CustomBuildField` for (most) of our serializable types. So we can either define it manually for each type or we can make use of sfinae (like you do here) to automatically only create the specialization for each of the required serializable types.
We also want to be able to further overload `CustomBuildField` for serializable types that can't use this generic specializat
...
(https://github.com/bitcoin/bitcoin/pull/28921#discussion_r1401997881)
I'm gonna try to summarize my understanding of this to make sure I actually got it right.
We want this specialization of `CustomBuildField` for (most) of our serializable types. So we can either define it manually for each type or we can make use of sfinae (like you do here) to automatically only create the specialization for each of the required serializable types.
We also want to be able to further overload `CustomBuildField` for serializable types that can't use this generic specializat
...
⚠️ quakemmo opened an issue: "importing a wallet containing an hdseed overwrites target wallet hdseed"
(https://github.com/bitcoin/bitcoin/issues/28927)
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Current behaviour
My basic idea was to have two nodes running with the same keys, if one fails, I can just rescan the blockchain on the other one and failover to it seamlessly.
I made a wallet on Node1, generated a few thousands addresses, saved the dumpwallet. Now I want to import the same keys to Node2, but I want it to be an HD wallet from the same seed, to avoid the headache of sendtoaddress, when
...
(https://github.com/bitcoin/bitcoin/issues/28927)
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Current behaviour
My basic idea was to have two nodes running with the same keys, if one fails, I can just rescan the blockchain on the other one and failover to it seamlessly.
I made a wallet on Node1, generated a few thousands addresses, saved the dumpwallet. Now I want to import the same keys to Node2, but I want it to be an HD wallet from the same seed, to avoid the headache of sendtoaddress, when
...
🤔 vasild reviewed a pull request: "rpc: add 'getnetmsgstats' RPC"
(https://github.com/bitcoin/bitcoin/pull/28926#pullrequestreview-1744577141)
Concept ACK
I was looking at this recently, and got some ideas how to simplify and better organize this, will share here. Thanks for taking charge!
(https://github.com/bitcoin/bitcoin/pull/28926#pullrequestreview-1744577141)
Concept ACK
I was looking at this recently, and got some ideas how to simplify and better organize this, will share here. Thanks for taking charge!
💬 vasild commented on pull request "rpc: add 'getnetmsgstats' RPC":
(https://github.com/bitcoin/bitcoin/pull/28926#discussion_r1402168125)
All these plus the `NUM_NET_MESSAGE_TYPES` constant can be avoided if the list of all message types is turned into a `std::array`. Consider this:
<details>
<summary>[patch] make the list of known message types a compile time constant</summary>
```diff
diff --git a/src/net.cpp b/src/net.cpp
index a2f80cbcf7..133abae117 100644
--- a/src/net.cpp
+++ b/src/net.cpp
@@ -3670,13 +3670,13 @@ CNode::CNode(NodeId idIn,
nLocalHostNonce{nLocalHostNonceIn},
m_recv_flood_size{node_
...
(https://github.com/bitcoin/bitcoin/pull/28926#discussion_r1402168125)
All these plus the `NUM_NET_MESSAGE_TYPES` constant can be avoided if the list of all message types is turned into a `std::array`. Consider this:
<details>
<summary>[patch] make the list of known message types a compile time constant</summary>
```diff
diff --git a/src/net.cpp b/src/net.cpp
index a2f80cbcf7..133abae117 100644
--- a/src/net.cpp
+++ b/src/net.cpp
@@ -3670,13 +3670,13 @@ CNode::CNode(NodeId idIn,
nLocalHostNonce{nLocalHostNonceIn},
m_recv_flood_size{node_
...
💬 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_r1402210346)
updated the commit message in [9a918a4](https://github.com/bitcoin/bitcoin/pull/28885/commits/9a918a4720ee091344a16dcd703d169ebdb7938e)
I also fixed the other ones that had refactor in the commit message since they weren't really refactors either
(https://github.com/bitcoin/bitcoin/pull/28885#discussion_r1402210346)
updated the commit message in [9a918a4](https://github.com/bitcoin/bitcoin/pull/28885/commits/9a918a4720ee091344a16dcd703d169ebdb7938e)
I also fixed the other ones that had refactor in the commit message since they weren't really refactors either
💬 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.