Bitcoin Core Github
44 subscribers
121K links
Download Telegram
⚠️ fanquake opened an issue: "test: consider Clang `type` sanitizer"
(https://github.com/bitcoin/bitcoin/issues/32495)
Clang 20 introduced an experimental type sanitizer that "detects violations of C/C++ type-based aliasing rules." See here for more info: https://clang.llvm.org/docs/TypeSanitizer.html. Someone might want to run this against our code, to see if it works, or detects real issues. This could likely be done with:
```bash
make -C depends/ CC=clang CXX=clang++ CXXFLAGS="-stdlib=libc++" NM=llvm-nm AR=llvm-ar RANLIB=llvm-ranlib STRIP=llvm-strip
cmake -B build --toolchain /bitcoin/depends/x86_64-pc-linux-
...
💬 maflcko commented on pull request "test: fix two intermittent failures in wallet_basic.py":
(https://github.com/bitcoin/bitcoin/pull/32483#issuecomment-2879883748)
review ACK e7ad86e1ca3b0b2f2795e91c2f9959486c67dd90 🎩

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: review ACK e7ad86e1ca3b
...
📝 fanquake opened a pull request: "depends: add Qt `-ltcg` for Darwin, drop it for Windows"
(https://github.com/bitcoin/bitcoin/pull/32496)
The related Windows patches were dropped in 5e794e62024eef612e1fbb71c76ea54d17435c14, and "Cross-compiling does not support LTO." (from #30997).

Using this with native Darwin builds works ok.
💬 luke-jr commented on pull request "qt: drop unused watch-only functionality":
(https://github.com/bitcoin/bitcoin/pull/32459#issuecomment-2879908057)
Shouldn't this be on the GUI repo?
💬 luke-jr commented on pull request "fs: remove `_POSIX_C_SOURCE` defining":
(https://github.com/bitcoin/bitcoin/pull/32460#issuecomment-2879919952)
>From what I can tell, compilers on Linux systems, will be defining _GNU_SOURCE

`_GNU_SOURCE` isn't supposed to ever be defined by default, and I don't see us defining it anywhere...?
💬 maflcko commented on issue "test: wallet_reorgsrestore.py failure under valgrind":
(https://github.com/bitcoin/bitcoin/issues/32493#issuecomment-2879940657)
Does it always happen, or did this start at some point in the git history?
👍 laanwj approved a pull request: "cmake: Add application manifests when cross-compiling for Windows"
(https://github.com/bitcoin/bitcoin/pull/32396#pullrequestreview-2839952072)
re-ACK c0f41523973e33a8cd104a8ebf4906a4d99f3eed
💬 fanquake commented on pull request "test: fix two intermittent failures in wallet_basic.py":
(https://github.com/bitcoin/bitcoin/pull/32483#issuecomment-2880021915)
@achow101 note that you've ACK'd a commit has not in this PR, but for an equivalent change.
fanquake closed an issue: "failure in wallet_basic.py --descriptors"
(https://github.com/bitcoin/bitcoin/issues/27249)
fanquake closed an issue: "qa: Failure in wallet_basic.py spendzeroconfchange test"
(https://github.com/bitcoin/bitcoin/issues/32456)
🚀 fanquake merged a pull request: "test: fix two intermittent failures in wallet_basic.py"
(https://github.com/bitcoin/bitcoin/pull/32483)
💬 l0rinc commented on pull request "refactor: reenable `implicit-integer-sign-change` check for `serialize.h`":
(https://github.com/bitcoin/bitcoin/pull/32296#discussion_r2088818525)
Nice, I like that a lot more - addressed the reformatting and unification in separate commits, let me know if you think this is better.
Pushed a rebase separately to simplify review from GitHub as well (I know you do range-diff), and to make sure the latest CI is run against it.
💬 fanquake commented on pull request "test: fix two intermittent failures in wallet_basic.py":
(https://github.com/bitcoin/bitcoin/pull/32483#issuecomment-2880037334)
Backported to `29.x` in #32292.
⚠️ Sjors opened an issue: "Dropping unused legacy wallet code"
(https://github.com/bitcoin-core/gui/issues/873)
Tracking pull request(s) that remove any remaining legacy wallet code, either here or in the main repo:

- https://github.com/bitcoin/bitcoin/pull/32459
💬 luke-jr commented on pull request "[DRAFT] Expose AssumeUTXO Load Snapshot Functionality To The GUI":
(https://github.com/bitcoin-core/gui/pull/870#issuecomment-2880052845)
Seems like this would be more appropriate for the first-run intro page?
💬 Sjors commented on pull request "qt: drop unused watch-only functionality":
(https://github.com/bitcoin/bitcoin/pull/32459#issuecomment-2880054651)
Doesn't seem worth moving, and this PR also touches the interface, but I opened a tracking issue on the GUI repo that points here. There's probably more than what I found here.
💬 maflcko commented on pull request "refactor: reenable `implicit-integer-sign-change` check for `serialize.h`":
(https://github.com/bitcoin/bitcoin/pull/32296#discussion_r2088837960)
nit in the second commit: Would be good to just set this to the final version `template<typename Stream> void Serialize(Stream& s, int8_t a ) { ser_writedata8(s, uint8_t(a)); }` from the beginning. Otherwise there will be three style-fixup commits on top of each other, making review harder and the git log and git blame depth tedious to follow.
💬 maflcko commented on issue "Dropping unused legacy wallet code":
(https://github.com/bitcoin-core/gui/issues/873#issuecomment-2880070206)
No objection, but not sure a tracking issue is needed for a single, simple pull request?
💬 TheCharlatan commented on pull request "kernel: Separate UTXO set access from validation functions":
(https://github.com/bitcoin/bitcoin/pull/32317#discussion_r2088847208)
Sweet, will apply this!
💬 Sjors commented on issue "Dropping unused legacy wallet code":
(https://github.com/bitcoin-core/gui/issues/873#issuecomment-2880140236)
I don't think I found all the code, regular GUI contributors might find more.