Bitcoin Core Github
44 subscribers
120K links
Download Telegram
🚀 fanquake merged a pull request: "ci: use LLVM/clang-16 in native_asan job"
(https://github.com/bitcoin/bitcoin/pull/27360)
👋 brunoerg's pull request is ready for review: "test: merge banning test from p2p_disconnect_ban to rpc_setban"
(https://github.com/bitcoin/bitcoin/pull/26863)
💬 liuyangc3 commented on issue "Build broken when enabling fuzzing on Apple M1 hw using homebrew llvm.":
(https://github.com/bitcoin/bitcoin/issues/27550#issuecomment-1531462324)
Yeah thanks for the advice, I will try more before ask help here. maybe is not the bitcoin code issue. will get back later thanks again
💬 fanquake commented on issue "Build broken when enabling fuzzing on Apple M1 hw using homebrew llvm.":
(https://github.com/bitcoin/bitcoin/issues/27550#issuecomment-1531463486)
Ok. We'll close this for now. Comment back when you've had a chance to test, and we can reopen if need be.
fanquake closed an issue: "Build broken when enabling fuzzing on Apple M1 hw using homebrew llvm."
(https://github.com/bitcoin/bitcoin/issues/27550)
💬 fanquake commented on pull request "net: use interruptible async getaddrinfo wrapper from libevent for DNS":
(https://github.com/bitcoin/bitcoin/pull/27505#issuecomment-1531483899)
In general, I'm ~0 on leaning into further libevent usage, especially for something like this. The upstream code is currently not necessarily very well maintained or tested. There is also at least one open issue which reports `evdns_getaddrinfo` just "crashing occasionally": https://github.com/libevent/libevent/issues/1130. Although it's not entirely clear if this is user error.
💬 brunoerg commented on pull request "fuzz: addrman, add coverage for `network` field in `Select()`":
(https://github.com/bitcoin/bitcoin/pull/27549#discussion_r1182566968)
Well, since it already imports `test/fuzz/util/net.h` there is no reason, gonna change it!
💬 laanwj commented on issue "Improve porting documentation for legacy-only wallet RPCs":
(https://github.com/bitcoin/bitcoin/issues/25363#issuecomment-1531539449)
Yes, lgtm, thanks!
laanwj closed an issue: "Improve porting documentation for legacy-only wallet RPCs"
(https://github.com/bitcoin/bitcoin/issues/25363)
💬 fjahr commented on pull request "Add ASM optimizations for MuHash3072":
(https://github.com/bitcoin/bitcoin/pull/19181#issuecomment-1531553456)
I have now tested SafeGCD vs SafeGCD+ASM (see https://github.com/fjahr/bitcoin/tree/pr21590-safegcd-asm) and the gains from including the ASM code are still substantial.

GCC 13.1 - SafeGCD only:

```
| ns/op | op/s | err% | total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
| 9,168.73 | 109,066.35 | 2.1% | 1.06 | `MuHash`
| 7,571.75 | 132,069.88 | 0.2% |
...
💬 real-or-random commented on pull request "Add ASM optimizations for MuHash3072":
(https://github.com/bitcoin/bitcoin/pull/19181#issuecomment-1531563808)
> the SafeGCD change only impacts the finalize operation and that has a much smaller impact on the overall computation time because of how we use MuHash in practice. When a new block comes in we do a div of all the spent TXOs and a mul of all the new UTXOs and then at the end we finalize once.

Okay, thanks for the numbers. I agree then, we shouldn't neglect this one.

> So I still have a hard time deciding which one is the more valuable change. Also, while ASM might be a bit intimidating, h
...
💬 dergoegge commented on pull request "net: use interruptible async getaddrinfo wrapper from libevent for DNS":
(https://github.com/bitcoin/bitcoin/pull/27505#issuecomment-1531564374)
As an alternative to this PR, could we give the DNS thread time to join cleanly (e.g. 5s) and if it doesn't we just detach it and let the OS handle the clean up? That risks memory leaks but that shouldn't really matter when the program is about to exit anyway.

I would also be fine with not addressing this at all, because it seems like this only happens when a system generally fails to make DNS requests? In the absence of nice solutions, it seems like that isn't our problem and the user should
...
🤔 mzumsande reviewed a pull request: "Add support for "partial" fuzzers that indicate usefulness"
(https://github.com/bitcoin/bitcoin/pull/27552#pullrequestreview-1409209797)
> Maybe it's worth experimenting a bit with to so how much impact it has;

Yes, I'm planning to play with that, I'd be really interested in whether there is a significant speedup.

I feel like ideally, this would be something a good fuzzing engine should be able to handle to some extent without user guidance - uninteresting cases should create fewer additional seeds added to the corpus, which should result in them being picked by the engine for mutation less (and there might be more sophisti
...
👍 pinheadmz approved a pull request: "test: Simplify feature_fastprune.py"
(https://github.com/bitcoin/bitcoin/pull/27553#pullrequestreview-1409227264)
ACK fa17767154e21e9ed00782a9e4cf9a3d1d66f5d1

Thanks for simplifying, dunno why I didn't think of `generateblock` for this

<details><summary>Show Signature</summary>

```
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

ACK fa17767154e21e9ed00782a9e4cf9a3d1d66f5d1
-----BEGIN PGP SIGNATURE-----

iQIzBAEBCAAdFiEE5hdzzW4BBA4vG9eM5+KYS2KJyToFAmRRHVcACgkQ5+KYS2KJ
yTrfcA/+NVtObvE0dL9V50sLrQ6Qep0hJ9MTxdL/nGsa8fL3IOGnH/WV0npEytC3
FjvAOCbD84CHe2iIvzKkSkT93yDElB9Mz6c35TFbhwHFrRHSGCUK+IFWNu
...
💬 furszy commented on pull request "Introduce `MockableDatabase` for wallet unit tests":
(https://github.com/bitcoin/bitcoin/pull/26715#discussion_r1182632309)
Missing return statement.
📝 hebasto opened a pull request: "test: Treat `bitcoin-wallet` binary in the same way as others"
(https://github.com/bitcoin/bitcoin/pull/27554)
This PR makes the `bitcoin-wallet` binary path customizable in the same way how it can be done now with other ones, including `bitcoind`, `bitcoin-cli` and `bitcoin-util`.
💬 pinheadmz commented on pull request "net: use interruptible async getaddrinfo wrapper from libevent for DNS":
(https://github.com/bitcoin/bitcoin/pull/27505#issuecomment-1531618658)
@fanquake @dergoegge good feedback, thanks. I'll look into a different approach moving the lib call `getaddrinfo` to a detachable thread before abandoning, and then we may just have to close https://github.com/bitcoin/bitcoin/issues/16778 as "won't fix"

There is another thought I had about name resolution moving forward: If bitcoin can make more interesting DNS queries (either using a library or just implementing some bare necessities) we could ask the DNS seeders for I2P and onion addresses
...
💬 fanquake commented on pull request "net: use interruptible async getaddrinfo wrapper from libevent for DNS":
(https://github.com/bitcoin/bitcoin/pull/27505#issuecomment-1531621948)
> either using a library

It's very unlikely we are going to add a new external dependency to make "interesting" DNS queries.
💬 pinheadmz commented on pull request "blockstorage: do not flush block to disk if it is already there":
(https://github.com/bitcoin/bitcoin/pull/27039#issuecomment-1531627646)
> 🐙 This pull request conflicts with the target branch and [needs rebase](https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#rebasing-changes).

done
💬 MarcoFalke commented on pull request "test: Treat `bitcoin-wallet` binary in the same way as others":
(https://github.com/bitcoin/bitcoin/pull/27554#discussion_r1182685633)
```suggestion
self.options.bitcoinwallet = os.getenv("BITCOINWALLET", default=fname("bitcoin-wallet"))
```

What about making this a function to reduce code bloat while touching this?