💬 Sjors commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1732943642)
I'm a bit confused where you want this code to go...

💬 Sjors commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#issuecomment-2312700483)
@laanwj I suggest adding a "Suggested Followups" section to the PR description, where you can list known issues that can be punted.

💬 hebasto commented on pull request "guix: Pointer Authentication and Branch Target Identification for aarch64 Linux (Guix)":
(https://github.com/bitcoin/bitcoin/pull/24123#issuecomment-2312711208)
It would be helpful if someone with BTI-enabled hardware could test the binaries and verified BTI during [runtime](https://wiki.debian.org/ToolChain/PACBTI).

Unfortunately, I'm unable to do it by myself, as my hardware supports only PAC, not BTI.

💬 github12101 commented on issue "b-msghand[4988] general protection fault":
(https://github.com/bitcoin/bitcoin/issues/30706#issuecomment-2312715012)
> Thank you for reporting this.
>
> While issues in `b-msghand` can be problematic and it's good to report them, I don't think this report contains any actionable details. The gbd backtrace only contains memory addresses and no further debug information.
>
> Unless you can share more information or you don't start to see more frequent segmentation faults, I don't think it's worth keeping this issue open.

Thanks for your reply. I am aware backtrace only contains ??? instead of anything u
...

💬 hodlinator commented on pull request "refactor: Testnet4 - Replace uint256S("str") -> uint256{"str"}":
(https://github.com/bitcoin/bitcoin/pull/30721#issuecomment-2312729226)
#29775 was merged into master as da083d4bbdb37737f5080fada97bd15f5a8bfb2d, and #30560 was merged into master as 21c2879f37ce336af6df878d43ab090eb9d02157.

I'm thinking CI running on 21c2879f37ce336af6df878d43ab090eb9d02157 upon merge, could detect a new scripted-diff, re-run it on 21c2879f37ce336af6df878d43ab090eb9d02157 and in theory detect issues with #29775 if it had been merged into master earlier (wouldn't have caught this case though, as da083d4bbdb37737f5080fada97bd15f5a8bfb2d was merge
...

💬 cdecker commented on pull request "chainparams: Remove seed.bitcoinstats.com":
(https://github.com/bitcoin/bitcoin/pull/30720#issuecomment-2312740874)
Great, I'm just testing it as we speak, and I'll open a new PR adding it, once its shown to work, if that's ok.

💬 maflcko commented on issue "b-msghand[4988] general protection fault":
(https://github.com/bitcoin/bitcoin/issues/30706#issuecomment-2312745148)
The guix debug symbols are split from the `bitcoind` you are using. So someone would have download them before demangling. I tried to do that, but for some reason I had no success. I am not a build system expert, so I am not sure where the problem lies.

👍 stickies-v approved a pull request: "bench: [refactor] iwyu"
(https://github.com/bitcoin/bitcoin/pull/30716#pullrequestreview-2263571847)
ACK fab0e834b8cf906c286ebbeed86eca8d65854f75

Reviewed by:
- checking that all changes are limited to updating includes and fwd declarations in `bench/` files
- checking the clang-tidy [CI job output](https://api.cirrus-ci.com/v1/task/4676750536343552/logs/ci.log) for suggestions on `bench/` files, yielding only one result for `bench/bench.h` (as already pointed out by other reviewers):
```
grep "bench\/.*should" -A 3
bench/bench.h should add these lines:
#include "nanobench.h" // for
...

💬 stickies-v commented on pull request "bench: [refactor] iwyu":
(https://github.com/bitcoin/bitcoin/pull/30716#discussion_r1732974884)
Review note: this include (and thus the new pragma) is necessary to compile `setup_common.cpp`. Without it, fails with

```
Undefined symbols for architecture arm64:
"_G_TEST_COMMAND_LINE_ARGUMENTS", referenced from:
BasicTestingSetup::BasicTestingSetup(ChainType, TestOpts) in libtest_util.a[11](libtest_util_a-setup_common.o)
"_G_TEST_GET_FULL_NAME", referenced from:
BasicTestingSetup::BasicTestingSetup(ChainType, TestOpts) in libtest_util.a[11](libtest_util_a-setup_common
...

📝 brunoerg opened a pull request: "fuzz: fix timeout in `crypto_fschacha20poly1305`"
(https://github.com/bitcoin/bitcoin/pull/30725)
This PR fixes a timeout in `crypto_fschacha20poly1305` by reducing the number of iterations. I left it running for a while and noticed it speeds up the target and do not impact coverage.

💬 brunoerg commented on pull request "fuzz: fix timeout in `crypto_fschacha20poly1305`":
(https://github.com/bitcoin/bitcoin/pull/30725#issuecomment-2312773626)
I think it can be tested by replacing the `ConsumeBool()` to `true` which causes a timeout quickly.
```diff
--- a/src/test/fuzz/crypto_chacha20poly1305.cpp
+++ b/src/test/fuzz/crypto_chacha20poly1305.cpp
@@ -130,7 +130,7 @@ FUZZ_TARGET(crypto_fschacha20poly1305)
// data).
InsecureRandomContext rng(provider.ConsumeIntegral<uint64_t>());

- LIMITED_WHILE(provider.ConsumeBool(), 10000)
+ LIMITED_WHILE(true, 10000)
{
// Mode:
```

Maybe this is a way to k
...

💬 instagibbs commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#issuecomment-2312787737)
Just posting in case someone else learns something. Not asking for any changes:

> Lastly, minimum time skips were increased to 100ms, meaning 100ms*100000=~166 minutes of simulated time. Since there are up to 8+120=128 peers, this means each peer can stall out the node for a full minute and we still have 40 minutes left for the honest peer to succeed in giving the tx.

After doing more study, I think this simulation "only" needs a little more than 7 minutes minimum time, since the internal
...

💬 github12101 commented on issue "b-msghand[4988] general protection fault":
(https://github.com/bitcoin/bitcoin/issues/30706#issuecomment-2312795052)
> The guix debug symbols are split from the `bitcoind` you are using. So someone would have download them before demangling. I tried to do that, but for some reason I had no success. I am not a build system expert, so I am not sure where the problem lies.

My knowledge goes only as far as adding `deb http://deb.debian.org/debian-debug/ bookworm-debug main` to the sources.list and then installing *-dbgsym packages. But that will not include bitcoind. If someone has any instructions how to get d
...

👍 hebasto approved a pull request: "build: add `standard branch-protection` to hardening flags for aarch64-linux"
(https://github.com/bitcoin/bitcoin/pull/30433#pullrequestreview-2263686690)
ACK 03171002e5de04953ecd220cce40313f42315fcb. Tested in both cross and native build scenarios.

💬 maflcko commented on pull request "fuzz: fix timeout in `crypto_fschacha20poly1305`":
(https://github.com/bitcoin/bitcoin/pull/30725#issuecomment-2312811224)
Please cross-link to the issue (https://github.com/bitcoin/bitcoin/issues/30505)

💬 0xB10C commented on issue "b-msghand[4988] general protection fault":
(https://github.com/bitcoin/bitcoin/issues/30706#issuecomment-2312820074)
>> Was there anything unusual in the debug log before the crash?

> Logs contains nothing else worth mentioning, machine continues to run as intended.

Do you mean the machine logs contain nothing worth mentioning or the Bitcoin Core `debug.log`?

Just in case you are not aware: Bitcoin Core writes to a `debug.log` file in your data directory (where the blockchain is stored too). It would be useful to know what Bitcoin Core logged at around `2024-08-23 23:47:55 BST`.

💬 hebasto commented on pull request "fuzz: Add missing fuzz targets to cmake build":
(https://github.com/bitcoin/bitcoin/pull/30712#discussion_r1733048320)
I've noticed the same -- https://github.com/hebasto/bitcoin/issues/341#issuecomment-2310510852:
> However, on my machine, I had to add `--timeout-factor=4`.

💬 maflcko commented on issue "b-msghand[4988] general protection fault":
(https://github.com/bitcoin/bitcoin/issues/30706#issuecomment-2312826024)
The debug symbols are separately produced by guix and the downloads are on https://bitcoincore.org/bin/bitcoin-core-27.1/. In this case it would be `x86_64-linux-gnu-debug`.

💬 maflcko commented on pull request "Pre-28.x branch off version bump and doc updates":
(https://github.com/bitcoin/bitcoin/pull/30719#issuecomment-2312839785)
> lgtm, should also do doc/release-notes/release-notes-27064.md

Added to the wiki, but still needs to be removed here.

👍 hebasto approved a pull request: "Pre-28.x branch off version bump and doc updates"
(https://github.com/bitcoin/bitcoin/pull/30719#pullrequestreview-2263736245)
ACK d3fd608100f6f4aff2a9f7a25b43f6c5ae065024.