Bitcoin Core Github
44 subscribers
120K links
Download Telegram
💬 maflcko commented on pull request "build: switch to using LLVM 17.x for macOS builds":
(https://github.com/bitcoin/bitcoin/pull/28880#issuecomment-1843367906)
I'll also try to reproduce outside of my riscv64 metal, because it is currently a bit slow and faster CPUs will only hit the market next year (or so).
💬 luke-jr commented on pull request "wallet, rpc: add anti-fee-sniping to `send` and `sendall`":
(https://github.com/bitcoin/bitcoin/pull/28944#discussion_r1417764868)
nit: blank line under this; ideally docs above
💬 maflcko commented on pull request "build: switch to using LLVM 17.x for macOS builds":
(https://github.com/bitcoin/bitcoin/pull/28880#issuecomment-1843435820)
Jup, same error with cross-compilation:

```
$ guix time-machine --commit=3778f778c37110460dd78088200cbd05eb0c49e4 -- build --target=riscv64-linux-gnu llvm
...
[ 89%] Linking CXX executable ../../bin/dsymutil
cd /tmp/guix-build-llvm-17.0.5.drv-0/source/build/tools/dsymutil && /gnu/store/ag7sflxmy9fpqxdrgwk6rlynvdxy5695-cmake-minimal-cross-3.24.2/bin/cmake -E cmake_link_script CMakeFiles/dsymutil.dir/link.txt --verbose=1
[ 89%] Building CXX object tools/llvm-isel-fuzzer/CMakeFiles/llvm-is
...
💬 instagibbs commented on pull request "test: Extends MEMPOOL msg functional test":
(https://github.com/bitcoin/bitcoin/pull/28485#issuecomment-1843441930)
ACK https://github.com/bitcoin/bitcoin/pull/28485/commits/97c0dfa8942c7fd62c920deee03b4d0c59aba641

tests line up with my understanding of the code
💬 theuni commented on pull request "[POC] C++20 `std::endian`":
(https://github.com/bitcoin/bitcoin/pull/28674#issuecomment-1843470806)
> Hmm, playing around with this code on godbolt, it hardly ever compiles down to bswaps. I'm not sure what changed from when I was initially investigating. Will have a look.

Ok, interesting, the problem is pretty easy to see here: https://godbolt.org/z/d5EGs8Ybs

GCC and MSVC both do the same thing: the optimization is done on the small static function, but it doesn't carry through when inlined. clang's output looks as expected. That explains why my test cases look good, but real world perf
...
💬 Sjors commented on pull request "test: Fix test by checking the actual exception instance":
(https://github.com/bitcoin/bitcoin/pull/28989#issuecomment-1843472589)
Looks like a reasonable fix, thanks!
💬 brunoerg commented on pull request "fuzz: p2p: Detect peer deadlocks":
(https://github.com/bitcoin/bitcoin/pull/29009#issuecomment-1843473953)
Concept ACK
💬 maflcko commented on pull request "[POC] C++20 `std::endian`":
(https://github.com/bitcoin/bitcoin/pull/28674#issuecomment-1843481759)
> clang can obviously do the right thing.

Are you sure? When I tried yesterday, it did not (https://github.com/bitcoin/bitcoin/pull/28674#issuecomment-1841445629)
🤔 ryanofsky reviewed a pull request: "Get rid of shutdown.cpp/shutdown.h, use SignalInterrupt directly"
(https://github.com/bitcoin/bitcoin/pull/28051#pullrequestreview-1767990381)
Updated 8a02ce59ffa16c73611aeda6caf8b54d85c6208f -> 4a3a2651420ca1808cb25aed8a33d57dfcd627f5 ([`pr/noshut.19`](https://github.com/ryanofsky/bitcoin/commits/pr/noshut.19) -> [`pr/noshut.20`](https://github.com/ryanofsky/bitcoin/commits/pr/noshut.20), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/noshut.19..pr/noshut.20)) with Qt windows bugfix
💬 ryanofsky commented on pull request "Get rid of shutdown.cpp/shutdown.h, use SignalInterrupt directly":
(https://github.com/bitcoin/bitcoin/pull/28051#discussion_r1417585924)
re: https://github.com/bitcoin/bitcoin/pull/28051#discussion_r1417160346

> Calling `app.node()` here before `app.createNode` results in a crash on windows when running `bitcoin-qt.exe`.

Great catch! This should be fixed now by passing a shutdown callback and delaying the call to app.node(). I didn't want to move the createNode call, because the interfaces::Node object is intentionally created after the splash screen, so the splash screen can show up without a delay. The delay wouldn't be s
...
💬 ryanofsky commented on pull request "Get rid of shutdown.cpp/shutdown.h, use SignalInterrupt directly":
(https://github.com/bitcoin/bitcoin/pull/28051#discussion_r1417833003)
re: https://github.com/bitcoin/bitcoin/pull/28051#discussion_r1417160672

> Nit: Could this be private?

Sure, switched to a private member in the latest push
💬 theuni commented on pull request "[POC] C++20 `std::endian`":
(https://github.com/bitcoin/bitcoin/pull/28674#issuecomment-1843504462)
> > clang can obviously do the right thing.
>
> Are you sure?

Absolutely not! I'm making this up as I go.

> When I tried yesterday, it did not ([#28674 (comment)](https://github.com/bitcoin/bitcoin/pull/28674#issuecomment-1841445629))

Thanks, I guess that's the test I just asked for.

Ok, I'll keep poking. But it seems like this approach likely won't work :(
💬 maflcko commented on pull request "[POC] C++20 `std::endian`":
(https://github.com/bitcoin/bitcoin/pull/28674#issuecomment-1843519987)
> Ok, I'll keep poking. But it seems like this approach likely won't work :(

What about hiding the function inside a compilation unit, where they are turned into `bswap`, and then rely on LTO to replace the `call internal_bswap_64` with `bswap`? (Haven't tried this)
💬 jonatack commented on pull request "rpc: encryptwallet help, mention HD seed rotation and backup requirement":
(https://github.com/bitcoin/bitcoin/pull/28980#issuecomment-1843570199)
Post-merge ACK
💬 hebasto commented on pull request "Replace Boost.Process with cpp-subprocess":
(https://github.com/bitcoin/bitcoin/pull/28981#issuecomment-1843624931)
Rebased on top of the merged #28989.
💬 maflcko commented on pull request "guix: update time-machine":
(https://github.com/bitcoin/bitcoin/pull/28580#issuecomment-1843630668)
I am trying to compile this on riscv64, however, it still does not seem to work.

```
dpkg --print-architecture && HOSTS="arm64-apple-darwin" V=1 VERBOSE=1 MAX_JOBS=$(nproc) ./contrib/guix/guix-build
riscv64
Found macOS SDK at '/bitcoin-core/depends/SDKs/Xcode-12.2-12B45b-extracted-SDK-with-libcxx-headers', using...
Checking that we can connect to the guix-daemon...

Hint: If this hangs, you may want to try turning your guix-daemon off and on
again.

accepted connection from pi
...
⚠️ hebasto opened an issue: "The `streams_tests/xor_file` test fails on Windows"
(https://github.com/bitcoin/bitcoin/issues/29014)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

```
>.\bitcoin-26.0\bin\test_bitcoin.exe -t streams_tests/xor_file
Running 1 test case...
unknown location(0): fatal error: in "streams_tests/xor_file": std::ios_base::failure[abi:cxx11]: AutoFile::write: file handle is nullptr: iostream error
test/streams_tests.cpp(29): last checkpoint

*** 1 failure is detected in the test module "Bitcoin Core Test Suite"

```

### Expected behav
...
💬 fanquake commented on issue "The `streams_tests/xor_file` test fails on Windows":
(https://github.com/bitcoin/bitcoin/issues/29014#issuecomment-1843638198)
> Download https://bitcoincore.org/bin/bitcoin-core-25.1/bitcoin-25.1-win64.zip

> What version of Bitcoin Core are you using?
> v26.0

Which version is it?
💬 hebasto commented on issue "The `streams_tests/xor_file` test fails on Windows":
(https://github.com/bitcoin/bitcoin/issues/29014#issuecomment-1843642317)
> > Download [bitcoincore.org/bin/bitcoin-core-25.1/bitcoin-25.1-win64.zip](https://bitcoincore.org/bin/bitcoin-core-25.1/bitcoin-25.1-win64.zip)
>
> > What version of Bitcoin Core are you using?
> > v26.0
>
> Which version is it?

https://bitcoincore.org/bin/bitcoin-core-26.0/bitcoin-26.0-win64.zip
💬 TheCharlatan commented on pull request "build: Introduce internal kernel library":
(https://github.com/bitcoin/bitcoin/pull/28690#issuecomment-1843661159)
Rebased 2086d1d4c3f40cce34647995ead4bff22967ffd8 -> 34970bde23dd87fc0fea5a1970880761f7184774 ([kernelInternalLib_11](https://github.com/TheCharlatan/bitcoin/tree/kernelInternalLib_11) -> [kernelInternalLib_12](https://github.com/TheCharlatan/bitcoin/tree/kernelInternalLib_12), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelInternalLib_11..kernelInternalLib_12))
* Fixed conflict with #27581