Bitcoin Core Github
42 subscribers
125K links
Download Telegram
👍 hebasto approved a pull request: "wallet: Allow user to navigate options while encrypting at creation"
(https://github.com/bitcoin-core/gui/pull/722#pullrequestreview-2058631259)
ACK cccddc03f0c625daeac7158eb20c1508aea5df39, tested on Ubuntu 24.04.
hebasto closed an issue: "Add go back button to `Confirm wallet encryption` window, add cancel button to `wallet to be encrypted` window"
(https://github.com/bitcoin-core/gui/issues/394)
🚀 hebasto merged a pull request: "wallet: Allow user to navigate options while encrypting at creation"
(https://github.com/bitcoin-core/gui/pull/722)
maflcko closed an issue: "decoderawtransaction should use hex or decimal, not both"
(https://github.com/bitcoin/bitcoin/issues/30067)
💬 maflcko commented on issue "decoderawtransaction should use hex or decimal, not both":
(https://github.com/bitcoin/bitcoin/issues/30067#issuecomment-2113105730)
Closing for now as duplicate
💬 ryanofsky commented on pull request "[refactor] Check CTxMemPool options in ctor":
(https://github.com/bitcoin/bitcoin/pull/28830#discussion_r1602025691)
> > Flatten avoid an unnecessary move:

> What do you mean with this?

I was suggesting returning `Options&&` instead of `Options` to be consistent with the validation.cpp flatten code, and to avoid two moves potentially happening on return: One move on the `return opts;` statement which the compiler can't elide because `opts` is not a local variable, and another move after the function returns to initialize `m_opts` member.

In practice, however, it looks like only one move will happen, b
...
👍 ryanofsky approved a pull request: "[refactor] Check CTxMemPool options in ctor"
(https://github.com/bitcoin/bitcoin/pull/28830#pullrequestreview-2058653021)
Code review ACK 33f2a708e392edb2501f555efef34a558da9d717. I actually didn't notice this push when I made my last two comments, so feel free to ignore the comments if you want to keep the current code.
💬 hebasto commented on pull request "Update about logo icon (colour) to denote the chain type of the QT instance in About/ Help Message Window/ Dialog":
(https://github.com/bitcoin-core/gui/pull/762#issuecomment-2113127729)
https://github.com/bitcoin-core/gui/pull/762#issuecomment-1946005139:
> Definitely more consistent this way.

Thank you @GBKS for your designer's opinion!
💬 TheCharlatan commented on pull request "[refactor] Check CTxMemPool options in ctor":
(https://github.com/bitcoin/bitcoin/pull/28830#issuecomment-2113137626)
Sorry for the many pushes, but I think it's important to get this consistent.

Updated 33f2a708e392edb2501f555efef34a558da9d717 -> 856c8563ca342303a83977146df22768bb6e2c7e ([mempoolArgs_9](https://github.com/TheCharlatan/bitcoin/tree/mempoolArgs_9) -> [mempoolArgs_10](https://github.com/TheCharlatan/bitcoin/tree/mempoolArgs_10), [compare](https://github.com/TheCharlatan/bitcoin/compare/mempoolArgs_9..mempoolArgs_10))

* Changed the constructor to be more like the `ChainstateManager`.
👍 pinheadmz approved a pull request: "p2p: detect addnode cjdns peers in GetAddedNodeInfo()"
(https://github.com/bitcoin/bitcoin/pull/30085#pullrequestreview-2058687122)
ACK d0b047494c28381942c09d0cca45baa323bfcffc

Built and tested on arm/macOS. It's a simple fix to recognize CJDNS addresses in `GetAddedNodeInfo()`. Otherwise `CService::IsValid()` will fail because the CJDNS prefix is in a reserved range. Confirmed the test fails on master and passes with the branch.

<details><summary>Show Signature</summary>

```
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

ACK d0b047494c28381942c09d0cca45baa323bfcffc
-----BEGIN PGP SIGNATURE-----

iQIzBAEBCA
...
👍 ryanofsky approved a pull request: "[refactor] Check CTxMemPool options in ctor"
(https://github.com/bitcoin/bitcoin/pull/28830#pullrequestreview-2058693860)
Code review ACK 33f2a708e392edb2501f555efef34a558da9d717, just tweaked since last review to be more consistent with validation.cpp Flatten code

> Sorry for the many pushes, but I think it's important to get this consistent.

Thanks for the update! I agree it's much nicer to be consistent. And sorry for the detour I caused by not fully understanding the original approach.
🤔 hebasto reviewed a pull request: "Update about logo icon (colour) to denote the chain type of the QT instance in About/ Help Message Window/ Dialog"
(https://github.com/bitcoin-core/gui/pull/762#pullrequestreview-2058694015)
If you will touch this branch, mind applying [`clang-format-diff.py`](https://github.com/bitcoin/bitcoin/blob/master/contrib/devtools/clang-format-diff.py)?
💬 hebasto commented on pull request "Update about logo icon (colour) to denote the chain type of the QT instance in About/ Help Message Window/ Dialog":
(https://github.com/bitcoin-core/gui/pull/762#discussion_r1602056777)
Why these two member functions are declared as slots?
💬 ryanofsky commented on pull request "serialization: Support for multiple parameters":
(https://github.com/bitcoin/bitcoin/pull/28929#issuecomment-2113159607)
@sipa, could you re-ack? Only change since your last review was adding a missing `const` ([compare](https://github.com/ryanofsky/bitcoin/compare/pr/iparams.8..pr/iparams.9))
⚠️ achow101 opened an issue: "stringop-overflow warning with GCC 14"
(https://github.com/bitcoin/bitcoin/issues/30114)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

When building with GCC 14, I noticed a new warning which is causing `-Werror` builds to fail

```
In file included from /usr/include/c++/14.1.1/string:51,
from ../../../src/crypto/sha256.h:10,
from ../../../src/hash.h:12,
from ../../../src/pubkey.h:10,
from ../../../src/addresstype.h:9,
from ../../
...
💬 sipa commented on pull request "serialization: Support for multiple parameters":
(https://github.com/bitcoin/bitcoin/pull/28929#issuecomment-2113178142)
utACK 8d491ae9ecf1948ea29f67b50ca7259123f602aa
💬 achow101 commented on issue "stringop-overflow warning with GCC 14":
(https://github.com/bitcoin/bitcoin/issues/30114#issuecomment-2113179206)
I think might be a GCC bug, I can't figure out what is actually wrong with this code that would trigger the warning.
💬 achow101 commented on pull request "crypto: disable asan for sha256_sse4 with clang and -O0":
(https://github.com/bitcoin/bitcoin/pull/30097#issuecomment-2113193720)
ACK 141df0a28810470e53fdbc6d32d3cb4020fe3ca1

It builds :tada:
💬 TheCharlatan commented on pull request "build: LLD based macOS toolchain":
(https://github.com/bitcoin/bitcoin/pull/21778#issuecomment-2113223051)
Guix build (aarch64):
```
cf99b6597c64c4d0cd75176d261a53f24779e8c3dbe9e891d9d166ae9d00d182 guix-build-f58a8678957e/output/aarch64-linux-gnu/SHA256SUMS.part
922e38e2b1a091f20caa014600936689fb9f14bf56fbd80e4c836c518897ff1f guix-build-f58a8678957e/output/aarch64-linux-gnu/bitcoin-f58a8678957e-aarch64-linux-gnu-debug.tar.gz
256c68e14a905aa6933a2dc83cad636a14ee99ac4661a287e7817efe421965ab guix-build-f58a8678957e/output/aarch64-linux-gnu/bitcoin-f58a8678957e-aarch64-linux-gnu.tar.gz
50eafd4c67
...
💬 achow101 commented on pull request "wallet: Implement independent BDB parser":
(https://github.com/bitcoin/bitcoin/pull/26606#discussion_r1602105484)
I think it's fixed this time.