💬 andrewtoth commented on pull request "http: replace WorkQueue and single threads handling for ThreadPool":
(https://github.com/bitcoin/bitcoin/pull/33689#discussion_r2475738139)
> But as you said, the thread sanitizer would likely complain about that. We’re accessing the same variable from different threads (main and worker).
It won't complain if we synchronize access to the same non-atomic variable, which we do here because we lock when calling `Submit`.
> The main issue, however, will probably be related to thread visibility; changes made in one thread aren’t guaranteed to be visible to another.
I don't think this is the main issue. If a future change breaks
...
(https://github.com/bitcoin/bitcoin/pull/33689#discussion_r2475738139)
> But as you said, the thread sanitizer would likely complain about that. We’re accessing the same variable from different threads (main and worker).
It won't complain if we synchronize access to the same non-atomic variable, which we do here because we lock when calling `Submit`.
> The main issue, however, will probably be related to thread visibility; changes made in one thread aren’t guaranteed to be visible to another.
I don't think this is the main issue. If a future change breaks
...
💬 umrashrf commented on issue "Can I compile on OSX Tahoe?":
(https://github.com/bitcoin/bitcoin/issues/33733#issuecomment-3464623966)
@l0rinc still fails
1. brew upgrade
2. brew reinstall boost
3. use clang from homebrew (update $PATH)
4. configure logs https://gist.github.com/umrashrf/6ca23317de227f3c8173a0031ced26f8
5. build logs https://gist.github.com/umrashrf/d4e0ef25921f78fe9fc8909bff043ccb
(https://github.com/bitcoin/bitcoin/issues/33733#issuecomment-3464623966)
@l0rinc still fails
1. brew upgrade
2. brew reinstall boost
3. use clang from homebrew (update $PATH)
4. configure logs https://gist.github.com/umrashrf/6ca23317de227f3c8173a0031ced26f8
5. build logs https://gist.github.com/umrashrf/d4e0ef25921f78fe9fc8909bff043ccb
💬 l0rinc commented on issue "Can I compile on OSX Tahoe?":
(https://github.com/bitcoin/bitcoin/issues/33733#issuecomment-3464678834)
Seems the llvm upgrade was successful, but you still have two boost installations, try removing the old one.
(https://github.com/bitcoin/bitcoin/issues/33733#issuecomment-3464678834)
Seems the llvm upgrade was successful, but you still have two boost installations, try removing the old one.
📝 l0rinc opened a pull request: "log: avoid collecting `GetSerializeSize` data when compact block logging is disabled"
(https://github.com/bitcoin/bitcoin/pull/33738)
The new accounting options introduced in https://github.com/bitcoin/bitcoin/pull/32582 can be quite heavy, and not needed when debug logging is disabled.
`PartiallyDownloadedBlock::FillBlock()` and `PeerManagerImpl::SendBlockTransactions()` accumulate transaction sizes for debug logging by calling `GetTotalSize()` in loops, which invokes expensive `GetSerializeSize()` serializations.
Guard the size calculations with `LogAcceptCategory()` checks so serialization only occurs when compact bl
...
(https://github.com/bitcoin/bitcoin/pull/33738)
The new accounting options introduced in https://github.com/bitcoin/bitcoin/pull/32582 can be quite heavy, and not needed when debug logging is disabled.
`PartiallyDownloadedBlock::FillBlock()` and `PeerManagerImpl::SendBlockTransactions()` accumulate transaction sizes for debug logging by calling `GetTotalSize()` in loops, which invokes expensive `GetSerializeSize()` serializations.
Guard the size calculations with `LogAcceptCategory()` checks so serialization only occurs when compact bl
...
⚠️ ukml opened an issue: "Link"
(https://github.com/bitcoin/bitcoin/issues/33739)
This link dos not work
(https://github.com/bitcoin/bitcoin/issues/33739)
This link dos not work
💬 ukml commented on issue "Link":
(https://github.com/bitcoin/bitcoin/issues/33739#issuecomment-3464792014)
This link does not work
(https://github.com/bitcoin/bitcoin/issues/33739#issuecomment-3464792014)
This link does not work
✅ pinheadmz closed an issue: "Link"
(https://github.com/bitcoin/bitcoin/issues/33739)
(https://github.com/bitcoin/bitcoin/issues/33739)
📝 fjahr opened a pull request: "RFC: bench: Add multi thread benchmarking"
(https://github.com/bitcoin/bitcoin/pull/33740)
This adds a rough way to run specific benchmarks with different numbers of worker threads to see how these impact performance. This is useful for me in the batch validation context and for potential refactorings of checkqueue but I am not sure if this is useful in many other contexts so I am leaving this as a RFC draft for now to see if there is any interest in merging something like this.
Example output:
```
$ build/bin/bench_bitcoin -filter=ConnectBlockAllSchnorr -min-time=1000 -scale-t
...
(https://github.com/bitcoin/bitcoin/pull/33740)
This adds a rough way to run specific benchmarks with different numbers of worker threads to see how these impact performance. This is useful for me in the batch validation context and for potential refactorings of checkqueue but I am not sure if this is useful in many other contexts so I am leaving this as a RFC draft for now to see if there is any interest in merging something like this.
Example output:
```
$ build/bin/bench_bitcoin -filter=ConnectBlockAllSchnorr -min-time=1000 -scale-t
...
💬 fjahr commented on pull request "checkqueue: set MAX_SCRIPTCHECK_THREADS to nCores - 1":
(https://github.com/bitcoin/bitcoin/pull/32692#issuecomment-3465177675)
> The reason for the existing limit of 15 is because benchmarks showed that even on an otherwise unloaded machine with more cores than that, adding more script validation threads was net slower after about that many. That benchmark dates from 2013, so there is no reason to assume it still holds, as both hardware and typical blockchain script usage has changed significantly, as well as other parts of the codebase. But it would still be worth investigating if there isn't just some number above whi
...
(https://github.com/bitcoin/bitcoin/pull/32692#issuecomment-3465177675)
> The reason for the existing limit of 15 is because benchmarks showed that even on an otherwise unloaded machine with more cores than that, adding more script validation threads was net slower after about that many. That benchmark dates from 2013, so there is no reason to assume it still holds, as both hardware and typical blockchain script usage has changed significantly, as well as other parts of the codebase. But it would still be worth investigating if there isn't just some number above whi
...
💬 fjahr commented on pull request "RFC: bench: Add multi thread benchmarking":
(https://github.com/bitcoin/bitcoin/pull/33740#issuecomment-3465206321)
I people could test the exact command as above it might be interesting to post the results here. For me at least, 13 worker threads are consistently delivering a better result than 15 worker threads.
(https://github.com/bitcoin/bitcoin/pull/33740#issuecomment-3465206321)
I people could test the exact command as above it might be interesting to post the results here. For me at least, 13 worker threads are consistently delivering a better result than 15 worker threads.
💬 Frenchanfry commented on issue "Limit "Bulk Dust" with a default filter or consensus..":
(https://github.com/bitcoin/bitcoin/issues/33737#issuecomment-3465272210)
I love this idea to keep spammers away; it also is fundamental when bitcoins value isn't compared relatively to the $. to keep the network clean and efficient.
1 satoshi = 1 satoshi.
(https://github.com/bitcoin/bitcoin/issues/33737#issuecomment-3465272210)
I love this idea to keep spammers away; it also is fundamental when bitcoins value isn't compared relatively to the $. to keep the network clean and efficient.
1 satoshi = 1 satoshi.
✅ pinheadmz closed an issue: "Limit "Bulk Dust" with a default filter or consensus.."
(https://github.com/bitcoin/bitcoin/issues/33737)
(https://github.com/bitcoin/bitcoin/issues/33737)
💬 pinheadmz commented on issue "Limit "Bulk Dust" with a default filter or consensus..":
(https://github.com/bitcoin/bitcoin/issues/33737#issuecomment-3465288829)
This should be posted on the [bitcoin-dev mailing list](https://groups.google.com/g/bitcoindev), the [Delving Bitcoin forum](https://delvingbitcoin.org/) or some other platform where broad, protocol-level concepts are discussed. Conceptual questions and most usage questions can be posted on [Stack Exchange](https://bitcoin.stackexchange.com/). The Bitcoin Core issue tracker is reserved for discussion about this specific software project only, its implementation and usage.
(https://github.com/bitcoin/bitcoin/issues/33737#issuecomment-3465288829)
This should be posted on the [bitcoin-dev mailing list](https://groups.google.com/g/bitcoindev), the [Delving Bitcoin forum](https://delvingbitcoin.org/) or some other platform where broad, protocol-level concepts are discussed. Conceptual questions and most usage questions can be posted on [Stack Exchange](https://bitcoin.stackexchange.com/). The Bitcoin Core issue tracker is reserved for discussion about this specific software project only, its implementation and usage.
🤔 fjahr reviewed a pull request: "[Draft/POC] Add secp256k1-based HPKE (Hybrid Public Key Encryption) For Payjoin v2"
(https://github.com/bitcoin/bitcoin/pull/32617#pullrequestreview-3396562602)
Just skimmed a bit to start. Before spending significant time on reviewing this, I think it would be great if this could be cleaned up to the point that it can be taken out of draft status. Potentially also consider splitting it into multiple commits.
(https://github.com/bitcoin/bitcoin/pull/32617#pullrequestreview-3396562602)
Just skimmed a bit to start. Before spending significant time on reviewing this, I think it would be great if this could be cleaned up to the point that it can be taken out of draft status. Potentially also consider splitting it into multiple commits.
💬 fjahr commented on pull request "[Draft/POC] Add secp256k1-based HPKE (Hybrid Public Key Encryption) For Payjoin v2":
(https://github.com/bitcoin/bitcoin/pull/32617#discussion_r2475742295)
There is a 2.5 but no 3?
(https://github.com/bitcoin/bitcoin/pull/32617#discussion_r2475742295)
There is a 2.5 but no 3?
💬 fjahr commented on pull request "[Draft/POC] Add secp256k1-based HPKE (Hybrid Public Key Encryption) For Payjoin v2":
(https://github.com/bitcoin/bitcoin/pull/32617#discussion_r2475743528)
There are two 2s here
(https://github.com/bitcoin/bitcoin/pull/32617#discussion_r2475743528)
There are two 2s here
💬 fjahr commented on pull request "[Draft/POC] Add secp256k1-based HPKE (Hybrid Public Key Encryption) For Payjoin v2":
(https://github.com/bitcoin/bitcoin/pull/32617#discussion_r2475734567)
2018?
(https://github.com/bitcoin/bitcoin/pull/32617#discussion_r2475734567)
2018?
💬 w0xlt commented on pull request "kernel: Introduce C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2476033021)
Should the height be validated here ?
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2476033021)
Should the height be validated here ?
💬 davidgumberg commented on pull request "p2p: Drop unsolicited CMPCTBLOCK from non-HB peer":
(https://github.com/bitcoin/bitcoin/pull/32606#issuecomment-3465392926)
Rebased to address reviewer feedback.
@ajtowns I'm thinking of this now less as a mitigation for the fingerprinting vector, and more just defense-in-depth / closing off attack surface for compact blocks. E.g. [CVE-2024-35202](https://bitcoincore.org/en/2024/10/08/disclose-blocktxn-crash/) and [CVE-2024-52921](https://bitcoincore.org/en/2024/10/08/disclose-mutated-blocks-hindering-propagation/) are more trivial to exploit because you don't need an HB slot.
An HB slot might be a low bar for
...
(https://github.com/bitcoin/bitcoin/pull/32606#issuecomment-3465392926)
Rebased to address reviewer feedback.
@ajtowns I'm thinking of this now less as a mitigation for the fingerprinting vector, and more just defense-in-depth / closing off attack surface for compact blocks. E.g. [CVE-2024-35202](https://bitcoincore.org/en/2024/10/08/disclose-blocktxn-crash/) and [CVE-2024-52921](https://bitcoincore.org/en/2024/10/08/disclose-mutated-blocks-hindering-propagation/) are more trivial to exploit because you don't need an HB slot.
An HB slot might be a low bar for
...
💬 w0xlt commented on pull request "kernel: Introduce C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2476043001)
Out of curiosity, why return a reference instead of an owned copy? It would avoid lifetime ambiguity
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2476043001)
Out of curiosity, why return a reference instead of an owned copy? It would avoid lifetime ambiguity