Bitcoin Core Github
42 subscribers
126K links
Download Telegram
⚠️ 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
...
🤔 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!
💬 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_
...
💬 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
💬 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)
👍 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
...
💬 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
...
🤔 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.
💬 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.
💬 ismaelsadeeq commented on pull request "Use Txid in COutpoint":
(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.
💬 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(
```
💬 fanquake commented on pull request "[26.x] Changes for rc3":
(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)
💬 brunoerg commented on pull request "rpc: add 'getnetmsgstats' RPC":
(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
...
👍 hebasto approved a pull request: "[26.x] Changes for rc3"
(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?
💬 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.
1
fanquake closed an issue: "win: non-static libssp in libbitcoinconsensus DLL"
(https://github.com/bitcoin/bitcoin/issues/28104)
1
🚀 fanquake merged a pull request: "build: Windows SSP roundup"
(https://github.com/bitcoin/bitcoin/pull/28461)