Bitcoin Core Github
44 subscribers
121K links
Download Telegram
💬 vasild commented on pull request "Logging cleanup":
(https://github.com/bitcoin/bitcoin/pull/29798#issuecomment-2034968337)
> See #27231.

There you mention:
> I'm going to open an alternate pull that does what you suggest...

Is the current PR doing what you meant? E.g. drop that help text and return an error for "0" ("none" already returns an error in `master`)?
💬 TheCharlatan commented on pull request "guix: bump time-machine to dc4842797bfdc5f9f3f5f725bf189c2b68bd6b5a":
(https://github.com/bitcoin/bitcoin/pull/29651#issuecomment-2034978341)
```
uname --machine && find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
riscv64
2cf27183bc9964708fe12465fa3bb216de6e109ecc9f7ead5d6eff29a178eca8 guix-build-cf5faf73c991/output/aarch64-linux-gnu/SHA256SUMS.part
ad97244ca2ebe9fe715bcfb9838d30436cb1e797d32cb6435615df5d35f564d9 guix-build-cf5faf73c991/output/aarch64-linux-gnu/bitcoin-cf5faf73c991-aarch64-linux-gnu-debug.tar.gz
ab34959b100669ebeca4c174660b26ca8eb6c93d33282459
...
⚠️ achow101 opened an issue: "Compilation failure when using `--enable-fuzz` and `--enable-debug` due to inline asm"
(https://github.com/bitcoin/bitcoin/issues/29801)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

Configure with
```
./configure --enable-fuzz --enable-debug --with-sanitizers=address,fuzzer,undefined,integer CC=clang CXX=clang++
```

Compile fails:
```
crypto/sha256_sse4.cpp:44:9: error: expected relocatable expression
44 | "shl $0x6,%2;"
| ^
<inline asm>:1:1882: note: instantiated into assembly here
crypto/sha256_sse4.cpp:44:9: error: expected r
...
💬 maflcko commented on issue "Compilation failure when using `--enable-fuzz` and `--enable-debug` due to inline asm":
(https://github.com/bitcoin/bitcoin/issues/29801#issuecomment-2034994885)
Recalling the IRC discussion, I presume this happens since `-O0` in https://github.com/bitcoin/bitcoin/pull/16435 ?
💬 maflcko commented on issue "Compilation failure when using `--enable-fuzz` and `--enable-debug` due to inline asm":
(https://github.com/bitcoin/bitcoin/issues/29801#issuecomment-2035002791)
Ref:

```
1782024-04-01T18:08:36 <cfields> can repro with: clang++ -std=c++20 -O0 -fsanitize=fuzzer -c crypto/sha256_sse4.cpp -o out.o
```

https://www.erisian.com.au/bitcoin-core-dev/log-2024-04-02.html
💬 fanquake commented on issue "Compilation failure when using `--enable-fuzz` and `--enable-debug` due to inline asm":
(https://github.com/bitcoin/bitcoin/issues/29801#issuecomment-2035003353)
The broken combination is `clang++ -std=c++20 -O0 -fsanitize=address -c crypto/sha256_sse4.cpp -o out.o`, pointed out by Cory. Happens for me with Clang-18.
💬 fanquake commented on issue "Compilation failure when using `--enable-fuzz` and `--enable-debug` due to inline asm":
(https://github.com/bitcoin/bitcoin/issues/29801#issuecomment-2035010355)
I guess the next step is to followup / file an issue with LLVM. I've got #29796 open which may "fix" this by just dropping `-O0`, depending on the outcome of discussion.
💬 fanquake commented on pull request "guix: Remove another leftover from #29648":
(https://github.com/bitcoin/bitcoin/pull/29797#issuecomment-2035021336)
Guix Build (aarch64):
```bash
8e7bddb8fa49c857bc9a815a7f27f2d60f8a2f8955966eff467ee86c0d0776f6 guix-build-3cb80febb876/output/aarch64-linux-gnu/SHA256SUMS.part
de0c0624b55418b6638b4852cbe4527c5fb4a2ccbb82d442e5dff0febc947a70 guix-build-3cb80febb876/output/aarch64-linux-gnu/bitcoin-3cb80febb876-aarch64-linux-gnu-debug.tar.gz
5f37064b7496125b90f7df1fda7df3e6545149274bf737361579c0e9be94f6f8 guix-build-3cb80febb876/output/aarch64-linux-gnu/bitcoin-3cb80febb876-aarch64-linux-gnu.tar.gz
02ad13
...
🤔 glozow reviewed a pull request: "Simplify network-adjusted time warning logic"
(https://github.com/bitcoin/bitcoin/pull/29623#pullrequestreview-1977282272)
approach ACK
💬 glozow commented on pull request "Simplify network-adjusted time warning logic":
(https://github.com/bitcoin/bitcoin/pull/29623#discussion_r1550035286)
needs doc?
💬 glozow commented on pull request "Simplify network-adjusted time warning logic":
(https://github.com/bitcoin/bitcoin/pull/29623#discussion_r1550033829)
nit: this is unnecessary
💬 glozow commented on pull request "Simplify network-adjusted time warning logic":
(https://github.com/bitcoin/bitcoin/pull/29623#discussion_r1550056609)
nit: documentation please, e.g. number of elements to track
💬 glozow commented on pull request "Simplify network-adjusted time warning logic":
(https://github.com/bitcoin/bitcoin/pull/29623#discussion_r1550045138)
+1, was going to say this should be removed in 2ef71c73582be554e565ada3f8a6ca77c2cd79f1, or say we are avoiding false warnings instead of preventing tampering of adjusted time
💬 glozow commented on pull request "Simplify network-adjusted time warning logic":
(https://github.com/bitcoin/bitcoin/pull/29623#discussion_r1550042121)
Or its own rpc commit?
💬 vostrnad commented on issue "IBD performance regression in 27.0rc1 on Windows":
(https://github.com/bitcoin/bitcoin/issues/29785#issuecomment-2035080712)
> Can you also let us know if you're using any particular config options etc.

Beyond the bare minimum (`-connect`, `-datadir`, RPC user/pass etc.) I'm just increasing dbcache and enabling pruning, but neither of those should kick in at this block height. I'll test again using the defaults just to be sure.

> how are you able to perform so many runs in such a short amount of time? Do you have access to a lot of compute?

I'm not doing full IBD, just to block 120,000. As much as I'd like to
...
💬 mzumsande commented on issue "IBD performance regression in 27.0rc1 on Windows":
(https://github.com/bitcoin/bitcoin/issues/29785#issuecomment-2035090717)
Does it also appear with `-reindex` (without a second node)?
💬 pablomartin4btc commented on pull request "refactor: interfaces, make 'createTransaction' less error-prone ":
(https://github.com/bitcoin-core/gui/pull/807#discussion_r1550107069)
It's ok, I got confused at first glance but then read more about how `util::Result` is being used in all our code.

https://github.com/bitcoin-core/gui/blob/0d509bab45d292caeaf34600e57b5928757c6005/src/util/result.h#L19-L33
💬 sipa commented on issue "IBD performance regression in 27.0rc1 on Windows":
(https://github.com/bitcoin/bitcoin/issues/29785#issuecomment-2035114828)
Or even with `-reindex-chainstate` (which does even less)?
💬 theuni commented on pull request "[RFC] Switch and/or align debugging flags (back) to `-Og`":
(https://github.com/bitcoin/bitcoin/pull/29796#issuecomment-2035139205)
Concept ACK.

I think msan is a good proxy for what we want enabled. [From its docs](https://releases.llvm.org/18.1.1/tools/clang/docs/MemorySanitizer.html):

> To get a reasonable performance add -O1 or higher. To get meaningful stack traces in error messages add -fno-omit-frame-pointer. To get perfect stack traces you may need to disable inlining (just use -O1) and tail call elimination (-fno-optimize-sibling-calls).


From [gcc's docs](https://gcc.gnu.org/onlinedocs/gcc/Optimize-Option
...
💬 drkhero commented on issue "rpc method removeprunedfunds should take an array of txids":
(https://github.com/bitcoin/bitcoin/issues/29466#issuecomment-2035144836)
@furszy @glozow Still waiting on this issue to be fixed. Ryan has been waiting for a while now. Thanks!
💬 davidgumberg commented on pull request "cli: Detect port errors in rpcconnect and rpcport":
(https://github.com/bitcoin/bitcoin/pull/29521#issuecomment-2035153866)
ACK https://github.com/bitcoin/bitcoin/pull/29521/commits/cb4f9fc5847a7e53fe45d54b1fddf504dee5af82

The changes in `bitcoin-cli.cpp` since the last ACK look good to me, and the test coverage is more extensive.

Tested with `make check` and running the functional test suite.

I also verified that passing bad ports to `rpcconnect` and `rpcport` trigger the warning.