✅ 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.

📝 theuni opened a pull request: "rpc: avoid copying into UniValue"
(https://github.com/bitcoin/bitcoin/pull/30115)
These are the simple (and hopefully obviously correct) copies that can be moves instead.

This is a follow-up from https://github.com/bitcoin/bitcoin/pull/30094#issuecomment-2108751842

As it turns out, there are hundreds of places where we copy UniValues needlessly. It should be the case that moves are always preferred over copies, so there should be no downside to these changes.

@willcl-ark, however, noticed that memory usage may increase in some cases. Logically this makes no sense to
...

💬 sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay)":
(https://github.com/bitcoin/bitcoin/pull/28765#discussion_r1602118753)
I think this can actually be seeded with anything, it doesn't have to be `m_k0`. IMO it'd better not be, to not repurpose something that is meant for something completely different

🚀 achow101 merged a pull request: "serialization: Support for multiple parameters"
(https://github.com/bitcoin/bitcoin/pull/28929)