💬 maflcko commented on pull request "fix: Make TxidFromString() respect string_view length":
(https://github.com/bitcoin/bitcoin/pull/30436#discussion_r1683831519)
> * So if there is a space in the string, or a 1 replaced with an I, it is treated like the end of input, and the resulting blob may have a combination of new and old values.

I don't think this is true. All "old" values will zeroed in the first line of `SetHex`.

The other shortcomings (no length check and no hex check) are true.

💬 paplorinc commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#issuecomment-2238277781)
Retested the speed gains after the recent minor changes, with a single run per commit:
> Summary
```python
'echo d7f94e && ./src/bitcoind -datadir=/mnt/hdd1/BitcoinData -dbcache=16384 -prune=550 -stopatheight=400000 -printtoconsole=0' ran
1.12 times faster than 'echo 0383de && ./src/bitcoind -datadir=/mnt/hdd1/BitcoinData -dbcache=16384 -prune=550 -stopatheight=400000 -printtoconsole=0'
```
i.e. 12% faster for the first 400000 blocks after the chang
...

📝 maflcko opened a pull request: " rest: Reject truncated hex txid early in getutxos parsing "
(https://github.com/bitcoin/bitcoin/pull/30482)
In `rest_getutxos` truncated txids such as `aa` or `ff` are accepted. This is brittle at best.

Fix it by rejecting any truncated (or overlarge) input.

----

Review note: This also starts a major refactor to rework hex parsing in Bitcoin Core, meaning that a few refactor commits are included as well. They are explained individually in the commit message and the work will be continued in the future.

⚠️ Sjors opened an issue: "guix: build multiprocess"
(https://github.com/bitcoin/bitcoin/issues/30483)
In order to run a Stratum v2 Template Provider as a "sidecar" via IPC, and to have `bitcoin-node` be reproducibly built, libmultiprocess needs to be built with guix.

Presumably this would happen as part of #10102 or #30437 eventually.

However if the TP "sidecar" is built on top of the Bitcoin Core codebase, like https://github.com/Sjors/bitcoin/pull/48 attempts, then _that_ binary should also be Guix built. So it makes sense to track this independently.

I made a naive attempt in https:/
...

💬 maflcko commented on pull request "fix: Make TxidFromString() respect string_view length":
(https://github.com/bitcoin/bitcoin/pull/30436#discussion_r1683934569)
A partial fix is in https://github.com/bitcoin/bitcoin/pull/30482. It will be continued in the future.

💬 maflcko commented on pull request "fix: Make TxidFromString() respect string_view length":
(https://github.com/bitcoin/bitcoin/pull/30436#discussion_r1683934828)
> Yes, those are known shortcomings. The function will be removed completely in the future, but this will be done in a follow-up.

A partial fix is in https://github.com/bitcoin/bitcoin/pull/30482. It will be continued in the future.

💬 maflcko commented on pull request "fix: Make TxidFromString() respect string_view length":
(https://github.com/bitcoin/bitcoin/pull/30436#discussion_r1683935476)
> Well, it will be removed, given that someone writes the code and reviews it.

A partial fix is in https://github.com/bitcoin/bitcoin/pull/30482. It will be continued in the future.

💬 maflcko commented on pull request "Early logging improvements":
(https://github.com/bitcoin/bitcoin/pull/30386#issuecomment-2238562006)
re-ACK b4dd7ab43e8cfc2c171f67588e4e1ec2705393c2 🕴

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: re-ACK b4dd7ab43e8cfc2c171f
...

💬 stickies-v commented on pull request "fix: Make TxidFromString() respect string_view length":
(https://github.com/bitcoin/bitcoin/pull/30436#discussion_r1683976467)
> it could iterate past the end of the of `string_view` and read uninitialized memory.

Right, because on master we rely on stopping when `HexDigit()` returns a `-1`, which is fine when the string is null-terminated but is not safe otherwise. Thanks for pointing this out, this also clarifies my misunderstanding about your [earlier comment](https://github.com/bitcoin/bitcoin/pull/30436/#pullrequestreview-2186195988).

⚠️ Arroz01 opened an issue: "New"
(https://github.com/bitcoin/bitcoin/issues/30484)

💬 maflcko commented on pull request "qa: Functional test improvements":
(https://github.com/bitcoin/bitcoin/pull/30463#discussion_r1683988688)
> In other places, `os.path.realpath(__file__)` is used in conjunction with relative paths in the `test/functional/` directory.

Thank you for the explanation. It makes sense, given that cmake creates all links recursively in this dir.

However, this leads back to my original question: Why not change the other affected places?

This still fails for me (and I expect it to fail in the cmake branch as well):


```
$ ln $PWD/../test/functional/interface_rest.py ./test/functional/
$
...

✅ maflcko closed an issue: "New"
(https://github.com/bitcoin/bitcoin/issues/30484)

:lock: fanquake locked an issue: "New"
(https://github.com/bitcoin/bitcoin/issues/30484)

💬 craigraw commented on issue "Feature Request: Broadcast Pool":
(https://github.com/bitcoin/bitcoin/issues/30471#issuecomment-2238694916)
> Do you disagree that it is harmless, or do you disagree that the same problem exists in a broadcast pool?

Both. A broadcast pool has direct access to policy rules, a wallet client generally does not.

> Also, why would a fee bump be unnecessary when the transaction doesn't even meet the min mempool fee?

Because it may still exist in a larger miner's mempool. Consider the following situation: Alice broadcasts a transaction at a fee rate of 10 sats/vb. Her min mempool fee increases late
...

💬 Sjors commented on pull request "Introduce waitFeesChanged() mining interface":
(https://github.com/bitcoin/bitcoin/pull/30443#issuecomment-2238698096)
Added `bool& tip_changed`.

🚀 fanquake merged a pull request: "depends: build FreeType with CMake"
(https://github.com/bitcoin/bitcoin/pull/29880)

💬 hebasto commented on pull request "depends: build zeromq with CMake":
(https://github.com/bitcoin/bitcoin/pull/29723#issuecomment-2238702885)
My Guix build:
```
x86_64
5cc76ca8f123e7ff2a365d6a94246a949c4976cf9ecfb9056a4e696a1b80b097 guix-build-e096f5d456f7/output/aarch64-linux-gnu/SHA256SUMS.part
1c6c8d2efba8f6cbe54b8e8bde00d962f9837ab76f801507a370e4d15ada0059 guix-build-e096f5d456f7/output/aarch64-linux-gnu/bitcoin-e096f5d456f7-aarch64-linux-gnu-debug.tar.gz
27110e79b03a3f22c24888f79e84c684eaddfb04b8595d200d6db5d689f260a2 guix-build-e096f5d456f7/output/aarch64-linux-gnu/bitcoin-e096f5d456f7-aarch64-linux-gnu.tar.gz
ddb3c16f6
...

🤔 glozow reviewed a pull request: "fuzz: Speed up PickValue in txorphan"
(https://github.com/bitcoin/bitcoin/pull/30474#pullrequestreview-2186411295)
concept ACK, thanks @maflcko!

💬 glozow commented on pull request "fuzz: Speed up PickValue in txorphan":
(https://github.com/bitcoin/bitcoin/pull/30474#discussion_r1683198029)
Agree that using a vector (i.e. the 3-line diff) does seem like it solves this problem in a simpler + faster way. Having duplicate outpoints should be fine and maybe even more coverage. Fuzzed with the 3-line diff for a bit.

💬 fanquake commented on issue "guix: build multiprocess":
(https://github.com/bitcoin/bitcoin/issues/30483#issuecomment-2238748982)
> That didn't work because curl is not accessible.

Can you elaborate? I did a build with `MULTIPROCESS=1` (https://github.com/fanquake/bitcoin/tree/guix_multiprocess) and it worked fine:
```bash
# time BASE_CACHE="/base_cache" SOURCES_PATH="/sources" SDK_PATH="/SDKs" HOSTS="x86_64-linux-gnu" ./contrib/guix/guix-build
e4e606e3a7076fe0e1f93b98ecdfbbfcd552a3c25f368684f3e94aab919058ce guix-build-e0e38abd03a0/output/dist-archive/bitcoin-e0e38abd03a0.tar.gz
e8b4c9f20abfe29fa1f0fa47515a28d169f
...