Bitcoin Core Github
44 subscribers
120K links
Download Telegram
hebasto closed a pull request: "guix: Do not set `C{PLUS}_INCLUDE_PATH` variables for Darwin builds"
(https://github.com/bitcoin/bitcoin/pull/31214)
💬 instagibbs commented on pull request "fuzz: Fix difficulty target generation in `p2p_headers_presync`":
(https://github.com/bitcoin/bitcoin/pull/31213#issuecomment-2455155318)
@marcofleon Ah, I didn't understand how pre-sync nBits checking works, my question makes no sense for pre-sync
🤔 murchandamus reviewed a pull request: "Remove mempoolfullrbf"
(https://github.com/bitcoin/bitcoin/pull/30592#pullrequestreview-2413508572)
ACK c189eec848e3c31f438151d4d3422718a29df3a3
💬 murchandamus commented on pull request "Remove mempoolfullrbf":
(https://github.com/bitcoin/bitcoin/pull/30592#discussion_r1828002169)
This is a bit confusing, because defaulting to `mempoolfullrbf=1` already made full replace-by-fee the standard behavior, but this reads as if removing the option makes it the standard behavior.
Perhaps something like this:

```suggestion
After the policy was broadly adopted by miners, starting with v28.0, the `mempoolfullrbf` startup option was set to
default to `1`. With widespread adoption of full replace-by-fee, it becomes the standard behavior on the network. Users no longer benefit f
...
💬 dergoegge commented on pull request "rpc: increase the defaults for -rpcthreads and -rpcworkqueue":
(https://github.com/bitcoin/bitcoin/pull/31215#discussion_r1828023493)
Maybe we should just make the default depend on how many cores are available? e.g. default: `num cores / 2`
💬 andrewtoth commented on pull request "rpc: increase the defaults for -rpcthreads and -rpcworkqueue":
(https://github.com/bitcoin/bitcoin/pull/31215#discussion_r1828025349)
From the linked issue, the user is saying the default of 4 is not enough for a raspi, which has 4 cores. So this would lower that number on a raspi.
💬 dergoegge commented on pull request "rpc: increase the defaults for -rpcthreads and -rpcworkqueue":
(https://github.com/bitcoin/bitcoin/pull/31215#discussion_r1828042992)
I see, at least something based on the users system instead of picking a random number?
💬 andrewtoth commented on pull request "rpc: increase the defaults for -rpcthreads and -rpcworkqueue":
(https://github.com/bitcoin/bitcoin/pull/31215#discussion_r1828046467)
Just number of cores would make sense as a default. At least it wouldn't be lower on raspis.
💬 maflcko commented on issue "intermittent issue in wallet_upgradewallet.py: AssertionError: bdb magic does not match bdb btree magic":
(https://github.com/bitcoin/bitcoin/issues/31210#issuecomment-2455258749)
Possibly related: https://github.com/containers/podman/issues/23808

I'll try tomorrow without a container.
💬 laanwj commented on pull request "rpc: increase the defaults for -rpcthreads and -rpcworkqueue":
(https://github.com/bitcoin/bitcoin/pull/31215#discussion_r1828088319)
i've always been kind of hestitant about adapting these values to the system automatically; it doesn't only depend on the CPU, but also the expected usage.

For a "fair" work queue it would indeed make sense to pick the number of CPU cores. However, bitcoin core's RPC threads often sleep for various reasons. Eg waiting for I/O, waiting for a lock in another thread, etc. So it's fine to have it a bit higher than the number of cores.
i'd definitely cap the minimum at 4, even for single-core CPU
...
👍 storopoli approved a pull request: "rpc: increase the defaults for -rpcthreads and -rpcworkqueue"
(https://github.com/bitcoin/bitcoin/pull/31215#pullrequestreview-2413651250)
Concept ACK
💬 laanwj commented on pull request "rpc: increase the defaults for -rpcthreads and -rpcworkqueue":
(https://github.com/bitcoin/bitcoin/pull/31215#issuecomment-2455273805)
ACK on increasing the default work queue size. i don't think the RPC work queue uses that much memory. It's fine to set the default a lot higher.

Not sure about the threads.
💬 instagibbs commented on pull request "Package validation: accept packages of size 1":
(https://github.com/bitcoin/bitcoin/pull/31096#discussion_r1828105140)
It also causes double-validation for single transaction package that is missing inputs.

Could special case this but not sure I love making things even harder to understand:
```
diff --git a/src/validation.cpp b/src/validation.cpp
index 3e48335255..c89050e895 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -1790,10 +1790,16 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptPackage(const Package& package,
// some of them may still be valid.

...
💬 laanwj commented on pull request "net: Use actual memory size in receive buffer accounting":
(https://github.com/bitcoin/bitcoin/pull/31164#discussion_r1828133259)
Done
💬 l0rinc commented on pull request "net: Use actual memory size in receive buffer accounting":
(https://github.com/bitcoin/bitcoin/pull/31164#discussion_r1828135621)
note: with clang I measured ~24: https://tc-imba.github.io/posts/cpp-sso
🤔 l0rinc reviewed a pull request: "net: Use actual memory size in receive buffer accounting"
(https://github.com/bitcoin/bitcoin/pull/31164#pullrequestreview-2413727212)
Concept ACK
💬 l0rinc commented on pull request "net: Use actual memory size in receive buffer accounting":
(https://github.com/bitcoin/bitcoin/pull/31164#discussion_r1828153937)
Could you please add a test in this commit that demonstrates the intended behavior? Or would that tie the implementation down too much?
💬 l0rinc commented on pull request "net: Use actual memory size in receive buffer accounting":
(https://github.com/bitcoin/bitcoin/pull/31164#discussion_r1828152759)
Since I think these are related pointers (unlike https://github.com/bitcoin/bitcoin/pull/31164#discussion_r1827884684), can we use simple pointer comparisons here? Or do I misunderstand how this works?
```C++
if (s.data() >= s_ptr && s.data() + s.size() <= s_ptr + sizeof(s)) {
```
💬 laanwj commented on pull request "net: Use actual memory size in receive buffer accounting":
(https://github.com/bitcoin/bitcoin/pull/31164#discussion_r1828173154)
interesting!
that also suggests that on 32-bit platforms, the size would still be 15 bytes for g++ (because it's the minimum bound), but 10 (3*4 minus 2 for size byte and zero byte) for clang
so it would trigger an allocation for message types of 11/12 chars there 😄
💬 laanwj commented on pull request "net: Use actual memory size in receive buffer accounting":
(https://github.com/bitcoin/bitcoin/pull/31164#discussion_r1828177808)
as i understand it, in the case that sso is not used, these are not related pointers