💬 willcl-ark commented on issue "Increasing self-hosted runner raw performance":
(https://github.com/bitcoin/bitcoin/issues/30852#issuecomment-2386850370)
> Apart from the inital clone+apt, the tasks are not IO-bound (at least not on Hetzner Cloud).

I know in docker you can cache apt, I wonder if we can do the same on podman (having difficulty finding that they support the exact same flags)? e.g. in docker you can:

```bash
RUN --mount=target=/var/lib/apt/lists,type=cache,sharing=locked \
--mount=target=/var/cache/apt,type=cache,sharing=locked \
```

We'd need likely to delineate between Ubuntu versions etc. and this may only help wi
...

💬 brunoerg commented on pull request "net: fuzz: bypass network magic and checksum validation":
(https://github.com/bitcoin/bitcoin/pull/31012#discussion_r1783412030)
Isn't `FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION` set when using `-DBUILD_FOR_FUZZING=ON`?

```
if(BUILD_FOR_FUZZING)
message(WARNING "BUILD_FOR_FUZZING=ON will disable all other targets and force BUILD_FUZZ_BINARY=ON.")
set(BUILD_DAEMON OFF)
set(BUILD_CLI OFF)
set(BUILD_TX OFF)
set(BUILD_UTIL OFF)
set(BUILD_UTIL_CHAINSTATE OFF)
set(BUILD_KERNEL_LIB OFF)
set(BUILD_WALLET_TOOL OFF)
set(BUILD_GUI OFF)
set(ENABLE_EXTERNAL_SIGNER OFF)
set(WITH_MINIUPNPC OFF)
set
...

💬 maflcko commented on issue "Increasing self-hosted runner raw performance":
(https://github.com/bitcoin/bitcoin/issues/30852#issuecomment-2386938805)
> I know in docker you can cache apt

Interesting. Though, I think the apt is rarely called, and would be just 30 seconds anyway (https://cirrus-ci.com/task/5200852661567488?logs=ci#L199). The reason the image cache exists is that network error are avoided and the expensive llvm build is cached.



> resilient in terms of accessing a remote cache dir (it always maintains a local cache

It would be nice if the local cache was also shared, but I guess this can be done in the future, if nee
...

💬 maflcko commented on pull request "net: fuzz: bypass network magic and checksum validation":
(https://github.com/bitcoin/bitcoin/pull/31012#discussion_r1783452154)
> DBUILD_FOR_FUZZING

This isn't set for the hfuzz net driver. It uses the `HFND_FUZZING_ENTRY_FUNCTION_CXX`

💬 brunoerg commented on pull request "net: fuzz: bypass network magic and checksum validation":
(https://github.com/bitcoin/bitcoin/pull/31012#discussion_r1783476135)
Yes, missed that. Just added it.

💬 ryanofsky commented on issue "RFC: Multiprocess binaries and packaging options":
(https://github.com/bitcoin/bitcoin/issues/30983#issuecomment-2387010267)
Here's an initial implementation of a `bitcoin` wrapper executable: commit 65e49f4579ee823284bf04f1295b59116acc727b [(branch)](https://github.com/ryanofsky/bitcoin/commits/pr/wrap)

This should be purely additive, and to willcl-ark's point, not break backwards compatibility. It just lets us add new binaries to a libexec/ directory instead of bin/.

📝 brunoerg converted_to_draft a pull request: "net: fuzz: bypass network magic and checksum validation"
(https://github.com/bitcoin/bitcoin/pull/31012)
Fixes #30957

We currently have some instructions on the documentation about how to fuzz the Bitcoin Core P2P layer using Honggfuzz NetDriver. It includes a patch to be applied to bypass network magic and checksum validation, that is outdated. However, we can change the code to bypass them using `FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION`. It also makes the `p2p_transport_serialization` fuzz target simpler since we do not need to assist the network magic and checksum creation anymore.

💬 brunoerg commented on pull request "net: fuzz: bypass network magic and checksum validation":
(https://github.com/bitcoin/bitcoin/pull/31012#issuecomment-2387029412)
Just moved it to draft to get some conceptual feedbacks first. I added a commit to bypass some asserts in `ConnmanTestMsg` that would fail since we're bypassing network magic and checksum validation. Also, I think we will have to change `p2p_transport_bidirectional` target since there are some sanity checks that would fail with the bypasses. Any thoughts?

💬 mooncityorg commented on pull request "build: have "make test" depend on "make all"":
(https://github.com/bitcoin/bitcoin/pull/31015#issuecomment-2387034541)
CMAKE_SKIP_TEST_ALL_DEPENDENCY is false? @bitcoin-core-merge-script

💬 antonilol commented on pull request "Add a "tx output spender" index":
(https://github.com/bitcoin/bitcoin/pull/24539#issuecomment-2387041009)
That's a good way to prevent attackers from inserting collisions in everyone's database, the chance of collision will be dependent on k, which is unknown to an attacker and different for everyone (except by chance or people copying each other's database). Shouldn't 8 bytes of randomness be enough (instead of 16)? Assuming that siphash's output is uniformly distributed, more randomness than the output size (8 bytes) feels unnecessary.

Also, the rpc description should be updated, https://github
...

💬 laanwj commented on pull request "build: have "make test" depend on "make all"":
(https://github.com/bitcoin/bitcoin/pull/31015#issuecomment-2387085025)
Concept ACK

💬 fjahr commented on pull request "doc: add testnet4 section header for config file":
(https://github.com/bitcoin/bitcoin/pull/31007#issuecomment-2387131238)
ACK 61cdb1c9d83778b95f4f9596f34617b7a191d0a5

💬 davidgumberg commented on pull request "Don't zero-after-free `DataStream`: ~25% faster IBD on some configurations":
(https://github.com/bitcoin/bitcoin/pull/30987#issuecomment-2387137498)
> I tested this PR on an AMD Ryzen 7950x machine with Ubuntu 24.04, with one local network peer and a gigabit internet connection.
>
> ```
> bitcoind -dbcache=30000 -stopatheight=863000 -blocksdir=/magnetic/.bitcoin -addnode=local-network
> ```
>
> Before: 5 hours 10 minutes After: 4 hours and 55 minutes
>
> Time includes 20 minutes to flush chainstate to disk during shutdown.

@Sjors Thanks for testing! I suspect that the biggest improvements will be seen on memory bandwidth constr
...

🤔 ismaelsadeeq reviewed a pull request: "test: switch MiniWallet padding unit from weight to vsize"
(https://github.com/bitcoin/bitcoin/pull/30718#pullrequestreview-2341539943)
post-merge ACK 940edd6ac24e921f639b1376fe7d35dc14c1887d

This PR simplify `_bulk_tx` a lot.

📝 mzumsande opened a pull request: "test: add missing sync to feature_fee_estimation.py"
(https://github.com/bitcoin/bitcoin/pull/31016)
This fixes a race:

- In the `test_estimate_dat_is_flushed_periodically` subtest, node 0 is isolated and creates 10 blocks (no sync).
- In `clear_estimates` the nodes are reconnected (but we don't wait for them to sync!)
- In the `sanity_check_rbf_estimates` subtest, node 1 generates another block and syncs with the other nodes. The sync fails if the generated block is at the same height as the tip of the node.

Fix this by adding a sync to `clear_estimates`.

Fixes #30990

💬 mzumsande commented on issue "ci: Intermittent issue in feature_fee_estimation.py in sanity_check_rbf_estimates (Block sync timed out after 2400s)":
(https://github.com/bitcoin/bitcoin/issues/30990#issuecomment-2387172634)
Looks like a sync was missing, see #31016 for a fix.

💬 Richard-23-y commented on issue "Having qt(@6) breaks build for qt@5 on macOS 15.0 and 13.7":
(https://github.com/bitcoin/bitcoin/issues/31009#issuecomment-2387189486)
Hello there

🤔 ismaelsadeeq reviewed a pull request: "test: add missing sync to feature_fee_estimation.py"
(https://github.com/bitcoin/bitcoin/pull/31016#pullrequestreview-2341572804)
Concept ACK

Thanks for solving the issue

Should also fix #30640

🤔 hodlinator reviewed a pull request: "Add a "tx output spender" index"
(https://github.com/bitcoin/bitcoin/pull/24539#pullrequestreview-2341436782)
Reviewed 93e1cafc3f8a7f5b36e495d7eb68c19acdab5809

Concept: I am curious about the justification for including this Index in Bitcoin Core vs implementing it as a third party index, especially since it is not required by anything inside of Core itself. It keeps the index in close sync with the chain state and de-duplicates effort among L2-implementations, but adds maintenance burden in Core.

Implementation: Appreciate the fresh SipHash take. Solves the DoS problem and has some nerd-sniping p
...

💬 hodlinator commented on pull request "Add a "tx output spender" index":
(https://github.com/bitcoin/bitcoin/pull/24539#discussion_r1783556338)
nit: More consistent with type name and snake_case variable naming convention.
```suggestion
bool TxoSpenderIndex::ReadTransaction(const CDiskTxPos& tx_pos, CTransactionRef& tx) const
```