✅ maflcko closed a pull request: "test: handle wallet_reorgrestore.py failed"
(https://github.com/bitcoin/bitcoin/pull/29395)

💬 maflcko commented on pull request "test: handle wallet_reorgrestore.py failed":
(https://github.com/bitcoin/bitcoin/pull/29395#issuecomment-1933738018)
I am not sure if your replies are ChatGPT genereated or not, but this clearly isn't the correct fix.

If your replies are ChatGPT generated, please don't. If anyone was interested in computer-generated hallucinations, they could do so themselves.

It is not the correct fix, because:

* The race isn't explained, either theoretically by pointing to the exact line(s) in the source code where it happens, or practically by adding a sleep to force the bug to appear for testing, as explained befo
...

✅ maflcko closed an issue: "Bitcoin ubuntu ppa, outdated"
(https://github.com/bitcoin/bitcoin/issues/29406)

💬 maflcko commented on issue "Bitcoin ubuntu ppa, outdated":
(https://github.com/bitcoin/bitcoin/issues/29406#issuecomment-1933755423)
The PPA was retired, because it didn't use the deterministically generated guix release builds. However, it seems unlikely to be maintained again, unless a new maintainer steps up to maintain it going forward.

In any case, there are endless other ways to get the latest version of Bitcoin Core:

* Self-compile any release tag. Ref: https://github.com/bitcoin/bitcoin/releases
* Download the deterministic release binary. Ref: https://bitcoincore.org/en/download/
* Use the snap, flat, guix or
...

💬 maflcko commented on pull request "test, assumeutxo: Add test to ensure failure when mempool not empty":
(https://github.com/bitcoin/bitcoin/pull/29394#discussion_r1482735029)
Ah right, the nodes are on different chains, so the utxo sets the miniwallet can use are different.

💬 maflcko commented on pull request "util: Faster std::byte (pre)vector (un)serialize":
(https://github.com/bitcoin/bitcoin/pull/29114#discussion_r1482747927)
Ah, will undo if I retouch.

💬 maflcko commented on pull request "util: Faster std::byte (pre)vector (un)serialize":
(https://github.com/bitcoin/bitcoin/pull/29114#issuecomment-1933790588)
> It's a nice cleanup, but I don't observe a big speedup on my machine after applying the diff and running the bench mentioned in the description on top of [fab4169](https://github.com/bitcoin/bitcoin/commit/fab41697a5448ef2861f65795bd63a4ccdda6a40).

Can you clarify if there is no speedup, or just a smaller one? Also, what it the speed for `uint8_t` on master vs `std::byte` on master vs `std::byte` after this pull?

💬 TheCharlatan commented on pull request "bitcoin-config.h includes cleanup":
(https://github.com/bitcoin/bitcoin/pull/29404#issuecomment-1933987554)
I validated the patch by running:
\> `iwyu_tool -p . -j $(nproc) -- -Xiwyu --max_line_length=180 -I/usr/lib/llvm-14/lib/clang/14/include/ -Xiwyu --mapping_file=/home/drgrid/bitcoin/contrib/devtools/iwyu/bitcoin.core.imp > iwyu_output.txt`
\> `cat iwyu_output.txt | awk '/should remove these lines:/ {file=$1} /- #include <config\/bitcoin-config.h>/ {split($NF, a, "-"); line=a[1]; if (!seen[file, line]++) print file " " a[1]}' | while read -r file line; do sed -i -e "$((line-1)),$((line+2))d" "sr
...

💬 TheCharlatan commented on pull request "RFC: build: remove confusing and inconsistent disable-asm option":
(https://github.com/bitcoin/bitcoin/pull/29407#issuecomment-1933988455)
Concept ACK

💬 brunoerg commented on pull request "p2p: Allow whitelisting manual connections":
(https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1482919503)
Yes, I'll address it.

💬 brunoerg commented on pull request "p2p: Allow whitelisting manual connections":
(https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1482929854)
Where is it calling `NetPermissions::Initialize` again?

💬 MarnixCroes commented on pull request "debugwindow: update session ID tooltip":
(https://github.com/bitcoin-core/gui/pull/788#issuecomment-1934067523)
> > When you have a v2 connection, there is always a session ID.
>
> What if the connection type is still `DETECTING`?
>
> The code reference:
>
> https://github.com/bitcoin-core/gui/blob/6737331c4ccc6da578bb44c809cc4e18f017ba51/src/node/connection_types.h#L83-L88

then there is no session ID property
as it is not a v2 peer (yet), right

📝 maflcko opened a pull request: " lint: Check for missing bitcoin-config.h includes "
(https://github.com/bitcoin/bitcoin/pull/29408)
Missing `bitcoin-config.h` includes are problematic, because the build could silently pass, but produce an unintended result. For example, a slower fallback algorithm could be picked, even though `bitcoin-config.h` indicates that a faster feature is available and should be used.

As the build succeeds silently, this problem is not possible to detect with iwyu.

Thus, fix this by using a linter based on grepping the source code.

📝 maflcko converted_to_draft a pull request: "lint: Check for missing bitcoin-config.h includes"
(https://github.com/bitcoin/bitcoin/pull/29408)
Missing `bitcoin-config.h` includes are problematic, because the build could silently pass, but produce an unintended result. For example, a slower fallback algorithm could be picked, even though `bitcoin-config.h` indicates that a faster feature is available and should be used.

As the build succeeds silently, this problem is not possible to detect with iwyu.

Thus, fix this by using a linter based on grepping the source code.

💬 maflcko commented on pull request "lint: Check for missing bitcoin-config.h includes":
(https://github.com/bitcoin/bitcoin/pull/29408#issuecomment-1934074211)
The idea to `grep` was taken from theuni (https://github.com/bitcoin/bitcoin/issues/26972#issuecomment-1930911069)

✅ maflcko closed an issue: "doc: List identifiers used from header in #if(def)"
(https://github.com/bitcoin/bitcoin/issues/26972)

💬 maflcko commented on issue "doc: List identifiers used from header in #if(def)":
(https://github.com/bitcoin/bitcoin/issues/26972#issuecomment-1934080395)
> I used a few combinations of git grep -L -E <tokens> to find all files which don't require bitcoin-config.h, and used that in a git grep -l bitcoin-config.h -- <those files> to find the files that don't require the include.

Iwyu should be able to find includes that are present, but not needed. So ideally, your result should match what iwyu finds.

While iwyu can find some *missing* includes, it can't find *all*. So I've written a lint check in https://github.com/bitcoin/bitcoin/pull/29408
...

💬 brunoerg commented on pull request "p2p: Allow whitelisting manual connections":
(https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1482969906)
Yes, we can call `Init` without setting all values in `Options`. I will change it to use `DEFAULT_WHITELISTFORCERELAY` and `DEFAULT_WHITELISTRELAY`.

💬 daniel-btc-nym commented on issue "getJsonToken assumes underlying string is null-terminated but requires end pointer":
(https://github.com/bitcoin/bitcoin/issues/28260#issuecomment-1934115970)
I still need to look into how MSAN is integrated into the current unit test suite. Questions like: how can I run the unit tests I added under MSAN? Are all unit tests run under MSAN with a certain make target ... the basics. I am very new to this repository here :)

💬 brunoerg commented on pull request "p2p: Allow whitelisting manual connections":
(https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1482974024)
Nice catch! I'll address it.