Bitcoin Core Github
44 subscribers
121K links
Download Telegram
💬 Sjors commented on issue "guix: Windows build is non-deterministic across build architectures":
(https://github.com/bitcoin/bitcoin/issues/32923#issuecomment-3055925920)
The most recent change in `prevector.h`, and the only one that hasn't been shipped in a release yet, was a40e9536588c366886de4f4b9d67b8665a509929. But that just removed the reverse iterator, which this doesn't use afaik.

You could try initiazing `std::vector<CRecipient> vecSend;` with the size of `recipients`, which might result in different code paths. Though even if that restored determinism it wouldn't fix the underlying issue.
💬 achow101 commented on issue "guix: Windows build is non-deterministic across build architectures":
(https://github.com/bitcoin/bitcoin/issues/32923#issuecomment-3055948842)
#32930 seems like it results in a different enough code path to workaround this issue.

Ultimately, this seems like an upstream compiler bug.
💬 Sjors commented on pull request "Resolve guix non-determinism with emplace_back instead of push_back":
(https://github.com/bitcoin/bitcoin/pull/32930#issuecomment-3055983620)
Concept ACK

I'll spin up a new aarch64 guix machine to test if this _actually_ makes determinism go away. Even if it doesn't the change itself is fine since it removes a line, but you'd have to change the commit message.

3add4b365c90c0943f18dd26d4a18c1e2ee5860e builds and passes the tests locally for me on Apple Silicon macOS 15.5. Not sure what's up with CI.
💬 maflcko commented on pull request "rpc, test: Fix JSON parsing errors in unloadwallet and getdescriptoractivity RPCs":
(https://github.com/bitcoin/bitcoin/pull/32845#discussion_r2196815142)
I think the function from a functionality perspective is fine, but symbols in the signature could be renamed for clarity.
👍 stratospher approved a pull request: "validation: invalid block handling followups"
(https://github.com/bitcoin/bitcoin/pull/32843#pullrequestreview-3004293734)
ACK e32e72039f02ea0b565e0ef5ab7fa40faeb65e4c.
💬 maflcko commented on pull request "Resolve guix non-determinism with emplace_back instead of push_back":
(https://github.com/bitcoin/bitcoin/pull/32930#issuecomment-3056019241)
Congrats, you are triggering another compiler bug. See 9999dbc1bd171931f02266d7c1a5cfd39f49238e for more details.
💬 Sjors commented on issue "Guix build fails on M4 macOS host with Ubuntu in UTM":
(https://github.com/bitcoin/bitcoin/issues/32759#issuecomment-3056031910)
Going to setup a fresh UTM instance, this time using Apple Virtualization instead of Qemu. Let's see if that (a) works and (b) doesn't break again after a few months.
💬 maflcko commented on pull request "Resolve guix non-determinism with emplace_back instead of push_back":
(https://github.com/bitcoin/bitcoin/pull/32930#discussion_r2196837764)
```suggestion
vecSend.emplace_back(CRecipient {DecodeDestination(rcp.address.toStdString()), rcp.amount, rcp.fSubtractFeeFromAmount});
```

I don't have an apple, but I guess this may be the patch to work around the xcode compiler bug. I don't know if that still works around the non-determinism.

As a completely different alternative, you could also try https://github.com/bitcoin/bitcoin/pull/32597#discussion_r2141169742 to remove the `std::string` construction from the header file a
...
💬 maflcko commented on pull request "log: Mitigate disk filling attacks by rate limiting LogPrintf, LogInfo, LogWarning, LogError":
(https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2196962104)
> I think there are a number of issues with this part of the test:

I also noticed those in my review, but also noticed you mentioned it already.
💬 maflcko commented on pull request "log: Mitigate disk filling attacks by rate limiting LogPrintf, LogInfo, LogWarning, LogError":
(https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2196975759)
yeah, i think the `std::hash` can be dropped here
🚀 fanquake merged a pull request: "test: Turn rpcauth.py test into functional test"
(https://github.com/bitcoin/bitcoin/pull/32881)
📝 maflcko opened a pull request: "test: Add missing convert_to_json_for_cli"
(https://github.com/bitcoin/bitcoin/pull/32932)
💬 maflcko commented on pull request "wallet, test: best block locator matches scan state follow-ups":
(https://github.com/bitcoin/bitcoin/pull/32580#issuecomment-3056445715)
silent conflict fixed in https://github.com/bitcoin/bitcoin/pull/32932
📝 maflcko opened a pull request: "log: Properly log warnings with warn loglevel in addrdb"
(https://github.com/bitcoin/bitcoin/pull/32933)
The logging in addrdb is confusing, because it uses `LogPrintf` (info level) to log warnings.

Fix this by properly using the `warn` level, where needed. Also, drop unused trailing `\n` while touching the lines.
💬 l0rinc commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2197128817)
> rename from xor to obfuscation is really needed or meaningful

My thinking was that `xor` describes the implementation while `obfuscation` describes the purpose - there are many xor uses throughout the codebase, but fewer places where we specifically need obfuscation.
I also noticed that our existing patterns already lean toward the obfuscation terminology: the printed messages reference `obfuscation key`, the database entry is `obfuscate_key`, and the parameter is `.obfuscate`.

> xor ap
...
⚠️ cyjseagull opened an issue: "Why bitcoin use so many RecursiveMutex"
(https://github.com/bitcoin/bitcoin/issues/32934)
### Please describe the feature you'd like to see added.

I have noticed that in the Bitcoin source code, almost all thread-safe sections use `RecursiveGuard`.
As we all know, `RecursiveGuard` is **exclusive**, which can **impact system performance and concurrency**.

At the same time, I have also noticed [another issue](https://github.com/bitcoin/bitcoin/issues/19303) specifically tracking the replacement of `RecursiveMutex` with `Mutex`, which would help reduce some of the locking overhead.


...
👍 fanquake approved a pull request: "test: Add missing convert_to_json_for_cli"
(https://github.com/bitcoin/bitcoin/pull/32932#pullrequestreview-3004869160)
ACK fa0528479d5e37833fa66395c94d4611aa9270f6
🚀 fanquake merged a pull request: "test: Add missing convert_to_json_for_cli"
(https://github.com/bitcoin/bitcoin/pull/32932)
💬 fanquake commented on pull request "qa: Avoid knock-on exception in assert_start_raises_init_error":
(https://github.com/bitcoin/bitcoin/pull/32929#issuecomment-3056708522)
(Test failure here was fixed in #32932)
💬 maflcko commented on pull request "clang-format: align brace-after-struct and *-class formatting":
(https://github.com/bitcoin/bitcoin/pull/32813#discussion_r2197233259)
in 7c9b3e1eae8f206753457149f1b1c837f6627d6d: How to reproduce the commit on non-macos? I don't have an apple, so I tried:

```
# clang-format-16 --version
Ubuntu clang-format version 16.0.6 (23ubuntu4)
```

However, the command:

```
clang-format-16 -dump-config > .clang-format
```

produces a different result:

```diff
# git diff -U0
diff --git a/src/.clang-format b/src/.clang-format
index 2f3f96ae2e..0822db5d11 100644
--- a/src/.clang-format
+++ b/src/.clang-format
@@ -0
...