Bitcoin Core Github
43 subscribers
123K links
Download Telegram
💬 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/
$
...
: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
...
👍 t-bast approved a pull request: "policy: Add PayToAnchor(P2A), `OP_TRUE <0x4e73>` as a standard output script for spending"
(https://github.com/bitcoin/bitcoin/pull/30352#pullrequestreview-2187787797)
ACK https://github.com/bitcoin/bitcoin/pull/30352/commits/8d956ccb86d70e2059a5e2ccd8e63b7f7c8e3842, thanks for pushing this!

I've read through the code and comments on this PR, and to me it makes sense to use a taproot 2-byte program, the cute trick of having it encode as "fees" in bech32m can be useful (since we need to arbitrarily choose 2 bytes anyway for the program), and it's a step towards ephemeral anchors which are much better than current anchor outputs :+1:

Regarding utxo bloat
...
💬 t-bast commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#issuecomment-2238752561)
Concept ACK :+1:
💬 fanquake commented on pull request "Stratum v2 Template Provider common functionality":
(https://github.com/bitcoin/bitcoin/pull/30475#issuecomment-2238759084)
Can you clarify what the point of PRs like this is, given you have a number of them open? They aren't reviewable, or mergable (given they are all based on various PRs, which them selves are in various stages of draft/WIP), and the continual pushes keep taking up all the CI resources in the repo (I thought we'd adjusted the CI so that you can use your own repo?).
💬 maflcko commented on issue "Feature Request: Broadcast Pool":
(https://github.com/bitcoin/bitcoin/issues/30471#issuecomment-2238799112)
The same problem exists in a broadcast pool. For example:

* Alice broadcasts at 10sat/vb, but the transaction isn't confirmed in $mempool_expiry time, and thus dropped from Alice's mempool (and broadcast pool). (The transaction may or may not be in a miner's mempool with larger expiry)
* Mallory broadcasts her at 9sat/vb.
* Alice's wallet discards her transaction using 10sat/vb, based on the hierarchy `confirmed > mempool > local` (or `confirmed > mempool > broadcast_pool > local`)

So I
...
💬 fanquake commented on pull request "depends: build expat with CMake":
(https://github.com/bitcoin/bitcoin/pull/29878#issuecomment-2238813040)
Rebased on master. 32-bit builds should also be fixed. No longer a version bump, just a Autotools -> CMake switch.
👋 fanquake's pull request is ready for review: "depends: build expat with CMake"
(https://github.com/bitcoin/bitcoin/pull/29878)
💬 craigraw commented on issue "Feature Request: Broadcast Pool":
(https://github.com/bitcoin/bitcoin/issues/30471#issuecomment-2238824451)
> and thus dropped from Alice's mempool (and broadcast pool)

This is where we differ. From my description in the OP: "Transactions in the broadcast pool are subject to almost all of the same rules as those in the mempool, except that they are not removed due to mempool size limitations." Therefore, in this scenario, the transaction would not be dropped from the broadcast pool.

I don't want to focus too deeply on whether the situation must be adversarial - as a wallet developer, I am always
...