💬 maflcko commented on pull request "refactor: Testnet4 - Replace uint256S("str") -> uint256{"str"}":
(https://github.com/bitcoin/bitcoin/pull/30721#issuecomment-2312650978)
When a commit from a pull request (which may contain a scripted-diff) is merged into another branch, the commit itself does not change, so it won't affect the execution or the result of the scripted-diff in that commit.

In theory, the commits could be rebased/cherry-picked instead of merged. However, that'd make auditing harder, because it makes it easier to inject malicious code accidentally or intentionally. Also, it wouldn't help here, because then the scripted diff check may permanently f
...

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